fix: review 1

review 1
loopbackio · Oct 17, 2018 · feef4e3 · feef4e3
1 parent acb0089
commit feef4e3
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 32 deletions.
diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts
@@ -33,7 +33,7 @@ import {
   Route,
   RouteEntry,
   RoutingTable,
-  StaticRoute,
+  StaticAssetsRoute,
 } from './router/routing-table';
 
 import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence';
@@ -47,7 +47,6 @@ import {
   Send,
 } from './types';
 import {ServerOptions} from 'https';
-import * as HttpErrors from 'http-errors';
 
 const debug = require('debug')('loopback:rest:server');
 
@@ -194,20 +193,8 @@ export class RestServer extends Context implements Server, HttpServerLike {
     this.bind(RestBindings.HANDLER).toDynamicValue(() => this.httpHandler);
 
     // LB4's static assets serving router
-    const staticAssetsRouter = new StaticRoute(
-      (req: Request, res: Response) => {
-        return new Promise((resolve, reject) => {
-          const onFinished = () => resolve();
-          res.once('finish', onFinished);
-          this._routerForStaticAssets.handle(req, res, (err: Error) => {
-            if (err) {
-              return reject(err);
-            }
-            // Express router called next, which means no route was matched
-            return reject(new HttpErrors.NotFound());
-          });
-        });
-      },
+    const staticAssetsRouter = new StaticAssetsRoute(
+      this._routerForStaticAssets,
     );
 
     this.httpHandler.registerRoute(staticAssetsRouter);

diff --git a/packages/rest/src/router/router-base.ts b/packages/rest/src/router/router-base.ts
@@ -49,11 +49,6 @@ export abstract class BaseRouter implements RestRouter {
     else return this.findRouteWithPathVars(request);
   }
 
-  getStaticAssetsRouter() {
-    const route = this.routesWithoutPathVars['/get/*'];
-    return createResolvedRoute(route, {});
-  }
-
   list() {
     let routes = Object.values(this.routesWithoutPathVars);
     routes = routes.concat(this.listRoutesWithPathVars());

diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts
@@ -22,6 +22,7 @@ import {inspect} from 'util';
 
 import {
   Request,
+  Response,
   PathParameterValues,
   OperationArgs,
   OperationRetval,
@@ -32,11 +33,13 @@ import {RestBindings} from '../keys';
 import {ControllerSpec} from '@loopback/openapi-v3';
 
 import * as assert from 'assert';
+import * as express from 'express';
 const debug = require('debug')('loopback:rest:routing-table');
 
 import {CoreBindings} from '@loopback/core';
 import {validateApiPath} from './openapi-path';
 import {TrieRouter} from './trie-router';
+import {RequestContext} from '../request-context';
 
 /**
  * A controller instance with open properties/methods
@@ -74,8 +77,6 @@ export interface RestRouter {
    */
   find(request: Request): ResolvedRoute | undefined;
 
-  getStaticAssetsRouter(): ResolvedRoute;
-
   /**
    * List all routes
    */
@@ -135,6 +136,8 @@ export class RoutingTable {
     return fullPath;
   }
 
+  private _staticAssetsRoute: ResolvedRoute;
+
   /**
    * Register a route
    * @param route A route entry
@@ -154,6 +157,9 @@ export class RoutingTable {
 
     validateApiPath(route.path);
     this._router.add(route);
+    if (route.path === '*') {
+      this._staticAssetsRoute = createResolvedRoute(route, {});
+    }
   }
 
   describeApiPaths(): PathObject {
@@ -184,8 +190,12 @@ export class RoutingTable {
       return found;
     }
 
-    const staticAssetsRouter = this._router.getStaticAssetsRouter();
-    return staticAssetsRouter;
+    debug(
+      'No API route found for %s %s, trying to find a static assets',
+      request.method,
+      request.path,
+    );
+    return this._staticAssetsRoute;
   }
 }
 
@@ -240,24 +250,42 @@ export interface ResolvedRoute extends RouteEntry {
   readonly schemas: SchemasObject;
 }
 
-export class StaticRoute implements RouteEntry {
+export class StaticAssetsRoute implements RouteEntry {
   public readonly verb: string = 'get';
   public readonly path: string = '*';
   public readonly spec: OperationObject = {responses: {}};
-
-  constructor(private _handler: Function) {}
+  private _handler: Function;
+
+  constructor(private _routerForStaticAssets: express.Router) {
+    this._handler = (request: Request, response: Response) => {
+      return new Promise((resolve, reject) => {
+        const onFinished = () => resolve();
+        response.once('finish', onFinished);
+        this._routerForStaticAssets.handle(request, response, (err: Error) => {
+          if (err) {
+            return reject(err);
+          }
+          // Express router called next, which means no route was matched
+          return reject(
+            new HttpErrors.NotFound(
+              `Endpoint "${request.method} ${request.path}" not found.`,
+            ),
+          );
+        });
+      });
+    };
+  }
 
   updateBindings(requestContext: Context): void {
     // no-op
   }
 
   async invokeHandler(
-    requestContext: Context,
+    requestContext: RequestContext,
     args: OperationArgs,
   ): Promise<OperationRetval> {
-    const req = await requestContext.get(RestBindings.Http.REQUEST);
-    const res = await requestContext.get(RestBindings.Http.RESPONSE);
-    return this._handler(req, res);
+    const {request, response} = requestContext;
+    return this._handler(request, response);
   }
 
   describe(): string {