Skip to content

Commit a8ffae9

Browse files
nlutsenkonlutsenko
committed
Remove direct mongo access from SchemaRouter.modify, Schema.deleteField.
1 parent 91e8506 commit a8ffae9

File tree

4 files changed

+95
-155
lines changed

4 files changed

+95
-155
lines changed

‎spec/Schema.spec.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ describe('Schema', () => {
483483
.then(schema => schema.deleteField('installationId', '_Installation'))
484484
.catch(error => {
485485
expect(error.code).toEqual(136);
486-
expect(error.error).toEqual('field installationId cannot be changed');
486+
expect(error.message).toEqual('field installationId cannot be changed');
487487
done();
488488
});
489489
});
@@ -493,7 +493,7 @@ describe('Schema', () => {
493493
.then(schema => schema.deleteField('field', 'NoClass'))
494494
.catch(error => {
495495
expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
496-
expect(error.error).toEqual('class NoClass does not exist');
496+
expect(error.message).toEqual('Class NoClass does not exist.');
497497
done();
498498
});
499499
});
@@ -504,7 +504,7 @@ describe('Schema', () => {
504504
.then(schema => schema.deleteField('missingField', 'HasAllPOD'))
505505
.fail(error => {
506506
expect(error.code).toEqual(255);
507-
expect(error.error).toEqual('field missingField does not exist, cannot delete');
507+
expect(error.message).toEqual('Field missingField does not exist, cannot delete.');
508508
done();
509509
});
510510
});
@@ -523,11 +523,11 @@ describe('Schema', () => {
523523
config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
524524
expect(err).toEqual(null);
525525
config.database.loadSchema()
526-
.then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database.adapter.database, 'test_'))
526+
.then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database))
527527
.then(() => config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
528528
expect(err).not.toEqual(null);
529529
done();
530-
}))
530+
}));
531531
});
532532
})
533533
});
@@ -538,7 +538,7 @@ describe('Schema', () => {
538538
var obj2 = hasAllPODobject();
539539
var p = Parse.Object.saveAll([obj1, obj2])
540540
.then(() => config.database.loadSchema())
541-
.then(schema => schema.deleteField('aString', 'HasAllPOD', config.database.adapter.database, 'test_'))
541+
.then(schema => schema.deleteField('aString', 'HasAllPOD', config.database))
542542
.then(() => new Parse.Query('HasAllPOD').get(obj1.id))
543543
.then(obj1Reloaded => {
544544
expect(obj1Reloaded.get('aString')).toEqual(undefined);
@@ -568,7 +568,7 @@ describe('Schema', () => {
568568
expect(obj1.get('aPointer').id).toEqual(obj1.id);
569569
})
570570
.then(() => config.database.loadSchema())
571-
.then(schema => schema.deleteField('aPointer', 'NewClass', config.database.adapter.database, 'test_'))
571+
.then(schema => schema.deleteField('aPointer', 'NewClass', config.database))
572572
.then(() => new Parse.Query('NewClass').get(obj1.id))
573573
.then(obj1 => {
574574
expect(obj1.get('aPointer')).toEqual(undefined);

‎spec/schemas.spec.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ describe('schemas', () => {
369369
}, (error, response, body) => {
370370
expect(response.statusCode).toEqual(400);
371371
expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
372-
expect(body.error).toEqual('class NoClass does not exist');
372+
expect(body.error).toEqual('Class NoClass does not exist.');
373373
done();
374374
});
375375
});
@@ -390,7 +390,7 @@ describe('schemas', () => {
390390
}, (error, response, body) => {
391391
expect(response.statusCode).toEqual(400);
392392
expect(body.code).toEqual(255);
393-
expect(body.error).toEqual('field aString exists, cannot update');
393+
expect(body.error).toEqual('Field aString exists, cannot update.');
394394
done();
395395
});
396396
})
@@ -412,7 +412,7 @@ describe('schemas', () => {
412412
}, (error, response, body) => {
413413
expect(response.statusCode).toEqual(400);
414414
expect(body.code).toEqual(255);
415-
expect(body.error).toEqual('field nonExistentKey does not exist, cannot delete');
415+
expect(body.error).toEqual('Field nonExistentKey does not exist, cannot delete.');
416416
done();
417417
});
418418
});

