Merge pull request #729 from ParsePlatform/nlutsenko.decouple.schema

Decouple and remove direct mongo access from Schema/SchemaRouter.
2016-02-29 23:53:01 -08:00
parent 382149b190 d1dbc1a035
commit 0b990b671a
6 changed files with 200 additions and 250 deletions
--- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js
+++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js
@@ -30,6 +30,17 @@ export class MongoStorageAdapter {
    });
  }

+  collectionExists(name: string) {
+    return this.connect().then(() => {
+      return this.database.listCollections({ name: name }).toArray();
+    }).then(collections => {
+      return collections.length > 0;
+    });
+  }
+
+  dropCollection(name: string) {
+    return this.collection(name).then(collection => collection.drop());
+  }
  // Used for testing only right now.
  collectionsContaining(match: string) {
    return this.connect().then(() => {
--- a/src/Controllers/DatabaseController.js
+++ b/src/Controllers/DatabaseController.js
@@ -38,10 +38,18 @@ DatabaseController.prototype.collection = function(className) {
  return this.rawCollection(className);
 };

+DatabaseController.prototype.collectionExists = function(className) {
+  return this.adapter.collectionExists(this.collectionPrefix + className);
+};
+
 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;
 }
--- a/src/Routers/SchemasRouter.js
+++ b/src/Routers/SchemasRouter.js
@@ -1,8 +1,8 @@
 // schemas.js

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

 import PromiseRouter from '../PromiseRouter';

@@ -49,10 +49,10 @@ function getAllSchemas(req) {
    return masterKeyRequiredResponse();
  }
  return req.config.database.collection('_SCHEMA')
-  .then(coll => coll.find({}).toArray())
-  .then(schemas => ({response: {
-    results: schemas.map(mongoSchemaToSchemaAPIResponse)
-  }}));
+    .then(coll => coll.find({}).toArray())
+    .then(schemas => ({response: {
+      results: schemas.map(mongoSchemaToSchemaAPIResponse)
+    }}));
 }

 function getOneSchema(req) {
@@ -60,15 +60,15 @@ function getOneSchema(req) {
    return masterKeyRequiredResponse();
  }
  return req.config.database.collection('_SCHEMA')
-  .then(coll => coll.findOne({'_id': req.params.className}))
-  .then(schema => ({response: mongoSchemaToSchemaAPIResponse(schema)}))
-  .catch(() => ({
-    status: 400,
-    response: {
-      code: 103,
-      error: 'class ' + req.params.className + ' does not exist',
-    }
-  }));
+    .then(coll => coll.findOne({'_id': req.params.className}))
+    .then(schema => ({response: mongoSchemaToSchemaAPIResponse(schema)}))
+    .catch(() => ({
+      status: 400,
+      response: {
+        code: 103,
+        error: 'class ' + req.params.className + ' does not exist',
+      }
+    }));
 }

 function createSchema(req) {
@@ -91,12 +91,12 @@ function createSchema(req) {
    });
  }
  return req.config.database.loadSchema()
