From 14b48ec59242532d031d1c10e2e30a9bec013fad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 21 Apr 2017 15:33:12 +0200 Subject: [PATCH] Add per-request child context --- packages/context/src/binding.ts | 17 +++-- packages/context/src/context.ts | 36 ++++++--- packages/context/src/resolver.ts | 2 +- .../context/test/acceptance/child-context.ts | 48 ++++++++++++ .../test/acceptance/class-level-bindings.ts | 2 +- packages/context/test/unit/binding.ts | 7 +- packages/loopback/index.ts | 5 ++ packages/loopback/lib/application.ts | 5 +- packages/loopback/lib/server.ts | 2 +- .../acceptance/routing/routing.acceptance.ts | 75 ++++++++++++++++++- packages/loopback/test/support/client.ts | 9 +++ 11 files changed, 182 insertions(+), 26 deletions(-) create mode 100644 packages/context/test/acceptance/child-context.ts diff --git a/packages/context/src/binding.ts b/packages/context/src/binding.ts index 4235eafd5b8d..9bd030905e59 100644 --- a/packages/context/src/binding.ts +++ b/packages/context/src/binding.ts @@ -16,7 +16,7 @@ export class Binding { // For bindings bound via toClass, this property contains the constructor function public valueConstructor: Constructor; - constructor(private readonly _context: Context, private readonly _key: string, public isLocked: boolean = false) {} + constructor(private readonly _key: string, public isLocked: boolean = false) {} get key() { return this._key; } get tagName() { return this._tagName; } @@ -32,7 +32,7 @@ export class Binding { * to check the type of the returned value to decide how to handle it. * * ``` - * const result = binding.getValue(); + * const result = binding.getValue(ctx); * if (isPromise(result)) { * result.then(doSomething) * } else { @@ -40,12 +40,13 @@ export class Binding { * } * ``` */ - getValue(): BoundValue | Promise { + getValue(ctx: Context): BoundValue | Promise { return Promise.reject(new Error(`No value was configured for binding ${this._key}.`)); } - lock() { + lock(): this { this.isLocked = true; + return this; } tag(tagName: string): this { @@ -88,7 +89,8 @@ export class Binding { * ``` */ toDynamicValue(factoryFn: () => BoundValue | Promise): this { - this.getValue = factoryFn; + // TODO(bajtos) allow factoryFn with @inject arguments + this.getValue = (ctx) => factoryFn(); return this; } @@ -100,12 +102,13 @@ export class Binding { * we can resolve them from the context. */ toClass(ctor: Constructor): this { - this.getValue = () => instantiateClass(ctor, this._context); + this.getValue = context => instantiateClass(ctor, context); this.valueConstructor = ctor; return this; } - unlock() { + unlock(): this { this.isLocked = false; + return this; } } diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index f1e8cdc98c70..ea6255755d17 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -10,7 +10,7 @@ import {isPromise} from './isPromise'; export class Context { private registry: Map; - constructor() { + constructor(private _parent?: Context) { this.registry = new Map(); } @@ -23,7 +23,7 @@ export class Context { throw new Error(`Cannot rebind key "${key}", associated binding is locked`); } - const binding = new Binding(this, key); + const binding = new Binding(key); this.registry.set(key, binding); return binding; } @@ -46,7 +46,8 @@ export class Context { bindings = Array.from(this.registry.values()); } - return bindings; + const parentBindings = this._parent && this._parent.find(pattern); + return this._mergeWithParent(bindings, parentBindings); } findByTag(pattern: string): Binding[] { @@ -58,13 +59,24 @@ export class Context { if (isMatch) bindings.push(binding); }); - return bindings; + + const parentBindings = this._parent && this._parent.findByTag(pattern); + return this._mergeWithParent(bindings, parentBindings); + } + + protected _mergeWithParent(childList: Binding[], parentList?: Binding[]) { + if (!parentList) return childList; + const additions = parentList.filter(parentBinding => { + // children bindings take precedence + return !childList.some(childBinding => childBinding.key === parentBinding.key); + }); + return childList.concat(additions); } get(key: string): Promise { try { const binding = this.getBinding(key); - return Promise.resolve(binding.getValue()); + return Promise.resolve(binding.getValue(this)); } catch (err) { return Promise.reject(err); } @@ -72,7 +84,7 @@ export class Context { getSync(key: string): BoundValue { const binding = this.getBinding(key); - const valueOrPromise = binding.getValue(); + const valueOrPromise = binding.getValue(this); if (isPromise(valueOrPromise)) { throw new Error( @@ -85,8 +97,14 @@ export class Context { getBinding(key: string): Binding { const binding = this.registry.get(key); - if (!binding) - throw new Error(`The key ${key} was not bound to any value.`); - return binding; + if (binding) { + return binding; + } + + if (this._parent) { + return this._parent.getBinding(key); + } + + throw new Error(`The key ${key} was not bound to any value.`); } } diff --git a/packages/context/src/resolver.ts b/packages/context/src/resolver.ts index e388b7549cbf..57365798acc9 100644 --- a/packages/context/src/resolver.ts +++ b/packages/context/src/resolver.ts @@ -60,7 +60,7 @@ export function resolveInjectedArguments(fn: Function, ctx: Context): BoundValue } const binding = ctx.getBinding(bindingKey); - const valueOrPromise = binding.getValue(); + const valueOrPromise = binding.getValue(ctx); if (isPromise(valueOrPromise)) { if (!asyncResolvers) asyncResolvers = []; asyncResolvers.push(valueOrPromise.then((v: BoundValue) => args[ix] = v)); diff --git a/packages/context/test/acceptance/child-context.ts b/packages/context/test/acceptance/child-context.ts new file mode 100644 index 000000000000..d28a968ba762 --- /dev/null +++ b/packages/context/test/acceptance/child-context.ts @@ -0,0 +1,48 @@ + +// Copyright IBM Corp. 2013,2017. All Rights Reserved. +// Node module: loopback +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {expect} from '@loopback/testlab'; +import {Context} from '../..'; + +describe('Context bindings - contexts inheritance', () => { + let parentCtx: Context; + let childCtx: Context; + + beforeEach('given a parent and a child context', createParentAndChildContext); + + it('child inherits values bound in parent', () => { + parentCtx.bind('foo').to('bar'); + expect(childCtx.getSync('foo')).to.equal('bar'); + }); + + it('child changes are not propagated to parent', () => { + childCtx.bind('foo').to('bar'); + expect(() => parentCtx.getSync('foo')).to.throw(/not bound/); + }); + + it('includes parent bindings when searching via find()', () => { + parentCtx.bind('foo').to('parent:foo'); + parentCtx.bind('bar').to('parent:bar'); + childCtx.bind('foo').to('child:foo'); + + const found = childCtx.find().map(b => b.getValue(childCtx)); + expect(found).to.deepEqual(['child:foo', 'parent:bar']); + }); + + it('includes parent bindings when searching via findByTag()', () => { + parentCtx.bind('foo').to('parent:foo').tag('a-tag'); + parentCtx.bind('bar').to('parent:bar').tag('a-tag'); + childCtx.bind('foo').to('child:foo').tag('a-tag'); + + const found = childCtx.findByTag('a-tag').map(b => b.getValue(childCtx)); + expect(found).to.deepEqual(['child:foo', 'parent:bar']); + }); + + function createParentAndChildContext() { + parentCtx = new Context(); + childCtx = new Context(parentCtx); + } +}); diff --git a/packages/context/test/acceptance/class-level-bindings.ts b/packages/context/test/acceptance/class-level-bindings.ts index 9932377adf6d..dc6dc4601916 100644 --- a/packages/context/test/acceptance/class-level-bindings.ts +++ b/packages/context/test/acceptance/class-level-bindings.ts @@ -77,7 +77,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { } const b = ctx.bind(INFO_CONTROLLER).toClass(InfoController); - const valueOrPromise = b.getValue(); + const valueOrPromise = b.getValue(ctx); expect(valueOrPromise).to.not.be.Promise(); expect(valueOrPromise as InfoController).to.have.property('appName', 'CodeHub'); }); diff --git a/packages/context/test/unit/binding.ts b/packages/context/test/unit/binding.ts index 22a616b1d931..789da618e6bf 100644 --- a/packages/context/test/unit/binding.ts +++ b/packages/context/test/unit/binding.ts @@ -9,6 +9,7 @@ import {Binding, Context} from '../..'; const key = 'foo'; describe('Binding', () => { + let ctx: Context; let binding: Binding; beforeEach(givenBinding); @@ -33,12 +34,12 @@ describe('Binding', () => { describe('to(value)', () => { it('returns the value synchronously', () => { binding.to('value'); - expect(binding.getValue()).to.equal('value'); + expect(binding.getValue(ctx)).to.equal('value'); }); }); function givenBinding() { - const ctx = new Context(); - binding = new Binding(ctx, key); + ctx = new Context(); + binding = new Binding(key); } }); diff --git a/packages/loopback/index.ts b/packages/loopback/index.ts index 042c1fc610be..390cce313a72 100644 --- a/packages/loopback/index.ts +++ b/packages/loopback/index.ts @@ -22,3 +22,8 @@ export * from '@loopback/openapi-spec'; export { inject, } from '@loopback/context'; + +export { + ServerRequest, + ServerResponse, +} from 'http'; diff --git a/packages/loopback/lib/application.ts b/packages/loopback/lib/application.ts index 17023ca2c6dd..05aa32500add 100644 --- a/packages/loopback/lib/application.ts +++ b/packages/loopback/lib/application.ts @@ -20,8 +20,9 @@ export class Application extends Context { } const ctorFactory = (req: http.ServerRequest, res: http.ServerResponse) => { - // TODO(bajtos) Create a new nested/child per-request Context - const requestContext = this; + const requestContext = new Context(this); + requestContext.bind('http.request').to(req); + requestContext.bind('http.response').to(res); return requestContext.get(b.key); }; const apiSpec = getApiSpec(ctor); diff --git a/packages/loopback/lib/server.ts b/packages/loopback/lib/server.ts index f259d947b30d..0f62b1c45d8c 100644 --- a/packages/loopback/lib/server.ts +++ b/packages/loopback/lib/server.ts @@ -44,7 +44,7 @@ export class Server extends Context { const apps = this.find('applications.*'); for (const appBinding of apps) { debug('Registering app controllers for %j', appBinding.key); - const app = await appBinding.getValue() as Application; + const app = await Promise.resolve(appBinding.getValue(this)) as Application; app.mountControllers(router); } diff --git a/packages/loopback/test/acceptance/routing/routing.acceptance.ts b/packages/loopback/test/acceptance/routing/routing.acceptance.ts index f7a2bae5f02c..0e4853e9ed80 100644 --- a/packages/loopback/test/acceptance/routing/routing.acceptance.ts +++ b/packages/loopback/test/acceptance/routing/routing.acceptance.ts @@ -3,11 +3,15 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Application, Server, api, OpenApiSpec, ParameterObject, OperationObject} from '../../..'; +import { + Application, Server, api, + OpenApiSpec, ParameterObject, OperationObject, + ServerRequest, ServerResponse, +} from '../../..'; import {Client} from './../../support/client'; import {expect} from 'testlab'; import {givenOpenApiSpec} from '@loopback/openapi-spec-builder'; -import {inject, Constructor} from '@loopback/context'; +import {inject, Constructor, Context} from '@loopback/context'; /* # Feature: Routing * - In order to build REST APIs @@ -90,6 +94,73 @@ describe('Routing', () => { expect(result).to.have.property('body', 'TestApp'); }); + it('creates a new child context for each request', async () => { + const app = givenAnApplication(); + app.bind('flag').to('original'); + + // create a special binding returning the current context instance + app.bind('context').getValue = ctx => ctx; + + const spec = givenOpenApiSpec() + .withOperationReturningString('put', '/flag', 'setFlag') + .withOperationReturningString('get', '/flag', 'getFlag') + .build(); + + @api(spec) + class FlagController { + constructor(@inject('context') private ctx: Context) { + } + + async setFlag(): Promise { + this.ctx.bind('flag').to('modified'); + return 'modified'; + } + + async getFlag(): Promise { + return this.ctx.get('flag'); + } + } + givenControllerInApp(app, FlagController); + + // Rebind "flag" to "modified". Since we are modifying + // the per-request child context, the change should + // be discarded after the request is done. + await whenIMakeRequestTo(app).put('/flag'); + // Get the value "flag" is bound to. + // This should return the original value. + const result = await whenIMakeRequestTo(app).get('/flag'); + expect(result).to.have.property('body', 'original'); + }); + + it('binds request and response objects', async () => { + const app = givenAnApplication(); + + const spec = givenOpenApiSpec() + .withOperationReturningString('get', '/status', 'getStatus') + .build(); + + @api(spec) + class StatusController { + constructor( + @inject('http.request') private request: ServerRequest, + @inject('http.response') private response: ServerResponse, + ) { + } + + async getStatus(): Promise { + this.response.statusCode = 202; // 202 Accepted + return this.request.method as string; + } + } + givenControllerInApp(app, StatusController); + + const result = await whenIMakeRequestTo(app).get('/status'); + expect(result).to.containDeep({ + body: 'GET', + status: 202, + }); + }); + /* ===== HELPERS ===== */ function givenAnApplication() { diff --git a/packages/loopback/test/support/client.ts b/packages/loopback/test/support/client.ts index 1de4c7c615d3..72b1ea9d7566 100644 --- a/packages/loopback/test/support/client.ts +++ b/packages/loopback/test/support/client.ts @@ -15,11 +15,20 @@ export class Client { } public async get(path : string) : Promise { + return this.request('get', path); + } + + public async put(path: string): Promise { + return this.request('put', path); + } + + public async request(verb: string, path: string): Promise { await this._ensureAppIsListening(); const url = 'http://localhost:' + this.app.config.port + path; const options = { uri: url, + method: verb, resolveWithFullResponse: true, };