refactor(rest): rework StaticAssetsRoute into ExternalExpressRoutes

Refactor the code handling static assets routes to make it open to
extension and allow adding additional types of external routes
later in the (near) future.

This commit is a pure refactoring. The only visible change is removal
of `StaticAssetsRoute` from the public API.

packages/rest/src/rest.server.ts

@@ -41,7 +41,7 @@ import {
   Route,
   RouteEntry,
   RoutingTable,
-  StaticAssetsRoute,
+  ExternalExpressRoutes,
   RedirectRoute,
 } from './router';
 import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence';
@@ -298,7 +298,7 @@ export class RestServer extends Context implements Server, HttpServerLike {
      * Check if there is custom router in the context
      */
     const router = this.getSync(RestBindings.ROUTER, {optional: true});
-    const routingTable = new RoutingTable(router, this._staticAssetRoute);
+    const routingTable = new RoutingTable(router, this._externalRoutes);
 
     this._httpHandler = new HttpHandler(this, routingTable);
     for (const b of this.find('controllers.*')) {
@@ -691,8 +691,10 @@ export class RestServer extends Context implements Server, HttpServerLike {
     );
   }
 
-  // The route for static assets
-  private _staticAssetRoute = new StaticAssetsRoute();
+  /*
+   * Registry of external routes & static assets
+   */
+  private _externalRoutes = new ExternalExpressRoutes();
 
   /**
    * Mount static assets to the REST server.
@@ -703,7 +705,7 @@ export class RestServer extends Context implements Server, HttpServerLike {
    * @param options Options for serve-static
    */
   static(path: PathParams, rootDir: string, options?: ServeStaticOptions) {
-    this._staticAssetRoute.registerAssets(path, rootDir, options);
+    this._externalRoutes.registerAssets(path, rootDir, options);
   }
 
   /**

packages/rest/src/router/static-assets-route.ts → packages/rest/src/router/external-express-routes.ts

@@ -5,7 +5,8 @@
 
 import {Context} from '@loopback/context';
 import {OperationObject, SchemasObject} from '@loopback/openapi-v3-types';
-import {RequestHandler, Router, static as serveStatic} from 'express';
+import * as express from 'express';
+import {RequestHandler} from 'express';
 import {PathParams} from 'express-serve-static-core';
 import * as HttpErrors from 'http-errors';
 import * as onFinished from 'on-finished';
@@ -21,30 +22,47 @@ import {
 } from '../types';
 import {ResolvedRoute, RouteEntry} from './route-entry';
 
-export class StaticAssetsRoute implements RouteEntry, ResolvedRoute {
-  // ResolvedRoute API
-  readonly pathParams: PathParameterValues = [];
-  readonly schemas: SchemasObject = {};
+export type ExpressRequestHandler = express.RequestHandler;
 
-  // RouteEntry implementation
-  readonly verb: string = 'get';
-  readonly path: string = '/*';
-  readonly spec: OperationObject = {
-    description: 'LoopBack static assets route',
-    'x-visibility': 'undocumented',
-    responses: {},
-  };
-
-  constructor(private readonly _expressRouter: Router = Router()) {}
+/**
+ * A registry of external, Express-style routes. These routes are invoked
+ * _after_ no LB4 route (controller or handler based) matched the incoming
+ * request.
+ *
+ * @private
+ */
+export class ExternalExpressRoutes {
+  protected _staticRoutes: express.Router = express.Router();
 
   public registerAssets(
     path: PathParams,
     rootDir: string,
     options?: ServeStaticOptions,
   ) {
-    this._expressRouter.use(path, serveStatic(rootDir, options));
+    this._staticRoutes.use(path, express.static(rootDir, options));
   }
 
+  find(request: Request): ResolvedRoute {
+    return new ExternalRoute(this._staticRoutes, request.method, request.url, {
+      description: 'LoopBack static assets route',
+      'x-visibility': 'undocumented',
+      responses: {},
+    });
+  }
+}
+
+class ExternalRoute implements RouteEntry, ResolvedRoute {
+  // ResolvedRoute API
+  readonly pathParams: PathParameterValues = [];
+  readonly schemas: SchemasObject = {};
+
+  constructor(
+    private readonly _staticAssets: express.Router,
+    public readonly verb: string,
+    public readonly path: string,
+    public readonly spec: OperationObject,
+  ) {}
+
   updateBindings(requestContext: Context): void {
     // no-op
   }
@@ -54,21 +72,21 @@ export class StaticAssetsRoute implements RouteEntry, ResolvedRoute {
     args: OperationArgs,
   ): Promise<OperationRetval> {
     const handled = await executeRequestHandler(
-      this._expressRouter,
+      this._staticAssets,
       request,
       response,
     );
+    if (handled) return;
 
-    if (!handled) {
-      // Express router called next, which means no route was matched
-      throw new HttpErrors.NotFound(
-        `Endpoint "${request.method} ${request.path}" not found.`,
-      );
-    }
+    // Express router called next, which means no route was matched
+    throw new HttpErrors.NotFound(
+      `Endpoint "${request.method} ${request.path}" not found.`,
+    );
   }
 
   describe(): string {
-    return 'final route to handle static assets';
+    // TODO(bajtos) provide better description for Express routes with spec
+    return `External Express route "${this.verb} ${this.path}"`;
   }
 }
 

packages/rest/src/router/index.ts

@@ -8,7 +8,7 @@ export * from './route-entry';
 export * from './base-route';
 export * from './controller-route';
 export * from './handler-route';
-export * from './static-assets-route';
+export * from './external-express-routes';
 export * from './redirect-route';
 
 // routers

packages/rest/src/router/routing-table.ts

@@ -22,7 +22,7 @@ import {
 import {validateApiPath} from './openapi-path';
 import {RestRouter} from './rest-router';
 import {ResolvedRoute, RouteEntry} from './route-entry';
-import {StaticAssetsRoute} from './static-assets-route';
+import {ExternalExpressRoutes} from './external-express-routes';
 import {TrieRouter} from './trie-router';
 
 const debug = debugFactory('loopback:rest:routing-table');
@@ -31,18 +31,10 @@ const debug = debugFactory('loopback:rest:routing-table');
  * Routing table
  */
 export class RoutingTable {
-  /**
-   * A route for static assets
-   */
-  private _staticAssetsRoute: StaticAssetsRoute;
   constructor(
     private readonly _router: RestRouter = new TrieRouter(),
-    staticAssetsRoute?: StaticAssetsRoute,
-  ) {
-    if (staticAssetsRoute) {
-      this._staticAssetsRoute = staticAssetsRoute;
-    }
-  }
+    private _externalRoutes?: ExternalExpressRoutes,
+  ) {}
 
   /**
    * Register a controller as the route
@@ -143,15 +135,14 @@ export class RoutingTable {
       return found;
     }
 
-    // this._staticAssetsRoute will be set only if app.static() was called
-    if (this._staticAssetsRoute) {
+    if (this._externalRoutes) {
       debug(
         'No API route found for %s %s, trying to find a static asset',
         request.method,
         request.path,
       );
 
-      return this._staticAssetsRoute;
+      return this._externalRoutes.find(request);
     }
 
     debug('No route found for %s %s', request.method, request.path);