Proper testing of the caching

parse-community · Aug 9, 2018 · ecc212b · ecc212b
1 parent c4f90b8
commit ecc212b
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 22 deletions.
diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js
@@ -61,11 +61,14 @@ describe('ParseLiveQueryServer', function() {
     jasmine.mockLibrary('../lib/LiveQuery/ParsePubSub', 'ParsePubSub', mockParsePubSub);
     spyOn(auth, 'getAuthForSessionToken').and.callFake(({ sessionToken, cacheController }) => {
       if (typeof sessionToken === 'undefined') {
-        return Promise.reject(/*new auth.Auth({ user: undefined })*/);
+        return Promise.reject();
       }
       if (sessionToken === null) {
         return Promise.reject();
       }
+      if (sessionToken === 'pleaseThrow') {
+        return Promise.reject();
+      }
       return Promise.resolve(new auth.Auth({ cacheController, user: { id: testUserId }}));
     });
     done();
@@ -1388,7 +1391,27 @@ describe('ParseLiveQueryServer', function() {
       expect(isMatched).toBe(false);
       done();
     });
+  });
 
+  it('should properly pull auth from cache', () => {
+    const parseLiveQueryServer = new ParseLiveQueryServer({});
+    const promise = parseLiveQueryServer.getAuthForSessionToken('sessionToken');
+    const secondPromise = parseLiveQueryServer.getAuthForSessionToken('sessionToken');
+    // should be in the cache
+    expect(parseLiveQueryServer.authCache.get('sessionToken')).toBe(promise);
+    // should be the same promise returned
+    expect(promise).toBe(secondPromise);
+    // the auth should be called only once
+    expect(auth.getAuthForSessionToken.calls.count()).toBe(1);
+  });
+
+  it('should delete from cache throwing auth calls', async () => {
+    const parseLiveQueryServer = new ParseLiveQueryServer({});
+    const promise = parseLiveQueryServer.getAuthForSessionToken('pleaseThrow');
+    expect(parseLiveQueryServer.authCache.get('pleaseThrow')).toBe(promise);
+    // after the promise finishes, it should have removed it from the cache
+    expect(await promise).toEqual({});
+    expect(parseLiveQueryServer.authCache.get('pleaseThrow')).toBe(undefined);
   });
 
   afterEach(function(){

diff --git a/src/LiveQuery/ParseLiveQueryServer.js b/src/LiveQuery/ParseLiveQueryServer.js
@@ -351,19 +351,17 @@ class ParseLiveQueryServer {
     if (fromCache) {
       return fromCache;
     }
-    try {
-      const authPromise = getAuthForSessionToken({ cacheController: this.cacheController, sessionToken: sessionToken })
-        .then((auth) => {
-          return { auth, userId: auth && auth.user && auth.user.id };
-        }, () => {
-          // If you can't continue, let's just wrap it up and delete it.
-          // Next time, one will try again
-          this.authCache.del(sessionToken);
-        });
-      this.authCache.set(sessionToken, authPromise);
-      return authPromise;
-    } catch(e) { /* ignore errors */ }
-    return Promise.resolve({});
+    const authPromise = getAuthForSessionToken({ cacheController: this.cacheController, sessionToken: sessionToken })
+      .then((auth) => {
+        return { auth, userId: auth && auth.user && auth.user.id };
+      }, () => {
+        // If you can't continue, let's just wrap it up and delete it.
+        // Next time, one will try again
+        this.authCache.del(sessionToken);
+        return {};
+      });
+    this.authCache.set(sessionToken, authPromise);
+    return authPromise;
   }
 
   async _matchesCLP(classLevelPermissions: ?any, object: any, client: any, requestId: number, op: string): any {
@@ -379,10 +377,10 @@ class ParseLiveQueryServer {
     }
     try {
       await SchemaController.validatePermission(classLevelPermissions, object.className, aclGroup, op);
-      return Promise.resolve(true);
+      return true;
     } catch(e) {
       logger.verbose(`Failed matching CLP for ${object.id} ${userId} ${e}`);
-      return Promise.resolve(false);
+      return false;
     }
     // TODO: handle roles permissions
     // Object.keys(classLevelPermissions).forEach((key) => {
@@ -403,22 +401,22 @@ class ParseLiveQueryServer {
       && typeof query.objectId === 'string' ? 'get' : 'find';
   }
 
-  async _matchesACL(acl: any, client: any, requestId: number): any {
+  async _matchesACL(acl: any, client: any, requestId: number): Promise<boolean> {
     // Return true directly if ACL isn't present, ACL is public read, or client has master key
     if (!acl || acl.getPublicReadAccess() || client.hasMasterKey) {
-      return Promise.resolve(true);
+      return true;
     }
     // Check subscription sessionToken matches ACL first
     const subscriptionInfo = client.getSubscriptionInfo(requestId);
     if (typeof subscriptionInfo === 'undefined') {
-      return Promise.resolve(false);
+      return false;
     }
 
     // TODO: get auth there and de-duplicate code below to work with the same Auth obj.
     const { auth, userId } = await this.getAuthForSessionToken(subscriptionInfo.sessionToken);
     const isSubscriptionSessionTokenMatched = acl.getReadAccess(userId);
     if (isSubscriptionSessionTokenMatched) {
-      return Promise.resolve(true);
+      return true;
     }
 
     // Check if the user has any roles that match the ACL
@@ -453,8 +451,7 @@ class ParseLiveQueryServer {
       } else  {
         return isRoleMatched;
       }
-    }).catch((error) => {
-      error;
+    }).catch(() => {
       return false;
     });
   }