From e40c1d39bd2f9c96a09747a4df5c5108e79dae44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 4 Mar 2019 09:17:47 +0100 Subject: [PATCH] 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 | 12 ++-- ...ts-route.ts => external-express-routes.ts} | 66 ++++++++++++------- packages/rest/src/router/index.ts | 2 +- packages/rest/src/router/routing-table.ts | 19 ++---- 4 files changed, 55 insertions(+), 44 deletions(-) rename packages/rest/src/router/{static-assets-route.ts => external-express-routes.ts} (61%) diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 206d880b570c..38eaf5065f0c 100644 --- a/packages/rest/src/rest.server.ts +++ b/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); } /** diff --git a/packages/rest/src/router/static-assets-route.ts b/packages/rest/src/router/external-express-routes.ts similarity index 61% rename from packages/rest/src/router/static-assets-route.ts rename to packages/rest/src/router/external-express-routes.ts index b3aa64b58ca1..bc2c16f9d3f2 100644 --- a/packages/rest/src/router/static-assets-route.ts +++ b/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 { 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}"`; } } diff --git a/packages/rest/src/router/index.ts b/packages/rest/src/router/index.ts index 559c8969b59f..20bfa22c760a 100644 --- a/packages/rest/src/router/index.ts +++ b/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 diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 9505ff91a498..f9ab053a0609 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/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);