fix: optimize serving static files

packages/rest/src/http-handler.ts

@@ -19,6 +19,8 @@ import {Request, Response} from './types';
 
 import {RestBindings} from './keys';
 import {RequestContext} from './request-context';
+import {PathParams} from 'express-serve-static-core';
+import {ServeStaticOptions} from 'serve-static';
 
 export class HttpHandler {
   protected _apiDefinitions: SchemasObject;
@@ -48,6 +50,14 @@ export class HttpHandler {
     this._apiDefinitions = Object.assign({}, this._apiDefinitions, defs);
   }
 
+  registerStaticAssets(
+    path: PathParams,
+    rootDir: string,
+    options?: ServeStaticOptions,
+  ) {
+    this._routes.registerStaticAssets(path, rootDir, options);
+  }
+
   getApiDefinitions() {
     return this._apiDefinitions;
   }

packages/rest/src/rest.server.ts

@@ -14,11 +14,12 @@ import {
 } from '@loopback/openapi-v3-types';
 import {AssertionError} from 'assert';
 import * as cors from 'cors';
+import * as debugFactory from 'debug';
 import * as express from 'express';
 import {PathParams} from 'express-serve-static-core';
 import {IncomingMessage, ServerResponse} from 'http';
+import {ServerOptions} from 'https';
 import {safeDump} from 'js-yaml';
-import * as pathToRegExp from 'path-to-regexp';
 import {ServeStaticOptions} from 'serve-static';
 import {HttpHandler} from './http-handler';
 import {RestBindings} from './keys';
@@ -34,7 +35,6 @@ import {
   RouteEntry,
   RoutingTable,
 } from './router/routing-table';
-
 import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence';
 import {
   FindRoute,
@@ -45,9 +45,8 @@ import {
   Response,
   Send,
 } from './types';
-import {ServerOptions} from 'https';
 
-const debug = require('debug')('loopback:rest:server');
+const debug = debugFactory('loopback:rest:server');
 
 export type HttpRequestListener = (
   req: IncomingMessage,
@@ -127,7 +126,6 @@ export class RestServer extends Context implements Server, HttpServerLike {
   protected _httpServer: HttpServer | undefined;
 
   protected _expressApp: express.Application;
-  protected _routerForStaticAssets: express.Router;
 
   get listening(): boolean {
     return this._httpServer ? this._httpServer.listening : false;
@@ -217,9 +215,6 @@ export class RestServer extends Context implements Server, HttpServerLike {
     };
     this._expressApp.use(cors(corsOptions));
 
-    // Place the assets router here before controllers
-    this._setupRouterForStaticAssets();
-
     // Set up endpoints for OpenAPI spec/ui
     this._setupOpenApiSpecEndpoints();
 
@@ -236,17 +231,6 @@ export class RestServer extends Context implements Server, HttpServerLike {
     );
   }
 
-  /**
-   * Set up an express router for all static assets so that middleware for
-   * all directories are invoked at the same phase
-   */
-  protected _setupRouterForStaticAssets() {
-    if (!this._routerForStaticAssets) {
-      this._routerForStaticAssets = express.Router();
-      this._expressApp.use(this._routerForStaticAssets);
-    }
-  }
-
   /**
    * Mount /openapi.json, /openapi.yaml for specs and /swagger-ui, /explorer
    * to redirect to externally hosted API explorer
@@ -626,18 +610,11 @@ export class RestServer extends Context implements Server, HttpServerLike {
    * See https://expressjs.com/en/4x/api.html#express.static
    * @param path The path(s) to serve the asset.
    * See examples at https://expressjs.com/en/4x/api.html#path-examples
-   * To avoid performance penalty, `/` is not allowed for now.
    * @param rootDir The root directory from which to serve static assets
    * @param options Options for serve-static
    */
   static(path: PathParams, rootDir: string, options?: ServeStaticOptions) {
-    const re = pathToRegExp(path, [], {end: false});
-    if (re.test('/')) {
-      throw new Error(
-        'Static assets cannot be mount to "/" to avoid performance penalty.',
-      );
-    }
-    this._routerForStaticAssets.use(path, express.static(rootDir, options));
+    this.httpHandler.registerStaticAssets(path, rootDir, options);
   }
 
   /**

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

@@ -27,6 +27,8 @@ import {
   OperationRetval,
 } from '../types';
 
+import * as express from 'express';
+
 import {ControllerSpec} from '@loopback/openapi-v3';
 
 import * as assert from 'assert';
@@ -35,6 +37,9 @@ 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';
+import {ServeStaticOptions} from 'serve-static';
+import {PathParams} from 'express-serve-static-core';
 
 /**
  * A controller instance with open properties/methods
@@ -84,6 +89,19 @@ export interface RestRouter {
 export class RoutingTable {
   constructor(private readonly _router: RestRouter = new TrieRouter()) {}
 
+  private _staticAssetsRoute: StaticAssetsRoute;
+
+  registerStaticAssets(
+    path: PathParams,
+    rootDir: string,
+    options?: ServeStaticOptions,
+  ) {
+    if (!this._staticAssetsRoute) {
+      this._staticAssetsRoute = new StaticAssetsRoute();
+    }
+    this._staticAssetsRoute.registerAssets(path, rootDir, options);
+  }
+
   /**
    * Register a controller as the route
    * @param spec
@@ -149,6 +167,7 @@ export class RoutingTable {
     }
 
     validateApiPath(route.path);
+
     this._router.add(route);
   }
 
@@ -181,6 +200,17 @@ export class RoutingTable {
       return found;
     }
 
+    // this._staticAssetsRoute will be set only if app.static() was called
+    if (this._staticAssetsRoute) {
+      debug(
+        'No API route found for %s %s, trying to find a static asset',
+        request.method,
+        request.path,
+      );
+
+      return this._staticAssetsRoute;
+    }
+
     debug('No route found for %s %s', request.method, request.path);
     throw new HttpErrors.NotFound(
       `Endpoint "${request.method} ${request.path}" not found.`,
@@ -271,6 +301,88 @@ export abstract class BaseRoute implements RouteEntry {
   }
 }
 
+export class StaticAssetsRoute implements RouteEntry, ResolvedRoute {
+  // ResolvedRoute API
+  readonly pathParams: PathParameterValues = [];
+  readonly schemas: SchemasObject = {};
+
+  // RouteEntry implementation
+  readonly verb: string = 'get';
+  readonly path: string = '/*';
+  readonly spec: OperationObject = {
+    description: 'LoopBack static assets route',
+    'x-visibility': 'undocumented',
+    responses: {},
+  };
+
+  private readonly _expressRouter: express.Router = express.Router();
+
+  public registerAssets(
+    path: PathParams,
+    rootDir: string,
+    options?: ServeStaticOptions,
+  ) {
+    this._expressRouter.use(path, express.static(rootDir, options));
+  }
+
+  updateBindings(requestContext: Context): void {
+    // no-op
+  }
+
+  async invokeHandler(
+    {request, response}: RequestContext,
+    args: OperationArgs,
+  ): Promise<OperationRetval> {
+    const handled = await executeRequestHandler(
+      this._expressRouter,
+      request,
+      response,
+    );
+
+    if (!handled) {
+      // 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';
+  }
+}
+
+/**
+ * Execute an Express-style callback-based request handler.
+ *
+ * @param handler
+ * @param request
+ * @param response
+ * @returns A promise resolved to:
+ * - `true` when the request was handled
+ * - `false` when the handler called `next()` to proceed to the next
+ *    handler (middleware) in the chain.
+ */
+function executeRequestHandler(
+  handler: express.RequestHandler,
+  request: Request,
+  response: express.Response,
+): Promise<boolean> {
+  return new Promise((resolve, reject) => {
+    const onceFinished = () => resolve(true);
+    response.once('finish', onceFinished);
+    handler(request, response, (err: Error) => {
+      response.removeListener('finish', onceFinished);
+      if (err) {
+        reject(err);
+      } else {
+        // Express router called next, which means no route was matched
+        resolve(false);
+      }
+    });
+  });
+}
+
 export function createResolvedRoute(
   route: RouteEntry,
   pathParams: PathParameterValues,

packages/rest/test/integration/rest.application.integration.ts

@@ -0,0 +1,48 @@
+// Copyright IBM Corp. 2017,2018. All Rights Reserved.
+// Node module: @loopback/rest
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+import {createRestAppClient, Client} from '@loopback/testlab';
+import {RestApplication} from '../..';
+import * as path from 'path';
+import * as fs from 'fs';
+
+const FIXTURES = path.resolve(__dirname, '../../../fixtures');
+
+describe('RestApplication (integration)', () => {
+  let restApp: RestApplication;
+  let client: Client;
+
+  beforeEach(givenAnApplication);
+  beforeEach(givenClient);
+  afterEach(async () => {
+    await restApp.stop();
+  });
+
+  it('serves static assets', async () => {
+    const root = FIXTURES;
+    const content = fs
+      .readFileSync(path.join(root, 'index.html'))
+      .toString('utf-8');
+    await client
+      .get('/index.html')
+      .expect(200)
+      .expect(content);
+  });
+
+  it('returns 404 if asset is not found', async () => {
+    await client.get('/404.html').expect(404);
+  });
+
+  function givenAnApplication() {
+    const root = FIXTURES;
+    restApp = new RestApplication({rest: {port: 0, host: '127.0.0.1'}});
+    restApp.static('/', root);
+  }
+
+  async function givenClient() {
+    await restApp.start();
+    return (client = createRestAppClient(restApp));
+  }
+});

packages/rest/test/integration/rest.server.integration.ts

@@ -80,37 +80,20 @@ describe('RestServer (integration)', () => {
       .expect(500);
   });
 
-  it('does not allow static assets to be mounted at /', async () => {
+  it('allows static assets to be mounted at /', async () => {
     const root = FIXTURES;
     const server = await givenAServer({
       rest: {
         port: 0,
       },
     });
 
-    expect(() => server.static('/', root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
-
-    expect(() => server.static('', root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
-
-    expect(() => server.static(['/'], root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
-
-    expect(() => server.static(['/html', ''], root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
-
-    expect(() => server.static(/.*/, root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
-
-    expect(() => server.static('/(.*)', root)).to.throw(
-      'Static assets cannot be mount to "/" to avoid performance penalty.',
-    );
+    expect(() => server.static('/', root)).to.not.throw();
+    expect(() => server.static('', root)).to.not.throw();
+    expect(() => server.static(['/'], root)).to.not.throw();
+    expect(() => server.static(['/html', ''], root)).to.not.throw();
+    expect(() => server.static(/.*/, root)).to.not.throw();
+    expect(() => server.static('/(.*)', root)).to.not.throw();
   });
 
   it('allows static assets via api', async () => {
@@ -164,7 +147,7 @@ describe('RestServer (integration)', () => {
       .expect(200, 'Hello');
   });
 
-  it('serve static assets if matches before other routes', async () => {
+  it('gives precedence to API routes over static assets', async () => {
     const root = FIXTURES;
     const server = await givenAServer({
       rest: {
@@ -174,12 +157,9 @@ describe('RestServer (integration)', () => {
     server.static('/html', root);
     server.handler(dummyRequestHandler);
 
-    const content = fs
-      .readFileSync(path.join(root, 'index.html'))
-      .toString('utf-8');
     await createClientForHandler(server.requestHandler)
       .get('/html/index.html')
-      .expect(200, content);
+      .expect(200, 'Hello');
   });
 
   it('allows cors', async () => {