Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions packages/rest/src/rest.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
Route,
RouteEntry,
RoutingTable,
StaticAssetsRoute,
ExternalExpressRoutes,
RedirectRoute,
} from './router';
import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence';
Expand Down Expand Up @@ -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.*')) {
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
}
Expand All @@ -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}"`;
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/rest/src/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 5 additions & 14 deletions packages/rest/src/router/routing-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand Down Expand Up @@ -143,15 +135,14 @@ export class RoutingTable {
return found;
}

// this._staticAssetsRoute will be set only if app.static() was called
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note this comment is no longer true, RestServer is always creating a new instance of StaticAssetsRoute/ExternalRoutes. I am not sure if that's a good thing and an intentional behavior. Either way, fixing it is out of scope of this pull request.

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);
Expand Down