From 58147569ef6722757fba5ee5b3685cceeed5e692 Mon Sep 17 00:00:00 2001 From: Kevin Delisle Date: Tue, 10 Oct 2017 14:10:01 -0400 Subject: [PATCH 1/3] feat(context): options function for bindings .options allows you to create multiple bindings that resolve different values for the same injection key(s). ```ts // in your application app.bind('foo').toClass(Magician).options({ alignment: 'good' }); app.bind('bar').toClass(Magician).options({ alignment: 'evil' }); // magician.ts class Magician { constructor(@inject('alignment') public alignment: string) { // The 'foo' version will have a 'good' alignment. // The 'bar' version will have a 'bad' alignment. } } ``` --- packages/context/src/binding.ts | 35 ++++++++++++++++++++++- packages/context/src/context.ts | 21 ++++++++++++++ packages/context/test/unit/binding.ts | 41 +++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/packages/context/src/binding.ts b/packages/context/src/binding.ts index 5141ccc28adf..6bca0e7c9882 100644 --- a/packages/context/src/binding.ts +++ b/packages/context/src/binding.ts @@ -92,6 +92,11 @@ export enum BindingType { PROVIDER = 'Provider', } +export type BindingDependencies = { + // tslint:disable-next-line:no-any + [key: string]: any; +}; + // FIXME(bajtos) The binding class should be parameterized by the value // type stored export class Binding { @@ -133,12 +138,16 @@ export class Binding { public readonly key: string; public readonly tags: Set = new Set(); + public scope: BindingScope = BindingScope.TRANSIENT; public type: BindingType; private _cache: BoundValue; private _getValue: (ctx?: Context) => BoundValue | Promise; + // Used to provide a child context at resolution time if .options + // is called. + protected subcontext: Context; // For bindings bound via toClass, this property contains the constructor // function public valueConstructor: Constructor; @@ -216,8 +225,16 @@ export class Binding { * ``` */ getValue(ctx: Context): BoundValue | Promise { - // First check cached value for non-transient + // Make sure we're not endlessly looping over our subcontext. + if (this.subcontext && this.subcontext !== ctx) { + // If the subcontext hasn't been bound to a parent, do so now. + if (!this.subcontext.hasParent()) { + this.subcontext.setParent(ctx); + } + return this.getValue(this.subcontext); + } if (this._cache !== undefined) { + // First check cached value for non-transient if (this.scope === BindingScope.SINGLETON) { return this._cache; } else if (this.scope === BindingScope.CONTEXT) { @@ -256,6 +273,22 @@ export class Binding { return this; } + /** + * Override any existing configuration on the context for + * this given binding and provide the given values for injection instead + * Any bindings not defined in the given dependencies will be resolved from + * the binding's context. + * @param deps + */ + options(deps: BindingDependencies): this { + // This context is currently unbound. + this.subcontext = new Context(); + for (const key in deps) { + this.subcontext.bind(key).to(deps[key]); + } + return this; + } + /** * Bind the key to a constant value. * diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index c2a66818f09d..fe6a1afbf258 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -203,6 +203,27 @@ export class Context { } return json; } + + /** + * Whether or not this context instance has a parent context instance. + * + * @returns {boolean} + * @memberof Context + */ + hasParent(): boolean { + return !!this._parent; + } + + /** + * Directly set the parent context of this context. + * + * @param {Context} ctx + * @memberof Context + */ + setParent(ctx: Context) { + // FIXME(kev): This is definitely open to circular linking (bad!) + this._parent = ctx; + } } function getDeepProperty(value: BoundValue, path: string) { diff --git a/packages/context/test/unit/binding.ts b/packages/context/test/unit/binding.ts index 2b9dcfe15f48..e5a23df2cf67 100644 --- a/packages/context/test/unit/binding.ts +++ b/packages/context/test/unit/binding.ts @@ -162,6 +162,47 @@ describe('Binding', () => { }); }); + describe('options(Dependencies))', () => { + it('binds dependencies without overlap', async () => { + const otherKey = 'bar'; + ctx + .bind(key) + .toClass(FakeProduct) + .options({ + config: { + foo: 'bar', + }, + }); + + ctx + .bind(otherKey) + .toClass(FakeProduct) + .options({ + config: { + foo: 'baz', + }, + }); + + const product1 = (await ctx.get(key)) as FakeProduct; + const product2 = (await ctx.get(otherKey)) as FakeProduct; + + function checkConfig(prod: FakeProduct, expected: string) { + expect(prod).to.not.be.null(); + expect(prod.config).to.not.be.null(); + expect(prod.config.foo).to.equal(expected); + } + + // Check in reverse order (in case the 2nd binding is masking the 1st) + checkConfig(product2, 'baz'); + checkConfig(product1, 'bar'); + }); + + class FakeProduct { + // tslint:disable-next-line:no-any + constructor(@inject('config') public config: any) {} + } + }); + function givenBinding() { ctx = new Context(); binding = new Binding(key); From 1caac72bedff669a911b135f1a7af6f6962bb83e Mon Sep 17 00:00:00 2001 From: Kevin Delisle Date: Tue, 17 Oct 2017 18:13:40 -0400 Subject: [PATCH 2/3] feat(context): setParent prevents circular references --- packages/context/src/context.ts | 22 +++++++++++-- packages/context/test/unit/context.ts | 45 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index fe6a1afbf258..f8044a9b334c 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -9,7 +9,7 @@ import {isPromise} from './is-promise'; export class Context { private registry: Map; - constructor(private _parent?: Context) { + constructor(protected _parent?: Context) { this.registry = new Map(); } @@ -221,8 +221,26 @@ export class Context { * @memberof Context */ setParent(ctx: Context) { - // FIXME(kev): This is definitely open to circular linking (bad!) + const circular = 'circular parent reference detected'; + if (this === ctx) { + throw new Error(circular); + } + const currentParent = this._parent; + let parent = ctx._parent; + // For now, set the parent. this._parent = ctx; + // Ensure that this instance is not the parent/grandparent/etc of ctx. + while (parent) { + if (parent === ctx) { + this._parent = currentParent; // Restore the previous parent (if any); + throw new Error(circular); + } + if (parent._parent) { + parent = parent._parent; + } else { + parent = undefined; + } + } } } diff --git a/packages/context/test/unit/context.ts b/packages/context/test/unit/context.ts index 65dbddb1a0d5..ed1ef93b4002 100644 --- a/packages/context/test/unit/context.ts +++ b/packages/context/test/unit/context.ts @@ -375,6 +375,51 @@ describe('Context', () => { }); }); + describe('getParent', () => { + it('links parent to child', () => { + const parent = new TestContext(); + const child = new TestContext(); + child.setParent(parent); + + expect(child.getParent()).to.equal(parent); + }); + + it('throws on self-linking', () => { + const context = new Context(); + expect.throws(() => { + context.setParent(context); + }); + expect(context.hasParent()).to.be.false(); + }); + + it('throws on circular linking', () => { + const grandparent = new TestContext(); + grandparent.name = 'grandma'; + const parent = new TestContext(); + parent.setParent(grandparent); + parent.name = 'mom'; + const child = new TestContext(); + child.name = 'daughter'; + child.setParent(parent); + expect.throws(() => { + grandparent.setParent(child); + }); + expect(grandparent.hasParent()).to.be.false(); + }); + + /** + * Custom extension to allow retrieval of parent for testing. + * @class TestContext + * @extends {Context} + */ + class TestContext extends Context { + public name: string; + getParent() { + return this._parent; + } + } + }); + describe('toJSON()', () => { it('converts to plain JSON object', () => { ctx From 25a13a922c23aff5eaca07c04077fdfe41cbc80d Mon Sep 17 00:00:00 2001 From: Kevin Delisle Date: Tue, 17 Oct 2017 18:16:48 -0400 Subject: [PATCH 3/3] fix(context): move test helpers out of tests --- packages/context/test/unit/binding.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/context/test/unit/binding.ts b/packages/context/test/unit/binding.ts index e5a23df2cf67..fd553e3a2aad 100644 --- a/packages/context/test/unit/binding.ts +++ b/packages/context/test/unit/binding.ts @@ -186,21 +186,10 @@ describe('Binding', () => { const product1 = (await ctx.get(key)) as FakeProduct; const product2 = (await ctx.get(otherKey)) as FakeProduct; - function checkConfig(prod: FakeProduct, expected: string) { - expect(prod).to.not.be.null(); - expect(prod.config).to.not.be.null(); - expect(prod.config.foo).to.equal(expected); - } - // Check in reverse order (in case the 2nd binding is masking the 1st) checkConfig(product2, 'baz'); checkConfig(product1, 'bar'); }); - - class FakeProduct { - // tslint:disable-next-line:no-any - constructor(@inject('config') public config: any) {} - } }); function givenBinding() { @@ -208,6 +197,17 @@ describe('Binding', () => { binding = new Binding(key); } + class FakeProduct { + // tslint:disable-next-line:no-any + constructor(@inject('config') public config: any) {} + } + + function checkConfig(prod: FakeProduct, expected: string) { + expect(prod).to.not.be.null(); + expect(prod.config).to.not.be.null(); + expect(prod.config.foo).to.equal(expected); + } + class MyProvider implements Provider { constructor(@inject('msg') private _msg: string) {} value(): string {