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

Decouple and remove direct mongo access from Schema/SchemaRouter.
parse-community · Mar 1, 2016 · 0b990b6 · 0b990b6
2 parents 382149b + d1dbc1a
commit 0b990b6
Show file tree

Hide file tree

Showing 6 changed files with 202 additions and 252 deletions.
diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js
@@ -1,3 +1,5 @@
+'use strict';
+
 var Config = require('../src/Config');
 var Schema = require('../src/Schema');
 var dd = require('deep-diff');
@@ -483,7 +485,7 @@ describe('Schema', () => {
     .then(schema => schema.deleteField('installationId', '_Installation'))
     .catch(error => {
       expect(error.code).toEqual(136);
-      expect(error.error).toEqual('field installationId cannot be changed');
+      expect(error.message).toEqual('field installationId cannot be changed');
       done();
     });
   });
@@ -493,7 +495,7 @@ describe('Schema', () => {
     .then(schema => schema.deleteField('field', 'NoClass'))
     .catch(error => {
       expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
-      expect(error.error).toEqual('class NoClass does not exist');
+      expect(error.message).toEqual('Class NoClass does not exist.');
       done();
     });
   });
@@ -504,32 +506,39 @@ describe('Schema', () => {
     .then(schema => schema.deleteField('missingField', 'HasAllPOD'))
     .fail(error => {
       expect(error.code).toEqual(255);
-      expect(error.error).toEqual('field missingField does not exist, cannot delete');
+      expect(error.message).toEqual('Field missingField does not exist, cannot delete.');
       done();
     });
   });
 
   it('drops related collection when deleting relation field', done => {
     var obj1 = hasAllPODobject();
     obj1.save()
-    .then(savedObj1 => {
-      var obj2 = new Parse.Object('HasPointersAndRelations');
-      obj2.set('aPointer', savedObj1);
-      var relation = obj2.relation('aRelation');
-      relation.add(obj1);
-      return obj2.save();
-    })
-    .then(() => {
-      config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
-        expect(err).toEqual(null);
-        config.database.loadSchema()
-        .then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database.adapter.database, 'test_'))
-        .then(() => config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
-          expect(err).not.toEqual(null);
-          done();
-        }))
+      .then(savedObj1 => {
+        var obj2 = new Parse.Object('HasPointersAndRelations');
+        obj2.set('aPointer', savedObj1);
+        var relation = obj2.relation('aRelation');
+        relation.add(obj1);
+        return obj2.save();
+      })
+      .then(() => config.database.collectionExists('_Join:aRelation:HasPointersAndRelations'))
+      .then(exists => {
+        if (!exists) {
+          fail('Relation collection should exist after save.');
+        }
+      })
+      .then(() => config.database.loadSchema())
+      .then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database))
+      .then(() => config.database.collectionExists('_Join:aRelation:HasPointersAndRelations'))
+      .then(exists => {
+        if (exists) {
+          fail('Relation collection should not exist after deleting relation field.');
+        }
+        done();
+      }, error => {
+        fail(error);
+        done();
       });
-    })
   });
 
   it('can delete string fields and resave as number field', done => {
@@ -538,7 +547,7 @@ describe('Schema', () => {
     var obj2 = hasAllPODobject();
     var p = Parse.Object.saveAll([obj1, obj2])
     .then(() => config.database.loadSchema())
-    .then(schema => schema.deleteField('aString', 'HasAllPOD', config.database.adapter.database, 'test_'))
+    .then(schema => schema.deleteField('aString', 'HasAllPOD', config.database))
     .then(() => new Parse.Query('HasAllPOD').get(obj1.id))
     .then(obj1Reloaded => {
       expect(obj1Reloaded.get('aString')).toEqual(undefined);
@@ -568,7 +577,7 @@ describe('Schema', () => {
       expect(obj1.get('aPointer').id).toEqual(obj1.id);
     })
     .then(() => config.database.loadSchema())
-    .then(schema => schema.deleteField('aPointer', 'NewClass', config.database.adapter.database, 'test_'))
+    .then(schema => schema.deleteField('aPointer', 'NewClass', config.database))
     .then(() => new Parse.Query('NewClass').get(obj1.id))
     .then(obj1 => {
       expect(obj1.get('aPointer')).toEqual(undefined);

diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js
@@ -1,3 +1,5 @@
+'use strict';
+
 var Parse = require('parse/node').Parse;
 var request = require('request');
 var dd = require('deep-diff');
@@ -369,7 +371,7 @@ describe('schemas', () => {
     }, (error, response, body) => {
       expect(response.statusCode).toEqual(400);
       expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
-      expect(body.error).toEqual('class NoClass does not exist');
+      expect(body.error).toEqual('Class NoClass does not exist.');
       done();
     });
   });
@@ -390,13 +392,13 @@ describe('schemas', () => {
       }, (error, response, body) => {
         expect(response.statusCode).toEqual(400);
         expect(body.code).toEqual(255);
-        expect(body.error).toEqual('field aString exists, cannot update');
+        expect(body.error).toEqual('Field aString exists, cannot update.');
         done();
       });
     })
   });
 
-  it('refuses to delete non-existant fields', done => {
+  it('refuses to delete non-existent fields', done => {
     var obj = hasAllPODobject();
     obj.save()
     .then(() => {
@@ -406,13 +408,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 +662,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();
       });
     });
@@ -710,28 +713,35 @@ describe('schemas', () => {
       }, (error, response, body) => {
         expect(response.statusCode).toEqual(200);
         expect(response.body).toEqual({});
-        config.database.adapter.database.collection('test__Join:aRelation:MyOtherClass', { strict: true }, (err, coll) => {
-          //Expect Join table to be gone
-          expect(err).not.toEqual(null);
-          config.database.adapter.database.collection('test_MyOtherClass', { strict: true }, (err, coll) => {
-            // Expect data table to be gone
-            expect(err).not.toEqual(null);
-            request.get({
-              url: 'http://localhost:8378/1/schemas/MyOtherClass',
-              headers: masterKeyHeaders,
-              json: true,
-            }, (error, response, body) => {
-              //Expect _SCHEMA entry to be gone.
-              expect(response.statusCode).toEqual(400);
-              expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
-              expect(body.error).toEqual('class MyOtherClass does not exist');
-              done();
-            });
+        config.database.collectionExists('_Join:aRelation:MyOtherClass').then(exists => {
+          if (exists) {
+            fail('Relation collection should be deleted.');
+            done();
+          }
+          return config.database.collectionExists('MyOtherClass');
+        }).then(exists => {
+          if (exists) {
+            fail('Class collection should be deleted.');
+            done();
+          }
+        }).then(() => {
+          request.get({
+            url: 'http://localhost:8378/1/schemas/MyOtherClass',
+            headers: masterKeyHeaders,
+            json: true,
+          }, (error, response, body) => {
+            //Expect _SCHEMA entry to be gone.
+            expect(response.statusCode).toEqual(400);
+            expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
+            expect(body.error).toEqual('class MyOtherClass does not exist');
+            done();
           });
         });
       });
+    }).then(() => {
     }, error => {
       fail(error);
+      done();
     });
   });
 });
diff --git 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(() => {

diff --git 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;
 }