From 0059ac4524b51329e1df769664c86ff5d6d4212a Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Sun, 3 May 2020 10:24:16 -0700 Subject: [PATCH] feat(express): make namespaces consistent and simplify chain invocations --- .../middleware-interceptor.acceptance.ts | 14 +++++---- packages/express/src/keys.ts | 21 +++++++++++++ .../express/src/middleware-interceptor.ts | 30 ++++++++++--------- packages/express/src/middleware.ts | 16 +++------- 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/packages/express/src/__tests__/acceptance/middleware-interceptor.acceptance.ts b/packages/express/src/__tests__/acceptance/middleware-interceptor.acceptance.ts index fa823c76d454..538d3f42bb30 100644 --- a/packages/express/src/__tests__/acceptance/middleware-interceptor.acceptance.ts +++ b/packages/express/src/__tests__/acceptance/middleware-interceptor.acceptance.ts @@ -144,10 +144,10 @@ describe('Middleware interceptor', () => { { global: true, injectConfiguration: false, - key: 'interceptors.middleware.spy', + key: 'globalInterceptors.middleware.spy', }, ); - expect(binding.key).to.eql('interceptors.middleware.spy'); + expect(binding.key).to.eql('globalInterceptors.middleware.spy'); return testFn(binding); }); @@ -163,7 +163,9 @@ describe('Middleware interceptor', () => { injectConfiguration: false, }, ); - expect(binding.key).to.eql('interceptors.middleware.namedSpyFactory'); + expect(binding.key).to.eql( + 'globalInterceptors.middleware.namedSpyFactory', + ); return testFn(binding); }); @@ -177,7 +179,7 @@ describe('Middleware interceptor', () => { injectConfiguration: false, }, ); - expect(binding.key).to.match(/^interceptors\.middleware\./); + expect(binding.key).to.match(/^globalInterceptors\.middleware\./); return testFn(binding); }); @@ -192,7 +194,9 @@ describe('Middleware interceptor', () => { const binding = createMiddlewareInterceptorBinding( SpyInterceptorProvider, ); - expect(binding.key).to.eql('interceptors.SpyInterceptorProvider'); + expect(binding.key).to.eql( + 'globalInterceptors.middleware.SpyInterceptorProvider', + ); helper.app.add(binding); return testFn(binding); }); diff --git a/packages/express/src/keys.ts b/packages/express/src/keys.ts index 7d00385d505b..518bf9dbb317 100644 --- a/packages/express/src/keys.ts +++ b/packages/express/src/keys.ts @@ -14,3 +14,24 @@ export namespace MiddlewareBindings { 'middleware.http.context', ); } + +/** + * Default namespaces for middleware + */ +export const MIDDLEWARE_NAMESPACE = 'middleware'; + +/** + * Default namespace for Express middleware based global interceptors + */ +export const GLOBAL_MIDDLEWARE_INTERCEPTOR_NAMESPACE = + 'globalInterceptors.middleware'; + +/** + * Default namespace for Express middleware based local interceptors + */ +export const MIDDLEWARE_INTERCEPTOR_NAMESPACE = 'globalInterceptors.middleware'; + +/** + * Default order group name for Express middleware based global interceptors + */ +export const DEFAULT_MIDDLEWARE_GROUP = 'middleware'; diff --git a/packages/express/src/middleware-interceptor.ts b/packages/express/src/middleware-interceptor.ts index 62796d3a13ab..8c0021e8eac4 100644 --- a/packages/express/src/middleware-interceptor.ts +++ b/packages/express/src/middleware-interceptor.ts @@ -21,13 +21,17 @@ import { InvocationContext, NamespacedReflect, Provider, - transformValueOrPromise, } from '@loopback/core'; import assert from 'assert'; import debugFactory from 'debug'; import onFinished from 'on-finished'; import {promisify} from 'util'; -import {MiddlewareBindings} from './keys'; +import { + DEFAULT_MIDDLEWARE_GROUP, + GLOBAL_MIDDLEWARE_INTERCEPTOR_NAMESPACE, + MiddlewareBindings, + MIDDLEWARE_INTERCEPTOR_NAMESPACE, +} from './keys'; import { ExpressMiddlewareFactory, ExpressRequestHandler, @@ -102,17 +106,11 @@ export function toInterceptor( const handlers = [firstHandler, ...additionalHandlers]; const interceptorList = handlers.map(handler => toInterceptor(handler)); return async (invocationCtx, next) => { - const middlewareCtx = await invocationCtx.get( - MiddlewareBindings.CONTEXT, - ); const middlewareChain = new GenericInterceptorChain( invocationCtx, interceptorList, ); - const result = middlewareChain.invokeInterceptors(); - return transformValueOrPromise(result, val => - val === middlewareCtx.response ? val : next(), - ); + return middlewareChain.invokeInterceptors(next); }; } @@ -362,16 +360,17 @@ export function registerExpressMiddlewareInterceptor( options = { injectConfiguration: true, global: true, - group: 'middleware', + group: DEFAULT_MIDDLEWARE_GROUP, ...options, }; if (!options.injectConfiguration) { let key = options.key; if (!key) { const name = buildName(middlewareFactory); - key = name - ? `interceptors.middleware.${name}` - : BindingKey.generate('interceptors.middleware'); + const namespace = options.global + ? GLOBAL_MIDDLEWARE_INTERCEPTOR_NAMESPACE + : MIDDLEWARE_INTERCEPTOR_NAMESPACE; + key = name ? `${namespace}.${name}` : BindingKey.generate(namespace); } const binding = ctx .bind(key!) @@ -412,9 +411,12 @@ export function createMiddlewareInterceptorBinding( group: 'middleware', ...options, }; + const namespace = options.global + ? GLOBAL_MIDDLEWARE_INTERCEPTOR_NAMESPACE + : MIDDLEWARE_INTERCEPTOR_NAMESPACE; const binding = createBindingFromClass(middlewareProviderClass, { defaultScope: BindingScope.SINGLETON, - namespace: 'interceptors', + namespace, }); if (options.global) { binding.tag({[ContextTags.GLOBAL_INTERCEPTOR_SOURCE]: 'route'}); diff --git a/packages/express/src/middleware.ts b/packages/express/src/middleware.ts index 8f2a7c3c28f2..775f782a4418 100644 --- a/packages/express/src/middleware.ts +++ b/packages/express/src/middleware.ts @@ -20,8 +20,8 @@ import { } from '@loopback/context'; import {extensionFilter, extensionFor} from '@loopback/core'; import debugFactory from 'debug'; +import {MIDDLEWARE_NAMESPACE} from './keys'; import { - buildName, createInterceptor, defineInterceptorProvider, toInterceptor, @@ -67,10 +67,7 @@ export function toMiddleware( return middlewareList[0](middlewareCtx, next); } const middlewareChain = new MiddlewareChain(middlewareCtx, middlewareList); - const result = middlewareChain.invokeInterceptors(); - return transformValueOrPromise(result, val => - val === middlewareCtx.response ? val : next(), - ); + return middlewareChain.invokeInterceptors(next); }; } @@ -112,11 +109,6 @@ export function registerExpressMiddleware( options = {injectConfiguration: true, ...options}; options.chain = options.chain ?? DEFAULT_MIDDLEWARE_CHAIN; if (!options.injectConfiguration) { - let key = options.key; - if (!key) { - const name = buildName(middlewareFactory); - key = name ? `interceptors.${name}` : BindingKey.generate('interceptors'); - } const middleware = createMiddleware(middlewareFactory, middlewareConfig); return registerMiddleware(ctx, middleware, options); } @@ -162,7 +154,7 @@ export function registerMiddleware( ctx.add(binding); return binding; } - const key = options.key ?? BindingKey.generate('middleware'); + const key = options.key ?? BindingKey.generate(MIDDLEWARE_NAMESPACE); return ctx .bind(key) .to(middleware as Middleware) @@ -183,7 +175,7 @@ export function createMiddlewareBinding( options.chain = options.chain ?? DEFAULT_MIDDLEWARE_CHAIN; const binding = createBindingFromClass(middlewareProviderClass, { defaultScope: BindingScope.TRANSIENT, - namespace: 'middleware', + namespace: MIDDLEWARE_NAMESPACE, key: options.key, }).apply(asMiddleware(options)); return binding;