From fb1dfaec0352c39bd30084ccbd5e59b52cc0d51b Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 14 Jan 2020 13:32:36 -0800 Subject: [PATCH] feat(context): index bindings by tag to speed up matching by tag Fixes https://github.com/strongloop/loopback-next/issues/4356 - create index for bindings by tag - optimize find bindings by tag - leverage findByTag for filterByTag to find matching bindings --- .../acceptance/interceptor.acceptance.ts | 8 +- .../src/__tests__/unit/context-view.unit.ts | 22 ++- .../src/__tests__/unit/context.unit.ts | 130 +++++++++++++++++ packages/context/src/context.ts | 137 +++++++++++++++++- packages/context/src/interceptor.ts | 13 +- .../request-context.integration.ts | 2 +- 6 files changed, 298 insertions(+), 14 deletions(-) diff --git a/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts b/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts index d39b8b56b018..7c328643c5b8 100644 --- a/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts @@ -186,8 +186,8 @@ describe('Interceptor', () => { } } - // No listeners yet - expect(ctx.listenerCount('bind')).to.eql(0); + // No invocation context related listeners yet + const listenerCount = ctx.listenerCount('bind'); const controller = new MyController(); // Run the invocation 5 times @@ -199,7 +199,7 @@ describe('Interceptor', () => { 'greet', ['John'], ); - // New listeners are added to `ctx` + // New listeners are added to `ctx` by the invocation context expect(ctx.listenerCount('bind')).to.be.greaterThan(count); // Wait until the invocation finishes @@ -208,7 +208,7 @@ describe('Interceptor', () => { // Listeners added by invocation context are gone now // There is one left for ctx.observers - expect(ctx.listenerCount('bind')).to.eql(1); + expect(ctx.listenerCount('bind')).to.eql(listenerCount + 1); }); it('invokes static interceptors', async () => { diff --git a/packages/context/src/__tests__/unit/context-view.unit.ts b/packages/context/src/__tests__/unit/context-view.unit.ts index ac52868a0566..20ce066e9e3a 100644 --- a/packages/context/src/__tests__/unit/context-view.unit.ts +++ b/packages/context/src/__tests__/unit/context-view.unit.ts @@ -7,6 +7,7 @@ import {expect} from '@loopback/testlab'; import { Binding, BindingScope, + BindingTag, compareBindingsByTag, Context, ContextView, @@ -16,7 +17,7 @@ import { describe('ContextView', () => { let app: Context; - let server: Context; + let server: ServerContext; let bindings: Binding[]; let taggedAsFoo: ContextView; @@ -27,6 +28,11 @@ describe('ContextView', () => { expect(taggedAsFoo.bindings).to.eql(bindings); }); + it('leverages findByTag for binding tag filter', () => { + expect(taggedAsFoo.bindings).to.eql(bindings); + expect(server.findByTagInvoked).to.be.true(); + }); + it('sorts matched bindings', () => { const view = new ContextView( server, @@ -199,9 +205,21 @@ describe('ContextView', () => { taggedAsFoo = server.createView(filterByTag('foo')); } + class ServerContext extends Context { + findByTagInvoked = false; + constructor(parent: Context, name: string) { + super(parent, name); + } + + _findByTagIndex(tag: BindingTag) { + this.findByTagInvoked = true; + return super._findByTagIndex(tag); + } + } + function givenContext() { app = new Context('app'); - server = new Context(app, 'server'); + server = new ServerContext(app, 'server'); bindings.push( server .bind('bar') diff --git a/packages/context/src/__tests__/unit/context.unit.ts b/packages/context/src/__tests__/unit/context.unit.ts index 338f7fdd0bbc..511158f8154b 100644 --- a/packages/context/src/__tests__/unit/context.unit.ts +++ b/packages/context/src/__tests__/unit/context.unit.ts @@ -9,9 +9,11 @@ import { BindingCreationPolicy, BindingKey, BindingScope, + BindingTag, BindingType, Context, ContextEventObserver, + filterByTag, isPromiseLike, Provider, } from '../..'; @@ -32,6 +34,16 @@ class TestContext extends Context { get eventListener() { return this.parentEventListener; } + get tagIndex() { + return this.bindingsIndexedByTag; + } + + findByTagInvoked = false; + + _findByTagIndex(tag: BindingTag | RegExp) { + this.findByTagInvoked = true; + return super._findByTagIndex(tag); + } } describe('Context constructor', () => { @@ -94,6 +106,18 @@ describe('Context', () => { 'Cannot rebind key "foo" to a locked binding', ); }); + + it('indexes a binding by tag', () => { + const binding = ctx.bind('foo').tag('a', {b: 1}); + assertBindingIndexedByTag(binding, 'a', 'b'); + }); + + it('indexes a binding by tag after being bound', () => { + const binding = ctx.bind('foo'); + assertBindingNotIndexedByTag(binding, 'a', 'b'); + binding.tag('a', {b: 1}); + assertBindingIndexedByTag(binding, 'a', 'b'); + }); }); describe('add', () => { @@ -111,6 +135,20 @@ describe('Context', () => { 'Cannot rebind key "foo" to a locked binding', ); }); + + it('indexes a binding by tag', () => { + const binding = new Binding('foo').to('bar').tag('a', {b: 1}); + ctx.add(binding); + assertBindingIndexedByTag(binding, 'a', 'b'); + }); + + it('indexes a binding by tag after being bound', () => { + const binding = new Binding('foo').to('bar'); + ctx.add(binding); + assertBindingNotIndexedByTag(binding, 'a', 'b'); + binding.tag('a', {b: 1}); + assertBindingIndexedByTag(binding, 'a', 'b'); + }); }); describe('contains', () => { @@ -191,6 +229,16 @@ describe('Context', () => { expect(result).to.be.false(); expect(ctx.contains('foo')).to.be.true(); }); + + it('removes indexes for a binding by tag', () => { + const binding = ctx + .bind('foo') + .to('bar') + .tag('a', {b: 1}); + assertBindingIndexedByTag(binding, 'a', 'b'); + ctx.unbind(binding.key); + assertBindingNotIndexedByTag(binding, 'a', 'b'); + }); }); describe('find', () => { @@ -276,6 +324,42 @@ describe('Context', () => { result = ctx.find(binding => binding.tagNames.includes('b')); expect(result).to.be.eql([b2, b3]); }); + + it('leverages binding index by tag', () => { + ctx.bind('foo'); + const b2 = ctx.bind('bar').tag('b'); + const b3 = ctx.bind('baz').tag('b'); + const result = ctx.find(filterByTag('b')); + expect(result).to.eql([b2, b3]); + expect(ctx.findByTagInvoked).to.be.true(); + }); + + it('leverages binding index by tag wildcard', () => { + ctx.bind('foo'); + const b2 = ctx.bind('bar').tag('b2'); + const b3 = ctx.bind('baz').tag('b3'); + const result = ctx.find(filterByTag('b?')); + expect(result).to.eql([b2, b3]); + expect(ctx.findByTagInvoked).to.be.true(); + }); + + it('leverages binding index by tag regexp', () => { + ctx.bind('foo'); + const b2 = ctx.bind('bar').tag('b2'); + const b3 = ctx.bind('baz').tag('b3'); + const result = ctx.find(filterByTag(/b\d/)); + expect(result).to.eql([b2, b3]); + expect(ctx.findByTagInvoked).to.be.true(); + }); + + it('leverages binding index by tag name/value pairs', () => { + ctx.bind('foo'); + const b2 = ctx.bind('bar').tag({a: 1}); + ctx.bind('baz').tag({a: 2, b: 1}); + const result = ctx.find(filterByTag({a: 1})); + expect(result).to.eql([b2]); + expect(ctx.findByTagInvoked).to.be.true(); + }); }); describe('findByTag with name pattern', () => { @@ -316,6 +400,34 @@ describe('Context', () => { expect(result).to.be.eql([b1]); }); + it('returns matching binding for multiple tags', () => { + const b1 = ctx + .bind('controllers.ProductController') + .tag({name: 'my-controller'}) + .tag('controller'); + ctx.bind('controllers.OrderController').tag('controller'); + ctx.bind('dataSources.mysql').tag({dbType: 'mysql'}); + const result = ctx.findByTag({ + name: 'my-controller', + controller: 'controller', + }); + expect(result).to.be.eql([b1]); + }); + + it('returns empty array if one of the tags does not match', () => { + ctx + .bind('controllers.ProductController') + .tag({name: 'my-controller'}) + .tag('controller'); + ctx.bind('controllers.OrderController').tag('controller'); + ctx.bind('dataSources.mysql').tag({dbType: 'mysql'}); + const result = ctx.findByTag({ + controller: 'controller', + name: 'your-controller', + }); + expect(result).to.be.eql([]); + }); + it('returns empty array if no matching tag value is found', () => { ctx.bind('controllers.ProductController').tag({name: 'my-controller'}); ctx.bind('controllers.OrderController').tag('controller'); @@ -888,4 +1000,22 @@ describe('Context', () => { function createContext() { ctx = new TestContext('app'); } + + function assertBindingIndexedByTag( + binding: Binding, + ...tags: string[] + ) { + for (const t of tags) { + expect(ctx.tagIndex.get(t)?.has(binding)).to.be.true(); + } + } + + function assertBindingNotIndexedByTag( + binding: Binding, + ...tags: string[] + ) { + for (const t of tags) { + expect(!!ctx.tagIndex.get(t)?.has(binding)).to.be.false(); + } + } }); diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index 49b471518fd9..dddfb27f23fd 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -6,7 +6,7 @@ import debugFactory from 'debug'; import {EventEmitter} from 'events'; import {v1 as uuidv1} from 'uuid'; -import {Binding, BindingTag} from './binding'; +import {Binding, BindingEventListener, BindingTag} from './binding'; import { ConfigurationResolver, DefaultConfigurationResolver, @@ -15,6 +15,7 @@ import { BindingFilter, filterByKey, filterByTag, + isBindingTagFilter, } from './binding-filter'; import {BindingAddress, BindingKey} from './binding-key'; import {BindingComparator} from './binding-sorter'; @@ -80,6 +81,24 @@ export class Context extends EventEmitter { */ protected readonly registry: Map = new Map(); + /** + * Index for bindings by tag names + */ + protected readonly bindingsIndexedByTag: Map< + string, + Set>> + > = new Map(); + + /** + * A listener for binding events + */ + private bindingEventListener: BindingEventListener; + + /** + * A listener to maintain tag index for bindings + */ + private tagIndexListener: ContextEventListener; + /** * Parent context */ @@ -144,6 +163,30 @@ export class Context extends EventEmitter { } this._parent = _parent; this.name = name ?? uuidv1(); + this.setupTagIndexForBindings(); + } + + /** + * Set up context/binding listeners and refresh index for bindings by tag + */ + private setupTagIndexForBindings() { + this.bindingEventListener = (binding, event) => { + if (event === 'tag') { + this.updateTagIndexForBinding(binding); + } + }; + this.tagIndexListener = (binding, context, event) => { + if (context !== this) return; + if (event === 'bind') { + this.updateTagIndexForBinding(binding); + binding.on('changed', this.bindingEventListener); + } else if (event === 'unbind') { + this.removeTagIndexForBinding(binding); + binding.removeListener('changed', this.bindingEventListener); + } + }; + this.on('bind', this.tagIndexListener); + this.on('unbind', this.tagIndexListener); } /** @@ -563,6 +606,8 @@ export class Context extends EventEmitter { this._parent.removeListener('unbind', this.parentEventListener); this.parentEventListener = undefined; } + this.removeListener('bind', this.tagIndexListener); + this.removeListener('unbind', this.tagIndexListener); } /** @@ -616,6 +661,32 @@ export class Context extends EventEmitter { } } + /** + * Remove tag index for the given binding + * @param binding - Binding object + */ + private removeTagIndexForBinding(binding: Readonly>) { + for (const [, bindings] of this.bindingsIndexedByTag) { + bindings.delete(binding); + } + } + + /** + * Update tag index for the given binding + * @param binding - Binding object + */ + private updateTagIndexForBinding(binding: Readonly>) { + this.removeTagIndexForBinding(binding); + for (const tag of binding.tagNames) { + let bindings = this.bindingsIndexedByTag.get(tag); + if (bindings == null) { + bindings = new Set(); + this.bindingsIndexedByTag.set(tag, bindings); + } + bindings.add(binding); + } + } + /** * Check if a binding exists with the given key in the local context without * delegating to the parent context @@ -667,6 +738,11 @@ export class Context extends EventEmitter { find( pattern?: string | RegExp | BindingFilter, ): Readonly>[] { + // Optimize if the binding filter is for tags + if (typeof pattern === 'function' && isBindingTagFilter(pattern)) { + return this._findByTagIndex(pattern.bindingTagPattern); + } + const bindings: Readonly>[] = []; const filter = filterByKey(pattern); @@ -698,6 +774,65 @@ export class Context extends EventEmitter { return this.find(filterByTag(tagFilter)); } + /** + * Find bindings by tag leveraging indexes + * @param tag - Tag name pattern or name/value pairs + */ + protected _findByTagIndex( + tag: BindingTag | RegExp, + ): Readonly>[] { + let tagNames: string[]; + // A flag to control if a union of matched bindings should be created + let union = false; + if (tag instanceof RegExp) { + // For wildcard/regexp, a union of matched bindings is desired + union = true; + // Find all matching tag names + tagNames = []; + for (const t of this.bindingsIndexedByTag.keys()) { + if (tag.test(t)) { + tagNames.push(t); + } + } + } else if (typeof tag === 'string') { + tagNames = [tag]; + } else { + tagNames = Object.keys(tag); + } + let filter: BindingFilter | undefined; + let bindings: Set>> | undefined; + for (const t of tagNames) { + const bindingsByTag = this.bindingsIndexedByTag.get(t); + if (bindingsByTag == null) break; // One of the tags is not found + filter = filter ?? filterByTag(tag); + const matched = new Set(Array.from(bindingsByTag).filter(filter)) as Set< + Readonly> + >; + if (!union && matched.size === 0) break; // One of the tag name/value is not found + if (bindings == null) { + // First set of bindings matching the tag + bindings = matched; + } else { + if (union) { + matched.forEach(b => bindings?.add(b)); + } else { + // Now need to find intersected bindings against visited tags + const intersection = new Set>>(); + bindings.forEach(b => { + if (matched.has(b)) { + intersection.add(b); + } + }); + bindings = intersection; + } + if (!union && bindings.size === 0) break; + } + } + const currentBindings = bindings == null ? [] : Array.from(bindings); + const parentBindings = this._parent && this._parent?._findByTagIndex(tag); + return this._mergeWithParent(currentBindings, parentBindings); + } + protected _mergeWithParent( childList: Readonly>[], parentList?: Readonly>[], diff --git a/packages/context/src/interceptor.ts b/packages/context/src/interceptor.ts index be0ef45b4210..8ea227367750 100644 --- a/packages/context/src/interceptor.ts +++ b/packages/context/src/interceptor.ts @@ -15,7 +15,6 @@ import assert from 'assert'; import debugFactory from 'debug'; import {Binding, BindingTemplate} from './binding'; import {bind} from './binding-decorator'; -import {filterByTag} from './binding-filter'; import {BindingSpec} from './binding-inspector'; import {sortBindingsByPhase} from './binding-sorter'; import {Context} from './context'; @@ -47,12 +46,14 @@ export class InterceptedInvocationContext extends InvocationContext { * ContextTags.GLOBAL_INTERCEPTOR) */ getGlobalInterceptorBindingKeys(): string[] { - const bindings: Readonly>[] = this.find( - binding => - filterByTag(ContextTags.GLOBAL_INTERCEPTOR)(binding) && - // Only include interceptors that match the source type of the invocation - this.applicableTo(binding), + let bindings: Readonly>[] = this.findByTag( + ContextTags.GLOBAL_INTERCEPTOR, ); + bindings = bindings.filter(binding => + // Only include interceptors that match the source type of the invocation + this.applicableTo(binding), + ); + this.sortGlobalInterceptorBindings(bindings); const keys = bindings.map(b => b.key); debug('Global interceptor binding keys:', keys); diff --git a/packages/rest/src/__tests__/integration/request-context.integration.ts b/packages/rest/src/__tests__/integration/request-context.integration.ts index 49231df5a818..066dd9f3b58d 100644 --- a/packages/rest/src/__tests__/integration/request-context.integration.ts +++ b/packages/rest/src/__tests__/integration/request-context.integration.ts @@ -142,7 +142,7 @@ describe('close', () => { .expect(200); } expect(observedCtx.contains('req.originalUrl')); - expect(server.listenerCount('bind')).to.eql(1); + expect(server.listenerCount('bind')).to.eql(2); }); });