From 29a748e50ca228f6c9873fbee926518f8fbc67ac Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 25 Mar 2019 10:37:37 -0700 Subject: [PATCH] fix(context): clear binding cache upon scope/getValue changes --- .../src/__tests__/unit/binding.unit.ts | 52 ++++++++++++- packages/context/src/binding.ts | 75 +++++++++++++++---- packages/context/src/context.ts | 29 +++---- packages/context/src/resolution-session.ts | 31 ++++++-- 4 files changed, 150 insertions(+), 37 deletions(-) diff --git a/packages/context/src/__tests__/unit/binding.unit.ts b/packages/context/src/__tests__/unit/binding.unit.ts index 28cb6837a4d0..55b700ab64ff 100644 --- a/packages/context/src/__tests__/unit/binding.unit.ts +++ b/packages/context/src/__tests__/unit/binding.unit.ts @@ -3,7 +3,7 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {expect} from '@loopback/testlab'; +import {expect, sinon, SinonSpy} from '@loopback/testlab'; import { Binding, BindingScope, @@ -214,6 +214,56 @@ describe('Binding', () => { }); }); + describe('cache', () => { + let spy: SinonSpy; + beforeEach(() => { + spy = sinon.spy(); + }); + + it('clears cache if scope changes', () => { + const indexBinding = ctx + .bind('index') + .toDynamicValue(spy) + .inScope(BindingScope.SINGLETON); + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + // Singleton + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + indexBinding.inScope(BindingScope.CONTEXT); + ctx.getSync(indexBinding.key); + sinon.assert.calledTwice(spy); + }); + + it('clears cache if _getValue changes', () => { + const providerSpy = sinon.spy(); + class IndexProvider implements Provider { + value() { + return providerSpy(); + } + } + const indexBinding = ctx + .bind('index') + .toDynamicValue(spy) + .inScope(BindingScope.SINGLETON); + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + // Singleton + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + + // Now change the value getter + indexBinding.toProvider(IndexProvider); + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + sinon.assert.calledOnce(providerSpy); + // Singleton + ctx.getSync(indexBinding.key); + sinon.assert.calledOnce(spy); + sinon.assert.calledOnce(providerSpy); + }); + }); + describe('toJSON()', () => { it('converts a keyed binding to plain JSON object', () => { const json = binding.toJSON(); diff --git a/packages/context/src/binding.ts b/packages/context/src/binding.ts index 3045bdee3f7c..a434b625e6b0 100644 --- a/packages/context/src/binding.ts +++ b/packages/context/src/binding.ts @@ -7,7 +7,11 @@ import * as debugModule from 'debug'; import {BindingAddress, BindingKey} from './binding-key'; import {Context} from './context'; import {Provider} from './provider'; -import {ResolutionSession} from './resolution-session'; +import { + asResolutionOptions, + ResolutionOptionsOrSession, + ResolutionSession, +} from './resolution-session'; import {instantiateClass} from './resolver'; import { BoundValue, @@ -162,9 +166,9 @@ export class Binding { private _cache: WeakMap; private _getValue: ( - ctx?: Context, - session?: ResolutionSession, - ) => ValueOrPromise; + ctx: Context, + optionsOrSession?: ResolutionOptionsOrSession, + ) => ValueOrPromise; private _valueConstructor?: Constructor; /** @@ -204,6 +208,15 @@ export class Binding { }); } + /** + * Clear the cache + */ + private _clearCache() { + if (!this._cache) return; + // WeakMap does not have a `clear` method + this._cache = new WeakMap(); + } + /** * This is an internal function optimized for performance. * Users should use `@inject(key)` or `ctx.get(key)` instead. @@ -228,7 +241,10 @@ export class Binding { * @param ctx Context for the resolution * @param session Optional session for binding and dependency resolution */ - getValue(ctx: Context, session?: ResolutionSession): ValueOrPromise { + getValue( + ctx: Context, + optionsOrSession?: ResolutionOptionsOrSession, + ): ValueOrPromise { /* istanbul ignore if */ if (debug.enabled) { debug('Get value for binding %s', this.key); @@ -246,11 +262,15 @@ export class Binding { } } } + optionsOrSession = asResolutionOptions(optionsOrSession); if (this._getValue) { let result = ResolutionSession.runWithBinding( - s => this._getValue(ctx, s), + s => { + const options = Object.assign({}, optionsOrSession, {session: s}); + return this._getValue(ctx, options); + }, this, - session, + optionsOrSession.session, ); return this._cacheValue(ctx, result); } @@ -317,6 +337,7 @@ export class Binding { * @param scope Binding scope */ inScope(scope: BindingScope): this { + if (this._scope !== scope) this._clearCache(); this._scope = scope; return this; } @@ -328,11 +349,26 @@ export class Binding { */ applyDefaultScope(scope: BindingScope): this { if (!this._scope) { - this._scope = scope; + this.inScope(scope); } return this; } + /** + * Set the `_getValue` function + * @param getValueFn getValue function + */ + private _setValueGetter( + getValueFn: ( + ctx: Context, + optionsOrSession?: ResolutionOptionsOrSession, + ) => ValueOrPromise, + ) { + // Clear the cache + this._clearCache(); + this._getValue = getValueFn; + } + /** * Bind the key to a constant value. The value must be already available * at binding time, it is not allowed to pass a Promise instance. @@ -372,7 +408,7 @@ export class Binding { debug('Bind %s to constant:', this.key, value); } this._type = BindingType.CONSTANT; - this._getValue = () => value; + this._setValueGetter(() => value); return this; } @@ -400,7 +436,7 @@ export class Binding { debug('Bind %s to dynamic value:', this.key, factoryFn); } this._type = BindingType.DYNAMIC_VALUE; - this._getValue = ctx => factoryFn(); + this._setValueGetter(ctx => factoryFn()); return this; } @@ -426,14 +462,14 @@ export class Binding { debug('Bind %s to provider %s', this.key, providerClass.name); } this._type = BindingType.PROVIDER; - this._getValue = (ctx, session) => { + this._setValueGetter((ctx, optionsOrSession) => { const providerOrPromise = instantiateClass>( providerClass, - ctx!, - session, + ctx, + asResolutionOptions(optionsOrSession).session, ); return transformValueOrPromise(providerOrPromise, p => p.value()); - }; + }); return this; } @@ -450,11 +486,20 @@ export class Binding { debug('Bind %s to class %s', this.key, ctor.name); } this._type = BindingType.CLASS; - this._getValue = (ctx, session) => instantiateClass(ctor, ctx!, session); + this._setValueGetter((ctx, optionsOrSession) => + instantiateClass( + ctor, + ctx, + asResolutionOptions(optionsOrSession).session, + ), + ); this._valueConstructor = ctor; return this; } + /** + * Unlock the binding + */ unlock(): this { this.isLocked = false; return this; diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index 3d32c221e321..dfc3f3e31556 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -6,11 +6,9 @@ import * as debugModule from 'debug'; import {EventEmitter} from 'events'; import {v1 as uuidv1} from 'uuid'; -import {ValueOrPromise} from '.'; import {Binding, BindingTag} from './binding'; import {BindingFilter, filterByKey, filterByTag} from './binding-filter'; import {BindingAddress, BindingKey} from './binding-key'; -import {ContextView} from './context-view'; import { ContextEventObserver, ContextEventType, @@ -18,8 +16,19 @@ import { Notification, Subscription, } from './context-observer'; -import {ResolutionOptions, ResolutionSession} from './resolution-session'; -import {BoundValue, getDeepProperty, isPromiseLike} from './value-promise'; +import {ContextView} from './context-view'; +import { + asResolutionOptions, + ResolutionOptions, + ResolutionOptionsOrSession, + ResolutionSession, +} from './resolution-session'; +import { + BoundValue, + getDeepProperty, + isPromiseLike, + ValueOrPromise, +} from './value-promise'; /** * Polyfill Symbol.asyncIterator as required by TypeScript for Node 8.x. @@ -766,22 +775,16 @@ export class Context extends EventEmitter { */ getValueOrPromise( keyWithPath: BindingAddress, - optionsOrSession?: ResolutionOptions | ResolutionSession, + optionsOrSession?: ResolutionOptionsOrSession, ): ValueOrPromise { const {key, propertyPath} = BindingKey.parseKeyWithPath(keyWithPath); - // backwards compatibility - if (optionsOrSession instanceof ResolutionSession) { - optionsOrSession = {session: optionsOrSession}; - } + optionsOrSession = asResolutionOptions(optionsOrSession); const binding = this.getBinding(key, optionsOrSession); if (binding == null) return undefined; - const boundValue = binding.getValue( - this, - optionsOrSession && optionsOrSession.session, - ); + const boundValue = binding.getValue(this, optionsOrSession); if (propertyPath === undefined || propertyPath === '') { return boundValue; } diff --git a/packages/context/src/resolution-session.ts b/packages/context/src/resolution-session.ts index 4bfc53cfa19b..1d2519fd0b72 100644 --- a/packages/context/src/resolution-session.ts +++ b/packages/context/src/resolution-session.ts @@ -3,19 +3,15 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT +import {DecoratorFactory} from '@loopback/metadata'; +import * as debugModule from 'debug'; import {Binding} from './binding'; import {Injection} from './inject'; -import {ValueOrPromise, BoundValue, tryWithFinally} from './value-promise'; -import * as debugModule from 'debug'; -import {DecoratorFactory} from '@loopback/metadata'; +import {BoundValue, tryWithFinally, ValueOrPromise} from './value-promise'; const debugSession = debugModule('loopback:context:resolver:session'); const getTargetName = DecoratorFactory.getTargetName; -// NOTE(bajtos) The following import is required to satisfy TypeScript compiler -// tslint:disable-next-line:no-unused -import {BindingKey} from './binding-key'; - /** * A function to be executed with the resolution session */ @@ -169,7 +165,7 @@ export class ResolutionSession { ); return { targetName: name, - bindingKey: injection.bindingSelector, + bindingSelector: injection.bindingSelector, // Cast to Object so that we don't have to expose InjectionMetadata metadata: injection.metadata as Object, }; @@ -343,3 +339,22 @@ export interface ResolutionOptions { */ optional?: boolean; } + +/** + * Resolution options or session + */ +export type ResolutionOptionsOrSession = ResolutionOptions | ResolutionSession; + +/** + * Normalize ResolutionOptionsOrSession to ResolutionOptions + * @param optionsOrSession resolution options or session + */ +export function asResolutionOptions( + optionsOrSession?: ResolutionOptionsOrSession, +): ResolutionOptions { + // backwards compatibility + if (optionsOrSession instanceof ResolutionSession) { + optionsOrSession = {session: optionsOrSession}; + } + return optionsOrSession || {}; +}