Skip to content

Commit

Permalink
fix(context): clear binding cache upon scope/getValue changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@raymondfeng
raymondfeng committed Mar 28, 2019
1 parent 124c078 commit 29a748e
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 37 deletions.
52 changes: 51 additions & 1 deletion packages/context/src/__tests__/unit/binding.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<number>('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<number> {
value() {
return providerSpy();
}
}
const indexBinding = ctx
.bind<number>('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();
Expand Down
75 changes: 60 additions & 15 deletions packages/context/src/binding.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -162,9 +166,9 @@ export class Binding<T = BoundValue> {

private _cache: WeakMap<Context, T>;
private _getValue: (
ctx?: Context,
session?: ResolutionSession,
) => ValueOrPromise<T>;
ctx: Context,
optionsOrSession?: ResolutionOptionsOrSession,
) => ValueOrPromise<T | undefined>;

private _valueConstructor?: Constructor<T>;
/**
Expand Down Expand Up @@ -204,6 +208,15 @@ export class Binding<T = BoundValue> {
});
}

/**
* 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.
Expand All @@ -228,7 +241,10 @@ export class Binding<T = BoundValue> {
* @param ctx Context for the resolution
* @param session Optional session for binding and dependency resolution
*/
getValue(ctx: Context, session?: ResolutionSession): ValueOrPromise<T> {
getValue(
ctx: Context,
optionsOrSession?: ResolutionOptionsOrSession,
): ValueOrPromise<T> {
/* istanbul ignore if */
if (debug.enabled) {
debug('Get value for binding %s', this.key);
Expand All @@ -246,11 +262,15 @@ export class Binding<T = BoundValue> {
}
}
}
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);
}
Expand Down Expand Up @@ -317,6 +337,7 @@ export class Binding<T = BoundValue> {
* @param scope Binding scope
*/
inScope(scope: BindingScope): this {
if (this._scope !== scope) this._clearCache();
this._scope = scope;
return this;
}
Expand All @@ -328,11 +349,26 @@ export class Binding<T = BoundValue> {
*/
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<T | undefined>,
) {
// 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.
Expand Down Expand Up @@ -372,7 +408,7 @@ export class Binding<T = BoundValue> {
debug('Bind %s to constant:', this.key, value);
}
this._type = BindingType.CONSTANT;
this._getValue = () => value;
this._setValueGetter(() => value);
return this;
}

Expand Down Expand Up @@ -400,7 +436,7 @@ export class Binding<T = BoundValue> {
debug('Bind %s to dynamic value:', this.key, factoryFn);
}
this._type = BindingType.DYNAMIC_VALUE;
this._getValue = ctx => factoryFn();
this._setValueGetter(ctx => factoryFn());
return this;
}

Expand All @@ -426,14 +462,14 @@ export class Binding<T = BoundValue> {
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<Provider<T>>(
providerClass,
ctx!,
session,
ctx,
asResolutionOptions(optionsOrSession).session,
);
return transformValueOrPromise(providerOrPromise, p => p.value());
};
});
return this;
}

Expand All @@ -450,11 +486,20 @@ export class Binding<T = BoundValue> {
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;
Expand Down
29 changes: 16 additions & 13 deletions packages/context/src/context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,29 @@
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,
ContextObserver,
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.
Expand Down Expand Up @@ -766,22 +775,16 @@ export class Context extends EventEmitter {
*/
getValueOrPromise<ValueType>(
keyWithPath: BindingAddress<ValueType>,
optionsOrSession?: ResolutionOptions | ResolutionSession,
optionsOrSession?: ResolutionOptionsOrSession,
): ValueOrPromise<ValueType | undefined> {
const {key, propertyPath} = BindingKey.parseKeyWithPath(keyWithPath);

// backwards compatibility
if (optionsOrSession instanceof ResolutionSession) {
optionsOrSession = {session: optionsOrSession};
}
optionsOrSession = asResolutionOptions(optionsOrSession);

const binding = this.getBinding<ValueType>(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;
}
Expand Down
31 changes: 23 additions & 8 deletions packages/context/src/resolution-session.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 || {};
}

0 comments on commit 29a748e

Please sign in to comment.