diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index 6a02009feb..d0e5095331 100644 --- 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,7 +506,7 @@ 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(); }); }); @@ -512,24 +514,31 @@ describe('Schema', () => { 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 index 36ba763787..145b2134c8 100644 --- 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 index 914d9bb030..742420c5b3 100644 --- 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 index 954b1a6f96..99ed564838 100644 --- 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; } diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index a748ad14fe..352b1caf44 100644 --- 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', - } - }); - } - if (!existingFields[submittedFieldName] && submittedFields[submittedFieldName].__op === 'Delete') { - return Promise.resolve({ - status: 400, - response: { - code: 255, - error: 'field ' + submittedFieldName + ' does not exist, cannot delete', - } - }); + let newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields); + let mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className); + if (!mongoObject.result) { + throw new Parse.Error(mongoObject.code, mongoObject.error); } - } - 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', - } - }); - } 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); + .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.`); } - }); + 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 { diff --git a/src/Schema.js b/src/Schema.js index 7f7d4701a7..5c8a94d183 100644 --- 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', - }); - } - - 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(prefix + '_Join:' + fieldName + ':' + className) - //Save the _SCHEMA object + .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].startsWith('relation<')) { + //For relations, drop the _Join table + return database.dropCollection(`_Join:${fieldName}:${className}`); + } + + // 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.