From 9ca73303386626a979b6aa25c2ab8625db4af82c Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Tue, 12 Apr 2016 18:53:03 -0700 Subject: [PATCH] Move field deletion logic into mongo adapter --- spec/Schema.spec.js | 9 ++- src/Adapters/Storage/Mongo/MongoCollection.js | 1 - .../Storage/Mongo/MongoStorageAdapter.js | 52 ++++++++++++++- src/Schema.js | 64 +++++++++---------- 4 files changed, 86 insertions(+), 40 deletions(-) diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index 1b84de1e38..7e87a72e41 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -684,12 +684,13 @@ describe('Schema', () => { .then(() => schema.deleteField('relationField', 'NewClass', config.database)) .then(() => schema.reloadData()) .then(() => { - expect(schema['data']['NewClass']).toEqual({ + const expectedSchema = { objectId: { type: 'String' }, updatedAt: { type: 'Date' }, createdAt: { type: 'Date' }, ACL: { type: 'ACL' } - }); + }; + expect(dd(schema.data.NewClass, expectedSchema)).toEqual(undefined); done(); }); }); @@ -716,6 +717,10 @@ describe('Schema', () => { done(); Parse.Object.enableSingleInstance(); }); + }) + .catch(error => { + fail(error); + done(); }); }); diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 12c9df668c..e29b68754b 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -28,7 +28,6 @@ export default class MongoCollection { var index = {}; index[key] = '2d'; - //TODO: condiser moving index creation logic into Schema.js return this._mongoCollection.createIndex(index) // Retry, but just once. .then(() => this._rawFind(query, { skip, limit, sort })); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index b39b2b56ed..b1238f3413 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -1,7 +1,7 @@ - -import MongoCollection from './MongoCollection'; -import MongoSchemaCollection from './MongoSchemaCollection'; +import MongoCollection from './MongoCollection'; +import MongoSchemaCollection from './MongoSchemaCollection'; import {parse as parseUrl, format as formatUrl} from '../../../vendor/mongodbUrl'; +import _ from 'lodash'; let mongodb = require('mongodb'); let MongoClient = mongodb.MongoClient; @@ -78,6 +78,52 @@ export class MongoStorageAdapter { }); }); } + + // Remove the column and all the data. For Relations, the _Join collection is handled + // specially, this function does not delete _Join columns. It should, however, indicate + // that the relation fields does not exist anymore. In mongo, this means removing it from + // the _SCHEMA collection. There should be no actual data in the collection under the same name + // as the relation column, so it's fine to attempt to delete it. If the fields listed to be + // deleted do not exist, this function should return successfully anyways. Checking for + // attempts to delete non-existent fields is the responsibility of Parse Server. + + // Pointer field names are passed for legacy reasons: the original mongo + // format stored pointer field names differently in the database, and therefore + // needed to know the type of the field before it could delete it. Future database + // adatpers should ignore the pointerFieldNames argument. All the field names are in + // fieldNames, they show up additionally in the pointerFieldNames database for use + // by the mongo adapter, which deals with the legacy mongo format. + + // This function is not obligated to delete fields atomically. It is given the field + // names in a list so that databases that are capable of deleting fields atomically + // may do so. + + // Returns a Promise. + + // This function currently accepts the collectionPrefix and adaptive collection as a paramater because it isn't + // actually capable of determining the location of it's own _SCHEMA collection without having + // the collectionPrefix. Also, Schemas.js, the caller of this function, only stores the collection + // itself, and not the prefix. Eventually Parse Server won't care what a SchemaCollection is and + // will just tell the DB adapter to do things and it will do them. + deleteFields(className: string, fieldNames, pointerFieldNames, collectionPrefix, adaptiveCollection) { + const nonPointerFieldNames = _.difference(fieldNames, pointerFieldNames); + const mongoFormatNames = nonPointerFieldNames.concat(pointerFieldNames.map(name => `_p_${name}`)); + const collectionUpdate = { '$unset' : {} }; + mongoFormatNames.forEach(name => { + collectionUpdate['$unset'][name] = null; + }); + + const schemaUpdate = { '$unset' : {} }; + fieldNames.forEach(name => { + schemaUpdate['$unset'][name] = null; + }); + + return adaptiveCollection.updateMany({}, collectionUpdate) + .then(updateResult => { + return this.schemaCollection(collectionPrefix) + }) + .then(schemaCollection => schemaCollection.updateSchema(className, schemaUpdate)); + } } export default MongoStorageAdapter; diff --git a/src/Schema.js b/src/Schema.js index 1aa33492da..385b3010e5 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -514,40 +514,36 @@ class Schema { } return this.reloadData() - .then(() => { - return this.hasClass(className) - .then(hasClass => { - if (!hasClass) { - throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); - } - if (!this.data[className][fieldName]) { - throw new Parse.Error(255, `Field ${fieldName} does not exist, cannot delete.`); - } - - if (this.data[className][fieldName].type == 'Relation') { - //For relations, drop the _Join table - return database.dropCollection(`_Join:${fieldName}:${className}`) - .then(() => { - return Promise.resolve(); - }, error => { - if (error.message == 'ns not found') { - return Promise.resolve(); - } - return Promise.reject(error); - }); - } - - // 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.adaptiveCollection(className) - .then(collection => { - let mongoFieldName = this.data[className][fieldName].type === 'Pointer' ? `_p_${fieldName}` : fieldName; - return collection.updateMany({}, { "$unset": { [mongoFieldName]: null } }); - }); - }) - // Save the _SCHEMA object - .then(() => this._collection.updateSchema(className, { $unset: { [fieldName]: null } })); - }); + .then(() => this.hasClass(className)) + .then(hasClass => { + if (!hasClass) { + throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); + } + if (!this.data[className][fieldName]) { + throw new Parse.Error(255, `Field ${fieldName} does not exist, cannot delete.`); + } + + if (this.data[className][fieldName].type == 'Relation') { + //For relations, drop the _Join table + return database.adaptiveCollection(className).then(collection => { + return database.adapter.deleteFields(className, [fieldName], [], database.collectionPrefix, collection); + }) + .then(() => database.dropCollection(`_Join:${fieldName}:${className}`)) + .catch(error => { + // 'ns not found' means collection was already gone. Ignore deletion attempt. + // TODO: 'ns not found' is a mongo implementation detail. Move it into mongo adapter. + if (error.message == 'ns not found') { + return Promise.resolve(); + } + return Promise.reject(error); + }); + } + + const fieldNames = [fieldName]; + const pointerFieldNames = this.data[className][fieldName].type === 'Pointer' ? [fieldName] : []; + return database.adaptiveCollection(className) + .then(collection => database.adapter.deleteFields(className, fieldNames, pointerFieldNames, database.collectionPrefix, collection)); + }); } // Validates an object provided in REST format.