From d934f3a8635dfade6fbe67cdaa610ffc250ba2e0 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 5 Feb 2016 09:42:35 -0800 Subject: [PATCH 1/5] Add class creation logic with validation --- ExportAdapter.js | 8 +- Schema.js | 199 +++++++++++++++++++++++++++++++++++++++++++- schemas.js | 4 +- spec/Schema.spec.js | 98 ++++++++++++++++++++++ 4 files changed, 298 insertions(+), 11 deletions(-) diff --git a/ExportAdapter.js b/ExportAdapter.js index 830b41807d..4a626fa2b9 100644 --- a/ExportAdapter.js +++ b/ExportAdapter.js @@ -60,13 +60,7 @@ ExportAdapter.prototype.connect = function() { 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 (className !== '_User' && - className !== '_Installation' && - className !== '_Session' && - className !== '_SCHEMA' && - className !== '_Role' && - !joinRegex.test(className) && - !otherRegex.test(className)) { + if (!Schema.classNameIsValid(className)) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, 'invalid className: ' + className); } diff --git a/Schema.js b/Schema.js index 66d1d45205..5514dca263 100644 --- a/Schema.js +++ b/Schema.js @@ -17,6 +17,137 @@ var Parse = require('parse/node').Parse; var transform = require('./transform'); +defaultColumns = { + // Contain the default columns for every parse object type (except _Join collection) + _Default: { + "objectId": {type:'String'}, + "createdAt": {type:'Date'}, + "updatedAt": {type:'Date'}, + "ACL": {type:'ACL'}, + }, + // The additional default columns for the _User collection (in addition to DefaultCols) + _User: { + "username": {type:'String'}, + "password": {type:'String'}, + "authData": {type:'Object'}, + "email": {type:'String'}, + "emailVerified": {type:'Boolean'}, + }, + // The additional default columns for the _User collection (in addition to DefaultCols) + _Installation: { + "installationId": {type:'String'}, + "deviceToken": {type:'String'}, + "channels": {type:'Array'}, + "deviceType": {type:'String'}, + "pushType": {type:'String'}, + "GCMSenderId": {type:'String'}, + "timeZone": {type:'String'}, + "localeIdentifier": {type:'String'}, + "badge": {type:'Number'}, + }, + // The additional default columns for the _User collection (in addition to DefaultCols) + _Role: { + "name": {type:'String'}, + "users": {type:'Relation',className:'_User'}, + "roles": {type:'Relation',className:'_Role'}, + }, + // The additional default columns for the _User collection (in addition to DefaultCols) + _Session: { + "restricted": {type:'Boolean'}, + "user": {type:'Pointer', className:'_User'}, + "installationId": {type:'String'}, + "sessionToken": {type:'String'}, + "expiresAt": {type:'Date'}, + "createdWith": {type:'Object'}, + }, +} + +// Valid classes must: +// Be one of _User, _Installation, _Role, _Session OR +// Be a join table OR +// Include only alpha-numeric and underscores, and not start with an underscore or number +var joinClassRegex = /^_Join:[A-Za-z0-9_]+:[A-Za-z0-9_]+/; +var classAndFieldRegex = /^[A-Za-z][A-Za-z0-9_]*$/; +function classNameIsValid(className) { + return ( + className === '_User' || + className === '_Installation' || + className === '_Session' || + className === '_SCHEMA' || //TODO: remove this, as _SCHEMA is not a valid class name for storing Parse Objects. + className === '_Role' || + joinClassRegex.test(className) || + classAndFieldRegex.test(className) + ); +} + +// Valid fields must be alpha-numeric, and not start with an underscore or number +function fieldNameIsValid(fieldName) { + return classAndFieldRegex.test(fieldName); +} + +// Checks that it's not trying to clobber one of the default fields of the class. +function fieldNameIsValidForClass(fieldName, className) { + if (!fieldNameIsValid(fieldName)) { + return false; + } + if (defaultColumns._Default[fieldName]) { + return false; + } + if (defaultColumns[className] && defaultColumns[className][fieldName]) { + return false; + } + return true; +} + +function invalidClassNameMessage(className) { + if (!className) { + className = ''; + } + return 'Invalid classname: ' + className + ', classnames can only have alphanumeric characters and _, and must start with an alpha character '; +} + +// Returns { error: "message", code: ### } if the type could not be +// converted, otherwise returns a returns { result: "mongotype" } +// where mongotype is suitable for inserting into mongo _SCHEMA collection +function schemaAPITypeToMongoFieldType(type) { + var invalidJsonError = { error: "invalid JSON", code: Parse.Error.INVALID_JSON }; + if (type.type == 'Pointer') { + if (!type.targetClass) { + return { error: 'type Pointer needs a class name', code: 135 }; + } else if (typeof type.targetClass !== 'string') { + return invalidJsonError; + } else if (!classNameIsValid(type.targetClass)) { + return { error: invalidClassNameMessage(type.targetClass), code: Parse.Error.INVALID_CLASS_NAME }; + } else { + return { result: '*' + type.targetClass }; + } + } + if (type.type == 'Relation') { + if (!type.targetClass) { + return { error: 'type Relation needs a class name', code: 135 }; + } else if (typeof type.targetClass !== 'string') { + return invalidJsonError; + } else if (!classNameIsValid(type.targetClass)) { + return { error: invalidClassNameMessage(type.targetClass), code: Parse.Error.INVALID_CLASS_NAME }; + } else { + return { result: 'relation<' + type.targetClass + '>' }; + } + } + if (typeof type.type !== 'string') { + return { error: "invalid JSON", code: Parse.Error.INVALID_JSON }; + } + switch (type.type) { + default : return { error: 'invalid field type: ' + type.type }; + case 'Number': return { result: 'number' }; + case 'String': return { result: 'string' }; + case 'Boolean': return { result: 'boolean' }; + case 'Date': return { result: 'date' }; + case 'Object': return { result: 'object' }; + case 'Array': return { result: 'array' }; + case 'GeoPoint': return { result: 'geopoint' }; + case 'File': return { result: 'file' }; + } +} // Create a schema from a Mongo collection and the exported schema format. // mongoSchema should be a list of objects, each with: @@ -71,9 +202,72 @@ Schema.prototype.reload = function() { return load(this.collection); }; +// 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 +// created schema, in mongo format. +// on success, and rejects with an error on fail. Ensure you +// have authorization (master key, or client class creation +// enabled) before calling this function. +Schema.prototype.addClassIfNotExists = function(className, fields) { + if (this.data[className]) { + return Promise.reject(new Parse.Error( + Parse.Error.DUPLICATE_VALUE, + 'class ' + className + ' already exists' + )); + } + + if (!classNameIsValid(className)) { + return Promise.reject({ + code: Parse.Error.INVALID_CLASS_NAME, + error: invalidClassNameMessage(className), + }); + return Promise.reject({ + code: Parse.Error.INVALID_CLASS_NAME, + }); + } + for (fieldName in fields) { + if (!fieldNameIsValid(fieldName)) { + return Promise.reject({ + code: Parse.Error.INVALID_KEY_NAME, + error: 'invalid field name: ' + fieldName, + }); + } + if (!fieldNameIsValidForClass(fieldName, className)) { + return Promise.reject({ + code: 136, + error: 'field ' + fieldName + ' cannot be added', + }); + } + } + + + + return this.collection.insertOne({ + _id: className, + objectId: 'string', + updatedAt: 'string', + createdAt: 'string', + }) + .then(result => result.ops[0]) + .catch(error => { + if (error.code === 11000) { //Mongo's duplicate key error + return Promise.reject({ + code: Parse.Error.INVALID_CLASS_NAME, + error: 'class ' + className + ' already exists', + }); + } + return Promise.reject(error); + }); +} + // Returns a promise that resolves successfully to the new schema -// object. +// object or fails with a reason. // If 'freeze' is true, refuse to update the schema. +// WARNING: this function has side-effects, and doesn't actually +// do any validation of the format of the className. You probably +// should use classNameIsValid or addClassIfNotExists or something +// like that instead. TODO: rename or remove this function. Schema.prototype.validateClassName = function(className, freeze) { if (this.data[className]) { return Promise.resolve(this); @@ -348,5 +542,6 @@ function getObjectType(obj) { module.exports = { - load: load + load: load, + classNameIsValid: classNameIsValid, }; diff --git a/schemas.js b/schemas.js index 62119a144e..4a8d50b4ee 100644 --- a/schemas.js +++ b/schemas.js @@ -5,7 +5,7 @@ var express = require('express'), var router = new PromiseRouter(); -function mongoFieldTypeToApiResponseType(type) { +function mongoFieldTypeToSchemaAPIType(type) { if (type[0] === '*') { return { type: 'Pointer', @@ -34,7 +34,7 @@ function mongoSchemaAPIResponseFields(schema) { fieldNames = Object.keys(schema).filter(key => key !== '_id'); response = {}; fieldNames.forEach(fieldName => { - response[fieldName] = mongoFieldTypeToApiResponseType(schema[fieldName]); + response[fieldName] = mongoFieldTypeToSchemaAPIType(schema[fieldName]); }); response.ACL = {type: 'ACL'}; response.createdAt = {type: 'Date'}; diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index abf178ab03..c142f628b8 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -131,4 +131,102 @@ describe('Schema', () => { }); }); }); + + it('can add classes without needing an object', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'String'} + })) + .then(result => { + expect(result).toEqual({ + _id: 'NewClass', + objectId: 'string', + updatedAt: 'string', + createdAt: 'string' + }) + done(); + }); + }); + + it('will fail to create a class if that class was already created by an object', done => { + config.database.loadSchema() + .then(schema => { + schema.validateObject('NewClass', {foo: 7}) + .then(() => { + schema.addClassIfNotExists('NewClass', { + foo: {type: 'String'} + }).catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME) + expect(error.error).toEqual('class NewClass already exists'); + done(); + }); + }); + }) + }); + + it('will resolve class creation races appropriately', done => { + // If two callers race to create the same schema, the response to the + // loser should be the same as if they hadn't been racing. Furthermore, + // The caller that wins the race should resolve it's promise before the + // caller that loses the race. + config.database.loadSchema() + .then(schema => { + var p1 = schema.addClassIfNotExists('NewClass', {foo: {type: 'String'}}); + var p2 = schema.addClassIfNotExists('NewClass', {foo: {type: 'String'}}); + var raceWinnerHasSucceeded = false; + var raceLoserHasFailed = false; + Promise.race([p1, p2]) //Use race because we expect the first completed promise to be the successful one + .then(response => { + raceWinnerHasSucceeded = true; + expect(raceLoserHasFailed).toEqual(false); + expect(response).toEqual({ + _id: 'NewClass', + objectId: 'string', + updatedAt: 'string', + createdAt: 'string' + }); + }); + Promise.all([p1,p2]) + .catch(error => { + expect(raceWinnerHasSucceeded).toEqual(true); + expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME); + expect(error.error).toEqual('class NewClass already exists'); + done(); + raceLoserHasFailed = true; + }); + }); + }); + + it('refuses to create classes with invalid names', done => { + config.database.loadSchema() + .then(schema => { + schema.addClassIfNotExists('_InvalidName', {foo: {type: 'String'}}) + .catch(error => { + expect(error.error).toEqual( + 'Invalid classname: _InvalidName, classnames can only have alphanumeric characters and _, and must start with an alpha character ' + ); + done(); + }); + }); + }); + + it('refuses to add fields with invalid names', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', {'0InvalidName': {type: 'String'}})) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_KEY_NAME); + expect(error.error).toEqual('invalid field name: 0InvalidName'); + done(); + }); + }); + + it('refuses to explicitly create the default fields', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('_Installation', {localeIdentifier: {type: 'String'}})) + .catch(error => { + expect(error.code).toEqual(136); + expect(error.error).toEqual('field localeIdentifier cannot be added'); + done(); + }); + }); }); From e6feefd223ffcf173d1f5d74c13dbd51529c14b3 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 5 Feb 2016 11:21:17 -0800 Subject: [PATCH 2/5] Add and test logic for adding fields to the DB --- ExportAdapter.js | 1 + Schema.js | 34 +++++++++-- spec/Schema.spec.js | 139 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 167 insertions(+), 7 deletions(-) diff --git a/ExportAdapter.js b/ExportAdapter.js index 4a626fa2b9..df417ac83a 100644 --- a/ExportAdapter.js +++ b/ExportAdapter.js @@ -494,6 +494,7 @@ ExportAdapter.prototype.smartFind = function(coll, where, options) { var index = {}; index[key] = '2d'; + //TODO: condiser moving index creation logic into Schema.js return coll.createIndex(index).then(() => { // Retry, but just once. return coll.find(where, options).toArray(); diff --git a/Schema.js b/Schema.js index 5514dca263..b0ea4d816d 100644 --- a/Schema.js +++ b/Schema.js @@ -137,7 +137,7 @@ function schemaAPITypeToMongoFieldType(type) { return { error: "invalid JSON", code: Parse.Error.INVALID_JSON }; } switch (type.type) { - default : return { error: 'invalid field type: ' + type.type }; + default: return { error: 'invalid field type: ' + type.type }; case 'Number': return { result: 'number' }; case 'String': return { result: 'string' }; case 'Boolean': return { result: 'boolean' }; @@ -241,14 +241,38 @@ Schema.prototype.addClassIfNotExists = function(className, fields) { } } - - - return this.collection.insertOne({ + var mongoObject = { _id: className, objectId: 'string', updatedAt: 'string', createdAt: 'string', - }) + }; + for (fieldName in defaultColumns[className]) { + validatedField = schemaAPITypeToMongoFieldType(defaultColumns[className][fieldName]); + if (validatedField.code) { + return Promise.reject(validatedField); + } + mongoObject[fieldName] = validatedField.result; + } + + for (fieldName in fields) { + validatedField = schemaAPITypeToMongoFieldType(fields[fieldName]); + if (validatedField.code) { + return Promise.reject(validatedField); + } + mongoObject[fieldName] = validatedField.result; + } + + var geoPoints = Object.keys(mongoObject).filter(key => mongoObject[key] === 'geopoint'); + + if (geoPoints.length > 1) { + return Promise.reject({ + code: Parse.Error.INCORRECT_TYPE, + error: 'currently, only one GeoPoint field may exist in an object. Adding ' + geoPoints[1] + ' when ' + geoPoints[0] + ' already exists.', + }); + } + + return this.collection.insertOne(mongoObject) .then(result => result.ops[0]) .catch(error => { if (error.code === 11000) { //Mongo's duplicate key error diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index c142f628b8..ce6f596875 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -1,6 +1,7 @@ // These tests check that the Schema operates correctly. var Config = require('../Config'); var Schema = require('../Schema'); +var dd = require('deep-diff'); var config = new Config('test'); @@ -142,7 +143,8 @@ describe('Schema', () => { _id: 'NewClass', objectId: 'string', updatedAt: 'string', - createdAt: 'string' + createdAt: 'string', + foo: 'string', }) done(); }); @@ -183,7 +185,8 @@ describe('Schema', () => { _id: 'NewClass', objectId: 'string', updatedAt: 'string', - createdAt: 'string' + createdAt: 'string', + foo: 'string', }); }); Promise.all([p1,p2]) @@ -229,4 +232,136 @@ describe('Schema', () => { done(); }); }); + + it('refuses to add fields with invalid types', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 7} + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_JSON); + expect(error.error).toEqual('invalid JSON'); + done(); + }); + }); + + it('refuses to add fields with invalid pointer types', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Pointer'}, + })) + .catch(error => { + expect(error.code).toEqual(135); + expect(error.error).toEqual('type Pointer needs a class name'); + done(); + }); + }); + + it('refuses to add fields with invalid pointer target', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Pointer', targetClass: 7}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_JSON); + expect(error.error).toEqual('invalid JSON'); + done(); + }); + }); + + it('refuses to add fields with invalid Relation type', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Relation', uselessKey: 7}, + })) + .catch(error => { + expect(error.code).toEqual(135); + expect(error.error).toEqual('type Relation needs a class name'); + done(); + }); + }); + + it('refuses to add fields with invalid relation target', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Relation', targetClass: 7}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_JSON); + expect(error.error).toEqual('invalid JSON'); + done(); + }); + }); + + it('refuses to add fields with uncreatable pointer target class', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Pointer', targetClass: 'not a valid class name'}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME); + expect(error.error).toEqual('Invalid classname: not a valid class name, classnames can only have alphanumeric characters and _, and must start with an alpha character '); + done(); + }); + }); + + it('refuses to add fields with uncreatable relation target class', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Relation', targetClass: 'not a valid class name'}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME); + expect(error.error).toEqual('Invalid classname: not a valid class name, classnames can only have alphanumeric characters and _, and must start with an alpha character '); + done(); + }); + }); + + it('will create classes', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + aNumber: {type: 'Number'}, + aString: {type: 'String'}, + aBool: {type: 'Boolean'}, + aDate: {type: 'Date'}, + aObject: {type: 'Object'}, + aArray: {type: 'Array'}, + aGeoPoint: {type: 'GeoPoint'}, + aFile: {type: 'File'}, + aPointer: {type: 'Pointer', targetClass: 'ThisClassDoesNotExistYet'}, + aRelation: {type: 'Relation', targetClass: 'NewClass'}, + })) + .then(mongoObj => { + expect(mongoObj).toEqual({ + _id: 'NewClass', + objectId: 'string', + createdAt: 'string', + updatedAt: 'string', + aNumber: 'number', + aString: 'string', + aBool: 'boolean', + aDate: 'date', + aObject: 'object', + aArray: 'array', + aGeoPoint: 'geopoint', + aFile: 'file', + aPointer: '*ThisClassDoesNotExistYet', + aRelation: 'relation', + }); + done(); + }); + }); + + it('refuses to create two geopoints', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + geo1: {type: 'GeoPoint'}, + geo2: {type: 'GeoPoint'}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INCORRECT_TYPE); + expect(error.error).toEqual('currently, only one GeoPoint field may exist in an object. Adding geo2 when geo1 already exists.'); + done(); + }); + }); }); From b741539e25369c6979389e3dc24d95beded3f24a Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 5 Feb 2016 12:48:36 -0800 Subject: [PATCH 3/5] More tests --- Schema.js | 16 ++++--------- spec/Schema.spec.js | 56 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/Schema.js b/Schema.js index b0ea4d816d..3f590ac7d2 100644 --- a/Schema.js +++ b/Schema.js @@ -100,9 +100,6 @@ function fieldNameIsValidForClass(fieldName, className) { } function invalidClassNameMessage(className) { - if (!className) { - className = ''; - } return 'Invalid classname: ' + className + ', classnames can only have alphanumeric characters and _, and must start with an alpha character '; } @@ -137,7 +134,7 @@ function schemaAPITypeToMongoFieldType(type) { return { error: "invalid JSON", code: Parse.Error.INVALID_JSON }; } switch (type.type) { - default: return { error: 'invalid field type: ' + type.type }; + default: return { error: 'invalid field type: ' + type.type, code: Parse.Error.INCORRECT_TYPE }; case 'Number': return { result: 'number' }; case 'String': return { result: 'string' }; case 'Boolean': return { result: 'boolean' }; @@ -211,10 +208,10 @@ Schema.prototype.reload = function() { // enabled) before calling this function. Schema.prototype.addClassIfNotExists = function(className, fields) { if (this.data[className]) { - return Promise.reject(new Parse.Error( - Parse.Error.DUPLICATE_VALUE, - 'class ' + className + ' already exists' - )); + return Promise.reject({ + code: Parse.Error.INVALID_CLASS_NAME, + error: 'class ' + className + ' already exists', + }); } if (!classNameIsValid(className)) { @@ -222,9 +219,6 @@ Schema.prototype.addClassIfNotExists = function(className, fields) { code: Parse.Error.INVALID_CLASS_NAME, error: invalidClassNameMessage(className), }); - return Promise.reject({ - code: Parse.Error.INVALID_CLASS_NAME, - }); } for (fieldName in fields) { if (!fieldNameIsValid(fieldName)) { diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index ce6f596875..a9cd62719e 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -155,9 +155,11 @@ describe('Schema', () => { .then(schema => { schema.validateObject('NewClass', {foo: 7}) .then(() => { - schema.addClassIfNotExists('NewClass', { + schema.reload() + .then(schema => schema.addClassIfNotExists('NewClass', { foo: {type: 'String'} - }).catch(error => { + })) + .catch(error => { expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME) expect(error.error).toEqual('class NewClass already exists'); done(); @@ -223,7 +225,17 @@ describe('Schema', () => { }); }); - it('refuses to explicitly create the default fields', done => { + it('refuses to explicitly create the default fields for custom classes', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', {objectId: {type: 'String'}})) + .catch(error => { + expect(error.code).toEqual(136); + expect(error.error).toEqual('field objectId cannot be added'); + done(); + }); + }); + + it('refuses to explicitly create the default fields for non-custom classes', done => { config.database.loadSchema() .then(schema => schema.addClassIfNotExists('_Installation', {localeIdentifier: {type: 'String'}})) .catch(error => { @@ -317,6 +329,18 @@ describe('Schema', () => { }); }); + it('refuses to add fields with unknown types', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('NewClass', { + foo: {type: 'Unknown'}, + })) + .catch(error => { + expect(error.code).toEqual(Parse.Error.INCORRECT_TYPE); + expect(error.error).toEqual('invalid field type: Unknown'); + done(); + }); + }); + it('will create classes', done => { config.database.loadSchema() .then(schema => schema.addClassIfNotExists('NewClass', { @@ -352,6 +376,32 @@ describe('Schema', () => { }); }); + it('creates the default fields for non-custom classes', done => { + config.database.loadSchema() + .then(schema => schema.addClassIfNotExists('_Installation', { + foo: {type: 'Number'}, + })) + .then(mongoObj => { + expect(mongoObj).toEqual({ + _id: '_Installation', + createdAt: 'string', + updatedAt: 'string', + objectId: 'string', + foo: 'number', + installationId: 'string', + deviceToken: 'string', + channels: 'array', + deviceType: 'string', + pushType: 'string', + GCMSenderId: 'string', + timeZone: 'string', + localeIdentifier: 'string', + badge: 'number', + }); + done(); + }); + }); + it('refuses to create two geopoints', done => { config.database.loadSchema() .then(schema => schema.addClassIfNotExists('NewClass', { From e2e0a26e9a7cefc69e501ff9d2cfbc12aef10083 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 5 Feb 2016 12:56:45 -0800 Subject: [PATCH 4/5] Updates tests to allow calls that race to create a schema to have the race loser return before the race winner. This test failed in mongo 2.6.11, and I don't know if thats because it's generally flaky or if that version of mongo makes less guarantees. --- spec/Schema.spec.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index a9cd62719e..ccc83525b6 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -170,19 +170,13 @@ describe('Schema', () => { it('will resolve class creation races appropriately', done => { // If two callers race to create the same schema, the response to the - // loser should be the same as if they hadn't been racing. Furthermore, - // The caller that wins the race should resolve it's promise before the - // caller that loses the race. + // race loser should be the same as if they hadn't been racing. config.database.loadSchema() .then(schema => { var p1 = schema.addClassIfNotExists('NewClass', {foo: {type: 'String'}}); var p2 = schema.addClassIfNotExists('NewClass', {foo: {type: 'String'}}); - var raceWinnerHasSucceeded = false; - var raceLoserHasFailed = false; Promise.race([p1, p2]) //Use race because we expect the first completed promise to be the successful one .then(response => { - raceWinnerHasSucceeded = true; - expect(raceLoserHasFailed).toEqual(false); expect(response).toEqual({ _id: 'NewClass', objectId: 'string', @@ -193,11 +187,9 @@ describe('Schema', () => { }); Promise.all([p1,p2]) .catch(error => { - expect(raceWinnerHasSucceeded).toEqual(true); expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME); expect(error.error).toEqual('class NewClass already exists'); done(); - raceLoserHasFailed = true; }); }); }); From a5440bcc404c343e7a87c5f55f8b329d9c3c29fb Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 5 Feb 2016 18:53:06 -0800 Subject: [PATCH 5/5] Address review comments --- Schema.js | 3 ++- schemas.js | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Schema.js b/Schema.js index 3f590ac7d2..25e301cdd9 100644 --- a/Schema.js +++ b/Schema.js @@ -76,7 +76,8 @@ function classNameIsValid(className) { className === '_SCHEMA' || //TODO: remove this, as _SCHEMA is not a valid class name for storing Parse Objects. className === '_Role' || joinClassRegex.test(className) || - classAndFieldRegex.test(className) + //Class names have the same constraints as field names, but also allow the previous additional names. + fieldNameIsValid(className) ); } diff --git a/schemas.js b/schemas.js index 4a8d50b4ee..875967cde7 100644 --- a/schemas.js +++ b/schemas.js @@ -32,10 +32,10 @@ function mongoFieldTypeToSchemaAPIType(type) { function mongoSchemaAPIResponseFields(schema) { fieldNames = Object.keys(schema).filter(key => key !== '_id'); - response = {}; - fieldNames.forEach(fieldName => { - response[fieldName] = mongoFieldTypeToSchemaAPIType(schema[fieldName]); - }); + response = fieldNames.reduce((obj, fieldName) => { + obj[fieldName] = mongoFieldTypeToSchemaAPIType(schema[fieldName]) + return obj; + }, {}); response.ACL = {type: 'ACL'}; response.createdAt = {type: 'Date'}; response.updatedAt = {type: 'Date'};