diff --git a/spec/.eslintrc.json b/spec/.eslintrc.json index 0814f305ce..d5b3e02c75 100644 --- a/spec/.eslintrc.json +++ b/spec/.eslintrc.json @@ -13,6 +13,7 @@ "Item": true, "Container": true, "equal": true, + "expectAsync": true, "notEqual": true, "it_only_db": true, "it_exclude_dbs": true, diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index ec40c1080d..e9471621cd 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -286,4 +286,27 @@ describe_only_db('mongo')('MongoStorageAdapter', () => { done(); }); }); + + it('getClass if exists', async () => { + const adapter = new MongoStorageAdapter({ uri: databaseURI }); + + const schema = { + fields: { + array: { type: 'Array' }, + object: { type: 'Object' }, + date: { type: 'Date' }, + }, + }; + + await adapter.createClass('MyClass', schema); + const myClassSchema = await adapter.getClass('MyClass'); + expect(myClassSchema).toBeDefined(); + }); + + it('getClass if not exists', async () => { + const adapter = new MongoStorageAdapter({ uri: databaseURI }); + await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith( + undefined + ); + }); }); diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index c557b472b1..b6fa7e2626 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -114,6 +114,33 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { }) .catch(done); }); + + it('getClass if exists', async () => { + const schema = { + fields: { + array: { type: 'Array' }, + object: { type: 'Object' }, + date: { type: 'Date' }, + }, + }; + await adapter.createClass('MyClass', schema); + const myClassSchema = await adapter.getClass('MyClass'); + expect(myClassSchema).toBeDefined(); + }); + + it('getClass if not exists', async () => { + const schema = { + fields: { + array: { type: 'Array' }, + object: { type: 'Object' }, + date: { type: 'Date' }, + }, + }; + await adapter.createClass('MyClass', schema); + await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith( + undefined + ); + }); }); describe_only_db('postgres')('PostgresStorageAdapter shutdown', () => { diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 10653a5f2e..637d9d67c7 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -1,5 +1,7 @@ const RedisCacheAdapter = require('../lib/Adapters/Cache/RedisCacheAdapter') .default; +const Config = require('../lib/Config'); + /* To run this test part of the complete suite set PARSE_SERVER_TEST_CACHE='redis' @@ -163,3 +165,168 @@ describe_only(() => { .then(done); }); }); + +describe_only(() => { + return process.env.PARSE_SERVER_TEST_CACHE === 'redis'; +})('Redis Performance', function() { + const cacheAdapter = new RedisCacheAdapter(); + let getSpy; + let putSpy; + + beforeEach(async () => { + await cacheAdapter.clear(); + getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); + putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); + await reconfigureServer({ + cacheAdapter, + enableSingleSchemaCache: true, + }); + }); + + it('test new object', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(3); + }); + + it('test new object multiple fields', async () => { + const container = new Container({ + dateField: new Date(), + arrayField: [], + numberField: 1, + stringField: 'hello', + booleanField: true, + }); + await container.save(); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(3); + }); + + it('test update existing fields', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + object.set('foo', 'barz'); + await object.save(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(0); + }); + + it('test add new field to existing object', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + object.set('new', 'barz'); + await object.save(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(1); + }); + + it('test add multiple fields to existing object', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + object.set({ + dateField: new Date(), + arrayField: [], + numberField: 1, + stringField: 'hello', + booleanField: true, + }); + await object.save(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(1); + }); + + it('test query', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + const query = new Parse.Query(TestObject); + await query.get(object.id); + expect(getSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(0); + }); + + it('test delete object', async () => { + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + await object.destroy(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(0); + }); + + it('test schema update class', async () => { + const container = new Container(); + await container.save(); + + getSpy.calls.reset(); + putSpy.calls.reset(); + + const config = Config.get('test'); + const schema = await config.database.loadSchema(); + await schema.reloadData(); + + const levelPermissions = { + find: { '*': true }, + get: { '*': true }, + create: { '*': true }, + update: { '*': true }, + delete: { '*': true }, + addField: { '*': true }, + protectedFields: { '*': [] }, + }; + + await schema.updateClass( + 'Container', + { + fooOne: { type: 'Number' }, + fooTwo: { type: 'Array' }, + fooThree: { type: 'Date' }, + fooFour: { type: 'Object' }, + fooFive: { type: 'Relation', targetClass: '_User' }, + fooSix: { type: 'String' }, + fooSeven: { type: 'Object' }, + fooEight: { type: 'String' }, + fooNine: { type: 'String' }, + fooTeen: { type: 'Number' }, + fooEleven: { type: 'String' }, + fooTwelve: { type: 'String' }, + fooThirteen: { type: 'String' }, + fooFourteen: { type: 'String' }, + fooFifteen: { type: 'String' }, + fooSixteen: { type: 'String' }, + fooEighteen: { type: 'String' }, + fooNineteen: { type: 'String' }, + }, + levelPermissions, + {}, + config.database + ); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(2); + }); +}); diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index b95783023e..716778ceba 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -1363,6 +1363,47 @@ describe('SchemaController', () => { .then(done) .catch(done.fail); }); + + it('setAllClasses return classes if cache fails', async () => { + const schema = await config.database.loadSchema(); + + spyOn(schema._cache, 'setAllClasses').and.callFake(() => + Promise.reject('Oops!') + ); + const errorSpy = spyOn(console, 'error').and.callFake(() => {}); + const allSchema = await schema.setAllClasses(); + + expect(allSchema).toBeDefined(); + expect(errorSpy).toHaveBeenCalledWith( + 'Error saving schema to cache:', + 'Oops!' + ); + }); + + it('should not throw on null field types', async () => { + const schema = await config.database.loadSchema(); + const result = await schema.enforceFieldExists( + 'NewClass', + 'fieldName', + null + ); + expect(result).toBeUndefined(); + }); + + it('ensureFields should throw when schema is not set', async () => { + const schema = await config.database.loadSchema(); + try { + schema.ensureFields([ + { + className: 'NewClass', + fieldName: 'fieldName', + type: 'String', + }, + ]); + } catch (e) { + expect(e.message).toBe('Could not add field fieldName'); + } + }); }); describe('Class Level Permissions for requiredAuth', () => { diff --git a/spec/SchemaCache.spec.js b/spec/SchemaCache.spec.js index a0ed509f2f..26897e03f7 100644 --- a/spec/SchemaCache.spec.js +++ b/spec/SchemaCache.spec.js @@ -33,28 +33,12 @@ describe('SchemaCache', () => { }); }); - it('does not return all schemas after a single schema is stored', done => { - const schemaCache = new SchemaCache(cacheController); - const schema = { - className: 'Class1', - }; - schemaCache - .setOneSchema(schema.className, schema) - .then(() => { - return schemaCache.getAllClasses(); - }) - .then(allSchemas => { - expect(allSchemas).toBeNull(); - done(); - }); - }); - it("doesn't persist cached data by default", done => { const schemaCache = new SchemaCache(cacheController); const schema = { className: 'Class1', }; - schemaCache.setOneSchema(schema.className, schema).then(() => { + schemaCache.setAllClasses([schema]).then(() => { const anotherSchemaCache = new SchemaCache(cacheController); return anotherSchemaCache.getOneSchema(schema.className).then(schema => { expect(schema).toBeNull(); @@ -68,7 +52,7 @@ describe('SchemaCache', () => { const schema = { className: 'Class1', }; - schemaCache.setOneSchema(schema.className, schema).then(() => { + schemaCache.setAllClasses([schema]).then(() => { const anotherSchemaCache = new SchemaCache(cacheController, 5000, true); return anotherSchemaCache.getOneSchema(schema.className).then(schema => { expect(schema).not.toBeNull(); @@ -76,4 +60,18 @@ describe('SchemaCache', () => { }); }); }); + + it('should not store if ttl is null', async () => { + const ttl = null; + const schemaCache = new SchemaCache(cacheController, ttl); + expect(await schemaCache.getAllClasses()).toBeNull(); + expect(await schemaCache.setAllClasses()).toBeNull(); + expect(await schemaCache.getOneSchema()).toBeNull(); + }); + + it('should convert string ttl to number', async () => { + const ttl = '5000'; + const schemaCache = new SchemaCache(cacheController, ttl); + expect(schemaCache.ttl).toBe(5000); + }); }); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 0b4bb2ab59..76efd6a06b 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -844,7 +844,6 @@ class DatabaseController { : schemaController.validatePermission(className, aclGroup, 'create') ) .then(() => schemaController.enforceClassExists(className)) - .then(() => schemaController.reloadData()) .then(() => schemaController.getOneSchema(className, true)) .then(schema => { transformAuthData(className, object, schema); diff --git a/src/Controllers/SchemaCache.js b/src/Controllers/SchemaCache.js index 007d51d308..55d70eabaa 100644 --- a/src/Controllers/SchemaCache.js +++ b/src/Controllers/SchemaCache.js @@ -1,6 +1,5 @@ const MAIN_SCHEMA = '__MAIN_SCHEMA'; const SCHEMA_CACHE_PREFIX = '__SCHEMA'; -const ALL_KEYS = '__ALL_KEYS'; import { randomString } from '../cryptoUtils'; import defaults from '../defaults'; @@ -24,17 +23,6 @@ export default class SchemaCache { } } - put(key, value) { - return this.cache.get(this.prefix + ALL_KEYS).then(allKeys => { - allKeys = allKeys || {}; - allKeys[key] = true; - return Promise.all([ - this.cache.put(this.prefix + ALL_KEYS, allKeys, this.ttl), - this.cache.put(key, value, this.ttl), - ]); - }); - } - getAllClasses() { if (!this.ttl) { return Promise.resolve(null); @@ -46,47 +34,26 @@ export default class SchemaCache { if (!this.ttl) { return Promise.resolve(null); } - return this.put(this.prefix + MAIN_SCHEMA, schema); - } - - setOneSchema(className, schema) { - if (!this.ttl) { - return Promise.resolve(null); - } - return this.put(this.prefix + className, schema); + return this.cache.put(this.prefix + MAIN_SCHEMA, schema); } getOneSchema(className) { if (!this.ttl) { return Promise.resolve(null); } - return this.cache.get(this.prefix + className).then(schema => { + return this.cache.get(this.prefix + MAIN_SCHEMA).then(cachedSchemas => { + cachedSchemas = cachedSchemas || []; + const schema = cachedSchemas.find(cachedSchema => { + return cachedSchema.className === className; + }); if (schema) { return Promise.resolve(schema); } - return this.cache.get(this.prefix + MAIN_SCHEMA).then(cachedSchemas => { - cachedSchemas = cachedSchemas || []; - schema = cachedSchemas.find(cachedSchema => { - return cachedSchema.className === className; - }); - if (schema) { - return Promise.resolve(schema); - } - return Promise.resolve(null); - }); + return Promise.resolve(null); }); } clear() { - // That clears all caches... - return this.cache.get(this.prefix + ALL_KEYS).then(allKeys => { - if (!allKeys) { - return; - } - const promises = Object.keys(allKeys).map(key => { - return this.cache.del(key); - }); - return Promise.all(promises); - }); + return this.cache.del(this.prefix + MAIN_SCHEMA); } } diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 0fac3022c8..01dc5f601f 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -549,29 +549,21 @@ export default class SchemaController { } reloadData(options: LoadSchemaOptions = { clearCache: false }): Promise { - let promise = Promise.resolve(); - if (options.clearCache) { - promise = promise.then(() => { - return this._cache.clear(); - }); - } if (this.reloadDataPromise && !options.clearCache) { return this.reloadDataPromise; } - this.reloadDataPromise = promise - .then(() => { - return this.getAllClasses(options).then( - allSchemas => { - this.schemaData = new SchemaData(allSchemas, this.protectedFields); - delete this.reloadDataPromise; - }, - err => { - this.schemaData = new SchemaData(); - delete this.reloadDataPromise; - throw err; - } - ); - }) + this.reloadDataPromise = this.getAllClasses(options) + .then( + allSchemas => { + this.schemaData = new SchemaData(allSchemas, this.protectedFields); + delete this.reloadDataPromise; + }, + err => { + this.schemaData = new SchemaData(); + delete this.reloadDataPromise; + throw err; + } + ) .then(() => {}); return this.reloadDataPromise; } @@ -579,26 +571,30 @@ export default class SchemaController { getAllClasses( options: LoadSchemaOptions = { clearCache: false } ): Promise> { - let promise = Promise.resolve(); if (options.clearCache) { - promise = this._cache.clear(); + return this.setAllClasses(); } - return promise - .then(() => { - return this._cache.getAllClasses(); - }) - .then(allClasses => { - if (allClasses && allClasses.length && !options.clearCache) { - return Promise.resolve(allClasses); - } - return this._dbAdapter - .getAllClasses() - .then(allSchemas => allSchemas.map(injectDefaultSchema)) - .then(allSchemas => { - return this._cache.setAllClasses(allSchemas).then(() => { - return allSchemas; - }); - }); + return this._cache.getAllClasses().then(allClasses => { + if (allClasses && allClasses.length) { + return Promise.resolve(allClasses); + } + return this.setAllClasses(); + }); + } + + setAllClasses(): Promise> { + return this._dbAdapter + .getAllClasses() + .then(allSchemas => allSchemas.map(injectDefaultSchema)) + .then(allSchemas => { + /* eslint-disable no-console */ + this._cache + .setAllClasses(allSchemas) + .catch(error => + console.error('Error saving schema to cache:', error) + ); + /* eslint-enable no-console */ + return allSchemas; }); } @@ -625,14 +621,15 @@ export default class SchemaController { if (cached && !options.clearCache) { return Promise.resolve(cached); } - return this._dbAdapter - .getClass(className) - .then(injectDefaultSchema) - .then(result => { - return this._cache.setOneSchema(className, result).then(() => { - return result; - }); - }); + return this.setAllClasses().then(allSchemas => { + const oneSchema = allSchemas.find( + schema => schema.className === className + ); + if (!oneSchema) { + return Promise.reject(undefined); + } + return oneSchema; + }); }); }); } @@ -745,6 +742,7 @@ export default class SchemaController { if (deletedFields.length > 0) { deletePromise = this.deleteFields(deletedFields, className, database); } + let enforceFields = []; return ( deletePromise // Delete Everything .then(() => this.reloadData({ clearCache: true })) // Reload our Schema, so we have all the new values @@ -755,9 +753,10 @@ export default class SchemaController { }); return Promise.all(promises); }) - .then(() => - this.setPermissions(className, classLevelPermissions, newSchema) - ) + .then(results => { + enforceFields = results.filter(result => !!result); + this.setPermissions(className, classLevelPermissions, newSchema); + }) .then(() => this._dbAdapter.setIndexesWithSchemaFormat( className, @@ -769,6 +768,7 @@ export default class SchemaController { .then(() => this.reloadData({ clearCache: true })) //TODO: Move this logic into the database adapter .then(() => { + this.ensureFields(enforceFields); const schema = this.schemaData[className]; const reloadedSchema: Schema = { className: className, @@ -936,62 +936,62 @@ export default class SchemaController { // If someone tries to create a new field with null/undefined as the value, return; if (!type) { - return Promise.resolve(this); + return undefined; } - return this.reloadData().then(() => { - const expectedType = this.getExpectedType(className, fieldName); - if (typeof type === 'string') { - type = { type }; + const expectedType = this.getExpectedType(className, fieldName); + if (typeof type === 'string') { + type = { type }; + } + + if (expectedType) { + if (!dbTypeMatchesObjectType(expectedType, type)) { + throw new Parse.Error( + Parse.Error.INCORRECT_TYPE, + `schema mismatch for ${className}.${fieldName}; expected ${typeToString( + expectedType + )} but got ${typeToString(type)}` + ); } + return undefined; + } - if (expectedType) { - if (!dbTypeMatchesObjectType(expectedType, type)) { - throw new Parse.Error( - Parse.Error.INCORRECT_TYPE, - `schema mismatch for ${className}.${fieldName}; expected ${typeToString( - expectedType - )} but got ${typeToString(type)}` - ); + return this._dbAdapter + .addFieldIfNotExists(className, fieldName, type) + .catch(error => { + if (error.code == Parse.Error.INCORRECT_TYPE) { + // Make sure that we throw errors when it is appropriate to do so. + throw error; } - return this; - } + // The update failed. This can be okay - it might have been a race + // condition where another client updated the schema in the same + // way that we wanted to. So, just reload the schema + return Promise.resolve(); + }) + .then(() => { + return { + className, + fieldName, + type, + }; + }); + } - return this._dbAdapter - .addFieldIfNotExists(className, fieldName, type) - .then( - () => { - // The update succeeded. Reload the schema - return this.reloadData({ clearCache: true }); - }, - error => { - if (error.code == Parse.Error.INCORRECT_TYPE) { - // Make sure that we throw errors when it is appropriate to do so. - throw error; - } - // The update failed. This can be okay - it might have been a race - // condition where another client updated the schema in the same - // way that we wanted to. So, just reload the schema - return this.reloadData({ clearCache: true }); - } - ) - .then(() => { - // Ensure that the schema now validates - const expectedType = this.getExpectedType(className, fieldName); - if (typeof type === 'string') { - type = { type }; - } - if (!expectedType || !dbTypeMatchesObjectType(expectedType, type)) { - throw new Parse.Error( - Parse.Error.INVALID_JSON, - `Could not add field ${fieldName}` - ); - } - // Remove the cached schema - this._cache.clear(); - return this; - }); - }); + ensureFields(fields: any) { + for (let i = 0; i < fields.length; i += 1) { + const { className, fieldName } = fields[i]; + let { type } = fields[i]; + const expectedType = this.getExpectedType(className, fieldName); + if (typeof type === 'string') { + type = { type: type }; + } + if (!expectedType || !dbTypeMatchesObjectType(expectedType, type)) { + throw new Parse.Error( + Parse.Error.INVALID_JSON, + `Could not add field ${fieldName}` + ); + } + } } // maintain compatibility @@ -1074,17 +1074,17 @@ export default class SchemaController { ); }); }) - .then(() => { - this._cache.clear(); - }); + .then(() => this._cache.clear()); } // Validates an object provided in REST format. // Returns a promise that resolves to the new schema if this object is // valid. - validateObject(className: string, object: any, query: any) { + async validateObject(className: string, object: any, query: any) { let geocount = 0; - let promise = this.enforceClassExists(className); + const schema = await this.enforceClassExists(className); + const promises = []; + for (const fieldName in object) { if (object[fieldName] === undefined) { continue; @@ -1096,14 +1096,12 @@ export default class SchemaController { if (geocount > 1) { // Make sure all field validation operations run before we return. // If not - we are continuing to run logic, but already provided response from the server. - return promise.then(() => { - return Promise.reject( - new Parse.Error( - Parse.Error.INCORRECT_TYPE, - 'there can only be one geopoint field in a class' - ) - ); - }); + return Promise.reject( + new Parse.Error( + Parse.Error.INCORRECT_TYPE, + 'there can only be one geopoint field in a class' + ) + ); } if (!expected) { continue; @@ -1112,13 +1110,18 @@ export default class SchemaController { // Every object has ACL implicitly. continue; } + promises.push(schema.enforceFieldExists(className, fieldName, expected)); + } + const results = await Promise.all(promises); + const enforceFields = results.filter(result => !!result); - promise = promise.then(schema => - schema.enforceFieldExists(className, fieldName, expected) - ); + if (enforceFields.length !== 0) { + await this.reloadData({ clearCache: true }); } - promise = thenValidateRequiredColumns(promise, className, object, query); - return promise; + this.ensureFields(enforceFields); + + const promise = Promise.resolve(schema); + return thenValidateRequiredColumns(promise, className, object, query); } // Validates that all the properties are set for the object