Skip to content

Commit 3563a5d

Browse files
drew-grossdrew-gross
committed
Add unique indexing for username/email
1 parent dff5a44 commit 3563a5d

File tree

3 files changed

+194
-39
lines changed

3 files changed

+194
-39
lines changed

‎spec/ParseAPI.spec.js

Lines changed: 108 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ var DatabaseAdapter = require('../src/DatabaseAdapter');
66
var request = require('request');
77
const Parse = require("parse/node");
88
let Config = require('../src/Config');
9+
let defaultColumns = require('../src/Controllers/SchemaController').defaultColumns;
10+
11+
const requiredUserFields = { fields: Object.assign({}, defaultColumns._Default, defaultColumns._User) };
12+
913

1014
describe('miscellaneous', function() {
1115
it('create a GameScore object', function(done) {
@@ -45,17 +49,116 @@ describe('miscellaneous', function() {
4549
});
4650
});
4751

48-
it('fail to create a duplicate username', function(done) {
49-
createTestUser(function(data) {
50-
createTestUser(function(data) {
51-
fail('Should not have been able to save duplicate username.');
52-
}, function(error) {
52+
fit('fail to create a duplicate username', done => {
53+
let numCreated = 0;
54+
let numFailed = 0;
55+
let p1 = createTestUser();
56+
p1.then(user => {
57+
numCreated++;
58+
expect(numCreated).toEqual(1);
59+
})
60+
.catch(error => {
61+
numFailed++;
62+
expect(numFailed).toEqual(1);
63+
expect(error.code).toEqual(Parse.Error.USERNAME_TAKEN);
64+
});
65+
let p2 = createTestUser();
66+
p2.then(user => {
67+
numCreated++;
68+
expect(numCreated).toEqual(1);
69+
})
70+
.catch(error => {
71+
numFailed++;
72+
expect(numFailed).toEqual(1);
73+
expect(error.code).toEqual(Parse.Error.USERNAME_TAKEN);
74+
});
75+
Parse.Promise.all([p1, p2])
76+
.then(() => {
77+
fail('one of the users should not have been created');
78+
done();
79+
})
80+
.catch(done);
81+
});
82+
83+
fit('ensure that if people already have duplicate users, they can still sign up new users', done => {
84+
let config = new Config('test');
85+
config.database.adapter.createObject('_User', { objectId: 'x', username: 'u' }, requiredUserFields)
86+
.then(() => config.database.adapter.createObject('_User', { objectId: 'y', username: 'u' }, requiredUserFields))
87+
.then(() => {
88+
let user = new Parse.User();
89+
user.setPassword('asdf');
90+
user.setUsername('zxcv');
91+
return user.signUp();
92+
})
93+
.then(() => {
94+
let user = new Parse.User();
95+
user.setPassword('asdf');
96+
user.setUsername('u');
97+
user.signUp()
98+
.catch(error => {
5399
expect(error.code).toEqual(Parse.Error.USERNAME_TAKEN);
54100
done();
55101
});
102+
})
103+
.catch(() => {
104+
fail('save should have succeeded');
105+
done();
56106
});
57107
});
58108

109+
fit('ensure that email is uniquely indexed', done => {
110+
let numCreated = 0;
111+
let numFailed = 0;
112+
113+
let user1 = new Parse.User();
114+
user1.setPassword('asdf');
115+
user1.setUsername('u1');
116+
user1.setEmail('[email protected]');
117+
let p1 = user1.signUp();
118+
p1.then(user => {
119+
numCreated++;
120+
expect(numCreated).toEqual(1);
121+
})
122+
.catch(error => {
123+
numFailed++;
124+
expect(numFailed).toEqual(1);
125+
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
126+
});
127+
128+
let user2 = new Parse.User();
129+
user2.setPassword('asdf');
130+
user2.setUsername('u2');
131+
user2.setEmail('[email protected]');
132+
let p2 = user2.signUp();
133+
p2.then(user => {
134+
numCreated++;
135+
expect(numCreated).toEqual(1);
136+
})
137+
.catch(error => {
138+
numFailed++;
139+
expect(numFailed).toEqual(1);
140+
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
141+
});
142+
Parse.Promise.all([p1, p2])
143+
.then(() => {
144+
fail('one of the users should not have been created');
145+
done();
146+
})
147+
.catch(done);
148+
});
149+
150+
it('ensure that if people already have duplicate emails, they can still sign up new users', done => {
151+
152+
});
153+
154+
it('ensure you get the right error if you have 2 users with the same email', done => {
155+
156+
});
157+
158+
it('ensure that if you try to sign up a user with a unique username and email, but duplicates in some other field that has a uniqueness constraint, you get a regular duplicate value error', done => {
159+
160+
});
161+
59162
it('succeed in logging in', function(done) {
60163
createTestUser(function(u) {
61164
expect(typeof u.id).toEqual('string');

‎spec/Uniqueness.spec.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ describe('Uniqueness', function() {
6666
});
6767
});
6868

69-
notWorking('doesnt let you enforce uniqueness on nullable fields', done => {
70-
// Coming later, once we have a way to specify that a field is non-nullable
71-
});
72-
7369
it('can do compound uniqueness', done => {
7470
let config = new Config('test');
7571
config.database.adapter.ensureUniqueness('CompoundUnique', ['k1', 'k2'], { fields: { k1: { type: 'String' }, k2: { type: 'String' } } })
@@ -102,4 +98,12 @@ describe('Uniqueness', function() {
10298
done();
10399
});
104100
});
101+
102+
it('adding a unique index to an existing field works even if it has nulls', done => {
103+
104+
});
105+
106+
it('adding a unique index to an existing field doesnt prevent you from adding new documents with nulls', done => {
107+
108+
});
105109
});

‎src/RestWrite.js

Lines changed: 78 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ 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+
1719
// query and data are both provided in REST API format. So data
1820
// types are encoded by plain old objects.
1921
// If query is null, this is a "create" and the data in data should be
@@ -356,44 +358,60 @@ RestWrite.prototype.transformUser = function() {
356358
}
357359
return;
358360
}
359-
return this.config.database.find(
360-
this.className, {
361-
username: this.data.username,
362-
objectId: {'$ne': this.objectId()}
363-
}, {limit: 1}).then((results) => {
364-
if (results.length > 0) {
365-
throw new Parse.Error(Parse.Error.USERNAME_TAKEN,
366-
'Account already exists for this username');
367-
}
368-
return Promise.resolve();
369-
});
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+
});
379+
}
380+
throw error;
381+
})
370382
}).then(() => {
371383
if (!this.data.email || this.data.email.__op === 'Delete') {
372384
return;
373385
}
374386
// Validate basic email address format
375387
if (!this.data.email.match(/^.+@.+$/)) {
376-
throw new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS,
377-
'Email address format is invalid.');
388+
throw new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS, 'Email address format is invalid.');
378389
}
379390
// Check for email uniqueness
380-
return this.config.database.find(
381-
this.className, {
382-
email: this.data.email,
383-
objectId: {'$ne': this.objectId()}
384-
}, {limit: 1}).then((results) => {
385-
if (results.length > 0) {
386-
throw new Parse.Error(Parse.Error.EMAIL_TAKEN,
387-
'Account already exists for this email ' +
388-
'address');
389-
}
390-
return Promise.resolve();
391-
}).then(() => {
392-
// We updated the email, send a new validation
393-
this.storage['sendVerificationEmail'] = true;
394-
this.config.userController.setEmailVerifyToken(this.data);
395-
return Promise.resolve();
396-
})
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+
});
406+
}
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);
413+
return;
414+
})
397415
});
398416
};
399417

