oaeproject/Hilary — pull request #663 by simong (2013-09-16 08:12:59+00:00)
Title: Intermittent test failure: Import users from CSV

```
1) Users Import users verify import external users:

actual expected

Nicolaas Matthijsuser-TGdDSdadW

at /Users/branden/Source/oaeproject/Hilary/node_modules/oae-principals/tests/test-users.js:564:76
at Request._callback (/Users/branden/Source/oaeproject/Hilary/node_modules/oae-rest/lib/util.js:203:16)
at Request.self.callback (/Users/branden/Source/oaeproject/Hilary/node_modules/request/request.js:129:22)
at Request.EventEmitter.emit (events.js:98:17)
at Request. (/Users/branden/Source/oaeproject/Hilary/node_modules/request/request.js:873:14)
at Request.EventEmitter.emit (events.js:117:20)
at IncomingMessage. (/Users/branden/Source/oaeproject/Hilary/node_modules/request/request.js:824:12)
at IncomingMessage.EventEmitter.emit (events.js:117:20)
at _stream_readable.js:910:16
at process._tickDomainCallback (node.js:459:13)
```

I did some code review, from **oae-principals/lib/api.user.js**:

```javascript
/*!
* Process an invidual user from the CSV file and create a new user if no user exists for the provided
* external id - authentication strategy combination.
*
* @param {String[][]} data Parsed CSV file
*/
var processUser = function(data) {
// Get the next user from the stack
var user = data.pop();

// Extract the password in case local authentication is used
var opts = {};
if (authenticationStrategy === AuthenticationConstants.providers.LOCAL) {
opts.password = user.splice(1, 1);
}

// Extract the basic profile data
var externalId = user[0];
// Construct the first name and last name into a display name
var displayName = util.format('%s %s', user[2], user[1]).trim();
opts.email = user[3];

// Check if the user already exists and create a new user if it doesn't
// TODO: Fix cross-dependency between the Authentication API and the Principals API
require('oae-authentication').getOrCreateUser(adminCtx, authenticationStrategy, externalId, displayName, opts, function(err, user) {

if (err) {
log(ctx).error({
'err': err,
'externalId': externalId,
'displayName': displayName
}, 'Failed to import user from CSV');
// If the user already existed and its displayName is the same as the external id, we update the user's display
// name to be the one provided in the CSV file
} else if (user.displayName === externalId && user.displayName !== displayName) {
log(ctx).info({
'externalId': externalId,
'user': user,
'displayName': displayName
}, 'Updating display name during import from CSV');
updateUser(adminCtx, user.id, {'displayName': displayName});
}

// Add a progress log statement every 25 imported users
if (data.length % 25 === 0) {
log(ctx).info({
'authenticationStrategy': authenticationStrategy,
'tenantAlias': tenantAlias
}, 'Importing users. ' + data.length + ' users left to import');
}

if (data.length === 0) {
// Send out an event indicating that the import has finished
PrincipalsEmitter.emit('postCSVUserImport');
} else {
// Process the next user
processUser(data);
}
});
};

// Process the first user
processUser(data);
});
```

In this block `updateUser` here is happening asynchronously to the whole CSV import cycle, so there is no guarantee the `updateUser` calls have actually finished at the time that the `postCSVUserImport` event is emitted.

Assigning to @nicolaasmatthijs , but if anyone with a clean working copy volunteers open cycles, feel free to re-assign as this should be a quick win.