From 309a1cfdf8071dbdfce758cf4b80fca22bbc3bf0 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 9 Feb 2018 21:34:13 -0800 Subject: [PATCH 1/3] feat(rest): allow factory for controller routes --- packages/core/src/keys.ts | 9 + packages/rest/src/http-handler.ts | 9 +- packages/rest/src/index.ts | 4 + packages/rest/src/rest.application.ts | 18 +- packages/rest/src/rest.server.ts | 27 ++- packages/rest/src/router/routing-table.ts | 201 ++++++++++++++---- .../acceptance/routing/routing.acceptance.ts | 83 +++++++- .../unit/router/controller-factory.test.ts | 41 ++++ .../test/unit/router/controller-route.unit.ts | 54 ++++- 9 files changed, 399 insertions(+), 47 deletions(-) create mode 100644 packages/rest/test/unit/router/controller-factory.test.ts diff --git a/packages/core/src/keys.ts b/packages/core/src/keys.ts index 21775387ffe6..a229a34b0352 100644 --- a/packages/core/src/keys.ts +++ b/packages/core/src/keys.ts @@ -5,6 +5,7 @@ import {BindingKey} from '@loopback/context'; import {Application, ControllerClass} from './application'; +import {ControllerInstance} from '../../rest/src'; /** * Namespace for core binding keys @@ -53,4 +54,12 @@ export namespace CoreBindings { * request context */ export const CONTROLLER_METHOD_META = 'controller.method.meta'; + + /** + * Binding key for the controller instance resolved in the current request + * context + */ + export const CONTROLLER_CURRENT = BindingKey.create( + 'controller.current', + ); } diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index df93f88676a4..a73d52448ec7 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -15,6 +15,7 @@ import { ResolvedRoute, RouteEntry, ControllerClass, + ControllerFactory, } from './router/routing-table'; import {ParsedRequest} from './internal-types'; @@ -33,8 +34,12 @@ export class HttpHandler { this.handleRequest = (req, res) => this._handleRequest(req, res); } - registerController(name: ControllerClass, spec: ControllerSpec) { - this._routes.registerController(name, spec); + registerController( + controllerCtor: ControllerClass, + spec: ControllerSpec, + controllerFactory?: ControllerFactory, + ) { + this._routes.registerController(controllerCtor, spec, controllerFactory); } registerRoute(route: RouteEntry) { diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index 30e64cbd5da2..91a65d12de64 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -14,6 +14,10 @@ export { ResolvedRoute, createResolvedRoute, parseRequestUrl, + ControllerClass, + ControllerInstance, + ControllerFactory, + createControllerFactory, } from './router/routing-table'; export * from './providers'; diff --git a/packages/rest/src/rest.application.ts b/packages/rest/src/rest.application.ts index 2c5f734b9c02..edb96239e419 100644 --- a/packages/rest/src/rest.application.ts +++ b/packages/rest/src/rest.application.ts @@ -10,7 +10,11 @@ import {Binding, Constructor} from '@loopback/context'; import {format} from 'util'; import {RestBindings} from './keys'; import {RestServer, HttpRequestListener, HttpServerLike} from './rest.server'; -import {ControllerClass, RouteEntry} from './router/routing-table'; +import { + RouteEntry, + ControllerClass, + ControllerFactory, +} from './router/routing-table'; import {OperationObject, OpenApiSpec} from '@loopback/openapi-v3-types'; export const ERR_NO_MULTI_SERVER = format( @@ -97,6 +101,7 @@ export class RestApplication extends Application implements HttpServerLike { * @param spec The OpenAPI spec describing the endpoint (operation) * @param controller Controller constructor * @param methodName The name of the controller method + * @param factory A factory function to create controller instance */ route( verb: string, @@ -104,6 +109,7 @@ export class RestApplication extends Application implements HttpServerLike { spec: OperationObject, controller: ControllerClass, methodName: string, + factory?: ControllerFactory, ): Binding; /** @@ -127,12 +133,20 @@ export class RestApplication extends Application implements HttpServerLike { spec?: OperationObject, controller?: ControllerClass, methodName?: string, + factory?: ControllerFactory, ): Binding { const server = this.restServer; if (typeof routeOrVerb === 'object') { return server.route(routeOrVerb); } else { - return server.route(routeOrVerb, path!, spec!, controller!, methodName!); + return server.route( + routeOrVerb, + path!, + spec!, + controller!, + methodName!, + factory, + ); } } diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 15629454729a..11b2b0d4bf80 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -10,6 +10,8 @@ import { Route, ControllerRoute, RouteEntry, + createControllerFactory, + ControllerFactory, ControllerClass, } from './router/routing-table'; import {ParsedRequest} from './internal-types'; @@ -243,7 +245,8 @@ export class RestServer extends Context implements Server, HttpServerLike { if (apiSpec.components && apiSpec.components.schemas) { this._httpHandler.registerApiDefinitions(apiSpec.components.schemas); } - this._httpHandler.registerController(ctor, apiSpec); + const controllerFactory = createControllerFactory(b.key); + this._httpHandler.registerController(ctor, apiSpec, controllerFactory); } for (const b of this.find('routes.*')) { @@ -292,7 +295,15 @@ export class RestServer extends Context implements Server, HttpServerLike { ); } - const route = new ControllerRoute(verb, path, spec, ctor); + const controllerFactory = createControllerFactory(b.key); + const route = new ControllerRoute( + verb, + path, + spec, + ctor, + undefined, + controllerFactory, + ); this._httpHandler.registerRoute(route); return; } @@ -374,6 +385,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * @param spec The OpenAPI spec describing the endpoint (operation) * @param controller Controller constructor * @param methodName The name of the controller method + * @param factory A factory function to create controller instance */ route( verb: string, @@ -381,6 +393,7 @@ export class RestServer extends Context implements Server, HttpServerLike { spec: OperationObject, controller: ControllerClass, methodName: string, + factory?: ControllerFactory, ): Binding; /** @@ -404,6 +417,7 @@ export class RestServer extends Context implements Server, HttpServerLike { spec?: OperationObject, controller?: ControllerClass, methodName?: string, + controllerFactory?: ControllerFactory, ): Binding { if (typeof routeOrVerb === 'object') { const r = routeOrVerb; @@ -439,7 +453,14 @@ export class RestServer extends Context implements Server, HttpServerLike { } return this.route( - new ControllerRoute(routeOrVerb, path, spec, controller, methodName), + new ControllerRoute( + routeOrVerb, + path, + spec, + controller, + methodName, + controllerFactory, + ), ); } diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 8dbc705dce65..404e58e4a81c 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -9,10 +9,12 @@ import { PathsObject, } from '@loopback/openapi-v3-types'; import { + BindingScope, Context, Constructor, - instantiateClass, invokeMethod, + instantiateClass, + ValueOrPromise, } from '@loopback/context'; import {ServerRequest} from 'http'; import * as HttpErrors from 'http-errors'; @@ -61,13 +63,42 @@ export function parseRequestUrl(request: ServerRequest): ParsedRequest { return parsedRequest; } +/** + * A controller instance with open properties/methods + */ // tslint:disable-next-line:no-any -export type ControllerClass = Constructor; +export type ControllerInstance = {[name: string]: any}; +/** + * A factory function to create controller instances synchronously or + * asynchronously + */ +export type ControllerFactory = ( + ctx: Context, +) => ValueOrPromise; + +/** + * Controller class + */ +export type ControllerClass = Constructor; + +/** + * Routing table + */ export class RoutingTable { private readonly _routes: RouteEntry[] = []; - registerController(controller: ControllerClass, spec: ControllerSpec) { + /** + * Register a controller as the route + * @param controller + * @param spec + * @param controllerFactory + */ + registerController( + controller: ControllerClass, + spec: ControllerSpec, + controllerFactory?: ControllerFactory, + ) { assert( typeof spec === 'object' && !!spec, 'API specification must be a non-null object', @@ -83,7 +114,14 @@ export class RoutingTable { for (const verb in spec.paths[p]) { const opSpec: OperationObject = spec.paths[p][verb]; const fullPath = RoutingTable.joinPath(basePath, p); - const route = new ControllerRoute(verb, fullPath, opSpec, controller); + const route = new ControllerRoute( + verb, + fullPath, + opSpec, + controller, + undefined, + controllerFactory, + ); this.registerRoute(route); } } @@ -98,6 +136,10 @@ export class RoutingTable { return fullPath; } + /** + * Register a route + * @param route A route entry + */ registerRoute(route: RouteEntry) { // TODO(bajtos) handle the case where opSpec.parameters contains $ref // See https://github.com/strongloop/loopback-next/issues/435 @@ -127,6 +169,10 @@ export class RoutingTable { return paths; } + /** + * Map a request to a route + * @param request + */ find(request: ParsedRequest): ResolvedRoute { for (const entry of this._routes) { const match = entry.match(request); @@ -139,14 +185,40 @@ export class RoutingTable { } } +/** + * An entry in the routing table + */ export interface RouteEntry { + /** + * http verb + */ readonly verb: string; + /** + * http path + */ readonly path: string; + /** + * OpenAPI operation spec + */ readonly spec: OperationObject; + /** + * Map an http request to a route + * @param request + */ match(request: ParsedRequest): ResolvedRoute | undefined; + /** + * Update bindings for the request context + * @param requestContext + */ updateBindings(requestContext: Context): void; + + /** + * A handler to invoke the resolved controller method + * @param requestContext + * @param args + */ invokeHandler( requestContext: Context, args: OperationArgs, @@ -155,15 +227,27 @@ export interface RouteEntry { describe(): string; } +/** + * A route with path parameters resolved + */ export interface ResolvedRoute extends RouteEntry { readonly pathParams: PathParameterValues; } +/** + * Base implementation of RouteEntry + */ export abstract class BaseRoute implements RouteEntry { public readonly verb: string; private readonly _keys: pathToRegexp.Key[] = []; private readonly _pathRegexp: RegExp; + /** + * Construct a new route + * @param verb http verb + * @param path http request path pattern + * @param spec OpenAPI operation spec + */ constructor( verb: string, public readonly path: string, @@ -259,55 +343,78 @@ export class Route extends BaseRoute { } } -type ControllerInstance = {[opName: string]: Function}; - +/** + * A route backed by a controller + */ export class ControllerRoute extends BaseRoute { + protected readonly _controllerCtor: ControllerClass; + protected readonly _controllerName: string; protected readonly _methodName: string; - + protected readonly _controllerFactory: ControllerFactory; + + /** + * Construct a controller based route + * @param verb http verb + * @param path http request path + * @param spec OpenAPI operation spec + * @param controllerCtor Controller class + * @param methodName Controller method name, default to `x-operation-name` + * @param controllerFactory A factory function to create a controller instance + */ constructor( verb: string, path: string, spec: OperationObject, - protected readonly _controllerCtor: ControllerClass, + controllerCtor: ControllerClass, methodName?: string, + controllerFactory?: ControllerFactory, ) { + const controllerName = spec['x-controller-name'] || controllerCtor.name; + methodName = methodName || spec['x-operation-name']; + + if (!methodName) { + throw new Error( + 'methodName must be provided either via the ControllerRoute argument ' + + 'or via "x-operation-name" extension field in OpenAPI spec. ' + + `Operation: "${verb} ${path}" ` + + `Controller: ${controllerName}.`, + ); + } + super( verb, path, // Add x-controller-name and x-operation-name if not present Object.assign( { - 'x-controller-name': _controllerCtor.name, + 'x-controller-name': controllerName, 'x-operation-name': methodName, - tags: [_controllerCtor.name], + tags: [controllerName], }, spec, ), ); - if (!methodName) { - methodName = this.spec['x-operation-name']; - } + this._controllerCtor = controllerCtor; + this._controllerName = controllerName || controllerCtor.name; + this._methodName = methodName; - if (!methodName) { - throw new Error( - 'methodName must be provided either via the ControllerRoute argument ' + - 'or via "x-operation-name" extension field in OpenAPI spec. ' + - `Operation: "${verb} ${path}" ` + - `Controller: ${this._controllerCtor.name}.`, - ); + if (controllerFactory == null) { + controllerFactory = createControllerFactory(controllerCtor); } - - this._methodName = methodName; + this._controllerFactory = controllerFactory; } describe(): string { - return `${this._controllerCtor.name}.${this._methodName}`; + return `${this._controllerName}.${this._methodName}`; } updateBindings(requestContext: Context) { - const ctor = this._controllerCtor; - requestContext.bind(CoreBindings.CONTROLLER_CLASS).to(ctor); + requestContext + .bind(CoreBindings.CONTROLLER_CURRENT) + .toDynamicValue(() => this._controllerFactory(requestContext)) + .inScope(BindingScope.SINGLETON); + requestContext.bind(CoreBindings.CONTROLLER_CLASS).to(this._controllerCtor); requestContext .bind(CoreBindings.CONTROLLER_METHOD_NAME) .to(this._methodName); @@ -317,7 +424,9 @@ export class ControllerRoute extends BaseRoute { requestContext: Context, args: OperationArgs, ): Promise { - const controller = await this._createControllerInstance(requestContext); + const controller = await requestContext.get( + 'controller.current', + ); if (typeof controller[this._methodName] !== 'function') { throw new HttpErrors.NotFound( `Controller method not found: ${this.describe()}`, @@ -331,16 +440,6 @@ export class ControllerRoute extends BaseRoute { args, ); } - - private async _createControllerInstance( - requestContext: Context, - ): Promise { - const valueOrPromise = instantiateClass( - this._controllerCtor, - requestContext, - ); - return (await Promise.resolve(valueOrPromise)) as ControllerInstance; - } } function describeOperationParameters(opSpec: OperationObject) { @@ -348,3 +447,33 @@ function describeOperationParameters(opSpec: OperationObject) { .map(p => (p && p.name) || '') .join(', '); } + +/** + * Create a controller factory function + * @param source The source can be one of the following: + * - A binding key + * - A controller class + * - A controller instance + */ +export function createControllerFactory( + source: ControllerClass | string | ControllerInstance, +): ControllerFactory { + if (typeof source === 'string') { + return ctx => ctx.get(source); + } else if (typeof source === 'function') { + const ctor = source as ControllerClass; + return async ctx => { + // By default, we get an instance of the controller from the context + // using `controllers.` as the key + let inst = await ctx.get(`controllers.${ctor.name}`, { + optional: true, + }); + if (inst === undefined) { + inst = await instantiateClass(ctor, ctx); + } + return inst; + }; + } else { + return ctx => source; + } +} diff --git a/packages/rest/test/acceptance/routing/routing.acceptance.ts b/packages/rest/test/acceptance/routing/routing.acceptance.ts index 1c4617c9870d..c26587917645 100644 --- a/packages/rest/test/acceptance/routing/routing.acceptance.ts +++ b/packages/rest/test/acceptance/routing/routing.acceptance.ts @@ -13,6 +13,9 @@ import { RestApplication, SequenceActions, HttpServerLike, + ControllerClass, + createControllerFactory, + ControllerFactory, } from '../../..'; import {api, get, post, param, requestBody} from '@loopback/openapi-v3'; @@ -27,8 +30,8 @@ import { import {expect, Client, createClientForHandler} from '@loopback/testlab'; import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder'; -import {inject, Context} from '@loopback/context'; -import {ControllerClass} from '../../../src/router/routing-table'; +import {inject, Context, BindingScope} from '@loopback/context'; + import {createUnexpectedHttpErrorLogger} from '../../helpers'; /* # Feature: Routing @@ -357,6 +360,32 @@ describe('Routing', () => { }); }); + it('binds the current controller', async () => { + const app = givenAnApplication(); + const server = await givenAServer(app); + const spec = anOpenApiSpec() + .withOperationReturningString('get', '/name', 'checkController') + .build(); + + @api(spec) + class GetCurrentController { + async checkController( + @inject('controllers.GetCurrentController') inst: GetCurrentController, + ): Promise { + return { + result: this === inst, + }; + } + } + givenControllerInApp(app, GetCurrentController); + + return whenIMakeRequestTo(server) + .get('/name') + .expect({ + result: true, + }); + }); + it('supports function-based routes', async () => { const app = givenAnApplication(); const server = await givenAServer(app); @@ -530,6 +559,27 @@ describe('Routing', () => { await client.get('/greet?name=world').expect(200, 'hello world'); }); + it('supports controller routes with factory defined via server.route()', async () => { + const app = givenAnApplication(); + const server = await givenAServer(app); + + class MyController { + greet(name: string) { + return `hello ${name}`; + } + } + + const spec = anOperationSpec() + .withParameter({name: 'name', in: 'query', type: 'string'}) + .build(); + + const factory = createControllerFactory(MyController); + server.route('get', '/greet', spec, MyController, 'greet', factory); + + const client = whenIMakeRequestTo(server); + await client.get('/greet?name=world').expect(200, 'hello world'); + }); + describe('RestApplication', () => { it('supports function-based routes declared via app.route()', async () => { const app = new RestApplication(); @@ -606,6 +656,33 @@ describe('Routing', () => { .expect(200, 'hello world'); }); + it('supports controller routes via app.route() with a factory', async () => { + const app = new RestApplication(); + + class MyController { + greet(name: string) { + return `hello ${name}`; + } + } + + class MySubController extends MyController { + greet(name: string) { + return super.greet(name) + '!'; + } + } + + const spec = anOperationSpec() + .withParameter({name: 'name', in: 'query', type: 'string'}) + .build(); + + const factory: ControllerFactory = ctx => new MySubController(); + app.route('get', '/greet', spec, MyController, 'greet', factory); + + await whenIMakeRequestTo(app) + .get('/greet?name=world') + .expect(200, 'hello world!'); + }); + it('provides httpHandler compatible with HTTP server API', async () => { const app = new RestApplication(); app.handler((sequence, req, res) => res.end('hello')); @@ -638,7 +715,7 @@ describe('Routing', () => { } function givenControllerInApp(app: Application, controller: ControllerClass) { - app.controller(controller); + app.controller(controller).inScope(BindingScope.CONTEXT); } function whenIMakeRequestTo(serverOrApp: HttpServerLike): Client { diff --git a/packages/rest/test/unit/router/controller-factory.test.ts b/packages/rest/test/unit/router/controller-factory.test.ts new file mode 100644 index 000000000000..dbfed0064892 --- /dev/null +++ b/packages/rest/test/unit/router/controller-factory.test.ts @@ -0,0 +1,41 @@ +// 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 {expect} from '@loopback/testlab'; +import {ControllerFactory, createControllerFactory} from '../../..'; +import {Context} from '@loopback/core'; + +describe('createControllerFactory', () => { + let ctx: Context; + + beforeEach(() => { + ctx = new Context(); + }); + + it('creates a factory with binding key', async () => { + ctx.bind('controllers.my-controller').toClass(MyController); + const factory = createControllerFactory('controllers.my-controller'); + const inst = await factory(ctx); + expect(inst).to.be.instanceof(MyController); + }); + + it('creates a factory with class', async () => { + const factory = createControllerFactory(MyController); + const inst = await factory(ctx); + expect(inst).to.be.instanceof(MyController); + }); + + it('creates a factory with an instance', async () => { + const factory = createControllerFactory(new MyController()); + const inst = await factory(ctx); + expect(inst).to.be.instanceof(MyController); + }); + + class MyController { + greet() { + return 'Hello'; + } + } +}); diff --git a/packages/rest/test/unit/router/controller-route.unit.ts b/packages/rest/test/unit/router/controller-route.unit.ts index ec4fec151182..8b1f3bab16c5 100644 --- a/packages/rest/test/unit/router/controller-route.unit.ts +++ b/packages/rest/test/unit/router/controller-route.unit.ts @@ -6,14 +6,66 @@ import {ControllerRoute} from '../../..'; import {expect} from '@loopback/testlab'; import {anOperationSpec} from '@loopback/openapi-spec-builder'; +import {ControllerFactory, createControllerFactory} from '../../..'; describe('ControllerRoute', () => { it('rejects routes with no methodName', () => { - class MyController {} const spec = anOperationSpec().build(); expect( () => new ControllerRoute('get', '/greet', spec, MyController), ).to.throw(/methodName must be provided.*"get \/greet".*MyController/); }); + + it('creates a factory', () => { + const spec = anOperationSpec().build(); + + const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + + expect(route._controllerFactory).to.be.a.Function(); + }); + + it('honors a factory', () => { + const spec = anOperationSpec().build(); + + const factory = createControllerFactory('controllers.my-controller'); + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + 'greet', + factory, + ); + + expect(route._controllerFactory).to.be.exactly(factory); + }); + + it('infers controllerName from the class', () => { + const spec = anOperationSpec().build(); + + const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + + expect(route._controllerName).to.eql(MyController.name); + }); + + it('honors controllerName from the spec', () => { + const spec = anOperationSpec().build(); + spec['x-controller-name'] = 'my-controller'; + + const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + + expect(route._controllerName).to.eql('my-controller'); + }); + + class MyController { + greet() { + return 'Hello'; + } + } + + class MyRoute extends ControllerRoute { + _controllerFactory: ControllerFactory; + _controllerName: string; + } }); From 7de8e002e6392986661a73f3fb2ef679cdfdd5d9 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 20 Mar 2018 10:00:08 -0700 Subject: [PATCH 2/3] feat(rest): add typing for controller instance/class/factory --- packages/rest/src/http-handler.ts | 6 +-- packages/rest/src/rest.application.ts | 20 ++++----- packages/rest/src/rest.server.ts | 23 +++++----- packages/rest/src/router/routing-table.ts | 42 +++++++++---------- .../acceptance/routing/routing.acceptance.ts | 9 +++- .../sequence/sequence.acceptance.ts | 9 +++- .../unit/router/controller-factory.test.ts | 6 ++- .../test/unit/router/controller-route.unit.ts | 8 ++-- 8 files changed, 69 insertions(+), 54 deletions(-) diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index a73d52448ec7..b5e3a41c659e 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -34,10 +34,10 @@ export class HttpHandler { this.handleRequest = (req, res) => this._handleRequest(req, res); } - registerController( - controllerCtor: ControllerClass, + registerController( + controllerCtor: ControllerClass, spec: ControllerSpec, - controllerFactory?: ControllerFactory, + controllerFactory?: ControllerFactory, ) { this._routes.registerController(controllerCtor, spec, controllerFactory); } diff --git a/packages/rest/src/rest.application.ts b/packages/rest/src/rest.application.ts index edb96239e419..2a17ea5aaed6 100644 --- a/packages/rest/src/rest.application.ts +++ b/packages/rest/src/rest.application.ts @@ -99,17 +99,17 @@ export class RestApplication extends Application implements HttpServerLike { * @param verb HTTP verb of the endpoint * @param path URL path of the endpoint * @param spec The OpenAPI spec describing the endpoint (operation) - * @param controller Controller constructor + * @param controllerCtor Controller constructor * @param methodName The name of the controller method - * @param factory A factory function to create controller instance + * @param controllerFactory A factory function to create controller instance */ - route( + route( verb: string, path: string, spec: OperationObject, - controller: ControllerClass, + controllerCtor: ControllerClass, methodName: string, - factory?: ControllerFactory, + controllerFactory?: ControllerFactory, ): Binding; /** @@ -127,13 +127,13 @@ export class RestApplication extends Application implements HttpServerLike { */ route(route: RouteEntry): Binding; - route( + route( routeOrVerb: RouteEntry | string, path?: string, spec?: OperationObject, - controller?: ControllerClass, + controllerCtor?: ControllerClass, methodName?: string, - factory?: ControllerFactory, + controllerFactory?: ControllerFactory, ): Binding { const server = this.restServer; if (typeof routeOrVerb === 'object') { @@ -143,9 +143,9 @@ export class RestApplication extends Application implements HttpServerLike { routeOrVerb, path!, spec!, - controller!, + controllerCtor!, methodName!, - factory, + controllerFactory, ); } } diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 11b2b0d4bf80..130dc1c2a480 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -13,6 +13,7 @@ import { createControllerFactory, ControllerFactory, ControllerClass, + ControllerInstance, } from './router/routing-table'; import {ParsedRequest} from './internal-types'; import {OpenApiSpec, OperationObject} from '@loopback/openapi-v3-types'; @@ -362,7 +363,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * ``` * */ - controller(controllerCtor: ControllerClass): Binding { + controller(controllerCtor: ControllerClass): Binding { return this.bind('controllers.' + controllerCtor.name).toClass( controllerCtor, ); @@ -383,17 +384,17 @@ export class RestServer extends Context implements Server, HttpServerLike { * @param verb HTTP verb of the endpoint * @param path URL path of the endpoint * @param spec The OpenAPI spec describing the endpoint (operation) - * @param controller Controller constructor + * @param controllerCtor Controller constructor * @param methodName The name of the controller method - * @param factory A factory function to create controller instance + * @param controllerFactory A factory function to create controller instance */ - route( + route( verb: string, path: string, spec: OperationObject, - controller: ControllerClass, + controllerCtor: ControllerClass, methodName: string, - factory?: ControllerFactory, + controllerFactory?: ControllerFactory, ): Binding; /** @@ -411,13 +412,13 @@ export class RestServer extends Context implements Server, HttpServerLike { */ route(route: RouteEntry): Binding; - route( + route( routeOrVerb: RouteEntry | string, path?: string, spec?: OperationObject, - controller?: ControllerClass, + controllerCtor?: ControllerClass, methodName?: string, - controllerFactory?: ControllerFactory, + controllerFactory?: ControllerFactory, ): Binding { if (typeof routeOrVerb === 'object') { const r = routeOrVerb; @@ -440,7 +441,7 @@ export class RestServer extends Context implements Server, HttpServerLike { }); } - if (!controller) { + if (!controllerCtor) { throw new AssertionError({ message: 'controller is required for a controller-based route', }); @@ -457,7 +458,7 @@ export class RestServer extends Context implements Server, HttpServerLike { routeOrVerb, path, spec, - controller, + controllerCtor, methodName, controllerFactory, ), diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 404e58e4a81c..841bb2b4e81e 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -67,20 +67,20 @@ export function parseRequestUrl(request: ServerRequest): ParsedRequest { * A controller instance with open properties/methods */ // tslint:disable-next-line:no-any -export type ControllerInstance = {[name: string]: any}; +export type ControllerInstance = {[name: string]: any} & object; /** * A factory function to create controller instances synchronously or * asynchronously */ -export type ControllerFactory = ( +export type ControllerFactory = ( ctx: Context, -) => ValueOrPromise; +) => ValueOrPromise; /** * Controller class */ -export type ControllerClass = Constructor; +export type ControllerClass = Constructor; /** * Routing table @@ -90,14 +90,14 @@ export class RoutingTable { /** * Register a controller as the route - * @param controller + * @param controllerCtor * @param spec * @param controllerFactory */ - registerController( - controller: ControllerClass, + registerController( + controllerCtor: ControllerClass, spec: ControllerSpec, - controllerFactory?: ControllerFactory, + controllerFactory?: ControllerFactory, ) { assert( typeof spec === 'object' && !!spec, @@ -118,7 +118,7 @@ export class RoutingTable { verb, fullPath, opSpec, - controller, + controllerCtor, undefined, controllerFactory, ); @@ -346,11 +346,11 @@ export class Route extends BaseRoute { /** * A route backed by a controller */ -export class ControllerRoute extends BaseRoute { - protected readonly _controllerCtor: ControllerClass; +export class ControllerRoute extends BaseRoute { + protected readonly _controllerCtor: ControllerClass; protected readonly _controllerName: string; protected readonly _methodName: string; - protected readonly _controllerFactory: ControllerFactory; + protected readonly _controllerFactory: ControllerFactory; /** * Construct a controller based route @@ -365,9 +365,9 @@ export class ControllerRoute extends BaseRoute { verb: string, path: string, spec: OperationObject, - controllerCtor: ControllerClass, + controllerCtor: ControllerClass, methodName?: string, - controllerFactory?: ControllerFactory, + controllerFactory?: ControllerFactory, ) { const controllerName = spec['x-controller-name'] || controllerCtor.name; methodName = methodName || spec['x-operation-name']; @@ -455,21 +455,21 @@ function describeOperationParameters(opSpec: OperationObject) { * - A controller class * - A controller instance */ -export function createControllerFactory( - source: ControllerClass | string | ControllerInstance, -): ControllerFactory { +export function createControllerFactory( + source: ControllerClass | string | T, +): ControllerFactory { if (typeof source === 'string') { - return ctx => ctx.get(source); + return ctx => ctx.get(source); } else if (typeof source === 'function') { - const ctor = source as ControllerClass; + const ctor = source as ControllerClass; return async ctx => { // By default, we get an instance of the controller from the context // using `controllers.` as the key - let inst = await ctx.get(`controllers.${ctor.name}`, { + let inst = await ctx.get(`controllers.${ctor.name}`, { optional: true, }); if (inst === undefined) { - inst = await instantiateClass(ctor, ctx); + inst = await instantiateClass(ctor, ctx); } return inst; }; diff --git a/packages/rest/test/acceptance/routing/routing.acceptance.ts b/packages/rest/test/acceptance/routing/routing.acceptance.ts index c26587917645..c0720e5daee1 100644 --- a/packages/rest/test/acceptance/routing/routing.acceptance.ts +++ b/packages/rest/test/acceptance/routing/routing.acceptance.ts @@ -16,6 +16,7 @@ import { ControllerClass, createControllerFactory, ControllerFactory, + ControllerInstance, } from '../../..'; import {api, get, post, param, requestBody} from '@loopback/openapi-v3'; @@ -675,7 +676,8 @@ describe('Routing', () => { .withParameter({name: 'name', in: 'query', type: 'string'}) .build(); - const factory: ControllerFactory = ctx => new MySubController(); + const factory: ControllerFactory = ctx => + new MySubController(); app.route('get', '/greet', spec, MyController, 'greet', factory); await whenIMakeRequestTo(app) @@ -714,7 +716,10 @@ describe('Routing', () => { return await app.getServer(RestServer); } - function givenControllerInApp(app: Application, controller: ControllerClass) { + function givenControllerInApp( + app: Application, + controller: ControllerClass, + ) { app.controller(controller).inScope(BindingScope.CONTEXT); } diff --git a/packages/rest/test/acceptance/sequence/sequence.acceptance.ts b/packages/rest/test/acceptance/sequence/sequence.acceptance.ts index 99f5d55f7c44..f9c49c18fd98 100644 --- a/packages/rest/test/acceptance/sequence/sequence.acceptance.ts +++ b/packages/rest/test/acceptance/sequence/sequence.acceptance.ts @@ -24,7 +24,10 @@ import {Application} from '@loopback/core'; import {expect, Client, createClientForHandler} from '@loopback/testlab'; import {anOpenApiSpec} from '@loopback/openapi-spec-builder'; import {inject, Context} from '@loopback/context'; -import {ControllerClass} from '../../../src/router/routing-table'; +import { + ControllerClass, + ControllerInstance, +} from '../../../src/router/routing-table'; const SequenceActions = RestBindings.SequenceActions; @@ -209,7 +212,9 @@ describe('Sequence', () => { server = await app.getServer(RestServer); } - function givenControllerInServer(controller: ControllerClass) { + function givenControllerInServer( + controller: ControllerClass, + ) { app.controller(controller); } diff --git a/packages/rest/test/unit/router/controller-factory.test.ts b/packages/rest/test/unit/router/controller-factory.test.ts index dbfed0064892..d736ed78c817 100644 --- a/packages/rest/test/unit/router/controller-factory.test.ts +++ b/packages/rest/test/unit/router/controller-factory.test.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {ControllerFactory, createControllerFactory} from '../../..'; +import {createControllerFactory} from '../../..'; import {Context} from '@loopback/core'; describe('createControllerFactory', () => { @@ -16,7 +16,9 @@ describe('createControllerFactory', () => { it('creates a factory with binding key', async () => { ctx.bind('controllers.my-controller').toClass(MyController); - const factory = createControllerFactory('controllers.my-controller'); + const factory = createControllerFactory( + 'controllers.my-controller', + ); const inst = await factory(ctx); expect(inst).to.be.instanceof(MyController); }); diff --git a/packages/rest/test/unit/router/controller-route.unit.ts b/packages/rest/test/unit/router/controller-route.unit.ts index 8b1f3bab16c5..3338cbdce55b 100644 --- a/packages/rest/test/unit/router/controller-route.unit.ts +++ b/packages/rest/test/unit/router/controller-route.unit.ts @@ -28,7 +28,9 @@ describe('ControllerRoute', () => { it('honors a factory', () => { const spec = anOperationSpec().build(); - const factory = createControllerFactory('controllers.my-controller'); + const factory = createControllerFactory( + 'controllers.my-controller', + ); const route = new MyRoute( 'get', '/greet', @@ -64,8 +66,8 @@ describe('ControllerRoute', () => { } } - class MyRoute extends ControllerRoute { - _controllerFactory: ControllerFactory; + class MyRoute extends ControllerRoute { + _controllerFactory: ControllerFactory; _controllerName: string; } }); From d48c062b4cce7f23a336f4123770e78bf3511e84 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 29 Mar 2018 15:23:27 -0700 Subject: [PATCH 3/3] refactor(rest): tidy up controller related methods - move controllFactory param next to controllerCtor - split createControllerFactory to three functions --- packages/core/src/keys.ts | 5 +- packages/rest/src/http-handler.ts | 4 +- packages/rest/src/index.ts | 4 +- packages/rest/src/rest.application.ts | 8 +- packages/rest/src/rest.server.ts | 17 ++--- packages/rest/src/router/routing-table.ts | 76 ++++++++++--------- .../acceptance/routing/routing.acceptance.ts | 25 +++--- .../integration/http-handler.integration.ts | 2 +- .../rest.server.open-api-spec.unit.ts | 17 ++++- .../unit/router/controller-factory.test.ts | 12 ++- .../test/unit/router/controller-route.unit.ts | 41 ++++++++-- .../test/unit/router/routing-table.unit.ts | 6 +- 12 files changed, 136 insertions(+), 81 deletions(-) diff --git a/packages/core/src/keys.ts b/packages/core/src/keys.ts index a229a34b0352..3050150bdefe 100644 --- a/packages/core/src/keys.ts +++ b/packages/core/src/keys.ts @@ -5,7 +5,6 @@ import {BindingKey} from '@loopback/context'; import {Application, ControllerClass} from './application'; -import {ControllerInstance} from '../../rest/src'; /** * Namespace for core binding keys @@ -59,7 +58,5 @@ export namespace CoreBindings { * Binding key for the controller instance resolved in the current request * context */ - export const CONTROLLER_CURRENT = BindingKey.create( - 'controller.current', - ); + export const CONTROLLER_CURRENT = BindingKey.create('controller.current'); } diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index b5e3a41c659e..7538a3451521 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -35,11 +35,11 @@ export class HttpHandler { } registerController( - controllerCtor: ControllerClass, spec: ControllerSpec, + controllerCtor: ControllerClass, controllerFactory?: ControllerFactory, ) { - this._routes.registerController(controllerCtor, spec, controllerFactory); + this._routes.registerController(spec, controllerCtor, controllerFactory); } registerRoute(route: RouteEntry) { diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index 91a65d12de64..450481d764fd 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -17,7 +17,9 @@ export { ControllerClass, ControllerInstance, ControllerFactory, - createControllerFactory, + createControllerFactoryForBinding, + createControllerFactoryForClass, + createControllerFactoryForInstance, } from './router/routing-table'; export * from './providers'; diff --git a/packages/rest/src/rest.application.ts b/packages/rest/src/rest.application.ts index 2a17ea5aaed6..93c23ab87cc5 100644 --- a/packages/rest/src/rest.application.ts +++ b/packages/rest/src/rest.application.ts @@ -100,16 +100,16 @@ export class RestApplication extends Application implements HttpServerLike { * @param path URL path of the endpoint * @param spec The OpenAPI spec describing the endpoint (operation) * @param controllerCtor Controller constructor - * @param methodName The name of the controller method * @param controllerFactory A factory function to create controller instance + * @param methodName The name of the controller method */ route( verb: string, path: string, spec: OperationObject, controllerCtor: ControllerClass, + controllerFactory: ControllerFactory, methodName: string, - controllerFactory?: ControllerFactory, ): Binding; /** @@ -132,8 +132,8 @@ export class RestApplication extends Application implements HttpServerLike { path?: string, spec?: OperationObject, controllerCtor?: ControllerClass, - methodName?: string, controllerFactory?: ControllerFactory, + methodName?: string, ): Binding { const server = this.restServer; if (typeof routeOrVerb === 'object') { @@ -144,8 +144,8 @@ export class RestApplication extends Application implements HttpServerLike { path!, spec!, controllerCtor!, + controllerFactory!, methodName!, - controllerFactory, ); } } diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 130dc1c2a480..3652f7947b68 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -10,10 +10,10 @@ import { Route, ControllerRoute, RouteEntry, - createControllerFactory, ControllerFactory, ControllerClass, ControllerInstance, + createControllerFactoryForBinding, } from './router/routing-table'; import {ParsedRequest} from './internal-types'; import {OpenApiSpec, OperationObject} from '@loopback/openapi-v3-types'; @@ -246,8 +246,8 @@ export class RestServer extends Context implements Server, HttpServerLike { if (apiSpec.components && apiSpec.components.schemas) { this._httpHandler.registerApiDefinitions(apiSpec.components.schemas); } - const controllerFactory = createControllerFactory(b.key); - this._httpHandler.registerController(ctor, apiSpec, controllerFactory); + const controllerFactory = createControllerFactoryForBinding(b.key); + this._httpHandler.registerController(apiSpec, ctor, controllerFactory); } for (const b of this.find('routes.*')) { @@ -296,13 +296,12 @@ export class RestServer extends Context implements Server, HttpServerLike { ); } - const controllerFactory = createControllerFactory(b.key); + const controllerFactory = createControllerFactoryForBinding(b.key); const route = new ControllerRoute( verb, path, spec, ctor, - undefined, controllerFactory, ); this._httpHandler.registerRoute(route); @@ -385,16 +384,16 @@ export class RestServer extends Context implements Server, HttpServerLike { * @param path URL path of the endpoint * @param spec The OpenAPI spec describing the endpoint (operation) * @param controllerCtor Controller constructor - * @param methodName The name of the controller method * @param controllerFactory A factory function to create controller instance + * @param methodName The name of the controller method */ route( verb: string, path: string, spec: OperationObject, controllerCtor: ControllerClass, + controllerFactory: ControllerFactory, methodName: string, - controllerFactory?: ControllerFactory, ): Binding; /** @@ -417,8 +416,8 @@ export class RestServer extends Context implements Server, HttpServerLike { path?: string, spec?: OperationObject, controllerCtor?: ControllerClass, - methodName?: string, controllerFactory?: ControllerFactory, + methodName?: string, ): Binding { if (typeof routeOrVerb === 'object') { const r = routeOrVerb; @@ -459,8 +458,8 @@ export class RestServer extends Context implements Server, HttpServerLike { path, spec, controllerCtor, - methodName, controllerFactory, + methodName, ), ); } diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 841bb2b4e81e..8d42f9b9d6c6 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -90,13 +90,13 @@ export class RoutingTable { /** * Register a controller as the route - * @param controllerCtor * @param spec + * @param controllerCtor * @param controllerFactory */ registerController( - controllerCtor: ControllerClass, spec: ControllerSpec, + controllerCtor: ControllerClass, controllerFactory?: ControllerFactory, ) { assert( @@ -119,7 +119,6 @@ export class RoutingTable { fullPath, opSpec, controllerCtor, - undefined, controllerFactory, ); this.registerRoute(route); @@ -358,16 +357,16 @@ export class ControllerRoute extends BaseRoute { * @param path http request path * @param spec OpenAPI operation spec * @param controllerCtor Controller class - * @param methodName Controller method name, default to `x-operation-name` * @param controllerFactory A factory function to create a controller instance + * @param methodName Controller method name, default to `x-operation-name` */ constructor( verb: string, path: string, spec: OperationObject, controllerCtor: ControllerClass, - methodName?: string, controllerFactory?: ControllerFactory, + methodName?: string, ) { const controllerName = spec['x-controller-name'] || controllerCtor.name; methodName = methodName || spec['x-operation-name']; @@ -395,14 +394,11 @@ export class ControllerRoute extends BaseRoute { ), ); + this._controllerFactory = + controllerFactory || createControllerFactoryForClass(controllerCtor); this._controllerCtor = controllerCtor; this._controllerName = controllerName || controllerCtor.name; this._methodName = methodName; - - if (controllerFactory == null) { - controllerFactory = createControllerFactory(controllerCtor); - } - this._controllerFactory = controllerFactory; } describe(): string { @@ -449,31 +445,41 @@ function describeOperationParameters(opSpec: OperationObject) { } /** - * Create a controller factory function - * @param source The source can be one of the following: - * - A binding key - * - A controller class - * - A controller instance + * Create a controller factory function for a given binding key + * @param key Binding key */ -export function createControllerFactory( - source: ControllerClass | string | T, +export function createControllerFactoryForBinding( + key: string, ): ControllerFactory { - if (typeof source === 'string') { - return ctx => ctx.get(source); - } else if (typeof source === 'function') { - const ctor = source as ControllerClass; - return async ctx => { - // By default, we get an instance of the controller from the context - // using `controllers.` as the key - let inst = await ctx.get(`controllers.${ctor.name}`, { - optional: true, - }); - if (inst === undefined) { - inst = await instantiateClass(ctor, ctx); - } - return inst; - }; - } else { - return ctx => source; - } + return ctx => ctx.get(key); +} + +/** + * Create a controller factory function for a given class + * @param controllerCtor Controller class + */ +export function createControllerFactoryForClass( + controllerCtor: ControllerClass, +): ControllerFactory { + return async ctx => { + // By default, we get an instance of the controller from the context + // using `controllers.` as the key + let inst = await ctx.get(`controllers.${controllerCtor.name}`, { + optional: true, + }); + if (inst === undefined) { + inst = await instantiateClass(controllerCtor, ctx); + } + return inst; + }; +} + +/** + * Create a controller factory function for a given instance + * @param controllerCtor Controller instance + */ +export function createControllerFactoryForInstance( + controllerInst: T, +): ControllerFactory { + return ctx => controllerInst; } diff --git a/packages/rest/test/acceptance/routing/routing.acceptance.ts b/packages/rest/test/acceptance/routing/routing.acceptance.ts index c0720e5daee1..6ee9d6fae8e4 100644 --- a/packages/rest/test/acceptance/routing/routing.acceptance.ts +++ b/packages/rest/test/acceptance/routing/routing.acceptance.ts @@ -14,9 +14,9 @@ import { SequenceActions, HttpServerLike, ControllerClass, - createControllerFactory, - ControllerFactory, ControllerInstance, + createControllerFactoryForClass, + createControllerFactoryForInstance, } from '../../..'; import {api, get, post, param, requestBody} from '@loopback/openapi-v3'; @@ -554,7 +554,14 @@ describe('Routing', () => { .withParameter({name: 'name', in: 'query', type: 'string'}) .build(); - server.route('get', '/greet', spec, MyController, 'greet'); + server.route( + 'get', + '/greet', + spec, + MyController, + createControllerFactoryForClass(MyController), + 'greet', + ); const client = whenIMakeRequestTo(server); await client.get('/greet?name=world').expect(200, 'hello world'); @@ -574,8 +581,8 @@ describe('Routing', () => { .withParameter({name: 'name', in: 'query', type: 'string'}) .build(); - const factory = createControllerFactory(MyController); - server.route('get', '/greet', spec, MyController, 'greet', factory); + const factory = createControllerFactoryForClass(MyController); + server.route('get', '/greet', spec, MyController, factory, 'greet'); const client = whenIMakeRequestTo(server); await client.get('/greet?name=world').expect(200, 'hello world'); @@ -650,7 +657,8 @@ describe('Routing', () => { .withParameter({name: 'name', in: 'query', type: 'string'}) .build(); - app.route('get', '/greet', spec, MyController, 'greet'); + const factory = createControllerFactoryForClass(MyController); + app.route('get', '/greet', spec, MyController, factory, 'greet'); await whenIMakeRequestTo(app) .get('/greet?name=world') @@ -676,9 +684,8 @@ describe('Routing', () => { .withParameter({name: 'name', in: 'query', type: 'string'}) .build(); - const factory: ControllerFactory = ctx => - new MySubController(); - app.route('get', '/greet', spec, MyController, 'greet', factory); + const factory = createControllerFactoryForInstance(new MySubController()); + app.route('get', '/greet', spec, MyController, factory, 'greet'); await whenIMakeRequestTo(app) .get('/greet?name=world') diff --git a/packages/rest/test/integration/http-handler.integration.ts b/packages/rest/test/integration/http-handler.integration.ts index 6e42723f786c..c03936de4b4c 100644 --- a/packages/rest/test/integration/http-handler.integration.ts +++ b/packages/rest/test/integration/http-handler.integration.ts @@ -454,7 +454,7 @@ describe('HttpHandler', () => { ctor: new (...args: any[]) => Object, spec: ControllerSpec, ) { - handler.registerController(ctor, spec); + handler.registerController(spec, ctor); } function givenClient() { diff --git a/packages/rest/test/unit/rest.server/rest.server.open-api-spec.unit.ts b/packages/rest/test/unit/rest.server/rest.server.open-api-spec.unit.ts index 2b05a5d4bc92..cbdd5e18399f 100644 --- a/packages/rest/test/unit/rest.server/rest.server.open-api-spec.unit.ts +++ b/packages/rest/test/unit/rest.server/rest.server.open-api-spec.unit.ts @@ -5,7 +5,12 @@ import {expect, validateApiSpec} from '@loopback/testlab'; import {Application} from '@loopback/core'; -import {RestServer, Route, RestComponent} from '../../..'; +import { + RestServer, + Route, + RestComponent, + createControllerFactoryForClass, +} from '../../..'; import {get, post, requestBody} from '@loopback/openapi-v3'; import {anOpenApiSpec} from '@loopback/openapi-spec-builder'; import {model, property} from '@loopback/repository'; @@ -63,6 +68,7 @@ describe('RestServer.getApiSpec()', () => { '/greet.json', {responses: {}}, MyController, + createControllerFactoryForClass(MyController), 'greet', ); expect(binding.key).to.eql('routes.get %2Fgreet%2Ejson'); @@ -88,7 +94,14 @@ describe('RestServer.getApiSpec()', () => { greet() {} } - server.route('get', '/greet', {responses: {}}, MyController, 'greet'); + server.route( + 'get', + '/greet', + {responses: {}}, + MyController, + createControllerFactoryForClass(MyController), + 'greet', + ); const spec = server.getApiSpec(); expect(spec.paths).to.eql({ diff --git a/packages/rest/test/unit/router/controller-factory.test.ts b/packages/rest/test/unit/router/controller-factory.test.ts index d736ed78c817..38e529ca3011 100644 --- a/packages/rest/test/unit/router/controller-factory.test.ts +++ b/packages/rest/test/unit/router/controller-factory.test.ts @@ -4,7 +4,11 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {createControllerFactory} from '../../..'; +import { + createControllerFactoryForBinding, + createControllerFactoryForClass, + createControllerFactoryForInstance, +} from '../../..'; import {Context} from '@loopback/core'; describe('createControllerFactory', () => { @@ -16,7 +20,7 @@ describe('createControllerFactory', () => { it('creates a factory with binding key', async () => { ctx.bind('controllers.my-controller').toClass(MyController); - const factory = createControllerFactory( + const factory = createControllerFactoryForBinding( 'controllers.my-controller', ); const inst = await factory(ctx); @@ -24,13 +28,13 @@ describe('createControllerFactory', () => { }); it('creates a factory with class', async () => { - const factory = createControllerFactory(MyController); + const factory = createControllerFactoryForClass(MyController); const inst = await factory(ctx); expect(inst).to.be.instanceof(MyController); }); it('creates a factory with an instance', async () => { - const factory = createControllerFactory(new MyController()); + const factory = createControllerFactoryForInstance(new MyController()); const inst = await factory(ctx); expect(inst).to.be.instanceof(MyController); }); diff --git a/packages/rest/test/unit/router/controller-route.unit.ts b/packages/rest/test/unit/router/controller-route.unit.ts index 3338cbdce55b..c2316b2229d6 100644 --- a/packages/rest/test/unit/router/controller-route.unit.ts +++ b/packages/rest/test/unit/router/controller-route.unit.ts @@ -3,10 +3,14 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {ControllerRoute} from '../../..'; +import { + ControllerRoute, + createControllerFactoryForClass, + createControllerFactoryForBinding, +} from '../../..'; import {expect} from '@loopback/testlab'; import {anOperationSpec} from '@loopback/openapi-spec-builder'; -import {ControllerFactory, createControllerFactory} from '../../..'; +import {ControllerFactory} from '../../..'; describe('ControllerRoute', () => { it('rejects routes with no methodName', () => { @@ -20,7 +24,14 @@ describe('ControllerRoute', () => { it('creates a factory', () => { const spec = anOperationSpec().build(); - const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + myControllerFactory, + 'greet', + ); expect(route._controllerFactory).to.be.a.Function(); }); @@ -28,7 +39,7 @@ describe('ControllerRoute', () => { it('honors a factory', () => { const spec = anOperationSpec().build(); - const factory = createControllerFactory( + const factory = createControllerFactoryForBinding( 'controllers.my-controller', ); const route = new MyRoute( @@ -36,8 +47,8 @@ describe('ControllerRoute', () => { '/greet', spec, MyController, - 'greet', factory, + 'greet', ); expect(route._controllerFactory).to.be.exactly(factory); @@ -46,7 +57,14 @@ describe('ControllerRoute', () => { it('infers controllerName from the class', () => { const spec = anOperationSpec().build(); - const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + myControllerFactory, + 'greet', + ); expect(route._controllerName).to.eql(MyController.name); }); @@ -55,7 +73,14 @@ describe('ControllerRoute', () => { const spec = anOperationSpec().build(); spec['x-controller-name'] = 'my-controller'; - const route = new MyRoute('get', '/greet', spec, MyController, 'greet'); + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + myControllerFactory, + 'greet', + ); expect(route._controllerName).to.eql('my-controller'); }); @@ -66,6 +91,8 @@ describe('ControllerRoute', () => { } } + const myControllerFactory = createControllerFactoryForClass(MyController); + class MyRoute extends ControllerRoute { _controllerFactory: ControllerFactory; _controllerName: string; diff --git a/packages/rest/test/unit/router/routing-table.unit.ts b/packages/rest/test/unit/router/routing-table.unit.ts index dd50c25fc0c4..4507dd055231 100644 --- a/packages/rest/test/unit/router/routing-table.unit.ts +++ b/packages/rest/test/unit/router/routing-table.unit.ts @@ -40,7 +40,7 @@ describe('RoutingTable', () => { } const spec = getControllerSpec(TestController); const table = new RoutingTable(); - table.registerController(TestController, spec); + table.registerController(spec, TestController); const paths = table.describeApiPaths(); const params = paths['/greet']['get'].parameters; expect(params).to.have.property('length', 1); @@ -59,7 +59,7 @@ describe('RoutingTable', () => { class TestController {} const table = new RoutingTable(); - table.registerController(TestController, spec); + table.registerController(spec, TestController); const request = givenRequest({ method: 'get', @@ -90,7 +90,7 @@ describe('RoutingTable', () => { class TestController {} const table = new RoutingTable(); - table.registerController(TestController, spec); + table.registerController(spec, TestController); const request = givenRequest({ method: 'get',