From d80d7f0fac7bd9ab7d0827e94d19a768b8fe4530 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Wed, 22 May 2019 17:26:40 -0500 Subject: [PATCH 01/17] Cache Improvements --- example.js | 20 +++++++ package.json | 2 +- spec/ParseQuery.spec.js | 30 ++++++++++ src/Controllers/DatabaseController.js | 1 - src/Controllers/SchemaCache.js | 17 ++---- src/Controllers/SchemaController.js | 84 +++++++++++---------------- 6 files changed, 92 insertions(+), 62 deletions(-) create mode 100644 example.js diff --git a/example.js b/example.js new file mode 100644 index 0000000000..ea49991264 --- /dev/null +++ b/example.js @@ -0,0 +1,20 @@ +var redis = require("redis"), + client = redis.createClient(); + +// if you'd like to select database 3, instead of 0 (default), call +// client.select(3, function() { /* ... */ }); + +client.on("error", function (err) { + console.log("Error " + err); +}); + +client.set("string key", "string val", redis.print); +client.hset("hash key", "hashtest 1", "some value", redis.print); +client.hset(["hash key", "hashtest 2", "some other value"], redis.print); +client.hkeys("hash key", function (err, replies) { + console.log(replies.length + " replies:"); + replies.forEach(function (reply, i) { + console.log(" " + i + ": " + reply); + }); + client.quit(); +}); diff --git a/package.json b/package.json index cb4205f948..48fe3dd682 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "lint": "flow && eslint --cache ./", "build": "babel src/ -d lib/ --copy-files", "watch": "babel --watch src/ -d lib/ --copy-files", - "test": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_STORAGE_ENGINE=mmapv1 TESTING=1 jasmine", + "test": "cross-env PARSE_SERVER_TEST_CACHE=redis TESTING=1 jasmine", "coverage": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_STORAGE_ENGINE=mmapv1 TESTING=1 nyc jasmine", "start": "node ./bin/parse-server", "prepare": "npm run build", diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 512fb82a13..116ebc97fd 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -6,6 +6,8 @@ const Parse = require('parse/node'); const request = require('../lib/request'); +const RedisCacheAdapter = require('../lib/Adapters/Cache/RedisCacheAdapter') + .default; const masterKeyHeaders = { 'X-Parse-Application-Id': 'test', @@ -32,6 +34,34 @@ describe('Parse.Query testing', () => { }); }); }); + fit('empty', done => done()); + + const cacheAdapter = new RedisCacheAdapter(); + fit('redis test', async () => { + try { + await reconfigureServer({ cacheAdapter: cacheAdapter }); + console.log('startTest'); + spyOn(cacheAdapter, 'get').and.callThrough(); + spyOn(cacheAdapter, 'del').and.callThrough(); + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + console.log(cacheAdapter.get.calls.count()); + console.log(cacheAdapter.del.calls.count()); + object.set('foo', 'barz'); + await object.save(); + console.log(cacheAdapter.get.calls.count()); + console.log(cacheAdapter.del.calls.count()); + object.set('new', 'barz'); + await object.save(); + console.log(cacheAdapter.get.calls.count()); + console.log(cacheAdapter.del.calls.count()); + await cacheAdapter.clear(); + console.log('done'); + } catch (e) { + console.log(e); + } + }); it('searching for null', function(done) { const baz = new TestObject({ foo: null }); 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..07f8bbe704 100644 --- a/src/Controllers/SchemaCache.js +++ b/src/Controllers/SchemaCache.js @@ -60,20 +60,15 @@ export default class SchemaCache { 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); }); } diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 0fac3022c8..54e4606259 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,25 @@ 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 => { + return this._cache.setAllClasses(allSchemas).then(() => { + return allSchemas; + }); }); } @@ -625,14 +616,9 @@ 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 => { + return allSchemas.find(schema => schema.className === className); + }); }); }); } @@ -766,7 +752,6 @@ export default class SchemaController { fullNewSchema ) ) - .then(() => this.reloadData({ clearCache: true })) //TODO: Move this logic into the database adapter .then(() => { const schema = this.schemaData[className]; @@ -987,8 +972,6 @@ export default class SchemaController { `Could not add field ${fieldName}` ); } - // Remove the cached schema - this._cache.clear(); return this; }); }); @@ -1282,6 +1265,9 @@ export default class SchemaController { // Checks if a given class is in the schema. hasClass(className: string) { + if (this.schemaData[className]) { + return Promise.resolve(this); + } return this.reloadData().then(() => !!this.schemaData[className]); } } From 3e4dd59e33f5a0bf51f5e4c0390841472cea8034 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 10:39:37 -0500 Subject: [PATCH 02/17] improve tests --- example.js | 20 --------------- package.json | 2 +- spec/ParseQuery.spec.js | 30 ----------------------- spec/RedisCacheAdapter.spec.js | 45 ++++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 51 deletions(-) delete mode 100644 example.js diff --git a/example.js b/example.js deleted file mode 100644 index ea49991264..0000000000 --- a/example.js +++ /dev/null @@ -1,20 +0,0 @@ -var redis = require("redis"), - client = redis.createClient(); - -// if you'd like to select database 3, instead of 0 (default), call -// client.select(3, function() { /* ... */ }); - -client.on("error", function (err) { - console.log("Error " + err); -}); - -client.set("string key", "string val", redis.print); -client.hset("hash key", "hashtest 1", "some value", redis.print); -client.hset(["hash key", "hashtest 2", "some other value"], redis.print); -client.hkeys("hash key", function (err, replies) { - console.log(replies.length + " replies:"); - replies.forEach(function (reply, i) { - console.log(" " + i + ": " + reply); - }); - client.quit(); -}); diff --git a/package.json b/package.json index 48fe3dd682..cb4205f948 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "lint": "flow && eslint --cache ./", "build": "babel src/ -d lib/ --copy-files", "watch": "babel --watch src/ -d lib/ --copy-files", - "test": "cross-env PARSE_SERVER_TEST_CACHE=redis TESTING=1 jasmine", + "test": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_STORAGE_ENGINE=mmapv1 TESTING=1 jasmine", "coverage": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_STORAGE_ENGINE=mmapv1 TESTING=1 nyc jasmine", "start": "node ./bin/parse-server", "prepare": "npm run build", diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 116ebc97fd..512fb82a13 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -6,8 +6,6 @@ const Parse = require('parse/node'); const request = require('../lib/request'); -const RedisCacheAdapter = require('../lib/Adapters/Cache/RedisCacheAdapter') - .default; const masterKeyHeaders = { 'X-Parse-Application-Id': 'test', @@ -34,34 +32,6 @@ describe('Parse.Query testing', () => { }); }); }); - fit('empty', done => done()); - - const cacheAdapter = new RedisCacheAdapter(); - fit('redis test', async () => { - try { - await reconfigureServer({ cacheAdapter: cacheAdapter }); - console.log('startTest'); - spyOn(cacheAdapter, 'get').and.callThrough(); - spyOn(cacheAdapter, 'del').and.callThrough(); - const object = new TestObject(); - object.set('foo', 'bar'); - await object.save(); - console.log(cacheAdapter.get.calls.count()); - console.log(cacheAdapter.del.calls.count()); - object.set('foo', 'barz'); - await object.save(); - console.log(cacheAdapter.get.calls.count()); - console.log(cacheAdapter.del.calls.count()); - object.set('new', 'barz'); - await object.save(); - console.log(cacheAdapter.get.calls.count()); - console.log(cacheAdapter.del.calls.count()); - await cacheAdapter.clear(); - console.log('done'); - } catch (e) { - console.log(e); - } - }); it('searching for null', function(done) { const baz = new TestObject({ foo: null }); diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 10653a5f2e..0515887b9c 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -99,6 +99,51 @@ describe_only(() => { .then(value => expect(value).not.toEqual(null)) .then(done); }); + + it('redis performance test', async () => { + const cacheAdapter = new RedisCacheAdapter(); + await reconfigureServer({ cacheAdapter: cacheAdapter }); + await cacheAdapter.clear(); + const spy = spyOn(cacheAdapter, 'get').and.callThrough(); + + // New Object + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + expect(spy.calls.count()).toBe(18); + spy.calls.reset(); + + // Update Existing Field + object.set('foo', 'barz'); + await object.save(); + expect(spy.calls.count()).toBe(8); + spy.calls.reset(); + + // Add New Field + object.set('new', 'barz'); + await object.save(); + expect(spy.calls.count()).toBe(13); + spy.calls.reset(); + + // Get Object + let query = new Parse.Query(TestObject); + await query.get(object.id); + expect(spy.calls.count()).toBe(4); + spy.calls.reset(); + + // Find Object + query = new Parse.Query(TestObject); + await query.find(); + expect(spy.calls.count()).toBe(4); + spy.calls.reset(); + + // Delete Object + await object.destroy(); + expect(spy.calls.count()).toBe(5); + spy.calls.reset(); + + await cacheAdapter.clear(); + }); }); describe_only(() => { From a603c83349401c1274d61df425b6d8b5045d7f3b Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 12:16:19 -0500 Subject: [PATCH 03/17] more tests --- spec/RedisCacheAdapter.spec.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 0515887b9c..332a27fe39 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -110,36 +110,46 @@ describe_only(() => { const object = new TestObject(); object.set('foo', 'bar'); await object.save(); - expect(spy.calls.count()).toBe(18); + expect(spy.calls.count()).toBe(8); spy.calls.reset(); // Update Existing Field object.set('foo', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(8); + expect(spy.calls.count()).toBe(5); spy.calls.reset(); // Add New Field object.set('new', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(13); + expect(spy.calls.count()).toBe(6); spy.calls.reset(); // Get Object let query = new Parse.Query(TestObject); await query.get(object.id); - expect(spy.calls.count()).toBe(4); + expect(spy.calls.count()).toBe(3); spy.calls.reset(); // Find Object query = new Parse.Query(TestObject); await query.find(); - expect(spy.calls.count()).toBe(4); + expect(spy.calls.count()).toBe(3); spy.calls.reset(); // Delete Object await object.destroy(); - expect(spy.calls.count()).toBe(5); + expect(spy.calls.count()).toBe(4); + spy.calls.reset(); + + const objects = []; + for (let i = 0; i < 100; i++) { + const obj = new TestObject(); + obj.set('number', i); + objects.push(obj); + } + await Parse.Object.saveAll(objects); + expect(spy.calls.count()).toBe(141); spy.calls.reset(); await cacheAdapter.clear(); From d67080b2cc19de71aec1a60e417c82ddeaf85976 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 12:50:24 -0500 Subject: [PATCH 04/17] clean-up --- spec/RedisCacheAdapter.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 332a27fe39..fb8b18a31b 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -153,6 +153,7 @@ describe_only(() => { spy.calls.reset(); await cacheAdapter.clear(); + await reconfigureServer(); }); }); From 6d8a75dbea33291b698f06a7e12299ac03016082 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 13:48:59 -0500 Subject: [PATCH 05/17] test with singlecache --- spec/RedisCacheAdapter.spec.js | 13 ++++++++----- src/Controllers/SchemaController.js | 8 +++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index fb8b18a31b..9bb0db3d82 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -102,7 +102,10 @@ describe_only(() => { it('redis performance test', async () => { const cacheAdapter = new RedisCacheAdapter(); - await reconfigureServer({ cacheAdapter: cacheAdapter }); + await reconfigureServer({ + cacheAdapter, + enableSingleSchemaCache: true, + }); await cacheAdapter.clear(); const spy = spyOn(cacheAdapter, 'get').and.callThrough(); @@ -122,7 +125,7 @@ describe_only(() => { // Add New Field object.set('new', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(6); + expect(spy.calls.count()).toBe(4); spy.calls.reset(); // Get Object @@ -134,12 +137,12 @@ describe_only(() => { // Find Object query = new Parse.Query(TestObject); await query.find(); - expect(spy.calls.count()).toBe(3); + expect(spy.calls.count()).toBe(2); spy.calls.reset(); // Delete Object await object.destroy(); - expect(spy.calls.count()).toBe(4); + expect(spy.calls.count()).toBe(3); spy.calls.reset(); const objects = []; @@ -149,7 +152,7 @@ describe_only(() => { objects.push(obj); } await Parse.Object.saveAll(objects); - expect(spy.calls.count()).toBe(141); + expect(spy.calls.count()).toBe(136); spy.calls.reset(); await cacheAdapter.clear(); diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 54e4606259..4a81173464 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -617,7 +617,13 @@ export default class SchemaController { return Promise.resolve(cached); } return this.setAllClasses().then(allSchemas => { - return allSchemas.find(schema => schema.className === className); + const oneSchema = allSchemas.find( + schema => schema.className === className + ); + if (!oneSchema) { + return Promise.reject(undefined); + } + return oneSchema; }); }); }); From eeec78dde5d166c9bd0520565fae80a6cd12f4b7 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 14:25:29 -0500 Subject: [PATCH 06/17] ensure indexes exists --- spec/SchemaCache.spec.js | 2 +- src/Controllers/SchemaController.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/SchemaCache.spec.js b/spec/SchemaCache.spec.js index a0ed509f2f..91ecfaf24d 100644 --- a/spec/SchemaCache.spec.js +++ b/spec/SchemaCache.spec.js @@ -68,7 +68,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(); diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 4a81173464..ce59719014 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -758,6 +758,7 @@ export default class SchemaController { fullNewSchema ) ) + .then(() => this.reloadData({ clearCache: true })) //TODO: Move this logic into the database adapter .then(() => { const schema = this.schemaData[className]; From 1f677536362c580de447eb028372b157b38d886a Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 15:44:21 -0500 Subject: [PATCH 07/17] remove ALL_KEYS --- spec/RedisCacheAdapter.spec.js | 8 ++++---- spec/SchemaCache.spec.js | 18 +---------------- src/Controllers/SchemaCache.js | 31 ++--------------------------- src/Controllers/SchemaController.js | 7 +------ 4 files changed, 8 insertions(+), 56 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 9bb0db3d82..c8511b99a5 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -113,13 +113,13 @@ describe_only(() => { const object = new TestObject(); object.set('foo', 'bar'); await object.save(); - expect(spy.calls.count()).toBe(8); + expect(spy.calls.count()).toBe(4); spy.calls.reset(); // Update Existing Field object.set('foo', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(5); + expect(spy.calls.count()).toBe(4); spy.calls.reset(); // Add New Field @@ -131,7 +131,7 @@ describe_only(() => { // Get Object let query = new Parse.Query(TestObject); await query.get(object.id); - expect(spy.calls.count()).toBe(3); + expect(spy.calls.count()).toBe(2); spy.calls.reset(); // Find Object @@ -152,7 +152,7 @@ describe_only(() => { objects.push(obj); } await Parse.Object.saveAll(objects); - expect(spy.calls.count()).toBe(136); + expect(spy.calls.count()).toBe(116); spy.calls.reset(); await cacheAdapter.clear(); diff --git a/spec/SchemaCache.spec.js b/spec/SchemaCache.spec.js index 91ecfaf24d..3bf6da92ef 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(); diff --git a/src/Controllers/SchemaCache.js b/src/Controllers/SchemaCache.js index 07f8bbe704..8b6371d076 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,14 +34,7 @@ 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) { @@ -74,14 +55,6 @@ export default class SchemaCache { 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 ce59719014..6d7d192803 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -1064,9 +1064,7 @@ export default class SchemaController { ); }); }) - .then(() => { - this._cache.clear(); - }); + .then(() => this.setAllClasses()); } // Validates an object provided in REST format. @@ -1272,9 +1270,6 @@ export default class SchemaController { // Checks if a given class is in the schema. hasClass(className: string) { - if (this.schemaData[className]) { - return Promise.resolve(this); - } return this.reloadData().then(() => !!this.schemaData[className]); } } From 98bbfe5a50c5bf0c4730473935e0f6061ce25169 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 16:16:08 -0500 Subject: [PATCH 08/17] Add Insert Test --- spec/RedisCacheAdapter.spec.js | 45 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index c8511b99a5..9f838cd722 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -107,43 +107,56 @@ describe_only(() => { enableSingleSchemaCache: true, }); await cacheAdapter.clear(); - const spy = spyOn(cacheAdapter, 'get').and.callThrough(); + const getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); + const putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); // New Object const object = new TestObject(); object.set('foo', 'bar'); await object.save(); - expect(spy.calls.count()).toBe(4); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(3); + getSpy.calls.reset(); + putSpy.calls.reset(); // Update Existing Field object.set('foo', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(4); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(0); + getSpy.calls.reset(); + putSpy.calls.reset(); // Add New Field object.set('new', 'barz'); await object.save(); - expect(spy.calls.count()).toBe(4); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(1); + getSpy.calls.reset(); + putSpy.calls.reset(); // Get Object let query = new Parse.Query(TestObject); await query.get(object.id); - expect(spy.calls.count()).toBe(2); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(0); + getSpy.calls.reset(); + putSpy.calls.reset(); // Find Object query = new Parse.Query(TestObject); await query.find(); - expect(spy.calls.count()).toBe(2); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(0); + getSpy.calls.reset(); + putSpy.calls.reset(); // Delete Object await object.destroy(); - expect(spy.calls.count()).toBe(3); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(0); + getSpy.calls.reset(); + putSpy.calls.reset(); const objects = []; for (let i = 0; i < 100; i++) { @@ -152,8 +165,10 @@ describe_only(() => { objects.push(obj); } await Parse.Object.saveAll(objects); - expect(spy.calls.count()).toBe(116); - spy.calls.reset(); + expect(getSpy.calls.count()).toBe(116); + expect(putSpy.calls.count()).toBe(20); + getSpy.calls.reset(); + putSpy.calls.reset(); await cacheAdapter.clear(); await reconfigureServer(); From 323e7130fb8f695e3ca44ebf9b3b1d38905353da Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 16:49:25 -0500 Subject: [PATCH 09/17] enableSingleSchemaCache default true --- spec/RedisCacheAdapter.spec.js | 5 +---- src/Options/Definitions.js | 4 ++-- src/Options/docs.js | 2 +- src/Options/index.js | 4 ++-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 9f838cd722..159b052d76 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -102,10 +102,7 @@ describe_only(() => { it('redis performance test', async () => { const cacheAdapter = new RedisCacheAdapter(); - await reconfigureServer({ - cacheAdapter, - enableSingleSchemaCache: true, - }); + await reconfigureServer({ cacheAdapter }); await cacheAdapter.clear(); const getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); const putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index b1564a7824..b5bbdb7a24 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -133,9 +133,9 @@ module.exports.ParseServerOptions = { enableSingleSchemaCache: { env: 'PARSE_SERVER_ENABLE_SINGLE_SCHEMA_CACHE', help: - 'Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request.', + 'Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request.', action: parsers.booleanParser, - default: false, + default: true, }, expireInactiveSessions: { env: 'PARSE_SERVER_EXPIRE_INACTIVE_SESSIONS', diff --git a/src/Options/docs.js b/src/Options/docs.js index 4ddd4f57b6..88639f368d 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -23,7 +23,7 @@ * @property {Number} emailVerifyTokenValidityDuration Email verification token validity duration, in seconds * @property {Boolean} enableAnonymousUsers Enable (or disable) anon users, defaults to true * @property {Boolean} enableExpressErrorHandler Enables the default express error handler for all errors - * @property {Boolean} enableSingleSchemaCache Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request. + * @property {Boolean} enableSingleSchemaCache Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request. * @property {Boolean} expireInactiveSessions Sets wether we should expire the inactive sessions, defaults to true * @property {String} fileKey Key for your files * @property {Adapter} filesAdapter Adapter module for the files sub-system diff --git a/src/Options/index.js b/src/Options/index.js index f995e46729..2f13efe637 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -149,8 +149,8 @@ export interface ParseServerOptions { :ENV: PARSE_SERVER_ENABLE_EXPERIMENTAL_DIRECT_ACCESS :DEFAULT: false */ directAccess: ?boolean; - /* Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request. - :DEFAULT: false */ + /* Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request. + :DEFAULT: true */ enableSingleSchemaCache: ?boolean; /* Enables the default express error handler for all errors :DEFAULT: false */ From 2e09b41c7e60a232222a69483b7303f8d67234c3 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 16:57:09 -0500 Subject: [PATCH 10/17] Revert "enableSingleSchemaCache default true" This reverts commit 323e7130fb8f695e3ca44ebf9b3b1d38905353da. --- spec/RedisCacheAdapter.spec.js | 5 ++++- src/Options/Definitions.js | 4 ++-- src/Options/docs.js | 2 +- src/Options/index.js | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 159b052d76..9f838cd722 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -102,7 +102,10 @@ describe_only(() => { it('redis performance test', async () => { const cacheAdapter = new RedisCacheAdapter(); - await reconfigureServer({ cacheAdapter }); + await reconfigureServer({ + cacheAdapter, + enableSingleSchemaCache: true, + }); await cacheAdapter.clear(); const getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); const putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index b5bbdb7a24..b1564a7824 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -133,9 +133,9 @@ module.exports.ParseServerOptions = { enableSingleSchemaCache: { env: 'PARSE_SERVER_ENABLE_SINGLE_SCHEMA_CACHE', help: - 'Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request.', + 'Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request.', action: parsers.booleanParser, - default: true, + default: false, }, expireInactiveSessions: { env: 'PARSE_SERVER_EXPIRE_INACTIVE_SESSIONS', diff --git a/src/Options/docs.js b/src/Options/docs.js index 88639f368d..4ddd4f57b6 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -23,7 +23,7 @@ * @property {Number} emailVerifyTokenValidityDuration Email verification token validity duration, in seconds * @property {Boolean} enableAnonymousUsers Enable (or disable) anon users, defaults to true * @property {Boolean} enableExpressErrorHandler Enables the default express error handler for all errors - * @property {Boolean} enableSingleSchemaCache Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request. + * @property {Boolean} enableSingleSchemaCache Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request. * @property {Boolean} expireInactiveSessions Sets wether we should expire the inactive sessions, defaults to true * @property {String} fileKey Key for your files * @property {Adapter} filesAdapter Adapter module for the files sub-system diff --git a/src/Options/index.js b/src/Options/index.js index 2f13efe637..f995e46729 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -149,8 +149,8 @@ export interface ParseServerOptions { :ENV: PARSE_SERVER_ENABLE_EXPERIMENTAL_DIRECT_ACCESS :DEFAULT: false */ directAccess: ?boolean; - /* Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to true, i.e. unique schema cache per request. - :DEFAULT: true */ + /* Use a single schema cache shared across requests. Reduces number of queries made to _SCHEMA, defaults to false, i.e. unique schema cache per request. + :DEFAULT: false */ enableSingleSchemaCache: ?boolean; /* Enables the default express error handler for all errors :DEFAULT: false */ From 52910fc2836e3196b297c4fd54f3f22c6f17cffc Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 20:46:43 -0500 Subject: [PATCH 11/17] further optimization --- spec/RedisCacheAdapter.spec.js | 23 +++++- src/Controllers/SchemaController.js | 119 ++++++++++++++++++++++++---- 2 files changed, 123 insertions(+), 19 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 9f838cd722..8e3e8ec31d 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -114,7 +114,7 @@ describe_only(() => { const object = new TestObject(); object.set('foo', 'bar'); await object.save(); - expect(getSpy.calls.count()).toBe(4); + expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(3); getSpy.calls.reset(); putSpy.calls.reset(); @@ -122,7 +122,7 @@ describe_only(() => { // Update Existing Field object.set('foo', 'barz'); await object.save(); - expect(getSpy.calls.count()).toBe(4); + expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(0); getSpy.calls.reset(); putSpy.calls.reset(); @@ -130,7 +130,7 @@ describe_only(() => { // Add New Field object.set('new', 'barz'); await object.save(); - expect(getSpy.calls.count()).toBe(4); + expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(1); getSpy.calls.reset(); putSpy.calls.reset(); @@ -165,11 +165,26 @@ describe_only(() => { objects.push(obj); } await Parse.Object.saveAll(objects); - expect(getSpy.calls.count()).toBe(116); + expect(getSpy.calls.count()).toBe(111); expect(putSpy.calls.count()).toBe(20); getSpy.calls.reset(); putSpy.calls.reset(); + // New Object Mutiple Fields + const container = new Container({ + dateField: new Date(), + arrayField: [], + numberField: 1, + stringField: 'hello', + booleanField: true, + pointerField: object, + }); + await container.save(); + expect(getSpy.calls.count()).toBe(3); + expect(putSpy.calls.count()).toBe(2); + getSpy.calls.reset(); + putSpy.calls.reset(); + await cacheAdapter.clear(); await reconfigureServer(); }); diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 6d7d192803..f96c6f2dd1 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -905,6 +905,70 @@ export default class SchemaController { return this._dbAdapter.setClassLevelPermissions(className, perms); } + // Returns a promise that resolves successfully to the new schema + // object if the provided className-fieldName-type tuple is valid. + // The className must already be validated. + // If 'freeze' is true, refuse to update the schema for this field. + ensureFieldExists( + className: string, + fieldName: string, + type: string | SchemaField + ) { + if (fieldName.indexOf('.') > 0) { + // subdocument key (x.y) => ok if x is of type 'object' + fieldName = fieldName.split('.')[0]; + type = 'Object'; + } + if (!fieldNameIsValid(fieldName)) { + throw new Parse.Error( + Parse.Error.INVALID_KEY_NAME, + `Invalid field name: ${fieldName}.` + ); + } + + // If someone tries to create a new field with null/undefined as the value, return; + if (!type) { + return undefined; + } + + 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; + } + + 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; + } + // 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, + }; + }); + } + // Returns a promise that resolves successfully to the new schema // object if the provided className-fieldName-type tuple is valid. // The className must already be validated. @@ -1064,15 +1128,17 @@ export default class SchemaController { ); }); }) - .then(() => this.setAllClasses()); + .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; @@ -1084,14 +1150,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; @@ -1100,13 +1164,38 @@ export default class SchemaController { // Every object has ACL implicitly. continue; } + promises.push(schema.ensureFieldExists(className, fieldName, expected)); + } + const results = await Promise.all(promises); + const toValidate = results.filter(result => !!result); + + if (toValidate.length !== 0) { + await this.reloadData({ clearCache: true }); + } - promise = promise.then(schema => - schema.enforceFieldExists(className, fieldName, expected) + for (let i = 0; i < toValidate.length; i += 1) { + const value = toValidate[i]; + // Ensure that the schema now validates + const expectedType = this.getExpectedType( + value.className, + value.fieldName ); + if (typeof value.type === 'string') { + value.type = { type: value.type }; + } + if (!expectedType || !dbTypeMatchesObjectType(expectedType, value.type)) { + throw new Parse.Error( + Parse.Error.INVALID_JSON, + `Could not add field ${value.fieldName}` + ); + } } - promise = thenValidateRequiredColumns(promise, className, object, query); - return promise; + return thenValidateRequiredColumns( + Promise.resolve(schema), + className, + object, + query + ); } // Validates that all the properties are set for the object From e0ba2a96021a620f512dfea3f335da249281e565 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Thu, 23 May 2019 21:38:45 -0500 Subject: [PATCH 12/17] refactor enforceFieldExists --- spec/RedisCacheAdapter.spec.js | 62 ++++++++++++++ src/Controllers/SchemaController.js | 127 ++++++---------------------- 2 files changed, 86 insertions(+), 103 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 8e3e8ec31d..93fddf3bac 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' @@ -151,6 +153,20 @@ describe_only(() => { getSpy.calls.reset(); putSpy.calls.reset(); + // Existing Object Mutiple Fields + 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); + getSpy.calls.reset(); + putSpy.calls.reset(); + // Delete Object await object.destroy(); expect(getSpy.calls.count()).toBe(3); @@ -185,6 +201,52 @@ describe_only(() => { getSpy.calls.reset(); putSpy.calls.reset(); + // Update Class Schema + 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); + getSpy.calls.reset(); + putSpy.calls.reset(); + await cacheAdapter.clear(); await reconfigureServer(); }); diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index f96c6f2dd1..e396f5a862 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -737,6 +737,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 @@ -747,9 +748,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, @@ -761,6 +763,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, @@ -909,7 +912,7 @@ export default class SchemaController { // object if the provided className-fieldName-type tuple is valid. // The className must already be validated. // If 'freeze' is true, refuse to update the schema for this field. - ensureFieldExists( + enforceFieldExists( className: string, fieldName: string, type: string | SchemaField @@ -969,83 +972,21 @@ export default class SchemaController { }); } - // Returns a promise that resolves successfully to the new schema - // object if the provided className-fieldName-type tuple is valid. - // The className must already be validated. - // If 'freeze' is true, refuse to update the schema for this field. - enforceFieldExists( - className: string, - fieldName: string, - type: string | SchemaField - ) { - if (fieldName.indexOf('.') > 0) { - // subdocument key (x.y) => ok if x is of type 'object' - fieldName = fieldName.split('.')[0]; - type = 'Object'; - } - if (!fieldNameIsValid(fieldName)) { - throw new Parse.Error( - Parse.Error.INVALID_KEY_NAME, - `Invalid field name: ${fieldName}.` - ); - } - - // If someone tries to create a new field with null/undefined as the value, return; - if (!type) { - return Promise.resolve(this); - } - - return this.reloadData().then(() => { + 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 = { 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 this; + if (!expectedType || !dbTypeMatchesObjectType(expectedType, type)) { + throw new Parse.Error( + Parse.Error.INVALID_JSON, + `Could not add field ${fieldName}` + ); } - - 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}` - ); - } - return this; - }); - }); + } } // maintain compatibility @@ -1164,38 +1105,18 @@ export default class SchemaController { // Every object has ACL implicitly. continue; } - promises.push(schema.ensureFieldExists(className, fieldName, expected)); + promises.push(schema.enforceFieldExists(className, fieldName, expected)); } const results = await Promise.all(promises); - const toValidate = results.filter(result => !!result); + const enforceFields = results.filter(result => !!result); - if (toValidate.length !== 0) { + if (enforceFields.length !== 0) { await this.reloadData({ clearCache: true }); } + this.ensureFields(enforceFields); - for (let i = 0; i < toValidate.length; i += 1) { - const value = toValidate[i]; - // Ensure that the schema now validates - const expectedType = this.getExpectedType( - value.className, - value.fieldName - ); - if (typeof value.type === 'string') { - value.type = { type: value.type }; - } - if (!expectedType || !dbTypeMatchesObjectType(expectedType, value.type)) { - throw new Parse.Error( - Parse.Error.INVALID_JSON, - `Could not add field ${value.fieldName}` - ); - } - } - return thenValidateRequiredColumns( - Promise.resolve(schema), - className, - object, - query - ); + const promise = Promise.resolve(schema); + return thenValidateRequiredColumns(promise, className, object, query); } // Validates that all the properties are set for the object From 0457699bd5fdd2b6d3a4c7119e307ce11c6ba234 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 24 May 2019 10:55:50 -0500 Subject: [PATCH 13/17] coverage improvements --- spec/MongoStorageAdapter.spec.js | 25 +++++++++++++++++++++++++ spec/PostgresStorageAdapter.spec.js | 29 +++++++++++++++++++++++++++++ spec/Schema.spec.js | 25 +++++++++++++++++++++++++ spec/SchemaCache.spec.js | 14 ++++++++++++++ src/Controllers/SchemaCache.js | 1 - 5 files changed, 93 insertions(+), 1 deletion(-) diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index ec40c1080d..5b41c7abba 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -286,4 +286,29 @@ 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 }); + try { + await adapter.getClass('UnknownClass'); + } catch (e) { + expect(e).toBeUndefined(); + } + }); }); diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index c557b472b1..cd3442aa12 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -114,6 +114,35 @@ 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); + try { + await adapter.getClass('UnknownClass'); + } catch (e) { + expect(e).toBeUndefined(); + } + }); }); describe_only_db('postgres')('PostgresStorageAdapter shutdown', () => { diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index b95783023e..0bebb44ef3 100644 --- a/spec/Schema.spec.js +++ b/spec/Schema.spec.js @@ -1734,4 +1734,29 @@ describe('Class Level Permissions for requiredAuth', () => { } ); }); + + 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 { + await schema.ensureFields([ + { + className: 'NewClass', + fieldName: 'fieldName', + type: 'String', + }, + ]); + } catch (e) { + expect(e.message).toBe('Could not add field fieldName'); + } + }); }); diff --git a/spec/SchemaCache.spec.js b/spec/SchemaCache.spec.js index 3bf6da92ef..26897e03f7 100644 --- a/spec/SchemaCache.spec.js +++ b/spec/SchemaCache.spec.js @@ -60,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/SchemaCache.js b/src/Controllers/SchemaCache.js index 8b6371d076..55d70eabaa 100644 --- a/src/Controllers/SchemaCache.js +++ b/src/Controllers/SchemaCache.js @@ -54,7 +54,6 @@ export default class SchemaCache { } clear() { - // That clears all caches... return this.cache.del(this.prefix + MAIN_SCHEMA); } } From 7a8be2bd98796a37b69a5bb95e625a1aa7a5a225 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 24 May 2019 13:48:04 -0500 Subject: [PATCH 14/17] improve tests --- spec/MongoStorageAdapter.spec.js | 4 +- spec/PostgresStorageAdapter.spec.js | 4 +- spec/RedisCacheAdapter.spec.js | 268 ++++++++++++++++------------ 3 files changed, 158 insertions(+), 118 deletions(-) diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index 5b41c7abba..6bcb8d49ba 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -307,8 +307,8 @@ describe_only_db('mongo')('MongoStorageAdapter', () => { const adapter = new MongoStorageAdapter({ uri: databaseURI }); try { await adapter.getClass('UnknownClass'); - } catch (e) { - expect(e).toBeUndefined(); + } catch (schema) { + expect(schema).toBeUndefined(); } }); }); diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index cd3442aa12..75487e001c 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -139,8 +139,8 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { await adapter.createClass('MyClass', schema); try { await adapter.getClass('UnknownClass'); - } catch (e) { - expect(e).toBeUndefined(); + } catch (schema) { + expect(schema).toBeUndefined(); } }); }); diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 93fddf3bac..c5f01d1df3 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -101,59 +101,153 @@ describe_only(() => { .then(value => expect(value).not.toEqual(null)) .then(done); }); +}); + +describe_only(() => { + return process.env.PARSE_SERVER_TEST_CACHE === 'redis'; +})('RedisCacheAdapter/KeyPromiseQueue', function() { + const KEY1 = 'key1'; + const KEY2 = 'key2'; + const VALUE = 'hello'; + + // number of chained ops on a single key + function getQueueCountForKey(cache, key) { + return cache.queue.queue[key][0]; + } + + // total number of queued keys + function getQueueCount(cache) { + return Object.keys(cache.queue.queue).length; + } + + it('it should clear completed operations from queue', done => { + const cache = new RedisCacheAdapter({ ttl: NaN }); + + // execute a bunch of operations in sequence + let promise = Promise.resolve(); + for (let index = 1; index < 100; index++) { + promise = promise.then(() => { + const key = `${index}`; + return cache + .put(key, VALUE) + .then(() => expect(getQueueCount(cache)).toEqual(0)) + .then(() => cache.get(key)) + .then(() => expect(getQueueCount(cache)).toEqual(0)) + .then(() => cache.clear()) + .then(() => expect(getQueueCount(cache)).toEqual(0)); + }); + } + + // at the end the queue should be empty + promise.then(() => expect(getQueueCount(cache)).toEqual(0)).then(done); + }); + + it('it should count per key chained operations correctly', done => { + const cache = new RedisCacheAdapter({ ttl: NaN }); + + let key1Promise = Promise.resolve(); + let key2Promise = Promise.resolve(); + for (let index = 1; index < 100; index++) { + key1Promise = cache.put(KEY1, VALUE); + key2Promise = cache.put(KEY2, VALUE); + // per key chain should be equal to index, which is the + // total number of operations on that key + expect(getQueueCountForKey(cache, KEY1)).toEqual(index); + expect(getQueueCountForKey(cache, KEY2)).toEqual(index); + // the total keys counts should be equal to the different keys + // we have currently being processed. + expect(getQueueCount(cache)).toEqual(2); + } + + // at the end the queue should be empty + Promise.all([key1Promise, key2Promise]) + .then(() => expect(getQueueCount(cache)).toEqual(0)) + .then(done); + }); +}); + +describe_only(() => { + return process.env.PARSE_SERVER_TEST_CACHE === 'redis'; +})('Redis Performance', function() { + const cacheAdapter = new RedisCacheAdapter(); + let getSpy; + let putSpy; - it('redis performance test', async () => { - const cacheAdapter = new RedisCacheAdapter(); - await reconfigureServer({ + function setUp() { + return reconfigureServer({ cacheAdapter, enableSingleSchemaCache: true, }); + } + + beforeEach(async () => { await cacheAdapter.clear(); - const getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); - const putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); + getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); + putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); + }); - // New Object + it('test new object', async () => { + await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); - expect(getSpy.calls.count()).toBe(3); + expect(getSpy.calls.count()).toBe(4); + expect(putSpy.calls.count()).toBe(3); + }); + + it('test new object multiple fields', async () => { + await setUp(); + 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 () => { + await setUp(); + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + getSpy.calls.reset(); putSpy.calls.reset(); - // Update Existing Field 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 () => { + await setUp(); + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + getSpy.calls.reset(); putSpy.calls.reset(); - // Add New Field object.set('new', 'barz'); await object.save(); expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(1); - getSpy.calls.reset(); - putSpy.calls.reset(); + }); - // Get Object - let query = new Parse.Query(TestObject); - await query.get(object.id); - expect(getSpy.calls.count()).toBe(2); - expect(putSpy.calls.count()).toBe(0); - getSpy.calls.reset(); - putSpy.calls.reset(); + it('test add multiple fields to existing object', async () => { + await setUp(); + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); - // Find Object - query = new Parse.Query(TestObject); - await query.find(); - expect(getSpy.calls.count()).toBe(2); - expect(putSpy.calls.count()).toBe(0); getSpy.calls.reset(); putSpy.calls.reset(); - // Existing Object Mutiple Fields object.set({ dateField: new Date(), arrayField: [], @@ -164,44 +258,58 @@ describe_only(() => { await object.save(); expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(1); + }); + + it('test query', async () => { + await setUp(); + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + getSpy.calls.reset(); putSpy.calls.reset(); - // Delete Object - await object.destroy(); - expect(getSpy.calls.count()).toBe(3); + 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 () => { + await setUp(); + 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 saveAll', async () => { + await setUp(); const objects = []; for (let i = 0; i < 100; i++) { - const obj = new TestObject(); - obj.set('number', i); - objects.push(obj); + const object = new TestObject(); + object.set('number', i); + objects.push(object); } await Parse.Object.saveAll(objects); - expect(getSpy.calls.count()).toBe(111); - expect(putSpy.calls.count()).toBe(20); - getSpy.calls.reset(); - putSpy.calls.reset(); + expect(getSpy.calls.count()).toBe(112); + expect(putSpy.calls.count()).toBe(41); + }); - // New Object Mutiple Fields - const container = new Container({ - dateField: new Date(), - arrayField: [], - numberField: 1, - stringField: 'hello', - booleanField: true, - pointerField: object, - }); + it('test schema update class', async () => { + await setUp(); + const container = new Container(); await container.save(); - expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(2); + getSpy.calls.reset(); putSpy.calls.reset(); - // Update Class Schema const config = Config.get('test'); const schema = await config.database.loadSchema(); await schema.reloadData(); @@ -244,73 +352,5 @@ describe_only(() => { ); expect(getSpy.calls.count()).toBe(3); expect(putSpy.calls.count()).toBe(2); - getSpy.calls.reset(); - putSpy.calls.reset(); - - await cacheAdapter.clear(); - await reconfigureServer(); - }); -}); - -describe_only(() => { - return process.env.PARSE_SERVER_TEST_CACHE === 'redis'; -})('RedisCacheAdapter/KeyPromiseQueue', function() { - const KEY1 = 'key1'; - const KEY2 = 'key2'; - const VALUE = 'hello'; - - // number of chained ops on a single key - function getQueueCountForKey(cache, key) { - return cache.queue.queue[key][0]; - } - - // total number of queued keys - function getQueueCount(cache) { - return Object.keys(cache.queue.queue).length; - } - - it('it should clear completed operations from queue', done => { - const cache = new RedisCacheAdapter({ ttl: NaN }); - - // execute a bunch of operations in sequence - let promise = Promise.resolve(); - for (let index = 1; index < 100; index++) { - promise = promise.then(() => { - const key = `${index}`; - return cache - .put(key, VALUE) - .then(() => expect(getQueueCount(cache)).toEqual(0)) - .then(() => cache.get(key)) - .then(() => expect(getQueueCount(cache)).toEqual(0)) - .then(() => cache.clear()) - .then(() => expect(getQueueCount(cache)).toEqual(0)); - }); - } - - // at the end the queue should be empty - promise.then(() => expect(getQueueCount(cache)).toEqual(0)).then(done); - }); - - it('it should count per key chained operations correctly', done => { - const cache = new RedisCacheAdapter({ ttl: NaN }); - - let key1Promise = Promise.resolve(); - let key2Promise = Promise.resolve(); - for (let index = 1; index < 100; index++) { - key1Promise = cache.put(KEY1, VALUE); - key2Promise = cache.put(KEY2, VALUE); - // per key chain should be equal to index, which is the - // total number of operations on that key - expect(getQueueCountForKey(cache, KEY1)).toEqual(index); - expect(getQueueCountForKey(cache, KEY2)).toEqual(index); - // the total keys counts should be equal to the different keys - // we have currently being processed. - expect(getQueueCount(cache)).toEqual(2); - } - - // at the end the queue should be empty - Promise.all([key1Promise, key2Promise]) - .then(() => expect(getQueueCount(cache)).toEqual(0)) - .then(done); }); }); From 0167c891094239243aa3c53ce6d034a11881c905 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 24 May 2019 14:16:09 -0500 Subject: [PATCH 15/17] remove flaky test --- spec/RedisCacheAdapter.spec.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index c5f01d1df3..5b4925529d 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -289,19 +289,6 @@ describe_only(() => { expect(putSpy.calls.count()).toBe(0); }); - it('test saveAll', async () => { - await setUp(); - const objects = []; - for (let i = 0; i < 100; i++) { - const object = new TestObject(); - object.set('number', i); - objects.push(object); - } - await Parse.Object.saveAll(objects); - expect(getSpy.calls.count()).toBe(112); - expect(putSpy.calls.count()).toBe(41); - }); - it('test schema update class', async () => { await setUp(); const container = new Container(); From 17e4d08c148bfc59970f825138de458e7b0cfc7d Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 24 May 2019 14:49:58 -0500 Subject: [PATCH 16/17] cleanup --- spec/RedisCacheAdapter.spec.js | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 5b4925529d..637d9d67c7 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -173,21 +173,17 @@ describe_only(() => { let getSpy; let putSpy; - function setUp() { - return reconfigureServer({ - cacheAdapter, - enableSingleSchemaCache: true, - }); - } - 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 () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -196,7 +192,6 @@ describe_only(() => { }); it('test new object multiple fields', async () => { - await setUp(); const container = new Container({ dateField: new Date(), arrayField: [], @@ -210,7 +205,6 @@ describe_only(() => { }); it('test update existing fields', async () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -225,7 +219,6 @@ describe_only(() => { }); it('test add new field to existing object', async () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -240,7 +233,6 @@ describe_only(() => { }); it('test add multiple fields to existing object', async () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -261,7 +253,6 @@ describe_only(() => { }); it('test query', async () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -276,7 +267,6 @@ describe_only(() => { }); it('test delete object', async () => { - await setUp(); const object = new TestObject(); object.set('foo', 'bar'); await object.save(); @@ -290,7 +280,6 @@ describe_only(() => { }); it('test schema update class', async () => { - await setUp(); const container = new Container(); await container.save(); From b4c4942f2df46b8f7a7303c2d1245456597f15b5 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 24 May 2019 16:26:26 -0500 Subject: [PATCH 17/17] Learned something new --- spec/.eslintrc.json | 1 + spec/MongoStorageAdapter.spec.js | 8 ++-- spec/PostgresStorageAdapter.spec.js | 8 ++-- spec/Schema.spec.js | 66 ++++++++++++++++++----------- src/Controllers/SchemaController.js | 11 +++-- 5 files changed, 56 insertions(+), 38 deletions(-) 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 6bcb8d49ba..e9471621cd 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -305,10 +305,8 @@ describe_only_db('mongo')('MongoStorageAdapter', () => { it('getClass if not exists', async () => { const adapter = new MongoStorageAdapter({ uri: databaseURI }); - try { - await adapter.getClass('UnknownClass'); - } catch (schema) { - expect(schema).toBeUndefined(); - } + await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith( + undefined + ); }); }); diff --git a/spec/PostgresStorageAdapter.spec.js b/spec/PostgresStorageAdapter.spec.js index 75487e001c..b6fa7e2626 100644 --- a/spec/PostgresStorageAdapter.spec.js +++ b/spec/PostgresStorageAdapter.spec.js @@ -137,11 +137,9 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => { }, }; await adapter.createClass('MyClass', schema); - try { - await adapter.getClass('UnknownClass'); - } catch (schema) { - expect(schema).toBeUndefined(); - } + await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith( + undefined + ); }); }); diff --git a/spec/Schema.spec.js b/spec/Schema.spec.js index 0bebb44ef3..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', () => { @@ -1734,29 +1775,4 @@ describe('Class Level Permissions for requiredAuth', () => { } ); }); - - 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 { - await schema.ensureFields([ - { - className: 'NewClass', - fieldName: 'fieldName', - type: 'String', - }, - ]); - } catch (e) { - expect(e.message).toBe('Could not add field fieldName'); - } - }); }); diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index e396f5a862..01dc5f601f 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -587,9 +587,14 @@ export default class SchemaController { .getAllClasses() .then(allSchemas => allSchemas.map(injectDefaultSchema)) .then(allSchemas => { - return this._cache.setAllClasses(allSchemas).then(() => { - return 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; }); }