From 028ef2a7b2e7e5ba35e5a3eb59d3f94402f34c86 Mon Sep 17 00:00:00 2001 From: Nikita Lutsenko Date: Mon, 29 Feb 2016 17:04:38 -0800 Subject: [PATCH] Remove dependency on raw mongo from SchemaRouter.delete. --- spec/schemas.spec.js | 9 +- .../Storage/Mongo/MongoStorageAdapter.js | 7 ++ src/Controllers/DatabaseController.js | 4 + src/Routers/SchemasRouter.js | 96 +++++++------------ 4 files changed, 50 insertions(+), 66 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 36ba7637..beb09d08 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -396,7 +396,7 @@ describe('schemas', () => { }) }); - it('refuses to delete non-existant fields', done => { + it('refuses to delete non-existent fields', done => { var obj = hasAllPODobject(); obj.save() .then(() => { @@ -406,13 +406,13 @@ describe('schemas', () => { json: true, body: { fields: { - nonExistantKey: {__op: "Delete"}, + nonExistentKey: {__op: "Delete"}, } } }, (error, response, body) => { expect(response.statusCode).toEqual(400); expect(body.code).toEqual(255); - expect(body.error).toEqual('field nonExistantKey does not exist, cannot delete'); + expect(body.error).toEqual('field nonExistentKey does not exist, cannot delete'); done(); }); }); @@ -660,7 +660,8 @@ describe('schemas', () => { }, (error, response, body) => { expect(response.statusCode).toEqual(400); expect(body.code).toEqual(255); - expect(body.error).toEqual('class HasAllPOD not empty, contains 1 objects, cannot drop schema'); + expect(body.error).toMatch(/HasAllPOD/); + expect(body.error).toMatch(/contains 1/); done(); }); }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 914d9bb0..86f56ea9 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -30,6 +30,13 @@ export class MongoStorageAdapter { }); } + dropCollection(name: string) { + return this.connect.then(() => this.collection(name).then(collection => collection.drop())); + } + + dropCollection(name: string) { + return this.collection(name).then(collection => collection.drop()); + } // Used for testing only right now. collectionsContaining(match: string) { return this.connect().then(() => { diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 954b1a6f..96c33931 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -42,6 +42,10 @@ DatabaseController.prototype.rawCollection = function(className) { return this.adapter.collection(this.collectionPrefix + className); }; +DatabaseController.prototype.dropCollection = function(className) { + return this.adapter.dropCollection(this.collectionPrefix + className); +}; + function returnsTrue() { return true; } diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index a748ad14..c496be34 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -184,20 +184,12 @@ function modifySchema(req) { } // A helper function that removes all join tables for a schema. Returns a promise. -var removeJoinTables = (database, prefix, mongoSchema) => { +var removeJoinTables = (database, mongoSchema) => { return Promise.all(Object.keys(mongoSchema) .filter(field => mongoSchema[field].startsWith('relation<')) .map(field => { - var joinCollectionName = prefix + '_Join:' + field + ':' + mongoSchema._id; - return new Promise((resolve, reject) => { - database.dropCollection(joinCollectionName, (err, results) => { - if (err) { - reject(err); - } else { - resolve(); - } - }) - }); + let collectionName = `_Join:${field}:${mongoSchema._id}`; + return database.dropCollection(collectionName); }) ); }; @@ -208,63 +200,43 @@ function deleteSchema(req) { } if (!Schema.classNameIsValid(req.params.className)) { - return Promise.resolve({ - status: 400, - response: { - code: Parse.Error.INVALID_CLASS_NAME, - error: Schema.invalidClassNameMessage(req.params.className), - } - }); + throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, Schema.invalidClassNameMessage(req.params.className)); } return req.config.database.collection(req.params.className) - .then(coll => new Promise((resolve, reject) => { - coll.count((err, count) => { - if (err) { - reject(err); - } else if (count > 0) { - resolve({ - status: 400, - response: { - code: 255, - error: 'class ' + req.params.className + ' not empty, contains ' + count + ' objects, cannot drop schema', + .then(collection => { + return collection.count() + .then(count => { + if (count > 0) { + throw new Parse.Error(255, `Class ${req.params.className} is not empty, contains ${count} objects, cannot drop schema.`); } - }); - } else { - coll.drop((err, reply) => { - if (err) { - reject(err); - } else { - // We've dropped the collection now, so delete the item from _SCHEMA - // and clear the _Join collections - req.config.database.collection('_SCHEMA') - .then(coll => new Promise((resolve, reject) => { - coll.findAndRemove({ _id: req.params.className }, [], (err, doc) => { - if (err) { - reject(err); - } else if (doc.value === null) { - //tried to delete non-existant class - resolve({ response: {}}); - } else { - removeJoinTables(req.config.database.adapter.database, req.config.database.collectionPrefix, doc.value) - .then(resolve, reject); - } - }); - })) - .then(resolve.bind(undefined, {response: {}}), reject); - } - }); + return collection.drop(); + }) + .then(() => { + // We've dropped the collection now, so delete the item from _SCHEMA + // and clear the _Join collections + return req.config.database.collection('_SCHEMA') + .then(coll => coll.findAndRemove({_id: req.params.className}, [])) + .then(doc => { + if (doc.value === null) { + //tried to delete non-existent class + return Promise.resolve(); + } + return removeJoinTables(req.config.database, doc.value); + }); + }) + }) + .then(() => { + // Success + return { response: {} }; + }, error => { + if (error.message == 'ns not found') { + // If they try to delete a non-existent class, that's fine, just let them. + return { response: {} }; } + + return Promise.reject(error); }); - })) - .catch( (error) => { - if (error.message == 'ns not found') { - // If they try to delete a non-existant class, thats fine, just let them. - return Promise.resolve({ response: {} }); - } - - return Promise.reject(error); - }); } export class SchemasRouter extends PromiseRouter {