Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(context): clear binding cache upon scope/getValue changes #2656

Merged
merged 2 commits into from
Apr 1, 2019
Merged

fix(context): clear binding cache upon scope/getValue changes #2656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7e8b21
feat(context): pass resolution options into binding.getValue()
raymondfeng Mar 25, 2019
815c23e
fix(context): clear binding cache upon scope or value getter changes
raymondfeng Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion packages/context/src/__tests__/unit/binding.unit.ts
View file
Open in desktop
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,68 @@ 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);
spy.resetHistory();

// Singleton
ctx.getSync(indexBinding.key);
sinon.assert.notCalled(spy);
spy.resetHistory();

indexBinding.inScope(BindingScope.CONTEXT);
ctx.getSync(indexBinding.key);
sinon.assert.calledOnce(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);
spy.resetHistory();

// Singleton
ctx.getSync(indexBinding.key);
sinon.assert.notCalled(spy);
spy.resetHistory();

// Now change the value getter
indexBinding.toProvider(IndexProvider);
ctx.getSync(indexBinding.key);
sinon.assert.notCalled(spy);
sinon.assert.calledOnce(providerSpy);
spy.resetHistory();
providerSpy.resetHistory();

// Singleton
ctx.getSync(indexBinding.key);
sinon.assert.notCalled(spy);
sinon.assert.notCalled(providerSpy);
});
});

describe('toJSON()', () => {
it('converts a keyed binding to plain JSON object', () => {
const json = binding.toJSON();
Expand Down
90 changes: 72 additions & 18 deletions packages/context/src/binding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import * as debugModule from 'debug';
import * as debugFactory from 'debug';
import {BindingAddress, BindingKey} from './binding-key';
import {Context} from './context';
import {Provider} from './provider';
import {ResolutionSession} from './resolution-session';
import {
asResolutionOptions,
ResolutionOptions,
ResolutionOptionsOrSession,
ResolutionSession,
} from './resolution-session';
import {instantiateClass} from './resolver';
import {
BoundValue,
Expand All @@ -18,7 +23,7 @@ import {
ValueOrPromise,
} from './value-promise';

const debug = debugModule('loopback:context:binding');
const debug = debugFactory('loopback:context:binding');

/**
* Scope for binding values
Expand Down Expand Up @@ -128,6 +133,11 @@ export type BindingTag = TagMap | string;
*/
export type BindingTemplate<T = unknown> = (binding: Binding<T>) => void;

type ValueGetter<T> = (
ctx: Context,
options: ResolutionOptions,
) => ValueOrPromise<T | undefined>;
bajtos marked this conversation as resolved.
Show resolved Hide resolved

/**
* Binding represents an entry in the `Context`. Each binding has a key and a
* corresponding value getter.
Expand Down Expand Up @@ -161,10 +171,7 @@ export class Binding<T = BoundValue> {
}

private _cache: WeakMap<Context, T>;
private _getValue: (
ctx?: Context,
session?: ResolutionSession,
) => ValueOrPromise<T>;
private _getValue: ValueGetter<T>;

private _valueConstructor?: Constructor<T>;
/**
Expand Down Expand Up @@ -204,6 +211,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 +244,25 @@ 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, session?: ResolutionSession): ValueOrPromise<T>;

/**
* Returns a value or promise for this binding in the given context. The
* resolved value can be `undefined` if `optional` is set to `true` in
* `options`.
* @param ctx Context for the resolution
* @param options Optional options for binding and dependency resolution
*/
getValue(
ctx: Context,
options?: ResolutionOptions,
): ValueOrPromise<T | undefined>;

// Implementation
getValue(
ctx: Context,
optionsOrSession?: ResolutionOptionsOrSession,
): ValueOrPromise<T | undefined> {
/* istanbul ignore if */
if (debug.enabled) {
debug('Get value for binding %s', this.key);
Expand All @@ -246,11 +280,15 @@ export class Binding<T = BoundValue> {
}
}
}
const options = asResolutionOptions(optionsOrSession);
if (this._getValue) {
let result = ResolutionSession.runWithBinding(
s => this._getValue(ctx, s),
s => {
const optionsWithSession = Object.assign({}, options, {session: s});
return this._getValue(ctx, optionsWithSession);
},
this,
session,
options.session,
);
return this._cacheValue(ctx, result);
}
Expand Down Expand Up @@ -317,6 +355,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 +367,21 @@ 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 getValue getValue function
*/
private _setValueGetter(getValue: ValueGetter<T>) {
// Clear the cache
this._clearCache();
this._getValue = getValue;
}

/**
* 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 +421,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 +449,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 +475,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, options) => {
const providerOrPromise = instantiateClass<Provider<T>>(
providerClass,
ctx!,
session,
ctx,
options.session,
);
return transformValueOrPromise(providerOrPromise, p => p.value());
};
});
return this;
}

Expand All @@ -450,11 +499,16 @@ 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, options) =>
instantiateClass(ctor, ctx, options.session),
);
this._valueConstructor = ctor;
return this;
}

/**
* Unlock the binding
*/
unlock(): this {
this.isLocked = false;
return this;
Expand Down
Loading