fix(mvc): Route order mechanism

Closes: #29
tsedio · May 9, 2019 · bba9d20 · bba9d20
1 parent a86ac81
commit bba9d20
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 41 deletions.
diff --git a/packages/common/src/filters/constants/index.ts b/packages/common/src/filters/constants/index.ts
@@ -60,13 +60,13 @@ FilterPreHandlers.set(EXPRESS_ERR, locals => locals.err);
 FilterPreHandlers.set(ENDPOINT_INFO, locals => {
   const op = FilterPreHandlers.get(EXPRESS_REQUEST)!;
 
-  return op(locals).getEndpoint();
+  return op(locals).ctx.endpoint;
 });
 /**
  * ResponseData PreHandler
  */
 FilterPreHandlers.set(RESPONSE_DATA, locals => {
   const op = FilterPreHandlers.get(EXPRESS_REQUEST)!;
 
-  return op(locals).getStoredData();
+  return op(locals).ctx.data;
 });
diff --git a/packages/common/src/mvc/class/ControllerBuilder.ts b/packages/common/src/mvc/class/ControllerBuilder.ts
@@ -5,6 +5,7 @@ import {ControllerProvider} from "../class/ControllerProvider";
 import {EndpointMetadata} from "../class/EndpointMetadata";
 import {bindEndpointMiddleware} from "../components/bindEndpointMiddleware";
 import {SendResponseMiddleware} from "../components/SendResponseMiddleware";
