Skip to content

Commit eaa9dd5

Browse files
drew-grossdrew-gross
committed
Finish unique indexes
1 parent c77b56e commit eaa9dd5

File tree

9 files changed

+84
-85
lines changed

9 files changed

+84
-85
lines changed

‎spec/ParseAPI.spec.js

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -80,32 +80,6 @@ describe('miscellaneous', function() {
8080
.catch(done);
8181
});
8282

83-
it('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 => {
99-
expect(error.code).toEqual(Parse.Error.USERNAME_TAKEN);
100-
done();
101-
});
102-
})
103-
.catch(() => {
104-
fail('save should have succeeded');
105-
done();
106-
});
107-
});
108-
10983
it('ensure that email is uniquely indexed', done => {
11084
let numCreated = 0;
11185
let numFailed = 0;
@@ -118,8 +92,7 @@ describe('miscellaneous', function() {
11892
p1.then(user => {
11993
numCreated++;
12094
expect(numCreated).toEqual(1);
121-
})
122-
.catch(error => {
95+
}, error => {
12396
numFailed++;
12497
expect(numFailed).toEqual(1);
12598
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
@@ -133,12 +106,12 @@ describe('miscellaneous', function() {
133106
p2.then(user => {
134107
numCreated++;
135108
expect(numCreated).toEqual(1);
136-
})
137-
.catch(error => {
109+
}, error => {
138110
numFailed++;
139111
expect(numFailed).toEqual(1);
140112
expect(error.code).toEqual(Parse.Error.EMAIL_TAKEN);
141113
});
114+
142115
Parse.Promise.all([p1, p2])
143116
.then(() => {
144117
fail('one of the users should not have been created');
@@ -147,6 +120,32 @@ describe('miscellaneous', function() {
147120
.catch(done);
148121
});
149122

123+
it('ensure that if people already have duplicate users, they can still sign up new users', done => {
124+
let config = new Config('test');
125+
config.database.adapter.createObject('_User', { objectId: 'x', username: 'u' }, requiredUserFields)
126+
.then(() => config.database.adapter.createObject('_User', { objectId: 'y', username: 'u' }, requiredUserFields))
127+
.then(() => {
128+
let user = new Parse.User();
129+
user.setPassword('asdf');
130+
user.setUsername('zxcv');
131+
return user.signUp();
132+
})
133+
.then(() => {
134+
let user = new Parse.User();
135+
user.setPassword('asdf');
136+
user.setUsername('u');
137+
user.signUp()
138+
.catch(error => {
139+
expect(error.code).toEqual(Parse.Error.USERNAME_TAKEN);
140+
done();
141+
});
142+
})
143+
.catch(() => {
144+
fail('save should have succeeded');
145+
done();
146+
});
147+
});
148+
150149
it('ensure that if people already have duplicate emails, they can still sign up new users', done => {
151150
let config = new Config('test');
152151
config.database.adapter.createObject('_User', { objectId: 'x', email: '[email protected]' }, requiredUserFields)
@@ -176,7 +175,7 @@ describe('miscellaneous', function() {
176175
});
177176
});
178177

179-
fit('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 => {
178+
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 => {
180179
let config = new Config('test');
181180
config.database.adapter.ensureUniqueness('_User', ['randomField'], requiredUserFields)
182181
.then(() => {

‎spec/Uniqueness.spec.js

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe('Uniqueness', function() {
1212
obj.save().then(() => {
1313
expect(obj.id).not.toBeUndefined();
1414
let config = new Config('test');
15-
return config.database.adapter.ensureUniqueness('UniqueField', ['unique'], { fields: { unique: { type: 'String' } } })
15+
return config.database.adapter.ensureUniqueness('UniqueField', ['unique'], { fields: { unique: { __type: 'String' } } })
1616
})
1717
.then(() => {
1818
let obj = new Parse.Object('UniqueField');
@@ -33,7 +33,10 @@ describe('Uniqueness', function() {
3333
.then(() => obj.save({ ptr: obj }))
3434
.then(() => {
3535
let config = new Config('test');
36-
return config.database.adapter.ensureUniqueness('UniquePointer', ['ptr'], { fields: { unique: { type: 'String' } } })
36+
return config.database.adapter.ensureUniqueness('UniquePointer', ['ptr'], { fields: {
37+
string: { __type: 'String' },
38+
ptr: { __type: 'Pointer', targetClass: 'UniquePointer' }
39+
} });
3740
})
3841
.then(() => {
3942
let newObj = new Parse.Object('UniquePointer')
@@ -58,7 +61,7 @@ describe('Uniqueness', function() {
5861
Parse.Object.saveAll([o1, o2])
5962
.then(() => {
6063
let config = new Config('test');
61-
return config.database.adapter.ensureUniqueness('UniqueFail', ['key'], { fields: { key: { type: 'String' } } });
64+
return config.database.adapter.ensureUniqueness('UniqueFail', ['key'], { fields: { key: { __type: 'String' } } });
6265
})
6366
.catch(error => {
6467
expect(error.code).toEqual(Parse.Error.DUPLICATE_VALUE);
@@ -68,7 +71,7 @@ describe('Uniqueness', function() {
6871

6972
it('can do compound uniqueness', done => {
7073
let config = new Config('test');
71-
config.database.adapter.ensureUniqueness('CompoundUnique', ['k1', 'k2'], { fields: { k1: { type: 'String' }, k2: { type: 'String' } } })
74+
config.database.adapter.ensureUniqueness('CompoundUnique', ['k1', 'k2'], { fields: { k1: { __type: 'String' }, k2: { __type: 'String' } } })
7275
.then(() => {
7376
let o1 = new Parse.Object('CompoundUnique');
7477
o1.set('k1', 'v1');
@@ -98,12 +101,4 @@ describe('Uniqueness', function() {
98101
done();
99102
});
100103
});
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-
});
109104
});

‎src/Adapters/Storage/Mongo/MongoCollection.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ export default class MongoCollection {
8383
return this._mongoCollection.deleteMany(query);
8484
}
8585

86+
_ensureSparseUniqueIndexInBackground(indexRequest) {
87+
return new Promise((resolve, reject) => {
88+
this._mongoCollection.ensureIndex(indexRequest, { unique: true, background: true, sparse: true }, (error, indexName) => {
89+
if (error) {
90+
reject(error);
91+
}
92+
resolve();
93+
});
94+
});
95+
}
96+
8697
drop() {
8798
return this._mongoCollection.drop();
8899
}

‎src/Adapters/Storage/Mongo/MongoSchemaCollection.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class MongoSchemaCollection {
139139
if (results.length === 1) {
140140
return mongoSchemaToParseSchema(results[0]);
141141
} else {
142-
return Promise.reject();
142+
throw undefined;
143143
}
144144
});
145145
}
@@ -166,9 +166,9 @@ class MongoSchemaCollection {
166166
.then(result => mongoSchemaToParseSchema(result.ops[0]))
167167
.catch(error => {
168168
if (error.code === 11000) { //Mongo's duplicate key error
169-
return Promise.reject();
169+
throw undefined;
170170
}
171-
return Promise.reject(error);
171+
throw error;
172172
});
173173
}
174174

@@ -198,17 +198,17 @@ class MongoSchemaCollection {
198198
if (type.type === 'GeoPoint') {
199199
// Make sure there are not other geopoint fields
200200
if (Object.keys(schema.fields).some(existingField => schema.fields[existingField].type === 'GeoPoint')) {
201-
return Promise.reject(new Parse.Error(Parse.Error.INCORRECT_TYPE, 'MongoDB only supports one GeoPoint field in a class.'));
201+
throw new Parse.Error(Parse.Error.INCORRECT_TYPE, 'MongoDB only supports one GeoPoint field in a class.');
202202
}
203203
}
204-
return Promise.resolve();
204+
return;
205205
}, error => {
206206
// If error is undefined, the schema doesn't exist, and we can create the schema with the field.
207207
// If some other error, reject with it.
208208
if (error === undefined) {
209-
return Promise.resolve();
209+
return;
210210
}
211-
throw Promise.reject(error);
211+
throw error;
212212
})
213213
.then(() => {
214214
// We use $exists and $set to avoid overwriting the field type if it

‎src/Adapters/Storage/Mongo/MongoStorageAdapter.js

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ export class MongoStorageAdapter {
102102
.catch(error => {
103103
// 'ns not found' means collection was already gone. Ignore deletion attempt.
104104
if (error.message == 'ns not found') {
105-
return Promise.resolve();
105+
return;
106106
}
107-
return Promise.reject(error);
107+
throw error;
108108
});
109109
}
110110

@@ -180,7 +180,7 @@ export class MongoStorageAdapter {
180180
throw new Parse.Error(Parse.Error.DUPLICATE_VALUE,
181181
'A duplicate value for a field with unique values was provided');
182182
}
183-
return Promise.reject(error);
183+
throw error;
184184
});
185185
}
186186

@@ -239,27 +239,22 @@ export class MongoStorageAdapter {
239239
// Create a unique index. Unique indexes on nullable fields are not allowed. Since we don't
240240
// currently know which fields are nullable and which aren't, we ignore that criteria.
241241
// As such, we shouldn't expose this function to users of parse until we have an out-of-band
242-
// Way of determining if a field is nullable.
242+
// Way of determining if a field is nullable. Undefined doesn't count against uniqueness,
243+
// which is why we use sparse indexes.
243244
ensureUniqueness(className, fieldNames, schema) {
244245
let indexCreationRequest = {};
245-
fieldNames.map(fieldName => transformKey(className, fieldName, schema)).forEach(transformedName => {
246-
indexCreationRequest[transformedName] = 1;
246+
let mongoFieldNames = fieldNames.map(fieldName => transformKey(className, fieldName, schema));
247+
mongoFieldNames.forEach(fieldName => {
248+
indexCreationRequest[fieldName] = 1;
247249
});
248250
return this.adaptiveCollection(className)
249-
.then(collection => {
250-
return new Promise((resolve, reject) => {
251-
collection._mongoCollection.ensureIndex(indexCreationRequest, { unique: true, background: true }, (err, indexName) => {
252-
if (err) {
253-
if (err.code === 11000) {
254-
reject(new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'Tried to force field uniqueness for a class that already has duplicates.'));
255-
} else {
256-
reject(err);
257-
}
258-
} else {
259-
resolve();
260-
}
261-
});
262-
})
251+
.then(collection => collection._ensureSparseUniqueIndexInBackground(indexCreationRequest))
252+
.catch(error => {
253+
if (error.code === 11000) {
254+
throw new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'Tried to ensure field uniqueness for a class that already has duplicates.');
255+
} else {
256+
throw error;
257+
}
263258
});
264259
}
265260

‎src/Controllers/DatabaseController.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ DatabaseController.prototype.validateClassName = function(className) {
8787
return Promise.resolve();
8888
}
8989
if (!SchemaController.classNameIsValid(className)) {
90-
const error = new Parse.Error(Parse.Error.INVALID_CLASS_NAME, 'invalid className: ' + className);
91-
return Promise.reject(error);
90+
return Promise.reject(new Parse.Error(Parse.Error.INVALID_CLASS_NAME, 'invalid className: ' + className));
9291
}
9392
return Promise.resolve();
9493
};

‎src/Controllers/UserController.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ export class UserController extends AdaptableController {
4343
if (!this.shouldVerifyEmails) {
4444
// Trying to verify email when not enabled
4545
// TODO: Better error here.
46-
return Promise.reject();
46+
throw undefined;
4747
}
4848
let database = this.config.database.WithoutValidation();
4949
return database.update('_User', {
5050
username: username,
5151
_email_verify_token: token
5252
}, {emailVerified: true}).then(document => {
5353
if (!document) {
54-
return Promise.reject();
54+
throw undefined;
5555
}
5656
return Promise.resolve(document);
5757
});
@@ -64,7 +64,7 @@ export class UserController extends AdaptableController {
6464
_perishable_token: token
6565
}, {limit: 1}).then(results => {
6666
if (results.length != 1) {
67-
return Promise.reject();
67+
throw undefined;
6868
}
6969
return results[0];
7070
});
@@ -85,7 +85,7 @@ export class UserController extends AdaptableController {
8585
var query = new RestQuery(this.config, Auth.master(this.config), '_User', where);
8686
return query.execute().then(function(result){
8787
if (result.results.length != 1) {
88-
return Promise.reject();
88+
throw undefined;
8989
}
9090
return result.results[0];
9191
})

‎src/RestWrite.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ RestWrite.prototype.getUserAndRoleACL = function() {
107107
return this.auth.getUserRoles().then((roles) => {
108108
roles.push(this.auth.user.id);
109109
this.runOptions.acl = this.runOptions.acl.concat(roles);
110-
return Promise.resolve();
110+
return;
111111
});
112-
}else{
112+
} else {
113113
return Promise.resolve();
114114
}
115115
};
@@ -121,7 +121,7 @@ RestWrite.prototype.validateClientClassCreation = function() {
121121
&& sysClass.indexOf(this.className) === -1) {
122122
return this.config.database.collectionExists(this.className).then((hasClass) => {
123123
if (hasClass === true) {
124-
return Promise.resolve();
124+
return;
125125
}
126126

127127
throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN,
@@ -311,7 +311,7 @@ RestWrite.prototype.handleAuthData = function(authData) {
311311
}
312312
}
313313
}
314-
return Promise.resolve();
314+
return;
315315
});
316316
}
317317

@@ -595,7 +595,7 @@ RestWrite.prototype.handleInstallation = function() {
595595
'deviceType may not be changed in this ' +
596596
'operation');
597597
}
598-
return Promise.resolve();
598+
return;
599599
});
600600
});
601601
}
@@ -807,7 +807,7 @@ RestWrite.prototype.runDatabaseOperation = function() {
807807
if (results.length > 0) {
808808
throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
809809
}
810-
throw error;
810+
throw new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'A duplicate value for a field with unique values was provided');
811811
});
812812
})
813813
.then(response => {

‎src/rest.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ function del(config, auth, className, objectId) {
6363
}).then(() => {
6464
if (!auth.isMaster) {
6565
return auth.getUserRoles();
66-
}else{
67-
return Promise.resolve();
66+
} else {
67+
return;
6868
}
6969
}).then(() => {
7070
var options = {};
@@ -81,7 +81,7 @@ function del(config, auth, className, objectId) {
8181
}, options);
8282
}).then(() => {
8383
triggers.maybeRunTrigger(triggers.Types.afterDelete, auth, inflatedObject, null, config);
84-
return Promise.resolve();
84+
return;
8585
});
8686
}
8787

0 commit comments

Comments
 (0)