From d4cf5994453f752a19e858d5439980d64382a39d Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 22 Apr 2016 16:50:13 -0700 Subject: [PATCH 1/3] Some cleanup for deleting one schema --- .../Storage/Mongo/MongoStorageAdapter.js | 24 +++++++++++---- src/Controllers/DatabaseController.js | 30 ++++++++----------- src/Controllers/SchemaController.js | 8 ++--- src/Routers/SchemasRouter.js | 2 +- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index c83200be52..1ad34998a7 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -70,7 +70,10 @@ export class MongoStorageAdapter { }); } - dropCollection(className: string) { + // Deletes a schema. Resolve if successful. If the schema doesn't + // exist, resolve with undefined. If schema exists, but can't be deleted for some other reason, + // reject with INTERNAL_SERVER_ERROR. + deleteOneSchema(className: string) { return this.collection(this._collectionPrefix + className).then(collection => collection.drop()) .catch(error => { // 'ns not found' means collection was already gone. Ignore deletion attempt. @@ -81,15 +84,24 @@ export class MongoStorageAdapter { }); } - // Used for testing only right now. - allCollections() { - return this.connect().then(() => { - return this.database.collections(); - }).then(collections => { + // Delete all data known to this adatper. Used for testing. + deleteAllSchemas() { + return this._allCollections().then(collections => { + let promises = collections.map(collection => collection.drop()); + return Promise.all(promises); + }); + } + + _allCollections() { + return this.connect() + .then(() => this.database.collections()) + .then(collections => { return collections.filter(collection => { if (collection.namespace.match(/\.system\./)) { return false; } + //TODO: If you have one app with a collection prefix that happens to be a prefix of another + // apps prefix, this will go very very badly. We should fix that somehow. return (collection.collectionName.indexOf(this._collectionPrefix) == 0); }); }); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 926b1b1717..fca0282967 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -381,11 +381,7 @@ DatabaseController.prototype.mongoFind = function(className, query, options = {} // Returns a promise. DatabaseController.prototype.deleteEverything = function() { this.schemaPromise = null; - - return this.adapter.allCollections().then(collections => { - let promises = collections.map(collection => collection.drop()); - return Promise.all(promises); - }); + return this.adapter.deleteAllSchemas(); }; // Finds the keys in a query. Returns a Set. REST format only @@ -652,21 +648,19 @@ DatabaseController.prototype.find = function(className, query, { DatabaseController.prototype.deleteSchema = function(className) { return this.collectionExists(className) - .then(exist => { - if (!exist) { - return Promise.resolve(); + .then(exist => { + if (!exist) { + return Promise.resolve(); + } + return this.adapter.adaptiveCollection(className) + .then(collection => collection.count()) + .then(count => { + if (count > 0) { + throw new Parse.Error(255, `Class ${className} is not empty, contains ${count} objects, cannot drop schema.`); } - return this.adapter.adaptiveCollection(className) - .then(collection => { - return collection.count() - .then(count => { - if (count > 0) { - throw new Parse.Error(255, `Class ${className} is not empty, contains ${count} objects, cannot drop schema.`); - } - return collection.drop(); - }) - }) + return this.adapter.deleteOneSchema(className); }) + }); } DatabaseController.prototype.addPointerPermissions = function(schema, className, operation, query, aclGroup = []) { diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index cf6b898a3e..1c993d724e 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -134,7 +134,7 @@ function validateCLP(perms, fields) { } return; } - + Object.keys(perms[operation]).forEach((key) => { verifyPermissionKey(key); let perm = perms[operation][key]; @@ -543,7 +543,7 @@ class SchemaController { if (this.data[className][fieldName].type == 'Relation') { //For relations, drop the _Join table return database.adapter.deleteFields(className, [fieldName], []) - .then(() => database.adapter.dropCollection(`_Join:${fieldName}:${className}`)); + .then(() => database.adapter.deleteOneSchema(`_Join:${fieldName}:${className}`)); } const fieldNames = [fieldName]; @@ -632,7 +632,7 @@ class SchemaController { found = true; } } - + if (found) { return Promise.resolve(); } @@ -640,7 +640,7 @@ class SchemaController { // No matching CLP, let's check the Pointer permissions // And handle those later let permissionField = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields'; - + // Reject create when write lockdown if (permissionField == 'writeUserFields' && operation == 'create') { throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index e7b8cfcd68..8ed14a839a 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -70,7 +70,7 @@ var removeJoinTables = (database, mongoSchema) => { .filter(field => mongoSchema[field].startsWith('relation<')) .map(field => { let collectionName = `_Join:${field}:${mongoSchema._id}`; - return database.adapter.dropCollection(collectionName); + return database.adapter.deleteOneSchema(collectionName); }) ); }; From 1786bb4841eb4684f0ed1e7dfa52e4ff100edd37 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 22 Apr 2016 17:01:00 -0700 Subject: [PATCH 2/3] tidyness --- src/Routers/SchemasRouter.js | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index 8ed14a839a..75891fd6a7 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -79,22 +79,19 @@ function deleteSchema(req) { if (!SchemaController.classNameIsValid(req.params.className)) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, SchemaController.invalidClassNameMessage(req.params.className)); } - return req.config.database.deleteSchema(req.params.className) - .then(() => { - // We've dropped the collection now, so delete the item from _SCHEMA - // and clear the _Join collections - return req.config.database.schemaCollection() - .then(coll => coll.findAndDeleteSchema(req.params.className)) - .then(document => { - if (document === null) { - //tried to delete non-existent class - return Promise.resolve(); - } - return removeJoinTables(req.config.database, document); - }); - }) - .then(() => ({ response: {} })); + .then(() => req.config.database.schemaCollection()) + // We've dropped the collection now, so delete the item from _SCHEMA + // and clear the _Join collections + .then(coll => coll.findAndDeleteSchema(req.params.className)) + .then(document => { + if (document === null) { + //tried to delete non-existent class + return Promise.resolve(); + } + return removeJoinTables(req.config.database, document); + }) + .then(() => ({ response: {} })); } export class SchemasRouter extends PromiseRouter { From 558c23739d203860b15c72ae822b15f293e7808f Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 25 Apr 2016 10:47:59 -0700 Subject: [PATCH 3/3] Remove _allCollections as Parse Server doesn't need it. --- .../Storage/Mongo/MongoStorageAdapter.js | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 1ad34998a7..803b3c63b7 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -10,6 +10,21 @@ let MongoClient = mongodb.MongoClient; const MongoSchemaCollectionName = '_SCHEMA'; const DefaultMongoURI = 'mongodb://localhost:27017/parse'; +const storageAdapterAllCollections = mongoAdapter => { + return mongoAdapter.connect() + .then(() => mongoAdapter.database.collections()) + .then(collections => { + return collections.filter(collection => { + if (collection.namespace.match(/\.system\./)) { + return false; + } + // TODO: If you have one app with a collection prefix that happens to be a prefix of another + // apps prefix, this will go very very badly. We should fix that somehow. + return (collection.collectionName.indexOf(mongoAdapter._collectionPrefix) == 0); + }); + }); +} + export class MongoStorageAdapter { // Private _uri: string; @@ -86,25 +101,8 @@ export class MongoStorageAdapter { // Delete all data known to this adatper. Used for testing. deleteAllSchemas() { - return this._allCollections().then(collections => { - let promises = collections.map(collection => collection.drop()); - return Promise.all(promises); - }); - } - - _allCollections() { - return this.connect() - .then(() => this.database.collections()) - .then(collections => { - return collections.filter(collection => { - if (collection.namespace.match(/\.system\./)) { - return false; - } - //TODO: If you have one app with a collection prefix that happens to be a prefix of another - // apps prefix, this will go very very badly. We should fix that somehow. - return (collection.collectionName.indexOf(this._collectionPrefix) == 0); - }); - }); + return storageAdapterAllCollections(this) + .then(collections => Promise.all(collections.map(collection => collection.drop()))); } // Remove the column and all the data. For Relations, the _Join collection is handled