From b0c4b8f6cec32eb8f3d5966ac7840b44c9ec00ce Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Tue, 9 Feb 2016 20:16:53 -0800 Subject: [PATCH] Drop _Join collection when deleting a relation field --- spec/Schema.spec.js | 61 +++++++++++++++++++++++++++++++++++++++++++- src/ExportAdapter.js | 2 -- src/Schema.js | 44 +++++++++++++++++++++++++++++++- 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index d6310ec8..9524ea1b 100644 --- a/spec/Schema.spec.js +++ b/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(); + })) + }); + }) + }); }); diff --git a/src/ExportAdapter.js b/src/ExportAdapter.js index 1676ccfb..7c2bd674 100644 --- a/src/ExportAdapter.js +++ b/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, diff --git a/src/Schema.js b/src/Schema.js index 9477bd39..33dedb99 100644 --- a/src/Schema.js +++ b/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);