I am part of a small team working on a fairly small website with user accounts; there are about 100 users at this time. And we are using Amazon Cognito for user management. On our website there is a summary page which displays a list/table of all users and various attributes. However, there's a hard limit on the number of items returned by the Amazon Cognito listUsers API call, in this case 60.
Luckily, the API call also returns a token to use to make subsequent calls if there are more users. Using those tokens, we're able to request a full list of all users.
Also, our website uses react-redux & the javascript library bluebird. In the context of this question, the component requesting a list of all users also dispatches actions (redux piece) and the CognitoIdentityServiceProvider from Amazon's aws-sdk is passed to bluebird Promise.promisfyAll (adds the async postfix to the listUser call). I am modifying this component to get the entire list of users.
As a rough draft of requesting the entire list of users, I used a recursive function to chain promises together until the pagination token returned from Amazon is undefined and store the results in some class variables. I used this forum post as reference.
This works but I don't feel great about the implementation; it should be okay for now in our use case as we have about 100 users but I do not know if this will scale well to thousands of users or more. I understand recursion just enough to be dangerous in that I do not know if this technique can cause problems when the number of promises/calls goes up. Questions of overhead and memory management come to mind. However, we shouldn't have to worry about the number of users skyrocketing into the thousands for at least a little while but I still want to learn about potentially preferable and/or safer methods of achieving the same thing.
The following snippets are from the component requesting the list of users:
import Promise from 'bluebird';
import { CognitoIdentityServiceProvider } from './services/aws';
export const fetchRegisteredUsers = () => (dispatch) => {
...
let allUsersTemp = [];
let paginationToken;
const getUserList = () => {
let tempUserTest = CognitoIdentityServiceProvider.listUsersAsync({
UserPoolId: process.env.COGNITO_USER_POOL_ID,
PaginationToken: paginationToken
});
return tempUserTest.then((tempUser) => {
allUsersTemp = allUsersTemp.concat(tempUser.Users);
paginationToken = tempUser.PaginationToken;
if(paginationToken) {
return getUserList();
} else {
return;
}
})
}
const adminUsers = CognitoIdentityServiceProvider.listUsersInGroupAsync(adminParams);
return Promise.all([adminUsers]).then(
([{ Users: adminUsers }]) => {
getUserList().then((item) => {
let allUsers = allUsersTemp;
const adminUsernames = adminUsers.map(user => user.Username);
allUsers.forEach(user => {
let mappedAttributes = {};
user.Attributes.forEach(attribute => mappedAttributes[attribute.Name] = attribute.Value);
user.Attributes = mappedAttributes;
user.isAdmin = adminUsernames.includes(user.Username);
});
dispatch({
type: FETCH_REGISTERED_USERS_SUCCESS,
payload: allUsers
});
});
}, ...
From what I understand the bluebird Promise.all call waits until every item passed to it can resolve before executing what's in the .then portion; before making any changes to the code, there were two lists, one for users & one for admins. The Promise.all waited until both promises were done, did some basic processing on the data, and returned the data in an action dispatch payload.
After my changes, the logic to process and return the list of users is executed after the recursive promise chain (getUserList) is complete.
My question: is this technique okay and can I use this as is? Or is it unsafe and, if so, what specifically is the problem? And is there a better way altogether?
My weakpoints are recursion and promises/bluebird so please feel free to critique the code on anything related to those topics
I understand recursion just enough to be dangerous in that I do not know if this technique can cause problems when the number of promises/calls goes up. Questions of overhead and memory management come to mind
Recursive approaches to make promise calls like your getUserList are OK.
The only possible memory problem is that your allUsersTemp array could be growing very large, until it exceeds the limits of your browser. You should however wonder long before that whether it would really be useful to display hundredths of thousands of user entries in a single large table. At those scales where memory would become problematic, you'd need more effective tools to manage your user base anyway than just listing all of them.
Regarding code style, I would recommend not to declare allUsersTemp and paginationToken as mutable higher-scope variables. Instead make them parameters of your recursive function and fulfill the promise with the result:
function getUserList (paginationToken, allUsersTemp = []) => {
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
return tempUserTest = CognitoIdentityServiceProvider.listUsersAsync({
UserPoolId: process.env.COGNITO_USER_POOL_ID,
PaginationToken: paginationToken
}).then(tempUser => {
const allUsers = allUsersTemp.concat(tempUser.Users);
const nextToken = tempUser.PaginationToken;
if (nextToken) {
return getUserList(nextToken, allUsers);
// ^^^^^^^^^^^^^^^^^^^
} else {
return allUsers;
// ^^^^^^^^
}
});
}
The
Promise.allwaited until both promises were done
No, you were passing an array with only a single promise to Promise.all, and were calling the second function after the first completed. To make the run concurrently, it should look like
return Promise.all([
CognitoIdentityServiceProvider.listUsersInGroupAsync(adminParams),
getUserList()
]).then(([{ Users: adminUsers }, allUsers]) => {
…
});