Drop _Join collection when deleting a relation field

spec/Schema.spec.js

@@ -4,6 +4,22 @@ var dd = require('deep-diff');
 
 var config = new Config('test');
 
+var hasAllPODobject = () => {
+  var obj = new Parse.Object('HasAllPOD');
+  obj.set('aNumber', 5);
+  obj.set('aString', 'string');
+  obj.set('aBool', true);
+  obj.set('aDate', new Date());
+  obj.set('aObject', {k1: 'value', k2: true, k3: 5});
+  obj.set('aArray', ['contents', true, 5]);
+  obj.set('aGeoPoint', new Parse.GeoPoint({latitude: 0, longitude: 0}));
+  obj.set('aFile', new Parse.File('f.txt', { base64: 'V29ya2luZyBhdCBQYXJzZSBpcyBncmVhdCE=' }));
+  var objACL = new Parse.ACL();
+  objACL.setPublicWriteAccess(false);
+  obj.setACL(objACL);
+  return obj;
+};
+
 describe('Schema', () => {
   it('can validate one object', (done) => {
     config.database.loadSchema().then((schema) => {
@@ -411,7 +427,6 @@ describe('Schema', () => {
     .then(schema => {
       return schema.addClassIfNotExists('NewClass', {})
       .then(() => {
-        console.log(Object.getPrototypeOf(schema));
         schema.hasClass('NewClass')
         .then(hasClass => {
           expect(hasClass).toEqual(true);
@@ -461,4 +476,48 @@ describe('Schema', () => {
       done();
     });
   });
+
+  it('refuses to delete fields from nonexistant classes', done => {
+    config.database.loadSchema()
+    .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');
+      done();
+    });
+  });
+
+  it('refuses to delete fields that dont exist', done => {
+    hasAllPODobject().save()
+    .then(() => config.database.loadSchema())
+    .then(schema => schema.deleteField('missingField', 'HasAllPOD'))
+    .fail(error => {
+      expect(error.code).toEqual(255);
+      expect(error.error).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.db.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
+        expect(err).toEqual(null);
+        config.database.loadSchema()
+        .then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database.db, 'test_'))
+        .then(() => config.database.db.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
+          expect(err).not.toEqual(null);
+          done();
+        }))
+      });
+    })
+  });
 });

src/ExportAdapter.js

@@ -43,8 +43,6 @@ ExportAdapter.prototype.connect = function() {
 
 // Returns a promise for a Mongo collection.
 // Generally just for internal use.
-var joinRegex = /^_Join:[A-Za-z0-9_]+:[A-Za-z0-9_]+/;
-var otherRegex = /^[A-Za-z][A-Za-z0-9_]*$/;
 ExportAdapter.prototype.collection = function(className) {
   if (!Schema.classNameIsValid(className)) {
     throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME,

src/Schema.js

@@ -413,7 +413,11 @@ Schema.prototype.validateField = function(className, key, type, freeze) {
 // to remove unused fields, if other writers are writing objects that include
 // this field, the field may reappear. Returns a Promise that resolves with
 // no object on success, or rejects with { code, error } on failure.
-Schema.prototype.deleteField = function(fieldName, className) {
+
+// 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) {
   if (!classNameIsValid(className)) {
     return Promise.reject({
       code: Parse.Error.INVALID_CLASS_NAME,
@@ -438,6 +442,37 @@ Schema.prototype.deleteField = function(fieldName, className) {
 
   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',
+        });
+      }
+
+      if (!schema.data[className][fieldName]) {
+        return Promise.reject({
+          code: 255,
+          error: 'field ' + fieldName + ' does not exist, cannot delete',
+        });
+      }
+
+      let p = null;
+      if (schema.data[className][fieldName].startsWith('relation')) {
+        //For relations, drop the _Join table
+
+        p = database.dropCollection(prefix + '_Join:' + fieldName + ':' + className);
+      } 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.
+        p = Promise.resolve();
+      }
+      return p.then(() => {
+        //Save the _SCHEMA object
+        return Promise.resolve();
+      });
+    });
   });
 }
 
@@ -509,6 +544,13 @@ Schema.prototype.getExpectedType = function(className, key) {
   return undefined;
 };
 
+// Checks if a given class is in the schema. Needs to load the
+// schema first, which is kinda janky. Hopefully we can refactor
+// and make this be a regular value. Parse would probably
+Schema.prototype.hasClass = function(className) {
+  return this.reload().then(newSchema => !!newSchema.data[className]);
+}
+
 // Helper function to check if a field is a pointer, returns true or false.
 Schema.prototype.isPointer = function(className, key) {
   var expected = this.getExpectedType(className, key);