diff --git a/packages/rest/fixtures/index.html b/packages/rest/fixtures/assets/index.html similarity index 100% rename from packages/rest/fixtures/index.html rename to packages/rest/fixtures/assets/index.html diff --git a/packages/rest/fixtures/other-assets/index.html b/packages/rest/fixtures/other-assets/index.html new file mode 100644 index 000000000000..67b34d7ab66e --- /dev/null +++ b/packages/rest/fixtures/other-assets/index.html @@ -0,0 +1,8 @@ + +
+ Other Assets +
+ +

Hello, World!

+ + diff --git a/packages/rest/fixtures/other-assets/robots.txt b/packages/rest/fixtures/other-assets/robots.txt new file mode 100644 index 000000000000..c2a49f4fb82f --- /dev/null +++ b/packages/rest/fixtures/other-assets/robots.txt @@ -0,0 +1,2 @@ +User-agent: * +Allow: / diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index 28ff43a89b8a..f4f81555d7b9 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -19,8 +19,6 @@ 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; @@ -50,14 +48,6 @@ 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; } diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index f6f4552b05b7..d1af982a68a5 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -34,6 +34,7 @@ import { Route, RouteEntry, RoutingTable, + StaticAssetsRoute, } from './router'; import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence'; import { @@ -270,7 +271,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); + const routingTable = new RoutingTable(router, this._staticAssetRoute); this._httpHandler = new HttpHandler(this, routingTable); for (const b of this.find('controllers.*')) { @@ -605,6 +606,9 @@ export class RestServer extends Context implements Server, HttpServerLike { ); } + // The route for static assets + private _staticAssetRoute = new StaticAssetsRoute(); + /** * Mount static assets to the REST server. * See https://expressjs.com/en/4x/api.html#express.static @@ -614,7 +618,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * @param options Options for serve-static */ static(path: PathParams, rootDir: string, options?: ServeStaticOptions) { - this.httpHandler.registerStaticAssets(path, rootDir, options); + this._staticAssetRoute.registerAssets(path, rootDir, options); } /** diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index c213f0fe2384..9505ff91a498 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -11,9 +11,7 @@ import { } from '@loopback/openapi-v3-types'; import * as assert from 'assert'; import * as debugFactory from 'debug'; -import {PathParams} from 'express-serve-static-core'; import * as HttpErrors from 'http-errors'; -import {ServeStaticOptions} from 'serve-static'; import {inspect} from 'util'; import {Request} from '../types'; import { @@ -33,19 +31,17 @@ const debug = debugFactory('loopback:rest:routing-table'); * Routing table */ export class RoutingTable { - constructor(private readonly _router: RestRouter = new TrieRouter()) {} - + /** + * A route for static assets + */ private _staticAssetsRoute: StaticAssetsRoute; - - registerStaticAssets( - path: PathParams, - rootDir: string, - options?: ServeStaticOptions, + constructor( + private readonly _router: RestRouter = new TrieRouter(), + staticAssetsRoute?: StaticAssetsRoute, ) { - if (!this._staticAssetsRoute) { - this._staticAssetsRoute = new StaticAssetsRoute(); + if (staticAssetsRoute) { + this._staticAssetsRoute = staticAssetsRoute; } - this._staticAssetsRoute.registerAssets(path, rootDir, options); } /** diff --git a/packages/rest/src/router/static-assets-route.ts b/packages/rest/src/router/static-assets-route.ts index e97588c76278..a6f79edd80a5 100644 --- a/packages/rest/src/router/static-assets-route.ts +++ b/packages/rest/src/router/static-assets-route.ts @@ -5,7 +5,7 @@ import {Context} from '@loopback/context'; import {OperationObject, SchemasObject} from '@loopback/openapi-v3-types'; -import * as express from 'express'; +import {Router, RequestHandler, static as serveStatic} from 'express'; import {PathParams} from 'express-serve-static-core'; import * as HttpErrors from 'http-errors'; import {ServeStaticOptions} from 'serve-static'; @@ -15,6 +15,7 @@ import { OperationRetval, PathParameterValues, Request, + Response, } from '../types'; import {ResolvedRoute, RouteEntry} from './route-entry'; @@ -32,14 +33,14 @@ export class StaticAssetsRoute implements RouteEntry, ResolvedRoute { responses: {}, }; - private readonly _expressRouter: express.Router = express.Router(); + constructor(private readonly _expressRouter: Router = Router()) {} public registerAssets( path: PathParams, rootDir: string, options?: ServeStaticOptions, ) { - this._expressRouter.use(path, express.static(rootDir, options)); + this._expressRouter.use(path, serveStatic(rootDir, options)); } updateBindings(requestContext: Context): void { @@ -81,9 +82,9 @@ export class StaticAssetsRoute implements RouteEntry, ResolvedRoute { * handler (middleware) in the chain. */ function executeRequestHandler( - handler: express.RequestHandler, + handler: RequestHandler, request: Request, - response: express.Response, + response: Response, ): Promise { return new Promise((resolve, reject) => { const onceFinished = () => resolve(true); diff --git a/packages/rest/test/integration/rest.application.integration.ts b/packages/rest/test/integration/rest.application.integration.ts index eb0e556d5312..cf8578b8f6d5 100644 --- a/packages/rest/test/integration/rest.application.integration.ts +++ b/packages/rest/test/integration/rest.application.integration.ts @@ -9,7 +9,7 @@ import * as path from 'path'; import * as fs from 'fs'; import {RestServer, RestServerConfig} from '../../src'; -const FIXTURES = path.resolve(__dirname, '../../../fixtures'); +const ASSETS = path.resolve(__dirname, '../../../fixtures/assets'); describe('RestApplication (integration)', () => { let restApp: RestApplication; @@ -23,10 +23,10 @@ describe('RestApplication (integration)', () => { it('serves static assets from root path', async () => { givenApplication(); - restApp.static('/', FIXTURES); + restApp.static('/', ASSETS); await restApp.start(); const content = fs - .readFileSync(path.join(FIXTURES, 'index.html')) + .readFileSync(path.join(ASSETS, '', 'index.html')) .toString('utf-8'); client = createRestAppClient(restApp); await client @@ -37,10 +37,10 @@ describe('RestApplication (integration)', () => { it('serves static assets from non-root path', async () => { givenApplication(); - restApp.static('/public', FIXTURES); + restApp.static('/public', ASSETS); await restApp.start(); const content = fs - .readFileSync(path.join(FIXTURES, 'index.html')) + .readFileSync(path.join(ASSETS, 'index.html')) .toString('utf-8'); client = createRestAppClient(restApp); await client @@ -51,7 +51,7 @@ describe('RestApplication (integration)', () => { it('returns 404 if asset is not found', async () => { givenApplication(); - restApp.static('/', FIXTURES); + restApp.static('/', ASSETS); await restApp.start(); client = createRestAppClient(restApp); await client.get('/404.html').expect(404); @@ -60,9 +60,9 @@ describe('RestApplication (integration)', () => { it('allows static assets via api after start', async () => { givenApplication(); await restApp.start(); - restApp.static('/', FIXTURES); + restApp.static('/', ASSETS); const content = fs - .readFileSync(path.join(FIXTURES, 'index.html')) + .readFileSync(path.join(ASSETS, 'index.html')) .toString('utf-8'); client = createRestAppClient(restApp); await client diff --git a/packages/rest/test/integration/rest.server.integration.ts b/packages/rest/test/integration/rest.server.integration.ts index 8933871f56a5..7abb66a404ad 100644 --- a/packages/rest/test/integration/rest.server.integration.ts +++ b/packages/rest/test/integration/rest.server.integration.ts @@ -12,14 +12,18 @@ import { httpsGetAsync, givenHttpServerConfig, } from '@loopback/testlab'; -import {RestBindings, RestServer, RestComponent} from '../..'; +import {RestBindings, RestServer, RestComponent, get} from '../..'; import {IncomingMessage, ServerResponse} from 'http'; import * as yaml from 'js-yaml'; import * as path from 'path'; import * as fs from 'fs'; +import * as util from 'util'; +const readFileAsync = util.promisify(fs.readFile); + import {RestServerConfig} from '../..'; const FIXTURES = path.resolve(__dirname, '../../../fixtures'); +const ASSETS = path.resolve(FIXTURES, 'assets'); describe('RestServer (integration)', () => { it('exports url property', async () => { @@ -81,7 +85,7 @@ describe('RestServer (integration)', () => { }); it('allows static assets to be mounted at /', async () => { - const root = FIXTURES; + const root = ASSETS; const server = await givenAServer({ rest: { port: 0, @@ -97,7 +101,7 @@ describe('RestServer (integration)', () => { }); it('allows static assets via api', async () => { - const root = FIXTURES; + const root = ASSETS; const server = await givenAServer({ rest: { port: 0, @@ -114,8 +118,57 @@ describe('RestServer (integration)', () => { .expect(200, content); }); + it('allows static assets to be mounted on multiple paths', async () => { + const root = ASSETS; + const server = await givenAServer({ + rest: { + port: 0, + }, + }); + + server.static('/html-0', root); + server.static('/html-1', root); + + const content = fs + .readFileSync(path.join(root, 'index.html')) + .toString('utf-8'); + await createClientForHandler(server.requestHandler) + .get('/html-0/index.html') + .expect('Content-Type', /text\/html/) + .expect(200, content); + await createClientForHandler(server.requestHandler) + .get('/html-1/index.html') + .expect('Content-Type', /text\/html/) + .expect(200, content); + }); + + it('merges different static asset directories when mounted on the same path', async () => { + const root = ASSETS; + const otherAssets = path.join(FIXTURES, 'other-assets'); + const server = await givenAServer({ + rest: { + port: 0, + }, + }); + + server.static('/html', root); + server.static('/html', otherAssets); + + let content = await readFileFromDirectory(root, 'index.html'); + await createClientForHandler(server.requestHandler) + .get('/html/index.html') + .expect('Content-Type', /text\/html/) + .expect(200, content); + + content = await readFileFromDirectory(otherAssets, 'robots.txt'); + await createClientForHandler(server.requestHandler) + .get('/html/robots.txt') + .expect('Content-Type', /text\/plain/) + .expect(200, content); + }); + it('allows static assets via api after start', async () => { - const root = FIXTURES; + const root = ASSETS; const server = await givenAServer({ rest: { port: 0, @@ -133,7 +186,7 @@ describe('RestServer (integration)', () => { }); it('allows non-static routes after assets', async () => { - const root = FIXTURES; + const root = ASSETS; const server = await givenAServer({ rest: { port: 0, @@ -148,7 +201,7 @@ describe('RestServer (integration)', () => { }); it('gives precedence to API routes over static assets', async () => { - const root = FIXTURES; + const root = ASSETS; const server = await givenAServer({ rest: { port: 0, @@ -162,6 +215,21 @@ describe('RestServer (integration)', () => { .expect(200, 'Hello'); }); + it('registers controllers defined later than static assets', async () => { + const root = ASSETS; + const server = await givenAServer({ + rest: { + port: 0, + }, + }); + server.static('/html', root); + server.controller(DummyController); + + await createClientForHandler(server.requestHandler) + .get('/html') + .expect(200, 'Hi'); + }); + it('allows cors', async () => { const server = await givenAServer({rest: {port: 0}}); server.handler(dummyRequestHandler); @@ -597,4 +665,21 @@ paths: response.write('Hello'); response.end(); } + + class DummyController { + constructor() {} + @get('/html', { + responses: {}, + }) + ping(): string { + return 'Hi'; + } + } + + function readFileFromDirectory( + dirname: string, + filename: string, + ): Promise { + return readFileAsync(path.join(dirname, filename), {encoding: 'utf-8'}); + } }); diff --git a/packages/rest/test/unit/router/routing-table.unit.ts b/packages/rest/test/unit/router/routing-table.unit.ts index 65cd54df66d7..8db08ca33384 100644 --- a/packages/rest/test/unit/router/routing-table.unit.ts +++ b/packages/rest/test/unit/router/routing-table.unit.ts @@ -18,6 +18,7 @@ import { RegExpRouter, TrieRouter, } from '../../..'; +import * as HttpErrors from 'http-errors'; describe('RoutingTable', () => { it('joins basePath and path', () => { @@ -187,6 +188,17 @@ function runTestsWithRouter(router: RestRouter) { expect(route.pathParams).to.containEql({arg1: '3', arg2: '2'}); }); + it('throws if router is not found', () => { + const table = givenRoutingTable(); + + const request = givenRequest({ + method: 'get', + url: '/hi', + }); + + expect(() => table.find(request)).to.throwError(HttpErrors.NotFound); + }); + function givenRequest(options?: ShotRequestOptions): Request { return stubExpressContext(options).request; }