chore(rest): address review comments

loopbackio · Apr 29, 2019 · 4acc08f · 4acc08f
1 parent a8517fa
commit 4acc08f
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 45 deletions.
diff --git a/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.acceptance.ts b/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.acceptance.ts
@@ -14,7 +14,6 @@ import {
 import {RestApplication} from '../../..';
 import {
   cache,
-  cachedResults,
   CachingInterceptorProvider,
   clearCache,
   status,
@@ -29,48 +28,39 @@ describe('caching interceptor', () => {
     await app.stop();
   });
 
-  context('toUpperCase with bound caching interceptor', () => {
+  beforeEach(clearCache);
+
+  context('as a binding key', () => {
     it('invokes the controller method if not cached', async () => {
       await client.get('/toUpperCase/Hello').expect(200, 'HELLO');
       expect(status.returnFromCache).to.be.false();
     });
 
     it('returns from cache without invoking the controller method', async () => {
+      await client.get('/toUpperCase/Hello').expect(200, 'HELLO');
       for (let i = 0; i <= 5; i++) {
         await client.get('/toUpperCase/Hello').expect(200, 'HELLO');
         expect(status.returnFromCache).to.be.true();
       }
     });
-
-    it('invokes the controller method after cache is cleared', async () => {
-      clearCache();
-      await client.get('/toUpperCase/Hello').expect(200, 'HELLO');
-      expect(status.returnFromCache).to.be.false();
-    });
   });
 
-  context('toLowerCase with cache interceptor function', () => {
+  context('as an interceptor function', () => {
     it('invokes the controller method if not cached', async () => {
       await client.get('/toLowerCase/Hello').expect(200, 'hello');
       expect(status.returnFromCache).to.be.false();
     });
 
     it('returns from cache without invoking the controller method', async () => {
+      await client.get('/toLowerCase/Hello').expect(200, 'hello');
       for (let i = 0; i <= 5; i++) {
         await client.get('/toLowerCase/Hello').expect(200, 'hello');
         expect(status.returnFromCache).to.be.true();
       }
     });
-
-    it('invokes the controller method after cache is cleared', async () => {
-      cachedResults.clear();
-      await client.get('/toLowerCase/Hello').expect(200, 'hello');
-      expect(status.returnFromCache).to.be.false();
-    });
   });
 
   async function givenAClient() {
-    clearCache();
     app = new RestApplication({rest: givenHttpServerConfig()});
     app.bind('caching-interceptor').toProvider(CachingInterceptorProvider);
     app.controller(StringCaseController);

diff --git a/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts b/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts
@@ -20,8 +20,7 @@ export const status = {
 };
 
 /**
- * An interceptor function that caches results. It uses `invocationContext`
- * to locate the http request
+ * In-memory cache
  */
 export const cachedResults = new Map<string, unknown>();
 
@@ -46,33 +45,14 @@ export class CachingInterceptorProvider implements Provider<Interceptor> {
     return <T>(
       invocationCtx: InvocationContext,
       next: () => ValueOrPromise<T>,
-    ) => this.intercept(invocationCtx, next);
-  }
-
-  async intercept<T>(
-    invocationCtx: InvocationContext,
-    next: () => ValueOrPromise<T>,
-  ) {
-    status.returnFromCache = false;
-
-    if (!this.request) {
-      // The method is not invoked by an http request, no caching
-      return await next();
-    }
-    const url = this.request.url;
-    const cachedValue = cachedResults.get(url);
-    if (cachedValue) {
-      status.returnFromCache = true;
-      return cachedValue as T;
-    }
-    const result = await next();
-    cachedResults.set(url, result);
-    return result;
+    ) => cache(invocationCtx, next);
   }
 }
 
 /**
- * An interceptor function for caching
+ * An interceptor function that caches results. It uses `invocationContext`
+ * to locate the http request
+ *
  * @param invocationCtx
  * @param next
  */

diff --git a/...est/src/__tests__/acceptance/caching-interceptor/global-caching-interceptor.acceptance.ts b/...est/src/__tests__/acceptance/caching-interceptor/global-caching-interceptor.acceptance.ts
@@ -3,7 +3,7 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {asInterceptor} from '@loopback/context';
+import {asGlobalInterceptor} from '@loopback/context';
 import {anOperationSpec} from '@loopback/openapi-spec-builder';
 import {get, param} from '@loopback/openapi-v3';
 import {
@@ -29,7 +29,7 @@ describe('global caching interceptor', () => {
     await app.stop();
   });
 
-  context('toUpperCase', () => {
+  context('caching invocation for controller methods', () => {
     it('invokes the controller method if not cached', async () => {
       await client.get('/toUpperCase/Hello').expect(200, 'HELLO');
       expect(status.returnFromCache).to.be.false();
@@ -49,7 +49,7 @@ describe('global caching interceptor', () => {
     });
   });
 
-  context('toLowerCase', () => {
+  context('caching invocation for route handler functions', () => {
     it('invokes the handler function if not cached', async () => {
       await client.get('/toLowerCase/Hello').expect(200, 'hello');
       expect(status.returnFromCache).to.be.false();
@@ -98,7 +98,7 @@ describe('global caching interceptor', () => {
     app
       .bind('caching-interceptor')
       .toProvider(CachingInterceptorProvider)
-      .apply(asInterceptor);
+      .apply(asGlobalInterceptor);
     app.controller(StringCaseController);
     app.route(
       'get',