+import {IPathMethod} from "../interfaces/IPathMethod";
 import {HandlerBuilder} from "./HandlerBuilder";
 
 export class ControllerBuilder {
@@ -28,16 +29,35 @@ export class ControllerBuilder {
     this.buildMiddlewares(injector, useBefore) // Controller before-middleware
       .buildEndpoints(injector) // All endpoints and his middlewares
       .buildMiddlewares(injector, useAfter) // Controller after-middleware
-      .buildSendResponse(injector) // Final middleware to send response
       .buildChildrenCtrls(injector); // Children controllers
 
     return this.provider.router;
   }
 
   private buildEndpoints(injector: InjectorService) {
     const {endpoints} = this.provider;
+    const pathsMethodsMap: Map<string, IPathMethod> = new Map();
 
-    endpoints.forEach((endpoint: EndpointMetadata) => this.buildEndpoint(injector, endpoint));
+    endpoints.forEach(({pathsMethods}) => {
+      pathsMethods.forEach(pathMethod => {
+        pathMethod.method = pathMethod.method || "use";
+
+        if (pathMethod.method !== "use") {
+          const key = pathMethod.method + "-" + pathMethod.path;
+
+          if (pathsMethodsMap.has(key)) {
+            pathsMethodsMap.get(key)!.isFinal = false;
+          }
+
+          pathMethod.isFinal = true;
+          pathsMethodsMap.set(key, pathMethod);
+        }
+      });
+    });
+
+    endpoints.forEach(endpoint => {
+      this.buildEndpoint(injector, endpoint);
+    });
 
     return this;
   }
@@ -48,11 +68,11 @@ export class ControllerBuilder {
       router,
       middlewares: {use}
     } = this.provider;
-
     // Endpoint lifecycle
     let handlers: any[] = [];
 
     handlers = handlers
+      .concat(bindEndpointMiddleware(endpoint))
       .concat(use) // Controller use-middlewares
       .concat(beforeMiddlewares) // Endpoint before-middlewares
       .concat(mldwrs) // Endpoint middlewares
@@ -61,16 +81,13 @@ export class ControllerBuilder {
       .filter((item: any) => !!item)
       .map((middleware: any) => HandlerBuilder.from(middleware).build(injector));
 
-    handlers = [bindEndpointMiddleware(endpoint)].concat(handlers);
+    const sendResponse = HandlerBuilder.from(SendResponseMiddleware).build(injector);
 
     // Add handlers to the router
-    pathsMethods.forEach(({path, method}) => {
-      if (!!method && router[method]) {
-        router[method](path, ...handlers);
-      } else {
-        const args: any[] = [path].concat(handlers);
-        router.use(...args);
-      }
+    pathsMethods.forEach(({path, method, isFinal}) => {
+      const localHandlers = isFinal ? handlers.concat(sendResponse) : handlers;
+
+      router[method!](path, ...localHandlers);
     });
 
     if (!pathsMethods.length) {
@@ -95,10 +112,6 @@ export class ControllerBuilder {
     });
   }
 
-  private buildSendResponse(injector: InjectorService) {
-    return this.buildMiddlewares(injector, [SendResponseMiddleware]);
-  }
-
   private buildMiddlewares(injector: InjectorService, middlewares: any[]) {
     const {router} = this.provider;
 

diff --git a/packages/common/src/mvc/class/EndpointMetadata.ts b/packages/common/src/mvc/class/EndpointMetadata.ts
@@ -1,7 +1,7 @@
 import {deepExtends, isArrayOrArrayClass, isPromise, Metadata, NotEnumerable, Storable, Store, Type} from "@tsed/core";
 import {ParamRegistry} from "../../filters/registries/ParamRegistry";
 import {EXPRESS_METHODS} from "../constants";
-import {ExpressPathMethod} from "../interfaces/ExpressPathMethod";
+import {IPathMethod} from "../interfaces/IPathMethod";
 import {PathParamsType} from "../interfaces/PathParamsType";
 
 /**
@@ -28,7 +28,7 @@ export class EndpointMetadata extends Storable {
   /**
    * Route strategy.
    */
-  public pathsMethods: ExpressPathMethod[] = [];
+  public pathsMethods: IPathMethod[] = [];
   /**
    * Endpoint inherited from parent class.
    */
@@ -57,7 +57,7 @@ export class EndpointMetadata extends Storable {
    */
   set httpMethod(value: string) {
     if (!this.pathsMethods[0]) {
-      this.pathsMethods[0] = {};
+      this.pathsMethods[0] = {} as any;
     }
 
     this.pathsMethods[0].method = value;
@@ -79,7 +79,7 @@ export class EndpointMetadata extends Storable {
    */
   set path(value: PathParamsType) {
     if (!this.pathsMethods[0]) {
-      this.pathsMethods[0] = {};
+      this.pathsMethods[0] = {} as any;
     }
     this.pathsMethods[0].path = value;
   }

diff --git a/packages/common/src/mvc/class/HandlerBuilder.ts b/packages/common/src/mvc/class/HandlerBuilder.ts
@@ -176,16 +176,11 @@ export class HandlerBuilder {
    * @param err
    */
   private async invoke(request: Express.Request, response: Express.Response, next: any, err?: any): Promise<any> {
-    const {hasEndpointInfo, hasNextFunction} = this.handlerMetadata;
+    const {hasNextFunction} = this.handlerMetadata;
     const {
-      ctx: {endpoint, container}
+      ctx: {container}
     } = request;
 
-    // Skip middleware when it use endpoint info without filled data
-    if (hasEndpointInfo && !endpoint) {
-      return next();
-    }
-
     next = this.buildNext(request, response, next);
 
     try {

diff --git a/packages/common/src/mvc/class/HandlerMetadata.ts b/packages/common/src/mvc/class/HandlerMetadata.ts
@@ -20,7 +20,6 @@ export class HandlerMetadata {
   readonly type: HandlerType = HandlerType.FUNCTION;
   readonly hasErrorParam: boolean = false;
   readonly hasNextFunction: boolean = false;
-  readonly hasEndpointInfo: boolean = false;
   handler: any;
 
   constructor(options: IHandlerOptions) {
@@ -35,7 +34,6 @@ export class HandlerMetadata {
       this.methodClassName = method;
       this.method = method;
       this.hasNextFunction = this.hasParamType(EXPRESS_NEXT_FN);
-      this.hasEndpointInfo = this.hasParamType(ENDPOINT_INFO);
       this.hasErrorParam = this.hasParamType(EXPRESS_ERR);
       this.injectable = (Metadata.get(PARAM_METADATA, target, method) || []).length > 0;
     }

diff --git a/packages/common/src/mvc/interfaces/ExpressPathMethod.ts b/packages/common/src/mvc/interfaces/ExpressPathMethod.ts
diff --git a/packages/common/src/mvc/interfaces/IPathMethod.ts b/packages/common/src/mvc/interfaces/IPathMethod.ts
@@ -0,0 +1,7 @@
+import {PathParamsType} from "./PathParamsType";
+
+export interface IPathMethod {
+  path: PathParamsType;
+  isFinal?: boolean;
+  method?: string;
+}
diff --git a/packages/common/src/mvc/utils/extendsRequest.ts b/packages/common/src/mvc/utils/extendsRequest.ts
@@ -33,6 +33,7 @@ if (!express.request.setEndpoint) {
      * @deprecated Use request.ctx.enpoint
      * @returns {any}
      */
+    // istanbul ignore next
     getEndpoint() {
       return this.ctx.endpoint;
     },

diff --git a/packages/common/test/mvc/class/ControllerBuilder.spec.ts b/packages/common/test/mvc/class/ControllerBuilder.spec.ts
@@ -52,7 +52,8 @@ describe("ControllerBuilder", () => {
     }];
     endpoint.pathsMethods.push({
       path: "/",
-      method: "get"
+      method: "get",
+      isFinal: true
     });
 
     // @ts-ignore
@@ -86,7 +87,7 @@ describe("ControllerBuilder", () => {
 
     // THEN
     result.should.to.eq(router);
-    router.use.callCount.should.deep.eq(4);
+    router.use.callCount.should.deep.eq(3);
     router.use.getCall(0).should.have.been.calledWithExactly(provider.middlewares.useBefore[0]); // controller
 
     // ENDPOINT
@@ -97,7 +98,8 @@ describe("ControllerBuilder", () => {
       endpoint.beforeMiddlewares[0],
       endpoint.middlewares[0],
       endpoint,
-      endpoint.afterMiddlewares[0]
+      endpoint.afterMiddlewares[0],
+      SendResponseMiddleware
     );
     router.use.getCall(2).should.have.been.calledWithExactly(provider.middlewares.useAfter[0]); // controller
   });
@@ -171,7 +173,6 @@ describe("ControllerBuilder", () => {
     );
 
     router.use.getCall(2).should.have.been.calledWithExactly(provider.middlewares.useAfter[0]); // controller
-    router.use.getCall(3).should.have.been.calledWithExactly(SendResponseMiddleware); // controller
   });
 
   it("should build controller (3)", () => {
@@ -239,6 +240,5 @@ describe("ControllerBuilder", () => {
     );
 
     router.use.getCall(2).should.have.been.calledWithExactly(provider.middlewares.useAfter[0]); // controller
-    router.use.getCall(3).should.have.been.calledWithExactly(SendResponseMiddleware); // controller
   });
 });