-  .then(schema => schema.addClassIfNotExists(className, req.body.fields))
-  .then(result => ({ response: mongoSchemaToSchemaAPIResponse(result) }))
-  .catch(error => ({
-    status: 400,
-    response: error,
-  }));
+    .then(schema => schema.addClassIfNotExists(className, req.body.fields))
+    .then(result => ({ response: mongoSchemaToSchemaAPIResponse(result) }))
+    .catch(error => ({
+      status: 400,
+      response: error,
+    }));
 }

 function modifySchema(req) {
@@ -112,92 +112,57 @@ function modifySchema(req) {
  var className = req.params.className;

  return req.config.database.loadSchema()
-  .then(schema => {
-    if (!schema.data[className]) {
-      return Promise.resolve({
-        status: 400,
-        response: {
-          code: Parse.Error.INVALID_CLASS_NAME,
-          error: 'class ' + req.params.className + ' does not exist',
+    .then(schema => {
+      if (!schema.data[className]) {
+        throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${req.params.className} does not exist.`);
+      }
+
+      let existingFields = schema.data[className];
+      Object.keys(submittedFields).forEach(name => {
+        let field = submittedFields[name];
+        if (existingFields[name] && field.__op !== 'Delete') {
+          throw new Parse.Error(255, `Field ${name} exists, cannot update.`);
+        }
+        if (!existingFields[name] && field.__op === 'Delete') {
+          throw new Parse.Error(255, `Field ${name} does not exist, cannot delete.`);
        }
      });
-    }
-    var existingFields = schema.data[className];

-    for (var submittedFieldName in submittedFields) {
-      if (existingFields[submittedFieldName] && submittedFields[submittedFieldName].__op !== 'Delete') {
-        return Promise.resolve({
-          status: 400,
-          response: {
-            code: 255,
-            error: 'field ' + submittedFieldName + ' exists, cannot update',
-          }
-        });
+      let newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields);
+      let mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className);
+      if (!mongoObject.result) {
+        throw new Parse.Error(mongoObject.code, mongoObject.error);
      }

-      if (!existingFields[submittedFieldName] && submittedFields[submittedFieldName].__op === 'Delete') {
-        return Promise.resolve({
-          status: 400,
-          response: {
-            code: 255,
-            error: 'field ' + submittedFieldName + ' does not exist, cannot delete',
-          }
-        });
-      }
-    }
-
-    var newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields);
-    var mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className);
-    if (!mongoObject.result) {
-      return Promise.resolve({
-        status: 400,
-        response: mongoObject,
+      // Finally we have checked to make sure the request is valid and we can start deleting fields.
+      // Do all deletions first, then a single save to _SCHEMA collection to handle all additions.
+      let deletionPromises = [];
+      Object.keys(submittedFields).forEach(submittedFieldName => {
+        if (submittedFields[submittedFieldName].__op === 'Delete') {
+          let promise = schema.deleteField(submittedFieldName, className, req.config.database);
+          deletionPromises.push(promise);
+        }
      });
-    }

-    // Finally we have checked to make sure the request is valid and we can start deleting fields.
-    // Do all deletions first, then a single save to _SCHEMA collection to handle all additions.
-    var deletionPromises = []
-    Object.keys(submittedFields).forEach(submittedFieldName => {
-      if (submittedFields[submittedFieldName].__op === 'Delete') {
-        var promise = req.config.database.connect()
-        .then(() => schema.deleteField(
-          submittedFieldName,
-          className,
-          req.config.database.adapter.database,
-          req.config.database.collectionPrefix
-        ));
-        deletionPromises.push(promise);
-      }
+      return Promise.all(deletionPromises)
+        .then(() => new Promise((resolve, reject) => {
+          schema.collection.update({_id: className}, mongoObject.result, {w: 1}, (err, docs) => {
+            if (err) {
+              reject(err);
+            }
+            resolve({ response: mongoSchemaToSchemaAPIResponse(mongoObject.result)});
+          })
+        }));
    });
-
-    return Promise.all(deletionPromises)
-    .then(() => new Promise((resolve, reject) => {
-      schema.collection.update({_id: className}, mongoObject.result, {w: 1}, (err, docs) => {
-        if (err) {
-          reject(err);
-        }
-        resolve({ response: mongoSchemaToSchemaAPIResponse(mongoObject.result)});
-      })
-    }));
-  });
 }

 // 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 +173,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 {
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -500,80 +500,47 @@ Schema.prototype.validateField = function(className, key, type, freeze) {

 // Passing the database and prefix is necessary in order to drop relation collections
 // and remove fields from objects. Ideally the database would belong to
-// a database adapter and this fuction would close over it or access it via member.
-Schema.prototype.deleteField = function(fieldName, className, database, prefix) {
+// a database adapter and this function would close over it or access it via member.
+Schema.prototype.deleteField = function(fieldName, className, database) {
  if (!classNameIsValid(className)) {
-    return Promise.reject({
-      code: Parse.Error.INVALID_CLASS_NAME,
-      error: invalidClassNameMessage(className),
-    });
+    throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, invalidClassNameMessage(className));
  }
-
  if (!fieldNameIsValid(fieldName)) {
-    return Promise.reject({
-      code: Parse.Error.INVALID_KEY_NAME,
-      error: 'invalid field name: ' + fieldName,
-    });
+    throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `invalid field name: ${fieldName}`);
  }
-
  //Don't allow deleting the default fields.
  if (!fieldNameIsValidForClass(fieldName, className)) {
-    return Promise.reject({
-      code: 136,
-      error: 'field ' + fieldName + ' cannot be changed',
-    });
+    throw new Parse.Error(136, `field ${fieldName} cannot be changed`);
  }

  return this.reload()
-  .then(schema => {
-    return schema.hasClass(className)
-    .then(hasClass => {
-      if (!hasClass) {
-        return Promise.reject({
-          code: Parse.Error.INVALID_CLASS_NAME,
-          error: 'class ' + className + ' does not exist',
-        });
-      }
+    .then(schema => {
+      return schema.hasClass(className)
+        .then(hasClass => {
+          if (!hasClass) {
+            throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`);
+          }
+          if (!schema.data[className][fieldName]) {
+            throw new Parse.Error(255, `Field ${fieldName} does not exist, cannot delete.`);
+          }

-      if (!schema.data[className][fieldName]) {
-        return Promise.reject({
-          code: 255,
-          error: 'field ' + fieldName + ' does not exist, cannot delete',
-        });
-      }
+          if (schema.data[className][fieldName].startsWith('relation<')) {
+            //For relations, drop the _Join table
+            return database.dropCollection(`_Join:${fieldName}:${className}`);
+          }

-      if (schema.data[className][fieldName].startsWith('relation<')) {
-        //For relations, drop the _Join table
-        return database.dropCollection(prefix + '_Join:' + fieldName + ':' + className)
-        //Save the _SCHEMA object
+          // for non-relations, remove all the data.
+          // This is necessary to ensure that the data is still gone if they add the same field.
+          return database.collection(className)
+            .then(collection => {
+              var mongoFieldName = schema.data[className][fieldName].startsWith('*') ? '_p_' + fieldName : fieldName;
+              return collection.update({}, { "$unset": { [mongoFieldName] : null } }, { multi: true });
+            });
+        })
+        // Save the _SCHEMA object
        .then(() => this.collection.update({ _id: className }, { $unset: {[fieldName]: null }}));
-      } else {
-        //for non-relations, remove all the data. This is necessary to ensure that the data is still gone
-        //if they add the same field.
-        return new Promise((resolve, reject) => {
-          database.collection(prefix + className, (err, coll) => {
-            if (err) {
-              reject(err);
-            } else {
-              var mongoFieldName = schema.data[className][fieldName].startsWith('*') ?
-                '_p_' + fieldName :
-                fieldName;
-              return coll.update({}, {
-                "$unset": { [mongoFieldName] : null },
-              }, {
-                multi: true,
-              })
-              //Save the _SCHEMA object
-              .then(() => this.collection.update({ _id: className }, { $unset: {[fieldName]: null }}))
-              .then(resolve)
-              .catch(reject);
-            }
-          });
-        });
-      }
    });
-  });
-}
+};

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