@@ -762,6 +780,36 @@ RestWrite.prototype.runDatabaseOperation = function() {
762780

763781
// Run a create
764782
return this.config.database.create(this.className, this.data, this.runOptions)
783+
.catch(error => {
784+
if (this.className !== '_User' || error.code !== Parse.Error.DUPLICATE_VALUE) {
785+
throw error;
786+
}
787+
// If this was a failed user creation due to username or email already taken, we need to
788+
// check whether it was username or email and return the appropriate error.
789+
790+
// TODO: See if we can later do this without additional queries by using named indexes.
791+
return this.config.database.find(
792+
this.className,
793+
{ username: this.data.username, objectId: {'$ne': this.objectId()} },
794+
{ limit: 1 }
795+
)
796+
.then(results => {
797+
if (results.length > 0) {
798+
throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
799+
}
800+
return this.config.database.find(
801+
this.className,
802+
{ email: this.data.email, objectId: {'$ne': this.objectId()} },
803+
{ limit: 1 }
804+
);
805+
})
806+
.then(results => {
807+
if (results.length > 0) {
808+
throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
809+
}
810+
throw error;
811+
});
812+
})
765813
.then(response => {
766814
response.objectId = this.data.objectId;
767815
response.createdAt = this.data.createdAt;

0 commit comments

Comments
 (0)