Skip to content

Commit d93d9a9

Browse files
committed
WIP on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (#1922)"
2 parents c454180 + 8342f3d commit d93d9a9

File tree

7 files changed

+149
-89
lines changed

7 files changed

+149
-89
lines changed

spec/ParseAPI.spec.js

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ let defaultColumns = require('../src/Controllers/SchemaController').defaultColum
1111
const requiredUserFields = { fields: Object.assign({}, defaultColumns._Default, defaultColumns._User) };
1212

1313

14-
describe('miscellaneous', function() {
14+
fdescribe('miscellaneous', function() {
1515
it('create a GameScore object', function(done) {
1616
var obj = new Parse.Object('GameScore');
1717
obj.set('score', 1337);
@@ -80,44 +80,53 @@ describe('miscellaneous', function() {
8080
.catch(done);
8181
});
8282

83-
it('ensure that email is uniquely indexed', done => {
84-
let numCreated = 0;
85-
let numFailed = 0;
86-
87-
let user1 = new Parse.User();
88-
user1.setPassword('asdf');
89-
user1.setUsername('u1');
90-
user1.setEmail('[email protected]');
91-
let p1 = user1.signUp();
92-
p1.then(user => {
93-
numCreated++;
94-
expect(numCreated).toEqual(1);
95-
}, error => {
96-
numFailed++;
97-
expect(numFailed).toEqual(1);
98-
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
99-
});
83+
fit('ensure that email is uniquely indexed', done => {
84+
DatabaseAdapter._indexBuildsCompleted('test')
85+
.then(() => {
86+
let numCreated = 0;
87+
let numFailed = 0;
88+
89+
let user1 = new Parse.User();
90+
user1.setPassword('asdf');
91+
user1.setUsername('u1');
92+
user1.setEmail('[email protected]');
93+
let p1 = user1.signUp();
94+
p1.then(user => {
95+
numCreated++;
96+
expect(numCreated).toEqual(1);
97+
}, error => {
98+
numFailed++;
99+
console.log(error);
100+
expect(numFailed).toEqual(1);
101+
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
102+
});
100103

101-
let user2 = new Parse.User();
102-
user2.setPassword('asdf');
103-
user2.setUsername('u2');
104-
user2.setEmail('[email protected]');
105-
let p2 = user2.signUp();
106-
p2.then(user => {
107-
numCreated++;
108-
expect(numCreated).toEqual(1);
109-
}, error => {
110-
numFailed++;
111-
expect(numFailed).toEqual(1);
112-
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
113-
});
104+
let user2 = new Parse.User();
105+
user2.setPassword('asdf');
106+
user2.setUsername('u2');
107+
user2.setEmail('[email protected]');
108+
let p2 = user2.signUp();
109+
p2.then(user => {
110+
numCreated++;
111+
expect(numCreated).toEqual(1);
112+
}, error => {
113+
numFailed++;
114+
console.log(error);
115+
expect(numFailed).toEqual(1);
116+
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
117+
});
114118

115-
Parse.Promise.all([p1, p2])
116-
.then(() => {
117-
fail('one of the users should not have been created');
118-
done();
119+
Parse.Promise.all([p1, p2])
120+
.then(() => {
121+
fail('one of the users should not have been created');
122+
done();
123+
})
124+
.catch(done);
119125
})
120-
.catch(done);
126+
.catch(error => {
127+
fail('index build failed')
128+
done();
129+
});
121130
});
122131

123132
it('ensure that if people already have duplicate users, they can still sign up new users', done => {

spec/helper.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ afterEach(function(done) {
111111
})
112112
.then(() => Parse.User.logOut())
113113
.then(() => {
114-
return TestUtils.destroyAllDataPermanently();
114+
//return TestUtils.destroyAllDataPermanently();
115115
}).then(() => {
116116
done();
117117
}, (error) => {
@@ -243,14 +243,16 @@ function mockFacebook() {
243243
facebook.validateAuthData = function(authData) {
244244
if (authData.id === '8675309' && authData.access_token === 'jenny') {
245245
return Promise.resolve();
246+
} else {
247+
throw undefined;
246248
}
247-
return Promise.reject();
248249
};
249250
facebook.validateAppId = function(appId, authData) {
250251
if (authData.access_token === 'jenny') {
251252
return Promise.resolve();
253+
} else {
254+
throw undefined;
252255
}
253-
return Promise.reject();
254256
};
255257
return facebook;
256258
}

src/Adapters/Storage/Mongo/MongoCollection.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,9 @@ export default class MongoCollection {
8888
this._mongoCollection.ensureIndex(indexRequest, { unique: true, background: true, sparse: true }, (error, indexName) => {
8989
if (error) {
9090
reject(error);
91+
} else {
92+
resolve();
9193
}
92-
resolve();
9394
});
9495
});
9596
}

src/DatabaseAdapter.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,16 @@
1717

1818
import DatabaseController from './Controllers/DatabaseController';
1919
import MongoStorageAdapter from './Adapters/Storage/Mongo/MongoStorageAdapter';
20+
import log from './logger';
21+
22+
var SchemaController = require('./Controllers/SchemaController');
2023

2124
let dbConnections = {};
2225
let appDatabaseURIs = {};
2326
let appDatabaseOptions = {};
27+
let indexBuildCreationPromises = {};
28+
29+
const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
2430

2531
function setAppDatabaseURI(appId, uri) {
2632
appDatabaseURIs[appId] = uri;
@@ -49,6 +55,11 @@ function destroyAllDataPermanently() {
4955
throw 'Only supported in test environment';
5056
}
5157

58+
//Super janky. Will be removed in a later PR.
59+
function _indexBuildsCompleted(appId) {
60+
return indexBuildCreationPromises[appId];
61+
}
62+
5263
function getDatabaseConnection(appId: string, collectionPrefix: string) {
5364
if (dbConnections[appId]) {
5465
return dbConnections[appId];
@@ -62,6 +73,57 @@ function getDatabaseConnection(appId: string, collectionPrefix: string) {
6273

6374
dbConnections[appId] = new DatabaseController(new MongoStorageAdapter(mongoAdapterOptions), {appId: appId});
6475

76+
// Kick off unique index build in the background (or ensure the unique index already exists)
77+
// A bit janky, will be fixed in a later PR.
78+
let p1 = dbConnections[appId].adapter.ensureUniqueness('_User', ['username'], requiredUserFields)
79+
.catch(error => {
80+
log.warn('Unable to ensure uniqueness for usernames: ', error);
81+
return Promise.reject();
82+
});
83+
84+
let p2 = dbConnections[appId].adapter.ensureUniqueness('_User', ['email'], requiredUserFields)
85+
.catch(error => {
86+
log.warn('Unabled to ensure uniqueness for user email addresses: ', error);
87+
return Promise.reject();
88+
})
89+
90+
indexBuildCreationPromises[appId] = p1.then(() => p2)
91+
.then(() => console.log('index build success'))
92+
.then(() => {
93+
let numCreated = 0;
94+
let numFailed = 0;
95+
96+
let user1 = new Parse.User();
97+
user1.setPassword('asdf');
98+
user1.setUsername('u1');
99+
user1.setEmail('[email protected]');
100+
let p1 = user1.signUp();
101+
p1.then(user => {
102+
numCreated++;
103+
console.log(numCreated)
104+
}, error => {
105+
numFailed++;
106+
console.log(error);
107+
console.log(numFailed)
108+
console.log(error.code)
109+
});
110+
111+
let user2 = new Parse.User();
112+
user2.setPassword('asdf');
113+
user2.setUsername('u2');
114+
user2.setEmail('[email protected]');
115+
let p2 = user2.signUp();
116+
p2.then(user => {
117+
numCreated++;
118+
console.log(numCreated)
119+
}, error => {
120+
numFailed++;
121+
console.log(error);
122+
console.log(numFailed)
123+
console.log(error.code)
124+
});
125+
})
126+
65127
return dbConnections[appId];
66128
}
67129

@@ -71,4 +133,5 @@ module.exports = {
71133
setAppDatabaseURI: setAppDatabaseURI,
72134
clearDatabaseSettings: clearDatabaseSettings,
73135
destroyAllDataPermanently: destroyAllDataPermanently,
136+
_indexBuildsCompleted,
74137
};

src/PromiseRouter.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
// components that external developers may be modifying.
77

88
import express from 'express';
9-
import url from 'url';
10-
import log from './logger';
9+
import url from 'url';
10+
import log from './logger';
1111

1212
export default class PromiseRouter {
1313
// Each entry should be an object with:

src/RestWrite.js

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ var triggers = require('./triggers');
1414
import RestQuery from './RestQuery';
1515
import _ from 'lodash';
1616

17-
const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
18-
1917
// query and data are both provided in REST API format. So data
2018
// types are encoded by plain old objects.
2119
// If query is null, this is a "create" and the data in data should be
@@ -358,61 +356,47 @@ RestWrite.prototype.transformUser = function() {
358356
}
359357
return;
360358
}
361-
// TODO: refactor this so that ensureUniqueness isn't called for every single sign up.
362-
return this.config.database.adapter.ensureUniqueness('_User', ['username'], requiredUserFields)
363-
.catch(error => {
364-
if (error.code === Parse.Error.DUPLICATE_VALUE) {
365-
// If they already have duplicate usernames or emails, the ensureUniqueness will fail,
366-
// and then nobody will be able to sign up D: so just use the old, janky
367-
// method to check for duplicate usernames.
368-
return this.config.database.find(
369-
this.className,
370-
{ username: this.data.username, objectId: {'$ne': this.objectId()} },
371-
{ limit: 1 }
372-
)
373-
.then(results => {
374-
if (results.length > 0) {
375-
throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
376-
}
377-
return;
378-
});
359+
// We need to a find to check for duplicate username in case they are missing the unique index on usernames
360+
// TODO: Check if there is a unique index, and if so, skip this query.
361+
return this.config.database.find(
362+
this.className,
363+
{ username: this.data.username, objectId: {'$ne': this.objectId()} },
364+
{ limit: 1 }
365+
)
366+
.then(results => {
367+
if (results.length > 0) {
368+
throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
379369
}
380-
throw error;
381-
})
382-
}).then(() => {
370+
return;
371+
});
372+
})
373+
.then(() => {
383374
if (!this.data.email || this.data.email.__op === 'Delete') {
384375
return;
385376
}
386377
// Validate basic email address format
387378
if (!this.data.email.match(/^.+@.+$/)) {
388379
throw new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS, 'Email address format is invalid.');
389380
}
390-
// Check for email uniqueness
391-
return this.config.database.adapter.ensureUniqueness('_User', ['email'], requiredUserFields)
392-
.catch(error => {
393-
// Same problem for email as above for username
394-
if (error.code === Parse.Error.DUPLICATE_VALUE) {
395-
return this.config.database.find(
396-
this.className,
397-
{ email: this.data.email, objectId: {'$ne': this.objectId()} },
398-
{ limit: 1 }
399-
)
400-
.then(results => {
401-
if (results.length > 0) {
402-
throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
403-
}
404-
return;
405-
});
381+
// Same problem for email as above for username
382+
return this.config.database.find(
383+
this.className,
384+
{ email: this.data.email, objectId: {'$ne': this.objectId()} },
385+
{ limit: 1 }
386+
)
387+
.then(results => {
388+
if (results.length > 0) {
389+
throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
406390
}
407-
throw error;
408-
})
409-
.then(() => {
410-
// We updated the email, send a new validation
411-
this.storage['sendVerificationEmail'] = true;
412-
this.config.userController.setEmailVerifyToken(this.data);
413391
return;
414-
})
415-
});
392+
});
393+
})
394+
.then(() => {
395+
// We updated the email, send a new validation
396+
this.storage['sendVerificationEmail'] = true;
397+
this.config.userController.setEmailVerifyToken(this.data);
398+
return;
399+
})
416400
};
417401

418402
RestWrite.prototype.createSessionTokenIfNeeded = function() {

src/middlewares.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ var handleParseErrors = function(err, req, res, next) {
235235
}
236236

237237
res.status(httpStatus);
238+
console.log(err);
238239
res.json({code: err.code, error: err.message});
239240
} else if (err.status && err.message) {
240241
res.status(err.status);

0 commit comments

Comments
 (0)