diff --git a/docs/img/binding-scopes.png b/docs/img/binding-scopes.png new file mode 100644 index 000000000000..41571682fb4f Binary files /dev/null and b/docs/img/binding-scopes.png differ diff --git a/docs/site/Dependency-injection.md b/docs/site/Dependency-injection.md index fa40c738dbc1..0eba170af562 100644 --- a/docs/site/Dependency-injection.md +++ b/docs/site/Dependency-injection.md @@ -313,6 +313,111 @@ Error: Circular dependency detected: lead ``` +## Dependency injection for bindings with different scopes + +Contexts can form a chain and bindings can be registered at different levels. +The binding scope controls not only how bound values are cached, but also how +its dependencies are resolved. + +Let's take a look at the following example: + +![binding-scopes](../img/binding-scopes.png) + +The corresponding code is: + +```ts +import {inject, Context, BindingScope} from '@loopback/context'; +import {RestBindings} from '@loopback/rest'; + +interface Logger() { + log(message: string); +} + +class PingController { + constructor(@inject('logger') private logger: Logger) {} +} + +class MyService { + constructor(@inject('logger') private logger: Logger) {} +} + +class ServerLogger implements Logger { + log(message: string) { + console.log('server: %s', message); + } +} + +class RequestLogger implements Logger { + // Inject the http request + constructor(@inject(RestBindings.Http.REQUEST) private req: Request) {} + log(message: string) { + console.log('%s: %s', this.req.url, message); + } +} + +const appCtx = new Context('application'); +appCtx + .bind('controllers.PingController') + .toClass(PingController) + .inScope(BindingScope.TRANSIENT); + +const serverCtx = new Context(appCtx, 'server'); +serverCtx + .bind('my-service') + .toClass(MyService) + .inScope(BindingScope.SINGLETON); + +serverCtx.bind('logger').toClass(ServerLogger); +``` + +Please note that `my-service` is a `SINGLETON` for the `server` context subtree +and it expects a `logger` to be injected. + +Now we create a new context per request: + +```ts +const requestCtx = new Context(serverCtx, 'request'); +requestCtx.bind('logger').toClass(RequestLogger); + +const myService = await requestCtx.get('my-service'); +// myService.logger should be an instance of `ServerLogger` instead of `RequestLogger` + +requestCtx.close(); +// myService survives as it's a singleton +``` + +Dependency injection for bindings in `SINGLETON` scope is resolved using the +owner context instead of the current one. This is needed to ensure that resolved +singleton bindings won't have dependencies from descendant contexts, which can +be closed before the owner context. The singleton cannot have dangling +references to values from the child context. + +The story is different for `PingController` as its binding scope is `TRANSIENT`. + +```ts +const requestCtx = new Context(serverCtx, 'request'); +requestCtx.bind('logger').toClass(RequestLogger); + +const pingController = await requestCtx.get( + 'controllers.PingController', +); +// pingController.logger should be an instance of `RequestLogger` instead of `ServerLogger` +``` + +A new instance of `PingController` is created for each invocation of +`await requestCtx.get('controllers.PingController')` and its +`logger` is injected to an instance of `RequestLogger` so that it can log +information (such as `url` or `request-id`) for the `request`. + +The following table illustrates how bindings and their dependencies are +resolved. + +| Code | Binding Scope | Resolution Context | Owner Context | Cache Context | Dependency | +| ------------------------------------------------ | ------------- | ------------------ | ------------- | ------------- | ----------------------- | +| requestCtx.get
('my-service') | SINGLETON | requestCtx | serverCtx | serverCtx | logger -> ServerLogger | +| serverCtx.get
('my-service') | SINGLETON | serverCtx | serverCtx | serverCtx | logger -> ServerLogger | +| requestCtx.get
('controllers.PingController') | TRANSIENT | requestCtx | appCtx | N/A | logger -> RequestLogger | + ## Additional resources - [Dependency Injection](https://en.wikipedia.org/wiki/Dependency_injection) on diff --git a/packages/context/src/__tests__/acceptance/class-level-bindings.acceptance.ts b/packages/context/src/__tests__/acceptance/class-level-bindings.acceptance.ts index 25a0a348a16a..046c83cd69dd 100644 --- a/packages/context/src/__tests__/acceptance/class-level-bindings.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/class-level-bindings.acceptance.ts @@ -5,13 +5,17 @@ import {expect} from '@loopback/testlab'; import { + BindingKey, + BindingScope, + Constructor, Context, - inject, - Setter, Getter, - Provider, + inject, Injection, + invokeMethod, + Provider, ResolutionSession, + Setter, } from '../..'; const INFO_CONTROLLER = 'controllers.info'; @@ -144,35 +148,186 @@ describe('Context bindings - Injecting dependencies of classes', () => { ); }); + const STORE_KEY = 'store'; + const HASH_KEY = BindingKey.create('hash'); + it('injects a getter function', async () => { - ctx.bind('key').to('value'); + ctx.bind(HASH_KEY).to('123'); class Store { - constructor(@inject.getter('key') public getter: Getter) {} + constructor(@inject.getter(HASH_KEY) public getter: Getter) {} } - ctx.bind('store').toClass(Store); - const store = ctx.getSync('store'); + ctx.bind(STORE_KEY).toClass(Store); + const store = ctx.getSync(STORE_KEY); expect(store.getter).to.be.Function(); - expect(await store.getter()).to.equal('value'); + expect(await store.getter()).to.equal('123'); // rebind the value to verify that getter always returns a fresh value - ctx.bind('key').to('new-value'); - expect(await store.getter()).to.equal('new-value'); + ctx.bind(HASH_KEY).to('456'); + expect(await store.getter()).to.equal('456'); + }); + + describe('in SINGLETON scope', () => { + it('throws if a getter cannot be resolved by the owning context', async () => { + class Store { + constructor(@inject.getter(HASH_KEY) public getter: Getter) {} + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + // Create the store instance from the child context + // Now the singleton references a `getter` to `hash` binding from + // the `requestCtx` + const store = requestCtx.getSync(STORE_KEY); + + // The `hash` injection of `Store` cannot be fulfilled by a binding (123) + // in `requestCtx` because `Store` is bound to `ctx` in `SINGLETON` scope + await expect(store.getter()).to.be.rejectedWith( + /The key 'hash' is not bound to any value in context .+/, + ); + + // Now bind `hash` to `456` to to ctx + ctx.bind(HASH_KEY).to('456'); + // The `hash` injection of `Store` can now be resolved + expect(await store.getter()).to.equal('456'); + }); + + it('throws if a value cannot be resolved by the owning context', async () => { + class Store { + constructor(@inject(HASH_KEY) public hash: string) {} + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + // The `hash` injection of `Store` cannot be fulfilled by a binding (123) + // in `requestCtx` because `Store` is bound to `ctx` in `SINGLETON` scope + await expect(requestCtx.get(STORE_KEY)).to.be.rejectedWith( + /The key 'hash' is not bound to any value in context .+/, + ); + + // Now bind `hash` to `456` to to ctx + ctx.bind(HASH_KEY).to('456'); + // The `hash` injection of `Store` can now be resolved + const store = await requestCtx.get(STORE_KEY); + expect(store.hash).to.equal('456'); + }); + + it('injects method parameters from a child context', async () => { + class Store { + private name = 'my-store'; + + static getHash(@inject(HASH_KEY) hash: string) { + return hash; + } + + getHashWithName(@inject(HASH_KEY) hash: string) { + return `${hash}: ${this.name}`; + } + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + let value = await invokeMethod(Store, 'getHash', requestCtx); + expect(value).to.equal('123'); + + const store = await requestCtx.get(STORE_KEY); + value = await invokeMethod(store, 'getHashWithName', requestCtx); + expect(value).to.equal('123: my-store'); + + // bind the value to to ctx + ctx.bind(HASH_KEY).to('456'); + + value = await invokeMethod(Store, 'getHash', ctx); + expect(value).to.equal('456'); + + value = await invokeMethod(store, 'getHashWithName', ctx); + expect(value).to.equal('456: my-store'); + }); + + it('injects only bindings from the owner context with @inject.tag', async () => { + class Store { + constructor(@inject.tag('feature') public features: string[]) {} + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + ctx + .bind('features.security') + .to('security') + .tag('feature'); + requestCtx + .bind('features.debug') + .to('debug') + .tag('feature'); + + const store = await requestCtx.get(STORE_KEY); + // For singleton bindings, the injected tagged bindings will be only from + // the owner context (`ctx`) instead of the current one (`requestCtx`) + // used to resolve the bound `Store` + expect(store.features).to.eql(['security']); + }); + + it('injects owner context with @inject.context', async () => { + class Store { + constructor(@inject.context() public context: Context) {} + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + const store = await requestCtx.get(STORE_KEY); + // For singleton bindings, the injected context will be the owner context + // (`ctx`) instead of the current one (`requestCtx`) used to resolve the + // bound `Store` + expect(store.context).to.equal(ctx); + }); + + it('injects setter of owner context with @inject.setter', async () => { + class Store { + constructor(@inject.setter(HASH_KEY) public setHash: Setter) {} + } + + const requestCtx = givenRequestContextWithSingletonStore(Store); + + const store = await requestCtx.get(STORE_KEY); + store.setHash('456'); + + // For singleton bindings, the injected setter will set value to the owner + // context (`ctx`) instead of the current one (`requestCtx`) used to + // resolve the bound `Store` + expect(await ctx.get(HASH_KEY)).to.equal('456'); + expect(await requestCtx.get(HASH_KEY)).to.equal('123'); + }); + + function givenRequestContextWithSingletonStore( + storeClass: Constructor, + ) { + // Create a child context of `ctx` + const requestCtx = new Context(ctx, 'child'); + // Bind `hash` to `123` in the `requestCtx` + requestCtx.bind(HASH_KEY).to('123'); + + // Bind `Store` as a singleton at parent level (`ctx`) + ctx + .bind(STORE_KEY) + .toClass(storeClass) + .inScope(BindingScope.SINGLETON); + return requestCtx; + } }); it('injects a setter function', async () => { class Store { - constructor(@inject.setter('key') public setter: Setter) {} + constructor(@inject.setter(HASH_KEY) public setter: Setter) {} } - ctx.bind('store').toClass(Store); - const store = ctx.getSync('store'); + ctx.bind(STORE_KEY).toClass(Store); + const store = ctx.getSync(STORE_KEY); expect(store.setter).to.be.Function(); store.setter('a-value'); - expect(ctx.getSync('key')).to.equal('a-value'); + expect(ctx.getSync(HASH_KEY)).to.equal('a-value'); }); it('creates getter from a value', () => { @@ -188,8 +343,8 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.getter('key') public getter: string) {} } - ctx.bind('store').toClass(Store); - expect(() => ctx.getSync('store')).to.throw( + ctx.bind(STORE_KEY).toClass(Store); + expect(() => ctx.getSync(STORE_KEY)).to.throw( 'The type of Store.constructor[0] (String) is not a Getter function', ); }); @@ -201,8 +356,8 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.setter('key') public setter: object) {} } - ctx.bind('store').toClass(Store); - expect(() => ctx.getSync('store')).to.throw( + ctx.bind(STORE_KEY).toClass(Store); + expect(() => ctx.getSync(STORE_KEY)).to.throw( 'The type of Store.constructor[0] (Object) is not a Setter function', ); }); @@ -223,8 +378,8 @@ describe('Context bindings - Injecting dependencies of classes', () => { class Store { constructor(@inject.context() public context: Context) {} } - ctx.bind('store').toClass(Store); - const store: Store = ctx.getSync('store'); + ctx.bind(STORE_KEY).toClass(Store); + const store: Store = ctx.getSync(STORE_KEY); expect(store.context).to.be.exactly(ctx); }); @@ -233,7 +388,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.tag('store:location') public locations: string[]) {} } - ctx.bind('store').toClass(Store); + ctx.bind(STORE_KEY).toClass(Store); ctx .bind('store.locations.sf') .to('San Francisco') @@ -242,7 +397,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { .bind('store.locations.sj') .to('San Jose') .tag('store:location'); - const store: Store = ctx.getSync('store'); + const store: Store = ctx.getSync(STORE_KEY); expect(store.locations).to.eql(['San Francisco', 'San Jose']); }); @@ -251,7 +406,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.tag(/.+:location:sj/) public locations: string[]) {} } - ctx.bind('store').toClass(Store); + ctx.bind(STORE_KEY).toClass(Store); ctx .bind('store.locations.sf') .to('San Francisco') @@ -260,7 +415,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { .bind('store.locations.sj') .to('San Jose') .tag('store:location:sj'); - const store: Store = ctx.getSync('store'); + const store: Store = ctx.getSync(STORE_KEY); expect(store.locations).to.eql(['San Jose']); }); @@ -269,8 +424,8 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.tag('store:location') public locations: string[]) {} } - ctx.bind('store').toClass(Store); - const store: Store = ctx.getSync('store'); + ctx.bind(STORE_KEY).toClass(Store); + const store: Store = ctx.getSync(STORE_KEY); expect(store.locations).to.eql([]); }); @@ -279,7 +434,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.tag('store:location') public locations: string[]) {} } - ctx.bind('store').toClass(Store); + ctx.bind(STORE_KEY).toClass(Store); ctx .bind('store.locations.sf') .to('San Francisco') @@ -288,7 +443,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { .bind('store.locations.sj') .toDynamicValue(async () => 'San Jose') .tag('store:location'); - const store = await ctx.get('store'); + const store = await ctx.get(STORE_KEY); expect(store.locations).to.eql(['San Francisco', 'San Jose']); }); @@ -314,7 +469,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { } } - ctx.bind('store').toClass(Store); + ctx.bind(STORE_KEY).toClass(Store); ctx .bind('store.locations.sf') .to('San Francisco') @@ -323,7 +478,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { .bind('store.locations.sj') .toProvider(LocationProvider) .tag('store:location'); - const store = await ctx.get('store'); + const store = await ctx.get(STORE_KEY); expect(store.locations).to.eql(['San Francisco', 'San Jose']); expect(resolutionPath).to.eql( 'store --> @Store.constructor[0] --> store.locations.sj --> ' + @@ -336,7 +491,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { constructor(@inject.tag('store:location') public locations: string[]) {} } - ctx.bind('store').toClass(Store); + ctx.bind(STORE_KEY).toClass(Store); ctx .bind('store.locations.sf') .to('San Francisco') @@ -345,7 +500,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { .bind('store.locations.sj') .toDynamicValue(() => Promise.reject(new Error('Bad'))) .tag('store:location'); - await expect(ctx.get('store')).to.be.rejectedWith('Bad'); + await expect(ctx.get(STORE_KEY)).to.be.rejectedWith('Bad'); }); function createContext() { diff --git a/packages/context/src/__tests__/unit/resolver.unit.ts b/packages/context/src/__tests__/unit/resolver.unit.ts index 3ce1f0b92acf..bd4603030f80 100644 --- a/packages/context/src/__tests__/unit/resolver.unit.ts +++ b/packages/context/src/__tests__/unit/resolver.unit.ts @@ -650,7 +650,7 @@ describe('method injection', () => { expect(() => { invokeMethod(TestClass.prototype, 'test', ctx); - }).to.throw(/The key .+ was not bound to any value/); + }).to.throw(/The key .+ is not bound to any value/); }); it('resolves arguments for a static method', () => { diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index 391cdbed00ad..3d32c221e321 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -555,7 +555,7 @@ export class Context extends EventEmitter { } /** - * Get the value bound to the given key, throw an error when no value was + * Get the value bound to the given key, throw an error when no value is * bound for the given key. * * @example @@ -586,7 +586,7 @@ export class Context extends EventEmitter { * * ```ts * // get "rest" property from the value bound to "config" - * // use "undefined" when not config was provided + * // use `undefined` when no config is provided * const config = await ctx.get('config#rest', { * optional: true * }); @@ -597,7 +597,7 @@ export class Context extends EventEmitter { * @param optionsOrSession Options or session for resolution. An instance of * `ResolutionSession` is accepted for backward compatibility. * @returns A promise of the bound value, or a promise of undefined when - * the optional binding was not found. + * the optional binding is not found. */ get( keyWithPath: BindingAddress, @@ -654,7 +654,7 @@ export class Context extends EventEmitter { * * ```ts * // get "rest" property from the value bound to "config" - * // use "undefined" when no config was provided + * // use "undefined" when no config is provided * const config = await ctx.getSync('config#rest', { * optional: true * }); @@ -664,7 +664,7 @@ export class Context extends EventEmitter { * (deeply) nested property to retrieve. * * @param optionsOrSession Options or session for resolution. An instance of * `ResolutionSession` is accepted for backward compatibility. - * @returns The bound value, or undefined when an optional binding was not found. + * @returns The bound value, or undefined when an optional binding is not found. */ getSync( keyWithPath: BindingAddress, @@ -732,7 +732,9 @@ export class Context extends EventEmitter { } if (options && options.optional) return undefined; - throw new Error(`The key ${key} was not bound to any value.`); + throw new Error( + `The key '${key}' is not bound to any value in context ${this.name}`, + ); } /** @@ -759,7 +761,7 @@ export class Context extends EventEmitter { * (deeply) nested property to retrieve. * @param optionsOrSession Options for resolution or a session * @returns The bound value or a promise of the bound value, depending - * on how the binding was configured. + * on how the binding is configured. * @internal */ getValueOrPromise( diff --git a/packages/context/src/resolver.ts b/packages/context/src/resolver.ts index bab217623ff2..0c1f2e9877a8 100644 --- a/packages/context/src/resolver.ts +++ b/packages/context/src/resolver.ts @@ -6,6 +6,7 @@ import {DecoratorFactory} from '@loopback/metadata'; import * as assert from 'assert'; import * as debugModule from 'debug'; +import {BindingScope} from './binding'; import {isBindingAddress} from './binding-filter'; import {BindingAddress} from './binding-key'; import {Context} from './context'; @@ -75,6 +76,40 @@ export function instantiateClass( }); } +/** + * If the scope of current binding is `SINGLETON`, reset the context + * to be the one that owns the current binding to make sure a singleton + * does not have dependencies injected from child contexts unless the + * injection is for method (excluding constructor) parameters. + */ +function resolveContext( + ctx: Context, + injection: Readonly, + session?: ResolutionSession, +) { + const currentBinding = session && session.currentBinding; + if ( + currentBinding == null || + currentBinding.scope !== BindingScope.SINGLETON + ) { + // No current binding or its scope is not `SINGLETON` + return ctx; + } + + const isConstructorOrPropertyInjection = + // constructor injection + !injection.member || + // property injection + typeof injection.methodDescriptorOrParameterIndex !== 'number'; + + if (isConstructorOrPropertyInjection) { + // Set context to the owner context of the current binding for constructor + // or property injections against a singleton + ctx = ctx.getOwnerContext(currentBinding.key)!; + } + return ctx; +} + /** * Resolve the value or promise for a given injection * @param ctx Context @@ -93,6 +128,8 @@ function resolve( ResolutionSession.describeInjection(injection), ); } + + ctx = resolveContext(ctx, injection, session); let resolved = ResolutionSession.runWithInjection( s => { if (injection.resolve) { diff --git a/packages/rest/src/__tests__/acceptance/routing/routing.acceptance.ts b/packages/rest/src/__tests__/acceptance/routing/routing.acceptance.ts index 0072468c53be..56b77ede2499 100644 --- a/packages/rest/src/__tests__/acceptance/routing/routing.acceptance.ts +++ b/packages/rest/src/__tests__/acceptance/routing/routing.acceptance.ts @@ -3,37 +3,32 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT +import {BindingScope, Constructor, Context, inject} from '@loopback/context'; +import {Application, CoreBindings} from '@loopback/core'; +import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder'; +import {api, get, param, post, requestBody} from '@loopback/openapi-v3'; +import { + OperationObject, + ParameterObject, + ResponseObject, +} from '@loopback/openapi-v3-types'; +import {Client, createClientForHandler, expect} from '@loopback/testlab'; import { + ControllerClass, + ControllerInstance, + createControllerFactoryForClass, + createControllerFactoryForInstance, + HttpServerLike, + RegExpRouter, Request, Response, + RestApplication, RestBindings, - RestServer, RestComponent, - RestApplication, + RestServer, SequenceActions, - HttpServerLike, - ControllerClass, - ControllerInstance, - createControllerFactoryForClass, - createControllerFactoryForInstance, } from '../../..'; - -import {api, get, post, param, requestBody} from '@loopback/openapi-v3'; - -import {Application, CoreBindings} from '@loopback/core'; - -import { - ParameterObject, - OperationObject, - ResponseObject, -} from '@loopback/openapi-v3-types'; - -import {expect, Client, createClientForHandler} from '@loopback/testlab'; -import {anOpenApiSpec, anOperationSpec} from '@loopback/openapi-spec-builder'; -import {inject, Context, BindingScope} from '@loopback/context'; - import {createUnexpectedHttpErrorLogger} from '../../helpers'; -import {RegExpRouter} from '../../..'; /* # Feature: Routing * - In order to build REST APIs @@ -361,30 +356,84 @@ describe('Routing', () => { }); }); - it('binds the current controller', async () => { - const app = givenAnApplication(); - const server = await givenAServer(app); - const spec = anOpenApiSpec() - .withOperationReturningString('get', '/name', 'checkController') - .build(); + describe('current controller', () => { + let app: Application; + let server: RestServer; + let controllerClass: Constructor; - @api(spec) - class GetCurrentController { - async checkController( - @inject('controllers.GetCurrentController') inst: GetCurrentController, - ): Promise { - return { - result: this === inst, - }; - } + beforeEach(setupApplicationAndServer); + beforeEach(setupController); + + it('binds current controller resolved from a transient binding', async () => { + givenControllerInApp(app, controllerClass); + + await whenIMakeRequestTo(server) + .get('/name') + .expect({ + count: 1, + isSingleton: false, + result: true, + }); + + // Make a second call + await whenIMakeRequestTo(server) + .get('/name') + .expect({ + count: 1, // The count is still 1 as it's from a new instance + isSingleton: false, + result: true, + }); + }); + + it('binds current controller resolved from a singleton binding', async () => { + app.controller(controllerClass).inScope(BindingScope.SINGLETON); + + await whenIMakeRequestTo(server) + .get('/name') + .expect({ + count: 1, + isSingleton: true, + result: true, + }); + + // Make a second call + await whenIMakeRequestTo(server) + .get('/name') + .expect({ + count: 2, // The count increases as the controller is singleton + isSingleton: true, + result: true, + }); + }); + + async function setupApplicationAndServer() { + app = givenAnApplication(); + server = await givenAServer(app); } - givenControllerInApp(app, GetCurrentController); - return whenIMakeRequestTo(server) - .get('/name') - .expect({ - result: true, - }); + async function setupController() { + const spec = anOpenApiSpec() + .withOperationReturningString('get', '/name', 'checkController') + .build(); + + @api(spec) + class GetCurrentController { + private count = 0; + async checkController( + @inject('controllers.GetCurrentController') + inst: GetCurrentController, + @inject(CoreBindings.CONTROLLER_CURRENT) + currentInst: GetCurrentController, + ): Promise { + return { + count: ++this.count, + isSingleton: this === inst, + result: this === currentInst, + }; + } + } + controllerClass = GetCurrentController; + } }); it('supports function-based routes', async () => { @@ -809,7 +858,7 @@ describe('Routing', () => { app: Application, controller: ControllerClass, ) { - app.controller(controller).inScope(BindingScope.CONTEXT); + return app.controller(controller); } function whenIMakeRequestTo(serverOrApp: HttpServerLike): Client { diff --git a/packages/rest/src/__tests__/unit/router/controller-route.unit.ts b/packages/rest/src/__tests__/unit/router/controller-route.unit.ts index 908a1f43345c..c8a2a45756b2 100644 --- a/packages/rest/src/__tests__/unit/router/controller-route.unit.ts +++ b/packages/rest/src/__tests__/unit/router/controller-route.unit.ts @@ -11,6 +11,7 @@ import { } from '../../..'; import {expect} from '@loopback/testlab'; import {anOperationSpec} from '@loopback/openapi-spec-builder'; +import {Context, CoreBindings, BindingScope} from '@loopback/core'; describe('ControllerRoute', () => { it('rejects routes with no methodName', () => { @@ -85,6 +86,59 @@ describe('ControllerRoute', () => { expect(route._controllerName).to.eql('my-controller'); }); + describe('updateBindings()', () => { + let appCtx: Context; + let requestCtx: Context; + + before(givenContextsAndControllerRoute); + + it('adds bindings to the request context', async () => { + expect(requestCtx.contains(CoreBindings.CONTROLLER_CURRENT)); + expect( + requestCtx.getBinding(CoreBindings.CONTROLLER_CURRENT).scope, + ).to.equal(BindingScope.SINGLETON); + expect(await requestCtx.get(CoreBindings.CONTROLLER_CLASS)).to.equal( + MyController, + ); + expect( + await requestCtx.get(CoreBindings.CONTROLLER_METHOD_NAME), + ).to.equal('greet'); + }); + + it('binds current controller to the request context as singleton', async () => { + const controller1 = await requestCtx.get(CoreBindings.CONTROLLER_CURRENT); + expect(controller1).instanceOf(MyController); + + const controller2 = await requestCtx.get(CoreBindings.CONTROLLER_CURRENT); + expect(controller2).to.be.exactly(controller1); + + const childCtx = new Context(requestCtx); + const controller3 = await childCtx.get(CoreBindings.CONTROLLER_CURRENT); + expect(controller3).to.be.exactly(controller1); + + await expect( + appCtx.get(CoreBindings.CONTROLLER_CURRENT), + ).to.be.rejectedWith(/The key .+ is not bound to any value/); + }); + + function givenContextsAndControllerRoute() { + const spec = anOperationSpec().build(); + + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + myControllerFactory, + 'greet', + ); + + appCtx = new Context('application'); + requestCtx = new Context(appCtx, 'request'); + route.updateBindings(requestCtx); + } + }); + class MyController { greet() { return 'Hello'; diff --git a/packages/rest/src/rest.component.ts b/packages/rest/src/rest.component.ts index f96d0b343e4b..1fbe2e711d4a 100644 --- a/packages/rest/src/rest.component.ts +++ b/packages/rest/src/rest.component.ts @@ -3,7 +3,7 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Binding, BindingScope, Constructor, inject} from '@loopback/context'; +import {Binding, Constructor, inject} from '@loopback/context'; import { Application, Component, @@ -53,9 +53,9 @@ export class RestComponent implements Component { * Add built-in body parsers */ bindings = [ - Binding.bind(RestBindings.REQUEST_BODY_PARSER) - .toClass(RequestBodyParser) - .inScope(BindingScope.SINGLETON), + // FIXME(rfeng): We now register request body parsers in TRANSIENT scope + // so that they can be bound at application or server level + Binding.bind(RestBindings.REQUEST_BODY_PARSER).toClass(RequestBodyParser), createBodyParserBinding( JsonBodyParser, RestBindings.REQUEST_BODY_PARSER_JSON, diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 26660f6c36cd..f64aaf93c25e 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -826,7 +826,7 @@ export function createBodyParserBinding( key || `${RestBindings.REQUEST_BODY_PARSER}.${parserClass.name}`; return Binding.bind(address) .toClass(parserClass) - .inScope(BindingScope.SINGLETON) + .inScope(BindingScope.TRANSIENT) .tag(REQUEST_BODY_PARSER_TAG); } diff --git a/packages/rest/src/router/controller-route.ts b/packages/rest/src/router/controller-route.ts index ee0d05430a76..50f9a7dbd716 100644 --- a/packages/rest/src/router/controller-route.ts +++ b/packages/rest/src/router/controller-route.ts @@ -4,12 +4,12 @@ // License text available at https://opensource.org/licenses/MIT import { - BindingScope, Constructor, Context, instantiateClass, invokeMethod, ValueOrPromise, + BindingScope, } from '@loopback/context'; import {CoreBindings} from '@loopback/core'; import {OperationObject} from '@loopback/openapi-v3-types'; @@ -100,6 +100,19 @@ export class ControllerRoute extends BaseRoute { } updateBindings(requestContext: Context) { + /* + * Bind current controller to the request context in `SINGLETON` scope. + * Within the same request, we always get the same instance of the + * current controller when `requestContext.get(CoreBindings.CONTROLLER_CURRENT)` + * is invoked. + * + * Please note the controller class itself can be bound to other scopes, + * such as SINGLETON or TRANSIENT (default) in the application or server + * context. + * + * - SINGLETON: all requests share the same instance of a given controller + * - TRANSIENT: each request has its own instance of a given controller + */ requestContext .bind(CoreBindings.CONTROLLER_CURRENT) .toDynamicValue(() => this._controllerFactory(requestContext))