‎src/Routers/SchemasRouter.js

Lines changed: 55 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// schemas.js
22

33
var express = require('express'),
4-
Parse = require('parse/node').Parse,
5-
Schema = require('../Schema');
4+
Parse = require('parse/node').Parse,
5+
Schema = require('../Schema');
66

77
import PromiseRouter from '../PromiseRouter';
88

@@ -49,26 +49,26 @@ function getAllSchemas(req) {
4949
return masterKeyRequiredResponse();
5050
}
5151
return req.config.database.collection('_SCHEMA')
52-
.then(coll => coll.find({}).toArray())
53-
.then(schemas => ({response: {
54-
results: schemas.map(mongoSchemaToSchemaAPIResponse)
55-
}}));
52+
.then(coll => coll.find({}).toArray())
53+
.then(schemas => ({response: {
54+
results: schemas.map(mongoSchemaToSchemaAPIResponse)
55+
}}));
5656
}
5757

5858
function getOneSchema(req) {
5959
if (!req.auth.isMaster) {
6060
return masterKeyRequiredResponse();
6161
}
6262
return req.config.database.collection('_SCHEMA')
63-
.then(coll => coll.findOne({'_id': req.params.className}))
64-
.then(schema => ({response: mongoSchemaToSchemaAPIResponse(schema)}))
65-
.catch(() => ({
66-
status: 400,
67-
response: {
68-
code: 103,
69-
error: 'class ' + req.params.className + ' does not exist',
70-
}
71-
}));
63+
.then(coll => coll.findOne({'_id': req.params.className}))
64+
.then(schema => ({response: mongoSchemaToSchemaAPIResponse(schema)}))
65+
.catch(() => ({
66+
status: 400,
67+
response: {
68+
code: 103,
69+
error: 'class ' + req.params.className + ' does not exist',
70+
}
71+
}));
7272
}
7373

7474
function createSchema(req) {
@@ -91,12 +91,12 @@ function createSchema(req) {
9191
});
9292
}
9393
return req.config.database.loadSchema()
94-
.then(schema => schema.addClassIfNotExists(className, req.body.fields))
95-
.then(result => ({ response: mongoSchemaToSchemaAPIResponse(result) }))
96-
.catch(error => ({
97-
status: 400,
98-
response: error,
99-
}));
94+
.then(schema => schema.addClassIfNotExists(className, req.body.fields))
95+
.then(result => ({ response: mongoSchemaToSchemaAPIResponse(result) }))
96+
.catch(error => ({
97+
status: 400,
98+
response: error,
99+
}));
100100
}
101101

