From 953f5ffe48be4c768cf2bc6ce87f91a43196074a Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 6 Jan 2020 12:48:16 -0800 Subject: [PATCH] feat(context): index bindings by tag to speed up matching by tag Fixes https://github.com/strongloop/loopback-next/issues/4356 - use one function to listen on bind/unbind events - set default maxListeners to Infinity - optimize find bindings by tag - leverage findByTag for filterByTag to find matching bindings --- .../acceptance/interceptor.acceptance.ts | 8 +- .../__tests__/unit/context-observer.unit.ts | 15 +- .../src/__tests__/unit/context-view.unit.ts | 22 +- .../src/__tests__/unit/context.unit.ts | 161 +++++++++-- packages/context/src/binding-filter.ts | 53 +++- packages/context/src/binding.ts | 8 + packages/context/src/context.ts | 258 ++++++++++++++---- packages/context/src/interceptor.ts | 13 +- packages/context/src/value-promise.ts | 2 +- .../request-context.integration.ts | 2 +- 10 files changed, 437 insertions(+), 105 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-observer.unit.ts b/packages/context/src/__tests__/unit/context-observer.unit.ts index 5e0603d35c28..444ad2c4f157 100644 --- a/packages/context/src/__tests__/unit/context-observer.unit.ts +++ b/packages/context/src/__tests__/unit/context-observer.unit.ts @@ -21,8 +21,8 @@ const setImmediateAsync = promisify(setImmediate); * for assertions */ class TestContext extends Context { - get parentEventListeners() { - return this._parentEventListeners; + get eventListener() { + return this.parentEventListener; } /** * Wait until the context event queue is empty or an error is thrown @@ -63,10 +63,9 @@ describe('Context', () => { it('registers observers on context with parent', () => { const childCtx = new TestContext(ctx, 'child'); - expect(childCtx.parentEventListeners).to.be.undefined(); + expect(childCtx.eventListener).to.be.undefined(); childCtx.subscribe(nonMatchingObserver); - expect(childCtx.parentEventListeners!.has('bind')).to.be.true(); - expect(childCtx.parentEventListeners!.has('unbind')).to.be.true(); + expect(childCtx.eventListener).to.be.a.Function(); expect(childCtx.isSubscribed(nonMatchingObserver)).to.true(); expect(ctx.isSubscribed(nonMatchingObserver)).to.false(); }); @@ -84,12 +83,8 @@ describe('Context', () => { it('un-registers observers on context chain during close', () => { const childCtx = new TestContext(ctx, 'child'); childCtx.subscribe(nonMatchingObserver); - const parentEventListeners = new Map(childCtx.parentEventListeners!); childCtx.close(); - for (const [event, listener] of parentEventListeners) { - expect(ctx.listeners(event)).to.not.containEql(listener); - } - expect(childCtx.parentEventListeners).to.be.undefined(); + expect(childCtx.eventListener).to.be.undefined(); expect(childCtx.isSubscribed(nonMatchingObserver)).to.false(); }); 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 32d04bd5880c..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 '../..'; @@ -29,8 +31,18 @@ class TestContext extends Context { const map = new Map(this.registry); return map; } - get parentEventListeners() { - return this._parentEventListeners; + get eventListener() { + return this.parentEventListener; + } + get tagIndex() { + return this.bindingsIndexedByTag; + } + + findByTagInvoked = false; + + _findByTagIndex(tag: BindingTag | RegExp) { + this.findByTagInvoked = true; + return super._findByTagIndex(tag); } } @@ -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'); @@ -759,17 +871,7 @@ describe('Context', () => { childCtx.subscribe(() => {}); // Now we have one observer expect(childCtx.observers!.size).to.eql(1); - // Two listeners are also added to the parent context - const parentEventListeners = childCtx.parentEventListeners!; - expect(parentEventListeners.size).to.eql(2); - - // The map contains listeners added to the parent context - // Take a snapshot into `copy` - const copy = new Map(parentEventListeners); - for (const [key, val] of copy.entries()) { - expect(val).to.be.a.Function(); - expect(ctx.listeners(key)).to.containEql(val); - } + expect(childCtx.eventListener).to.be.a.Function(); // Now clear subscriptions childCtx.close(); @@ -777,9 +879,7 @@ describe('Context', () => { // observers are gone expect(childCtx.observers).to.be.undefined(); // listeners are removed from parent context - for (const [key, val] of copy.entries()) { - expect(ctx.listeners(key)).to.not.containEql(val); - } + expect(childCtx.eventListener).to.be.undefined(); }); it('keeps parent and bindings', () => { @@ -791,6 +891,17 @@ describe('Context', () => { }); }); + describe('maxListeners', () => { + it('defaults to Infinity', () => { + expect(ctx.getMaxListeners()).to.equal(Infinity); + }); + + it('can be changed', () => { + ctx.setMaxListeners(128); + expect(ctx.getMaxListeners()).to.equal(128); + }); + }); + describe('toJSON() and inspect()', () => { beforeEach(setupBindings); @@ -889,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/binding-filter.ts b/packages/context/src/binding-filter.ts index 7d28ea85670c..82d2138f062b 100644 --- a/packages/context/src/binding-filter.ts +++ b/packages/context/src/binding-filter.ts @@ -5,6 +5,7 @@ import {Binding, BindingTag} from './binding'; import {BindingAddress} from './binding-key'; +import {MapObject} from './value-promise'; /** * A function that filters bindings. It returns `true` to select a given @@ -59,27 +60,63 @@ export function isBindingAddress( return typeof bindingSelector !== 'function'; } +/** + * Binding filter function that uses tags + */ +export interface BindingTagFilter extends BindingFilter { + bindingTag: BindingTag | RegExp; +} + +/** + * Type guard for BindingTagFilter + * @param filter - A BindingFilter function + */ +export function isBindingTagFilter( + filter?: BindingFilter, +): filter is BindingTagFilter { + return filter != null && 'bindingTag' in filter; +} + /** * Create a binding filter for the tag pattern * @param tagPattern - Binding tag name, regexp, or object */ export function filterByTag(tagPattern: BindingTag | RegExp): BindingFilter { - if (typeof tagPattern === 'string' || tagPattern instanceof RegExp) { - const regexp = - typeof tagPattern === 'string' - ? wildcardToRegExp(tagPattern) - : tagPattern; - return b => Array.from(b.tagNames).some(t => regexp!.test(t)); + let filter: BindingFilter; + let regex: RegExp | undefined = undefined; + if (tagPattern instanceof RegExp) { + // RegExp for tag names + regex = tagPattern; + } + if ( + typeof tagPattern === 'string' && + (tagPattern.includes('*') || tagPattern.includes('?')) + ) { + // Wildcard tag name + regex = wildcardToRegExp(tagPattern); + } + + if (regex != null) { + // RegExp or wildcard match + filter = b => b.tagNames.some(t => regex!.test(t)); + } else if (typeof tagPattern === 'string') { + // Plain tag string match + filter = b => b.tagNames.includes(tagPattern); } else { - return b => { + // Match tag name/value pairs + const tagMap = tagPattern as MapObject; + filter = b => { for (const t in tagPattern) { // One tag name/value does not match - if (b.tagMap[t] !== tagPattern[t]) return false; + if (b.tagMap[t] !== tagMap[t]) return false; } // All tag name/value pairs match return true; }; } + // Set up binding tag for the filter + (filter as BindingTagFilter).bindingTag = regex ?? tagPattern; + return filter; } /** diff --git a/packages/context/src/binding.ts b/packages/context/src/binding.ts index 6e43cab6960a..cfe5f37d8ae6 100644 --- a/packages/context/src/binding.ts +++ b/packages/context/src/binding.ts @@ -140,6 +140,14 @@ export type BindingTag = TagMap | string; */ export type BindingTemplate = (binding: Binding) => void; +/** + * Event listeners for binding events + */ +export type BindingEventListener = ( + binding: Binding, + event: string, +) => void; + type ValueGetter = ( ctx: Context, options: ResolutionOptions, diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index 97ca962ad469..bb50f506d7ba 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -6,12 +6,17 @@ 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, } from './binding-config'; -import {BindingFilter, filterByKey, filterByTag} from './binding-filter'; +import { + BindingFilter, + filterByKey, + filterByTag, + isBindingTagFilter, +} from './binding-filter'; import {BindingAddress, BindingKey} from './binding-key'; import {BindingComparator} from './binding-sorter'; import { @@ -53,6 +58,15 @@ import {iterator, multiple} from 'p-event'; const debug = debugFactory('loopback:context'); +/** + * Synchronous event listener for the `Context` as en event emitter + */ +export type ContextEventListener = ( + binding: Readonly>, // Binding to be added or removed + context: Context, // The source context + event: string, // 'bind' or 'unbind' +) => void; + /** * Context provides an implementation of Inversion of Control (IoC) container */ @@ -67,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 */ @@ -75,17 +107,9 @@ export class Context extends EventEmitter { protected configResolver: ConfigurationResolver; /** - * Event listeners for parent context keyed by event names. It keeps track - * of listeners from this context against its parent so that we can remove - * these listeners when this context is closed. + * A listener to watch parent context events */ - protected _parentEventListeners: - | Map< - string, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (...args: any[]) => void - > - | undefined; + protected parentEventListener?: ContextEventListener; /** * A list of registered context observers. The Set will be created when the @@ -128,12 +152,41 @@ export class Context extends EventEmitter { */ constructor(_parent?: Context | string, name?: string) { super(); + // The number of listeners can grow with the number of child contexts + // For example, each request can add a listener to the RestServer and the + // listener is removed when the request processing is finished. + // See https://github.com/strongloop/loopback-next/issues/4363 + this.setMaxListeners(Infinity); if (typeof _parent === 'string') { name = _parent; _parent = undefined; } 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); } /** @@ -161,8 +214,23 @@ export class Context extends EventEmitter { private setupEventHandlersIfNeeded() { if (this.notificationQueue != null) return; - this.addParentEventListener('bind'); - this.addParentEventListener('unbind'); + if (this._parent != null) { + /** + * Add an event listener to its parent context so that this context will + * be notified of parent events, such as `bind` or `unbind`. + */ + this.parentEventListener = ( + binding: Readonly>, + context: Context, + event: string, + ) => { + this.handleParentEvent(binding, context, event); + }; + + // Listen on the parent context events + this._parent.on('bind', this.parentEventListener!); + this._parent.on('unbind', this.parentEventListener!); + } // The following are two async functions. Returned promises are ignored as // they are long-running background tasks. @@ -177,47 +245,32 @@ export class Context extends EventEmitter { } } - /** - * Add an event listener to its parent context so that this context will - * be notified of parent events, such as `bind` or `unbind`. - * @param event - Event name - */ - private addParentEventListener(event: string) { - if (this._parent == null) return; - - // Keep track of parent event listeners so that we can remove them - this._parentEventListeners = this._parentEventListeners ?? new Map(); - if (this._parentEventListeners.has(event)) return; - - const parentEventListener = ( - binding: Readonly>, - context: Context, - ) => { - // Propagate the event to this context only if the binding key does not - // exist in this context. The parent binding is shadowed if there is a - // binding with the same key in this one. - if (this.contains(binding.key)) { - this._debug( - 'Event %s %s is not re-emitted from %s to %s', - event, - binding.key, - context.name, - this.name, - ); - return; - } + private handleParentEvent( + binding: Readonly>, + context: Context, + event: string, + ) { + // Propagate the event to this context only if the binding key does not + // exist in this context. The parent binding is shadowed if there is a + // binding with the same key in this one. + if (this.contains(binding.key)) { this._debug( - 'Re-emitting %s %s from %s to %s', + 'Event %s %s is not re-emitted from %s to %s', event, binding.key, context.name, this.name, ); - this.emit(event, binding, context); - }; - this._parentEventListeners.set(event, parentEventListener); - // Listen on the parent context events - this._parent.on(event, parentEventListener); + return; + } + this._debug( + 'Re-emitting %s %s from %s to %s', + event, + binding.key, + context.name, + this.name, + ); + this.emit(event, binding, context, event); } /** @@ -357,9 +410,9 @@ export class Context extends EventEmitter { this.registry.set(key, binding); if (existingBinding !== binding) { if (existingBinding != null) { - this.emit('unbind', existingBinding, this); + this.emit('unbind', existingBinding, this, 'unbind'); } - this.emit('bind', binding, this); + this.emit('bind', binding, this, 'bind'); } return this; } @@ -505,7 +558,7 @@ export class Context extends EventEmitter { if (binding?.isLocked) throw new Error(`Cannot unbind key "${key}" of a locked binding`); this.registry.delete(key); - this.emit('unbind', binding, this); + this.emit('unbind', binding, this, 'unbind'); return true; } @@ -548,12 +601,13 @@ export class Context extends EventEmitter { }); this.notificationQueue = undefined; } - if (this._parent && this._parentEventListeners) { - for (const [event, listener] of this._parentEventListeners) { - this._parent.removeListener(event, listener); - } - this._parentEventListeners = undefined; + if (this._parent && this.parentEventListener) { + this._parent.removeListener('bind', this.parentEventListener); + this._parent.removeListener('unbind', this.parentEventListener); + this.parentEventListener = undefined; } + this.removeListener('bind', this.tagIndexListener); + this.removeListener('unbind', this.tagIndexListener); } /** @@ -607,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 @@ -658,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.bindingTag); + } + const bindings: Readonly>[] = []; const filter = filterByKey(pattern); @@ -689,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/context/src/value-promise.ts b/packages/context/src/value-promise.ts index 99f036500437..73dcbb54cd97 100644 --- a/packages/context/src/value-promise.ts +++ b/packages/context/src/value-promise.ts @@ -29,7 +29,7 @@ export type BoundValue = any; */ export type ValueOrPromise = T | PromiseLike; -export type MapObject = {[name: string]: T}; +export type MapObject = Record; /** * Check whether a value is a Promise-like instance. 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); }); });