Cleanup delete schema (#1604)

* Some cleanup for deleting one schema

* tidyness

* Remove _allCollections as Parse Server doesn't need it.

Author: drew-gross

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;
@@ -70,7 +85,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,18 +99,10 @@ export class MongoStorageAdapter {
     });
   }
 
-  // Used for testing only right now.
-  allCollections() {
-    return this.connect().then(() => {
-      return this.database.collections();
-    }).then(collections => {
-      return collections.filter(collection => {
-        if (collection.namespace.match(/\.system\./)) {
-          return false;
-        }
-        return (collection.collectionName.indexOf(this._collectionPrefix) == 0);
-      });
-    });
+  // Delete all data known to this adatper. Used for testing.
+  deleteAllSchemas() {
+    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

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 = []) {

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,15 +632,15 @@ class SchemaController {
         found = true;
       }
     }
-     
+
     if (found) {
       return Promise.resolve();
     }
 
     // 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,

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);
     })
   );
 };
@@ -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 {