From f81045fc9e7cd6d77910bc7fc36139d15e6d3473 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 24 Jan 2019 09:44:42 -0800 Subject: [PATCH] chore(context): review - 8 --- packages/context/src/context-listener.ts | 9 +- packages/context/src/context-view.ts | 30 ++++--- packages/context/src/context.ts | 32 +++---- packages/context/src/inject.ts | 87 ++++++++++++------ .../acceptance/context-view.acceptance.ts | 89 +++++++++---------- .../context/test/unit/context-view.unit.ts | 65 +++++++------- .../context/test/unit/inject-view.unit.ts | 6 +- 7 files changed, 177 insertions(+), 141 deletions(-) diff --git a/packages/context/src/context-listener.ts b/packages/context/src/context-listener.ts index 8c5952337b6d..d26a60b871f9 100644 --- a/packages/context/src/context-listener.ts +++ b/packages/context/src/context-listener.ts @@ -8,9 +8,10 @@ import {BindingFilter} from './binding-filter'; import {ValueOrPromise} from './value-promise'; /** - * Context event types + * Context event types. We support `bind` and `unbind` for now but + * keep it open for new types */ -export type ContextEventType = string; +export type ContextEventType = 'bind' | 'unbind' | string; /** * Listeners of context bind/unbind events @@ -22,7 +23,9 @@ export interface ContextEventListener { filter?: BindingFilter; /** - * Listen on `bind` or `unbind` + * Listen on `bind`, `unbind`, or other events + * @param eventType Context event type + * @param binding The binding as event source */ listen( eventType: ContextEventType, diff --git a/packages/context/src/context-view.ts b/packages/context/src/context-view.ts index 82495422c87a..a9cdf5c4a692 100644 --- a/packages/context/src/context-view.ts +++ b/packages/context/src/context-view.ts @@ -15,8 +15,9 @@ import { } from './context-listener'; import {Getter} from './inject'; import {ResolutionSession} from './resolution-session'; -import {resolveList} from './value-promise'; +import {resolveList, ValueOrPromise} from './value-promise'; const debug = debugFactory('loopback:context:view'); +const nextTick = promisify(process.nextTick); /** * `ContextView` provides a view for a given context chain to maintain a live @@ -34,7 +35,7 @@ export class ContextView implements ContextEventListener { private _subscription: Subscription | undefined; constructor( - protected readonly ctx: Context, + protected readonly context: Context, public readonly filter: BindingFilter, ) {} @@ -42,19 +43,18 @@ export class ContextView implements ContextEventListener { * Start listening events from the context */ open() { - debug('Start listening on changes of context %s', this.ctx.name); - return (this._subscription = this.ctx.subscribe(this)); + debug('Start listening on changes of context %s', this.context.name); + return (this._subscription = this.context.subscribe(this)); } /** * Stop listening events from the context */ close() { - debug('Stop listening on changes of context %s', this.ctx.name); - if (this._subscription && !this._subscription.closed) { - this._subscription.unsubscribe(); - this._subscription = undefined; - } + debug('Stop listening on changes of context %s', this.context.name); + if (!this._subscription || this._subscription.closed) return; + this._subscription.unsubscribe(); + this._subscription = undefined; } /** @@ -74,7 +74,7 @@ export class ContextView implements ContextEventListener { */ protected findBindings() { debug('Finding matching bindings'); - this._cachedBindings = this.ctx.find(this.filter); + this._cachedBindings = this.context.find(this.filter); return this._cachedBindings; } @@ -98,11 +98,13 @@ export class ContextView implements ContextEventListener { * Resolve values for the matching bindings * @param session Resolution session */ - resolve(session?: ResolutionSession) { + resolve(session?: ResolutionSession): ValueOrPromise { debug('Resolving values'); - // We don't cache values with this method + // We don't cache values with this method as it returns `ValueOrPromise` + // for inject `resolve` function to allow `getSync` but `this._cachedValues` + // expects `T[]` return resolveList(this.bindings, b => { - return b.getValue(this.ctx, ResolutionSession.fork(session)); + return b.getValue(this.context, ResolutionSession.fork(session)); }); } @@ -113,7 +115,7 @@ export class ContextView implements ContextEventListener { async values(): Promise { debug('Reading values'); // Wait for the next tick so that context event notification can be emitted - await promisify(process.nextTick)(); + await nextTick(); if (this._cachedValues == null) { this._cachedValues = await this.resolve(); } diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index f6cc42ae31b5..78aafd1e4687 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -47,10 +47,18 @@ export class Context { /** * Create a new context. For example, * ```ts - * const ctx1 = new Context(); - * const ctx2 = new Context(ctx1); - * const ctx3 = new Context('ctx3'); - * const ctx4 = new Context(ctx3, 'ctx4'); + * // Create a new root context, let the framework to create a unique name + * const rootCtx = new Context(); + * + * // Create a new child context inheriting bindings from `rootCtx` + * const childCtx = new Context(rootCtx); + * + * // Create another root context called "application" + * const appCtx = new Context('application'); + * + * // Create a new child context called "request" and inheriting bindings + * // from `appCtx` + * const reqCtx = new Context(appCtx, 'request'); * ``` * @param _parent The optional parent context * @param name Name of the context, if not provided, a `uuid` will be @@ -65,13 +73,6 @@ export class Context { this.name = name || uuidv1(); } - /** - * Get the parent context - */ - get parent() { - return this._parent; - } - /** * Create a binding with the given key in the context. If a locked binding * already exists with the same key, an error will be thrown. @@ -129,12 +130,13 @@ export class Context { unbind(key: BindingAddress): boolean { key = BindingKey.validate(key); const binding = this.registry.get(key); + // If not found, return `false` if (binding == null) return false; if (binding && binding.isLocked) throw new Error(`Cannot unbind key "${key}" of a locked binding`); - const found = this.registry.delete(key); + this.registry.delete(key); this.notifyListeners('unbind', binding); - return found; + return true; } /** @@ -570,14 +572,14 @@ export class Context { */ class ContextSubscription implements Subscription { constructor( - protected ctx: Context, + protected context: Context, protected listener: ContextEventListener, ) {} private _closed = false; unsubscribe() { - this.ctx.unsubscribe(this.listener); + this.context.unsubscribe(this.listener); this._closed = true; } diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts index 7f99fde61d31..6af664ae81ed 100644 --- a/packages/context/src/inject.ts +++ b/packages/context/src/inject.ts @@ -111,7 +111,7 @@ export function inject( resolve?: ResolverFunction, ) { if (typeof bindingSelector === 'function' && !resolve) { - resolve = resolveByFilter; + resolve = resolveValuesByFilter; } metadata = Object.assign({decorator: '@inject'}, metadata); return function markParameterOrPropertyAsInjected( @@ -293,7 +293,7 @@ export namespace inject { metadata?: InjectionMetadata, ) { metadata = Object.assign({decorator: '@inject.view'}, metadata); - return inject(bindingFilter, metadata, resolveByFilter); + return inject(bindingFilter, metadata, resolveAsContextView); }; /** @@ -332,7 +332,7 @@ function resolveAsGetter( } const bindingSelector = injection.bindingSelector; if (!isBindingAddress(bindingSelector)) { - return resolveByFilter(ctx, injection, session); + return resolveAsGetterByFilter(ctx, injection, session); } // We need to clone the session for the getter as it will be resolved later session = ResolutionSession.fork(session); @@ -420,46 +420,75 @@ function inspectTargetType(injection: Readonly) { } /** - * Resolve values by a binding filter function + * Resolve an array of bound values matching the filter function for `@inject`. * @param ctx Context object * @param injection Injection information * @param session Resolution session */ -function resolveByFilter( +function resolveValuesByFilter( ctx: Context, injection: Readonly, session?: ResolutionSession, ) { const targetType = inspectTargetType(injection); - const decorator = - (injection.metadata && injection.metadata.decorator) || '@inject'; - const targetName = ResolutionSession.describeInjection(injection)!.targetName; - if (decorator === '@inject.view') { - if (targetType && targetType !== ContextView) { - throw new Error( - `The type of ${targetName} (${targetType.name}) is not ContextView`, - ); - } - } else if (decorator === '@inject') { - if (targetType !== Array) { - throw new Error( - `The type of ${targetName} (${targetType.name}) is not Array`, - ); - } + + if (targetType !== Array) { + const targetName = ResolutionSession.describeInjection(injection)! + .targetName; + throw new Error( + `The type of ${targetName} (${targetType.name}) is not Array`, + ); } + const bindingFilter = injection.bindingSelector as BindingFilter; const view = new ContextView(ctx, bindingFilter); - const autoOpen = injection.metadata!.autoOpen; + return view.resolve(session); +} - if (targetType === Function) { - if (autoOpen !== false) view.open(); - return view.asGetter(); - } else if (targetType === ContextView) { - if (autoOpen !== false) view.open(); - return view; - } else { - return view.resolve(session); +/** + * Resolve to a getter function that returns an array of bound values matching + * the filter function for `@inject.getter`. + * + * @param ctx Context object + * @param injection Injection information + * @param session Resolution session + */ +function resolveAsGetterByFilter( + ctx: Context, + injection: Readonly, + session?: ResolutionSession, +) { + const bindingFilter = injection.bindingSelector as BindingFilter; + const view = new ContextView(ctx, bindingFilter); + view.open(); + return view.asGetter(); +} + +/** + * Resolve to an instance of `ContextView` by the binding filter function + * for `@inject.view` + * @param ctx Context object + * @param injection Injection information + * @param session Resolution session + */ +function resolveAsContextView( + ctx: Context, + injection: Readonly, + session?: ResolutionSession, +) { + const targetType = inspectTargetType(injection); + if (targetType && targetType !== ContextView) { + const targetName = ResolutionSession.describeInjection(injection)! + .targetName; + throw new Error( + `The type of ${targetName} (${targetType.name}) is not ContextView`, + ); } + + const bindingFilter = injection.bindingSelector as BindingFilter; + const view = new ContextView(ctx, bindingFilter); + view.open(); + return view; } /** diff --git a/packages/context/test/acceptance/context-view.acceptance.ts b/packages/context/test/acceptance/context-view.acceptance.ts index 78198311c4ae..aba4a040dcbf 100644 --- a/packages/context/test/acceptance/context-view.acceptance.ts +++ b/packages/context/test/acceptance/context-view.acceptance.ts @@ -10,30 +10,32 @@ import { inject, } from '../..'; -describe('ContextView - watches matching bindings', () => { - let server: Context; - let contextWatcher: ContextView; - beforeEach(givenControllerWatcher); +let app: Context; +let server: Context; + +describe('ContextView', () => { + let contextView: ContextView; + beforeEach(givenViewForControllers); it('watches matching bindings', async () => { - // We have ctx: 1, parent: 2 + // We have server: 1, app: 2 expect(await getControllers()).to.eql(['1', '2']); server.unbind('controllers.1'); - // Now we have parent: 2 + // Now we have app: 2 expect(await getControllers()).to.eql(['2']); - server.parent!.unbind('controllers.2'); + app.unbind('controllers.2'); // All controllers are gone from the context chain expect(await getControllers()).to.eql([]); - // Add a new controller - ctx: 3 + // Add a new controller - server: 3 givenController(server, '3'); expect(await getControllers()).to.eql(['3']); }); - function givenControllerWatcher() { - server = givenServerWithinAnApp(); - contextWatcher = server.createView(filterByTag('controller')); + function givenViewForControllers() { + givenServerWithinAnApp(); + contextView = server.createView(filterByTag('controller')); givenController(server, '1'); - givenController(server.parent!, '2'); + givenController(app, '2'); } function givenController(_ctx: Context, _name: string) { @@ -48,16 +50,15 @@ describe('ContextView - watches matching bindings', () => { async function getControllers() { // tslint:disable-next-line:no-any - return (await contextWatcher.values()).map((v: any) => v.name); + return (await contextView.values()).map((v: any) => v.name); } }); describe('@inject.* - injects a live collection of matching bindings', async () => { - let ctx: Context; beforeEach(givenPrimeNumbers); class MyControllerWithGetter { - @inject.getter(filterByTag('prime'), {autoOpen: true}) + @inject.getter(filterByTag('prime')) getter: Getter; } @@ -74,65 +75,64 @@ describe('@inject.* - injects a live collection of matching bindings', async () } it('injects as getter', async () => { - ctx.bind('my-controller').toClass(MyControllerWithGetter); - const inst = await ctx.get('my-controller'); + server.bind('my-controller').toClass(MyControllerWithGetter); + const inst = await server.get('my-controller'); const getter = inst.getter; expect(await getter()).to.eql([3, 5]); // Add a new binding that matches the filter - givenPrime(ctx, 7); + givenPrime(server, 7); // The getter picks up the new binding expect(await getter()).to.eql([3, 7, 5]); }); it('injects as values', async () => { - ctx.bind('my-controller').toClass(MyControllerWithValues); - const inst = await ctx.get('my-controller'); + server.bind('my-controller').toClass(MyControllerWithValues); + const inst = await server.get('my-controller'); expect(inst.values).to.eql([3, 5]); }); it('injects as a view', async () => { - ctx.bind('my-controller').toClass(MyControllerWithView); - const inst = await ctx.get('my-controller'); + server.bind('my-controller').toClass(MyControllerWithView); + const inst = await server.get('my-controller'); const view = inst.view; expect(await view.values()).to.eql([3, 5]); // Add a new binding that matches the filter // Add a new binding that matches the filter - givenPrime(ctx, 7); + givenPrime(server, 7); // The view picks up the new binding expect(await view.values()).to.eql([3, 7, 5]); - ctx.unbind('prime.7'); + server.unbind('prime.7'); expect(await view.values()).to.eql([3, 5]); }); function givenPrimeNumbers() { - ctx = givenServerWithinAnApp(); - givenPrime(ctx, 3); - givenPrime(ctx.parent!, 5); + givenServerWithinAnApp(); + givenPrime(server, 3); + givenPrime(app, 5); } - function givenPrime(_ctx: Context, p: number) { - _ctx + function givenPrime(ctx: Context, p: number) { + ctx .bind(`prime.${p}`) .to(p) .tag('prime'); } }); -describe('ContextEventListener - listens on matching bindings', () => { - let server: Context; +describe('ContextEventListener', () => { let contextListener: MyListenerForControllers; beforeEach(givenControllerListener); it('receives notifications of matching binding events', async () => { - // We have ctx: 1, parent: 2 + // We have server: 1, app: 2 expect(await getControllers()).to.eql(['1', '2']); server.unbind('controllers.1'); - // Now we have parent: 2 + // Now we have app: 2 expect(await getControllers()).to.eql(['2']); - server.parent!.unbind('controllers.2'); + app.unbind('controllers.2'); // All controllers are gone from the context chain expect(await getControllers()).to.eql([]); - // Add a new controller - ctx: 3 + // Add a new controller - server: 3 givenController(server, '3'); expect(await getControllers()).to.eql(['3']); }); @@ -150,21 +150,21 @@ describe('ContextEventListener - listens on matching bindings', () => { } function givenControllerListener() { - server = givenServerWithinAnApp(); + givenServerWithinAnApp(); contextListener = new MyListenerForControllers(); server.subscribe(contextListener); givenController(server, '1'); - givenController(server.parent!, '2'); + givenController(app, '2'); } - function givenController(_ctx: Context, _name: string) { + function givenController(ctx: Context, controllerName: string) { class MyController { - name = _name; + name = controllerName; } - _ctx - .bind(`controllers.${_name}`) + ctx + .bind(`controllers.${controllerName}`) .toClass(MyController) - .tag('controller', {name: _name}); + .tag('controller', {name: controllerName}); } async function getControllers() { @@ -176,7 +176,6 @@ describe('ContextEventListener - listens on matching bindings', () => { }); function givenServerWithinAnApp() { - const parent = new Context('app'); - const ctx = new Context(parent, 'server'); - return ctx; + app = new Context('app'); + server = new Context(app, 'server'); } diff --git a/packages/context/test/unit/context-view.unit.ts b/packages/context/test/unit/context-view.unit.ts index 8b2922886c18..801feac0aa87 100644 --- a/packages/context/test/unit/context-view.unit.ts +++ b/packages/context/test/unit/context-view.unit.ts @@ -7,7 +7,9 @@ import {expect} from '@loopback/testlab'; import {Binding, BindingScope, filterByTag, Context, ContextView} from '../..'; describe('ContextView', () => { - let ctx: Context; + let app: Context; + let server: Context; + let bindings: Binding[]; let contextView: ContextView; @@ -28,11 +30,11 @@ describe('ContextView', () => { it('reloads bindings after reset', async () => { contextView.reset(); - const abcBinding = ctx + const abcBinding = server .bind('abc') .to('ABC') .tag('abc'); - const xyzBinding = ctx + const xyzBinding = server .bind('xyz') .to('XYZ') .tag('foo'); @@ -43,11 +45,11 @@ describe('ContextView', () => { }); it('reloads bindings if context bindings are added', async () => { - const abcBinding = ctx + const abcBinding = server .bind('abc') .to('ABC') .tag('abc'); - const xyzBinding = ctx + const xyzBinding = server .bind('xyz') .to('XYZ') .tag('foo'); @@ -58,18 +60,18 @@ describe('ContextView', () => { }); it('reloads bindings if context bindings are removed', async () => { - ctx.unbind('bar'); + server.unbind('bar'); expect(await contextView.values()).to.eql(['FOO']); }); it('reloads bindings if context bindings are rebound', async () => { - ctx.bind('bar').to('BAR'); // No more tagged with `foo` + server.bind('bar').to('BAR'); // No more tagged with `foo` expect(await contextView.values()).to.eql(['FOO']); }); it('reloads bindings if parent context bindings are added', async () => { - const xyzBinding = ctx - .parent!.bind('xyz') + const xyzBinding = app + .bind('xyz') .to('XYZ') .tag('foo'); expect(contextView.bindings).to.containEql(xyzBinding); @@ -77,39 +79,38 @@ describe('ContextView', () => { }); it('reloads bindings if parent context bindings are removed', async () => { - ctx.parent!.unbind('foo'); + app.unbind('foo'); expect(await contextView.values()).to.eql(['BAR']); }); it('stops watching', async () => { expect(await contextView.values()).to.eql(['BAR', 'FOO']); contextView.close(); - ctx.parent!.unbind('foo'); + app.unbind('foo'); expect(await contextView.values()).to.eql(['BAR', 'FOO']); }); function givenContextView() { bindings = []; - ctx = givenContext(bindings); - contextView = ctx.createView(filterByTag('foo')); + givenContext(); + contextView = server.createView(filterByTag('foo')); } -}); -function givenContext(bindings: Binding[] = []) { - const parent = new Context('app'); - const ctx = new Context(parent, 'server'); - bindings.push( - ctx - .bind('bar') - .toDynamicValue(() => Promise.resolve('BAR')) - .tag('foo', 'bar') - .inScope(BindingScope.SINGLETON), - ); - bindings.push( - parent - .bind('foo') - .to('FOO') - .tag('foo', 'bar'), - ); - return ctx; -} + function givenContext() { + app = new Context('app'); + server = new Context(app, 'server'); + bindings.push( + server + .bind('bar') + .toDynamicValue(() => Promise.resolve('BAR')) + .tag('foo', 'bar') + .inScope(BindingScope.SINGLETON), + ); + bindings.push( + app + .bind('foo') + .to('FOO') + .tag('foo', 'bar'), + ); + } +}); diff --git a/packages/context/test/unit/inject-view.unit.ts b/packages/context/test/unit/inject-view.unit.ts index 94b0b53a4333..eaee9e89400c 100644 --- a/packages/context/test/unit/inject-view.unit.ts +++ b/packages/context/test/unit/inject-view.unit.ts @@ -21,7 +21,7 @@ describe('@inject.view', async () => { }); class MyControllerWithGetter { - @inject.view(filterByTag('foo'), {autoOpen: true}) + @inject.view(filterByTag('foo')) getter: Getter; } @@ -77,7 +77,7 @@ describe('@inject with filter function', async () => { }); class MyControllerWithGetter { - @inject.getter(filterByTag('foo'), {autoOpen: true}) + @inject.getter(filterByTag('foo')) getter: Getter; } @@ -94,7 +94,7 @@ describe('@inject with filter function', async () => { } class MyControllerWithGetter2 { - @inject(filterByTag('foo'), {autoOpen: true}) + @inject(filterByTag('foo')) getter: Getter; }