feat(express): make namespaces consistent and simplify chain invocations

loopbackio · May 3, 2020 · 0059ac4 · 0059ac4
1 parent 4453e01
commit 0059ac4
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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<CTX extends Context = InvocationContext>(
   const handlers = [firstHandler, ...additionalHandlers];
   const interceptorList = handlers.map(handler => toInterceptor<CTX>(handler));
   return async (invocationCtx, next) => {
-    const middlewareCtx = await invocationCtx.get<MiddlewareContext>(
-      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<CFG>(
   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<CFG>(
     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
@@ -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<CFG>(
   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;