102102
function modifySchema(req) {
@@ -112,75 +112,48 @@ function modifySchema(req) {
112112
var className = req.params.className;
113113

114114
return req.config.database.loadSchema()
115-
.then(schema => {
116-
if (!schema.data[className]) {
117-
return Promise.resolve({
118-
status: 400,
119-
response: {
120-
code: Parse.Error.INVALID_CLASS_NAME,
121-
error: 'class ' + req.params.className + ' does not exist',
115+
.then(schema => {
116+
if (!schema.data[className]) {
117+
throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${req.params.className} does not exist.`);
118+
}
119+
120+
let existingFields = schema.data[className];
121+
Object.keys(submittedFields).forEach(name => {
122+
let field = submittedFields[name];
123+
if (existingFields[name] && field.__op !== 'Delete') {
124+
throw new Parse.Error(255, `Field ${name} exists, cannot update.`);
125+
}
126+
if (!existingFields[name] && field.__op === 'Delete') {
127+
throw new Parse.Error(255, `Field ${name} does not exist, cannot delete.`);
122128
}
123129
});
124-
}
125-
var existingFields = schema.data[className];
126-
127-
for (var submittedFieldName in submittedFields) {
128-
if (existingFields[submittedFieldName] && submittedFields[submittedFieldName].__op !== 'Delete') {
129-
return Promise.resolve({
130-
status: 400,
131-
response: {
132-
code: 255,
133-
error: 'field ' + submittedFieldName + ' exists, cannot update',
134-
}
135-
});
136-
}
137130

138-
if (!existingFields[submittedFieldName] && submittedFields[submittedFieldName].__op === 'Delete') {
139-
return Promise.resolve({
140-
status: 400,
141-
response: {
142-
code: 255,
143-
error: 'field ' + submittedFieldName + ' does not exist, cannot delete',
144-
}
145-
});
131+
let newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields);
132+
let mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className);
133+
if (!mongoObject.result) {
134+
throw new Parse.Error(mongoObject.code, mongoObject.error);
146135
}
147-
}
148136

149-
var newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields);
150-
var mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className);
151-
if (!mongoObject.result) {
152-
return Promise.resolve({
153-
status: 400,
154-
response: mongoObject,
137+
// Finally we have checked to make sure the request is valid and we can start deleting fields.
138+
// Do all deletions first, then a single save to _SCHEMA collection to handle all additions.
139+
let deletionPromises = [];
140+
Object.keys(submittedFields).forEach(submittedFieldName => {
141+
if (submittedFields[submittedFieldName].__op === 'Delete') {
142+
let promise = schema.deleteField(submittedFieldName, className, req.config.database);
143+
deletionPromises.push(promise);
144+
}
155145
});
156-
}
157146

158-
// Finally we have checked to make sure the request is valid and we can start deleting fields.
159-
// Do all deletions first, then a single save to _SCHEMA collection to handle all additions.
160-
var deletionPromises = []
161-
Object.keys(submittedFields).forEach(submittedFieldName => {
162-
if (submittedFields[submittedFieldName].__op === 'Delete') {
163-
var promise = req.config.database.connect()
164-
.then(() => schema.deleteField(
165-
submittedFieldName,
166-
className,
167-
req.config.database.adapter.database,
168-
req.config.database.collectionPrefix
169-
));
170-
deletionPromises.push(promise);
171-
}
147+
return Promise.all(deletionPromises)
148+
.then(() => new Promise((resolve, reject) => {
149+
schema.collection.update({_id: className}, mongoObject.result, {w: 1}, (err, docs) => {
150+
if (err) {
151+
reject(err);
152+
}
153+
resolve({ response: mongoSchemaToSchemaAPIResponse(mongoObject.result)});
154+
})
155+
}));
172156
});
173-
174-
return Promise.all(deletionPromises)
175-
.then(() => new Promise((resolve, reject) => {
176-
schema.collection.update({_id: className}, mongoObject.result, {w: 1}, (err, docs) => {
177-
if (err) {
178-
reject(err);
179-
}
180-
resolve({ response: mongoSchemaToSchemaAPIResponse(mongoObject.result)});
181-
})
182-
}));
183-
});
184157
}
185158

186159
// A helper function that removes all join tables for a schema. Returns a promise.

‎src/Schema.js

Lines changed: 30 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -500,80 +500,47 @@ Schema.prototype.validateField = function(className, key, type, freeze) {
500500

501501
// Passing the database and prefix is necessary in order to drop relation collections
502502
// and remove fields from objects. Ideally the database would belong to
503-
// a database adapter and this fuction would close over it or access it via member.
504-
Schema.prototype.deleteField = function(fieldName, className, database, prefix) {
503+
// a database adapter and this function would close over it or access it via member.
504+
Schema.prototype.deleteField = function(fieldName, className, database) {
505505
if (!classNameIsValid(className)) {
506-
return Promise.reject({
507-
code: Parse.Error.INVALID_CLASS_NAME,
508-
error: invalidClassNameMessage(className),
509-
});
506+
throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, invalidClassNameMessage(className));
510507
}
511-
512508
if (!fieldNameIsValid(fieldName)) {
513-
return Promise.reject({
514-
code: Parse.Error.INVALID_KEY_NAME,
515-
error: 'invalid field name: ' + fieldName,
516-
});
509+
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `invalid field name: ${fieldName}`);
517510
}
518-
519511
//Don't allow deleting the default fields.
520512
if (!fieldNameIsValidForClass(fieldName, className)) {
521-
return Promise.reject({
522-
code: 136,
523-
error: 'field ' + fieldName + ' cannot be changed',
524-
});
513+
throw new Parse.Error(136, `field ${fieldName} cannot be changed`);
525514
}
526515

527516
return this.reload()
528-
.then(schema => {
529-
return schema.hasClass(className)
530-
.then(hasClass => {
531-
if (!hasClass) {
532-
return Promise.reject({
533-
code: Parse.Error.INVALID_CLASS_NAME,
534-
error: 'class ' + className + ' does not exist',
535-
});
536-
}
537-
538-
if (!schema.data[className][fieldName]) {
539-
return Promise.reject({
540-
code: 255,
541-
error: 'field ' + fieldName + ' does not exist, cannot delete',
542-
});
543-
}
544-
545-
if (schema.data[className][fieldName].startsWith('relation<')) {
546-
//For relations, drop the _Join table
547-
return database.dropCollection(prefix + '_Join:' + fieldName + ':' + className)
548-
//Save the _SCHEMA object
517+
.then(schema => {
518+
return schema.hasClass(className)
519+
.then(hasClass => {
520+
if (!hasClass) {
521+
throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`);
522+
}
523+
if (!schema.data[className][fieldName]) {
524+
throw new Parse.Error(255, `Field ${fieldName} does not exist, cannot delete.`);
525+
}
526+
527+
if (schema.data[className][fieldName].startsWith('relation<')) {
528+
//For relations, drop the _Join table
529+
return database.dropCollection(`_Join:${fieldName}:${className}`);
530+
}
531+
532+
// for non-relations, remove all the data.
533+
// This is necessary to ensure that the data is still gone if they add the same field.
534+
return database.collection(className)
535+
.then(collection => {
536+
var mongoFieldName = schema.data[className][fieldName].startsWith('*') ? '_p_' + fieldName : fieldName;
537+
return collection.update({}, { "$unset": { [mongoFieldName] : null } }, { multi: true });
538+
});
539+
})
540+
// Save the _SCHEMA object
549541
.then(() => this.collection.update({ _id: className }, { $unset: {[fieldName]: null }}));
550-
} else {
551-
//for non-relations, remove all the data. This is necessary to ensure that the data is still gone
552-
//if they add the same field.
553-
return new Promise((resolve, reject) => {
554-
database.collection(prefix + className, (err, coll) => {
555-
if (err) {
556-
reject(err);
557-
} else {
558-
var mongoFieldName = schema.data[className][fieldName].startsWith('*') ?
559-
'_p_' + fieldName :
560-
fieldName;
561-
return coll.update({}, {
562-
"$unset": { [mongoFieldName] : null },
563-
}, {
564-
multi: true,
565-
})
566-
//Save the _SCHEMA object
567-
.then(() => this.collection.update({ _id: className }, { $unset: {[fieldName]: null }}))
568-
.then(resolve)
569-
.catch(reject);
570-
}
571-
});
572-
});
573-
}
574542
});
575-
});
576-
}
543+
};
577544

578545
// Given a schema promise, construct another schema promise that
579546
// validates this field once the schema loads.

0 commit comments

Comments
 (0)