From e81680bf514e8ab480bd7f5427d731c5d5fbfef6 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 13:58:55 -0700 Subject: [PATCH 1/7] Tidy up schemas router --- src/Routers/SchemasRouter.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index 574aeb09d4..ca3c779fc7 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -72,19 +72,8 @@ function modifySchema(req) { let className = req.params.className; return req.config.database.loadSchema() - .then(schema => { - return schema.updateClass(className, submittedFields, req.body.classLevelPermissions, req.config.database); - }).then((result) => { - return Promise.resolve({response: result}); - }); -} - -function getSchemaPermissions(req) { - var className = req.params.className; - return req.config.database.loadSchema() - .then(schema => { - return Promise.resolve({response: schema.perms[className]}); - }); + .then(schema => schema.updateClass(className, submittedFields, req.body.classLevelPermissions, req.config.database)) + .then(result => ({response: result})); } // A helper function that removes all join tables for a schema. Returns a promise. From 6c7fc898683169119f34ea24061f8b1821646f8f Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 14:16:23 -0700 Subject: [PATCH 2/7] de-duplicate logic for injecting default schemas --- src/Routers/SchemasRouter.js | 14 ++------------ src/Schema.js | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index ca3c779fc7..a663e33382 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -14,20 +14,10 @@ function classNameMismatchResponse(bodyClass, pathClass) { ); } -function injectDefaultSchema(schema) { - let defaultSchema = Schema.defaultColumns[schema.className]; - if (defaultSchema) { - Object.keys(defaultSchema).forEach((key) => { - schema.fields[key] = defaultSchema[key]; - }); - } - return schema; -} - function getAllSchemas(req) { return req.config.database.schemaCollection() .then(collection => collection.getAllSchemas()) - .then(schemas => schemas.map(injectDefaultSchema)) + .then(schemas => schemas.map(Schema.injectDefaultSchema)) .then(schemas => ({ response: { results: schemas } })); } @@ -35,7 +25,7 @@ function getOneSchema(req) { const className = req.params.className; return req.config.database.schemaCollection() .then(collection => collection.findSchema(className)) - .then(injectDefaultSchema) + .then(Schema.injectDefaultSchema) .then(schema => ({ response: schema })) .catch(error => { if (error === undefined) { diff --git a/src/Schema.js b/src/Schema.js index 880a224f86..002911ac64 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -201,6 +201,16 @@ const fieldTypeIsInvalid = ({ type, targetClass }) => { return undefined; } +const injectDefaultSchema = schema => ({ + className: schema.className, + fields: { + ...defaultColumns._Default, + ...(defaultColumns[schema.className] || {}), + ...schema.fields, + }, + classLevelPermissions: schema.classLevelPermissions, +}) + // Stores the entire schema of the app in a weird hybrid format somewhere between // the mongo format and the Parse format. Soon, this will all be Parse format. class Schema { @@ -221,13 +231,8 @@ class Schema { this.data = {}; this.perms = {}; return this._collection.getAllSchemas().then(allSchemas => { - allSchemas.forEach(schema => { - const parseFormatSchema = { - ...defaultColumns._Default, - ...(defaultColumns[schema.className] || {}), - ...schema.fields, - } - this.data[schema.className] = parseFormatSchema; + allSchemas.map(injectDefaultSchema).forEach(schema => { + this.data[schema.className] = schema.fields; this.perms[schema.className] = schema.classLevelPermissions; }); }); @@ -822,4 +827,5 @@ export { buildMergedSchemaObject, systemClasses, defaultColumns, + injectDefaultSchema, }; From a5d57dc324c4113eb39af3944a58313bd3ff60fd Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 14:19:26 -0700 Subject: [PATCH 3/7] Remove no-op promise --- src/Schema.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Schema.js b/src/Schema.js index 002911ac64..4475937ebb 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -252,9 +252,6 @@ class Schema { } return this._collection.addSchema(className, fields, classLevelPermissions) - .then(res => { - return Promise.resolve(res); - }) .catch(error => { if (error === undefined) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} already exists.`); From 87e6413296ab4832b2ba86109b006814eb3b49fc Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 14:27:27 -0700 Subject: [PATCH 4/7] use hasClass --- src/Schema.js | 81 +++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/src/Schema.js b/src/Schema.js index 4475937ebb..712330f3b6 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -262,40 +262,42 @@ class Schema { } updateClass(className, submittedFields, classLevelPermissions, database) { - if (!this.data[className]) { - throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); - } - let existingFields = Object.assign(this.data[className], {_id: 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.`); + return this.hasClass(className) + .then(hasClass => { + if (!hasClass) { + throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); } - }); - - let newSchema = buildMergedSchemaObject(existingFields, submittedFields); - let validationError = this.validateSchemaData(className, newSchema, classLevelPermissions); - if (validationError) { - throw new Parse.Error(validationError.code, validationError.error); - } + let existingFields = Object.assign(this.data[className], {_id: 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.`); + } + }); - // 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 deletePromises = []; - let insertedFields = []; - Object.keys(submittedFields).forEach(fieldName => { - if (submittedFields[fieldName].__op === 'Delete') { - const promise = this.deleteField(fieldName, className, database); - deletePromises.push(promise); - } else { - insertedFields.push(fieldName); + let newSchema = buildMergedSchemaObject(existingFields, submittedFields); + let validationError = this.validateSchemaData(className, newSchema, classLevelPermissions); + if (validationError) { + throw new Parse.Error(validationError.code, validationError.error); } - }); - return Promise.all(deletePromises) // Delete Everything + // 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 deletePromises = []; + let insertedFields = []; + Object.keys(submittedFields).forEach(fieldName => { + if (submittedFields[fieldName].__op === 'Delete') { + const promise = this.deleteField(fieldName, className, database); + deletePromises.push(promise); + } else { + insertedFields.push(fieldName); + } + }); + + return Promise.all(deletePromises) // Delete Everything .then(() => this.reloadData()) // Reload our Schema, so we have all the new values .then(() => { let promises = insertedFields.map(fieldName => { @@ -304,15 +306,14 @@ class Schema { }); return Promise.all(promises); }) - .then(() => { - return this.setPermissions(className, classLevelPermissions) - }) + .then(() => this.setPermissions(className, classLevelPermissions)) //TODO: Move this logic into the database adapter - .then(() => { - return { className: className, - fields: this.data[className], - classLevelPermissions: this.perms[className] } - }); + .then(() => ({ + className: className, + fields: this.data[className], + classLevelPermissions: this.perms[className] + })); + }) } @@ -639,9 +640,7 @@ class Schema { 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. + // Checks if a given class is in the schema. hasClass(className) { return this.reloadData().then(() => !!(this.data[className])); } From f7e7368ad58968b9c1ce49f019ceb19a257e540c Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 15:24:04 -0700 Subject: [PATCH 5/7] Make getAllSchemas part of SchemaController --- .../Storage/Mongo/MongoSchemaCollection.js | 7 ++----- .../Storage/Mongo/MongoStorageAdapter.js | 7 +++++++ src/Controllers/DatabaseController.js | 4 ++-- src/Routers/SchemasRouter.js | 7 +++---- src/Schema.js | 20 +++++++++++++------ 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoSchemaCollection.js b/src/Adapters/Storage/Mongo/MongoSchemaCollection.js index 28fe615d84..4f82197efb 100644 --- a/src/Adapters/Storage/Mongo/MongoSchemaCollection.js +++ b/src/Adapters/Storage/Mongo/MongoSchemaCollection.js @@ -1,6 +1,6 @@ import MongoCollection from './MongoCollection'; -import * as transform from './MongoTransform'; +import * as transform from './MongoTransform'; function mongoFieldToParseSchemaField(type) { if (type[0] === '*') { @@ -128,10 +128,7 @@ class MongoSchemaCollection { this._collection = collection; } - // Return a promise for all schemas known to this adapter, in Parse format. In case the - // schemas cannot be retrieved, returns a promise that rejects. Requirements fot the - // rejection reason are TBD. - getAllSchemas() { + _fetchAllSchemasFrom_SCHEMA() { return this._collection._rawFind({}) .then(schemas => schemas.map(mongoSchemaToParseSchema)); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 66342e0b9e..785cb89d4b 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -127,6 +127,13 @@ export class MongoStorageAdapter { .then(schemaCollection => schemaCollection.updateSchema(className, schemaUpdate)); } + // Return a promise for all schemas known to this adapter, in Parse format. In case the + // schemas cannot be retrieved, returns a promise that rejects. Requirements for the + // rejection reason are TBD. + getAllSchemas() { + return this.schemaCollection().then(schemasCollection => schemasCollection._fetchAllSchemasFrom_SCHEMA()); + } + // TODO: As yet not particularly well specified. Creates an object. Does it really need the schema? // or can it fetch the schema itself? Also the schema is not currently a Parse format schema, and it // should be, if we are passing it at all. diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index d0e92acb5b..6c6d7b9b47 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -67,7 +67,7 @@ DatabaseController.prototype.loadSchema = function(acceptor = () => true) { if (!this.schemaPromise) { this.schemaPromise = this.schemaCollection().then(collection => { delete this.schemaPromise; - return Schema.load(collection); + return Schema.load(collection, this.adapter); }); return this.schemaPromise; } @@ -78,7 +78,7 @@ DatabaseController.prototype.loadSchema = function(acceptor = () => true) { } this.schemaPromise = this.schemaCollection().then(collection => { delete this.schemaPromise; - return Schema.load(collection); + return Schema.load(collection, this.adapter); }); return this.schemaPromise; }); diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index a663e33382..964be2e399 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -15,10 +15,9 @@ function classNameMismatchResponse(bodyClass, pathClass) { } function getAllSchemas(req) { - return req.config.database.schemaCollection() - .then(collection => collection.getAllSchemas()) - .then(schemas => schemas.map(Schema.injectDefaultSchema)) - .then(schemas => ({ response: { results: schemas } })); + return req.config.database.loadSchema() + .then(schemaController => schemaController.getAllSchemas()) + .then(schemas => ({ response: { results: schemas } })); } function getOneSchema(req) { diff --git a/src/Schema.js b/src/Schema.js index 712330f3b6..66db1753b8 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -213,13 +213,15 @@ const injectDefaultSchema = schema => ({ // Stores the entire schema of the app in a weird hybrid format somewhere between // the mongo format and the Parse format. Soon, this will all be Parse format. -class Schema { +class SchemaController { _collection; + _dbAdapter; data; perms; - constructor(collection) { + constructor(collection, databaseAdapter) { this._collection = collection; + this._dbAdapter = databaseAdapter; // this.data[className][fieldName] tells you the type of that field, in mongo format this.data = {}; @@ -230,14 +232,20 @@ class Schema { reloadData() { this.data = {}; this.perms = {}; - return this._collection.getAllSchemas().then(allSchemas => { - allSchemas.map(injectDefaultSchema).forEach(schema => { + return this.getAllSchemas() + .then(allSchemas => { + allSchemas.forEach(schema => { this.data[schema.className] = schema.fields; this.perms[schema.className] = schema.classLevelPermissions; }); }); } + getAllSchemas() { + return this._dbAdapter.getAllSchemas() + .then(allSchemas => allSchemas.map(injectDefaultSchema)); + } + // Create a new class that includes the three default fields. // ACL is an implicit column that does not get an entry in the // _SCHEMAS database. Returns a promise that resolves with the @@ -673,8 +681,8 @@ class Schema { } // Returns a promise for a new Schema. -function load(collection) { - let schema = new Schema(collection); +function load(collection, dbAdapter) { + let schema = new SchemaController(collection, dbAdapter); return schema.reloadData().then(() => schema); } From b2aa6491742c753a98a5f29806edc39e4cd86d9e Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 15:40:10 -0700 Subject: [PATCH 6/7] Move getOneSchema logic onto schema controller --- .../Storage/Mongo/MongoSchemaCollection.js | 5 +---- .../Storage/Mongo/MongoStorageAdapter.js | 7 ++++++ src/Routers/SchemasRouter.js | 22 +++++++++---------- src/Schema.js | 6 ++++- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoSchemaCollection.js b/src/Adapters/Storage/Mongo/MongoSchemaCollection.js index 4f82197efb..ea0cb23740 100644 --- a/src/Adapters/Storage/Mongo/MongoSchemaCollection.js +++ b/src/Adapters/Storage/Mongo/MongoSchemaCollection.js @@ -133,10 +133,7 @@ class MongoSchemaCollection { .then(schemas => schemas.map(mongoSchemaToParseSchema)); } - // Return a promise for the schema with the given name, in Parse format. If - // this adapter doesn't know about the schema, return a promise that rejects with - // undefined as the reason. - findSchema(name: string) { + _fechOneSchemaFrom_SCHEMA(name: string) { return this._collection._rawFind(_mongoSchemaQueryFromNameQuery(name), { limit: 1 }).then(results => { if (results.length === 1) { return mongoSchemaToParseSchema(results[0]); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 785cb89d4b..05438c6b2a 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -134,6 +134,13 @@ export class MongoStorageAdapter { return this.schemaCollection().then(schemasCollection => schemasCollection._fetchAllSchemasFrom_SCHEMA()); } + // Return a promise for the schema with the given name, in Parse format. If + // this adapter doesn't know about the schema, return a promise that rejects with + // undefined as the reason. + getOneSchema(className) { + return this.schemaCollection().then(schemasCollection => schemasCollection._fechOneSchemaFrom_SCHEMA(className)); + } + // TODO: As yet not particularly well specified. Creates an object. Does it really need the schema? // or can it fetch the schema itself? Also the schema is not currently a Parse format schema, and it // should be, if we are passing it at all. diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index 964be2e399..b7af27c8b7 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -22,17 +22,17 @@ function getAllSchemas(req) { function getOneSchema(req) { const className = req.params.className; - return req.config.database.schemaCollection() - .then(collection => collection.findSchema(className)) - .then(Schema.injectDefaultSchema) - .then(schema => ({ response: schema })) - .catch(error => { - if (error === undefined) { - throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); - } else { - throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Database adapter error.'); - } - }); + return req.config.database.loadSchema() + .then(schemaController => schemaController.getOneSchema(className)) + .then(schema => ({ response: schema })) + .catch(error => { + console.log(error); + if (error === undefined) { + throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); + } else { + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Database adapter error.'); + } + }); } function createSchema(req) { diff --git a/src/Schema.js b/src/Schema.js index 66db1753b8..b3ac443aba 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -246,6 +246,11 @@ class SchemaController { .then(allSchemas => allSchemas.map(injectDefaultSchema)); } + getOneSchema(className) { + return this._dbAdapter.getOneSchema(className) + .then(injectDefaultSchema); + } + // Create a new class that includes the three default fields. // ACL is an implicit column that does not get an entry in the // _SCHEMAS database. Returns a promise that resolves with the @@ -831,5 +836,4 @@ export { buildMergedSchemaObject, systemClasses, defaultColumns, - injectDefaultSchema, }; From 618c824bd55032fb4cbd2c4e3e5d3d18604b6989 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Mon, 18 Apr 2016 15:47:05 -0700 Subject: [PATCH 7/7] remove log --- src/Routers/SchemasRouter.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index b7af27c8b7..d169ea6554 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -26,7 +26,6 @@ function getOneSchema(req) { .then(schemaController => schemaController.getOneSchema(className)) .then(schema => ({ response: schema })) .catch(error => { - console.log(error); if (error === undefined) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); } else {