From edfa1df454f0e76946c231a4573822cb870affde Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Fri, 11 Oct 2019 15:27:15 -0500 Subject: [PATCH] Cleanup Schema cache per request (#6126) * remove enableSingleSchemaCache from test * clear schema cache per request --- .gitignore | 3 + spec/RedisCacheAdapter.spec.js | 103 ++++++++++++++---- src/Adapters/Cache/RedisCacheAdapter/index.js | 13 +++ src/PromiseRouter.js | 14 ++- 4 files changed, 112 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index b7d6200e..e4e19156 100644 --- a/.gitignore +++ b/.gitignore @@ -56,3 +56,6 @@ lib/ # Folder created by FileSystemAdapter /files + +# Redis Dump +dump.rdb diff --git a/spec/RedisCacheAdapter.spec.js b/spec/RedisCacheAdapter.spec.js index 3b1c78af..9ae53cf6 100644 --- a/spec/RedisCacheAdapter.spec.js +++ b/spec/RedisCacheAdapter.spec.js @@ -172,16 +172,18 @@ describe_only(() => { let cacheAdapter; let getSpy; let putSpy; + let delSpy; beforeEach(async () => { cacheAdapter = new RedisCacheAdapter(); - await cacheAdapter.clear(); await reconfigureServer({ cacheAdapter, - enableSingleSchemaCache: true, }); + await cacheAdapter.clear(); + getSpy = spyOn(cacheAdapter, 'get').and.callThrough(); putSpy = spyOn(cacheAdapter, 'put').and.callThrough(); + delSpy = spyOn(cacheAdapter, 'del').and.callThrough(); }); it('test new object', async () => { @@ -189,7 +191,11 @@ describe_only(() => { object.set('foo', 'bar'); await object.save(); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(3); + expect(delSpy.calls.count()).toBe(1); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test new object multiple fields', async () => { @@ -202,7 +208,11 @@ describe_only(() => { }); await container.save(); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(3); + expect(delSpy.calls.count()).toBe(1); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test update existing fields', async () => { @@ -216,7 +226,11 @@ describe_only(() => { object.set('foo', 'barz'); await object.save(); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(2); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test saveAll / destroyAll', async () => { @@ -234,14 +248,18 @@ describe_only(() => { } await Parse.Object.saveAll(objects); expect(getSpy.calls.count()).toBe(21); - expect(putSpy.calls.count()).toBe(10); + expect(putSpy.calls.count()).toBe(11); getSpy.calls.reset(); putSpy.calls.reset(); await Parse.Object.destroyAll(objects); expect(getSpy.calls.count()).toBe(11); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(3); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test saveAll / destroyAll batch', async () => { @@ -259,14 +277,18 @@ describe_only(() => { } await Parse.Object.saveAll(objects, { batchSize: 5 }); expect(getSpy.calls.count()).toBe(22); - expect(putSpy.calls.count()).toBe(5); + expect(putSpy.calls.count()).toBe(7); getSpy.calls.reset(); putSpy.calls.reset(); await Parse.Object.destroyAll(objects, { batchSize: 5 }); expect(getSpy.calls.count()).toBe(12); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(2); + expect(delSpy.calls.count()).toBe(5); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test add new field to existing object', async () => { @@ -280,7 +302,11 @@ describe_only(() => { object.set('new', 'barz'); await object.save(); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(1); + expect(putSpy.calls.count()).toBe(2); + expect(delSpy.calls.count()).toBe(2); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test add multiple fields to existing object', async () => { @@ -300,7 +326,11 @@ describe_only(() => { }); await object.save(); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(1); + expect(putSpy.calls.count()).toBe(2); + expect(delSpy.calls.count()).toBe(2); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test user', async () => { @@ -310,7 +340,11 @@ describe_only(() => { await user.signUp(); expect(getSpy.calls.count()).toBe(8); - expect(putSpy.calls.count()).toBe(1); + expect(putSpy.calls.count()).toBe(2); + expect(delSpy.calls.count()).toBe(1); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test allowClientCreation false', async () => { @@ -318,16 +352,18 @@ describe_only(() => { await object.save(); await reconfigureServer({ cacheAdapter, - enableSingleSchemaCache: true, allowClientClassCreation: false, }); + await cacheAdapter.clear(); + getSpy.calls.reset(); putSpy.calls.reset(); + delSpy.calls.reset(); object.set('foo', 'bar'); await object.save(); expect(getSpy.calls.count()).toBe(4); - expect(putSpy.calls.count()).toBe(1); + expect(putSpy.calls.count()).toBe(2); getSpy.calls.reset(); putSpy.calls.reset(); @@ -335,7 +371,11 @@ describe_only(() => { const query = new Parse.Query(TestObject); await query.get(object.id); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(2); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test query', async () => { @@ -345,11 +385,16 @@ describe_only(() => { getSpy.calls.reset(); putSpy.calls.reset(); + delSpy.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); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(1); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test query include', async () => { @@ -368,7 +413,11 @@ describe_only(() => { await query.get(object.id); expect(getSpy.calls.count()).toBe(4); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(3); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('query relation without schema', async () => { @@ -388,7 +437,11 @@ describe_only(() => { expect(objects[0].id).toBe(child.id); expect(getSpy.calls.count()).toBe(2); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(3); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test delete object', async () => { @@ -398,10 +451,15 @@ describe_only(() => { getSpy.calls.reset(); putSpy.calls.reset(); + delSpy.calls.reset(); await object.destroy(); expect(getSpy.calls.count()).toBe(2); - expect(putSpy.calls.count()).toBe(0); + expect(putSpy.calls.count()).toBe(1); + expect(delSpy.calls.count()).toBe(1); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(0); }); it('test schema update class', async () => { @@ -410,6 +468,7 @@ describe_only(() => { getSpy.calls.reset(); putSpy.calls.reset(); + delSpy.calls.reset(); const config = Config.get('test'); const schema = await config.database.loadSchema(); @@ -452,6 +511,10 @@ describe_only(() => { config.database ); expect(getSpy.calls.count()).toBe(3); - expect(putSpy.calls.count()).toBe(2); + expect(putSpy.calls.count()).toBe(3); + expect(delSpy.calls.count()).toBe(0); + + const keys = await cacheAdapter.getAllKeys(); + expect(keys.length).toBe(1); }); }); diff --git a/src/Adapters/Cache/RedisCacheAdapter/index.js b/src/Adapters/Cache/RedisCacheAdapter/index.js index 7c771a23..cf0bb716 100644 --- a/src/Adapters/Cache/RedisCacheAdapter/index.js +++ b/src/Adapters/Cache/RedisCacheAdapter/index.js @@ -96,6 +96,19 @@ export class RedisCacheAdapter { }) ); } + + // Used for testing + async getAllKeys() { + return new Promise((resolve, reject) => { + this.client.keys('*', (err, keys) => { + if (err) { + reject(err); + } else { + resolve(keys); + } + }); + }); + } } export default RedisCacheAdapter; diff --git a/src/PromiseRouter.js b/src/PromiseRouter.js index 755b5e5e..d67bf30d 100644 --- a/src/PromiseRouter.js +++ b/src/PromiseRouter.js @@ -153,6 +153,7 @@ function makeExpressHandler(appId, promiseHandler) { promiseHandler(req) .then( result => { + clearSchemaCache(req); if (!result.response && !result.location && !result.text) { log.error( 'the handler did not include a "response" or a "location" field' @@ -186,13 +187,18 @@ function makeExpressHandler(appId, promiseHandler) { } res.json(result.response); }, - error => next(error) + error => { + clearSchemaCache(req); + next(error); + } ) .catch(e => { + clearSchemaCache(req); log.error(`Error generating response. ${inspect(e)}`, { error: e }); next(e); }); } catch (e) { + clearSchemaCache(req); log.error(`Error handling request: ${inspect(e)}`, { error: e }); next(e); } @@ -210,3 +216,9 @@ function maskSensitiveUrl(req) { } return maskUrl; } + +function clearSchemaCache(req) { + if (req.config && !req.config.enableSingleSchemaCache) { + req.config.database.schemaCache.clear(); + } +}