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',