Cleanup Schema cache per request (#6126)

* remove enableSingleSchemaCache from test * clear schema cache per request
parse-community · Oct 11, 2019 · edfa1df · edfa1df
1 parent f26008f
commit edfa1df
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 21 deletions.
diff --git 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
@@ -172,24 +172,30 @@ 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 () => {
     const object = new TestObject();
     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,32 +340,42 @@ 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 () => {
     const object = new TestObject();
     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();
 
     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
@@ -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
@@ -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();
+  }
+}