Skip to content

Commit

Permalink
feat(context): index bindings by tag to speed up matching by tag
Browse files Browse the repository at this point in the history 
Fixes #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
  • Loading branch information
@raymondfeng
raymondfeng committed Jan 14, 2020
1 parent ee24aad commit 953f5ff
Show file tree
Hide file tree
Showing 10 changed files with 437 additions and 105 deletions.
8 changes: 4 additions & 4 deletions packages/context/src/__tests__/acceptance/interceptor.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 () => {
Expand Down
15 changes: 5 additions & 10 deletions packages/context/src/__tests__/unit/context-observer.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});

Expand Down
22 changes: 20 additions & 2 deletions packages/context/src/__tests__/unit/context-view.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {expect} from '@loopback/testlab';
import {
Binding,
BindingScope,
BindingTag,
compareBindingsByTag,
Context,
ContextView,
Expand All @@ -16,7 +17,7 @@ import {

describe('ContextView', () => {
let app: Context;
let server: Context;
let server: ServerContext;

let bindings: Binding<unknown>[];
let taggedAsFoo: ContextView;
Expand All @@ -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,
Expand Down Expand Up @@ -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')
Expand Down
161 changes: 145 additions & 16 deletions packages/context/src/__tests__/unit/context.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import {
BindingCreationPolicy,
BindingKey,
BindingScope,
BindingTag,
BindingType,
Context,
ContextEventObserver,
filterByTag,
isPromiseLike,
Provider,
} from '../..';
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -759,27 +871,15 @@ 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();

// 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', () => {
Expand All @@ -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);

Expand Down Expand Up @@ -889,4 +1000,22 @@ describe('Context', () => {
function createContext() {
ctx = new TestContext('app');
}

function assertBindingIndexedByTag(
binding: Binding<unknown>,
...tags: string[]
) {
for (const t of tags) {
expect(ctx.tagIndex.get(t)?.has(binding)).to.be.true();
}
}

function assertBindingNotIndexedByTag(
binding: Binding<unknown>,
...tags: string[]
) {
for (const t of tags) {
expect(!!ctx.tagIndex.get(t)?.has(binding)).to.be.false();
}
}
});
Loading

0 comments on commit 953f5ff

Please sign in to comment.