From ef28cee692c1c2fbd7653e01d00a316938711ba8 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 16 Apr 2019 21:45:10 -0700 Subject: [PATCH] chore(rest): address review comments --- .../caching-interceptor.acceptance.ts | 22 ++++---------- .../caching-interceptor.ts | 30 ++++--------------- .../global-caching-interceptor.acceptance.ts | 8 ++--- 3 files changed, 15 insertions(+), 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 index 4a5a7cd955fd..8585183a46ec 100644 --- 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 index 5999be1a1628..641df275e98c 100644 --- 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(); @@ -46,33 +45,14 @@ export class CachingInterceptorProvider implements Provider { return ( invocationCtx: InvocationContext, next: () => ValueOrPromise, - ) => this.intercept(invocationCtx, next); - } - - async intercept( - invocationCtx: InvocationContext, - next: () => ValueOrPromise, - ) { - 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/packages/rest/src/__tests__/acceptance/caching-interceptor/global-caching-interceptor.acceptance.ts b/packages/rest/src/__tests__/acceptance/caching-interceptor/global-caching-interceptor.acceptance.ts index d3abe773cee9..3c0f0d3700a8 100644 --- a/packages/rest/src/__tests__/acceptance/caching-interceptor/global-caching-interceptor.acceptance.ts +++ b/packages/rest/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',