Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve context.find/findByTag() and interceptor perf #4377

Merged
merged 3 commits into from
Jan 27, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/context/src/context-observation.ts
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ class RequestContext extends Context {
* Wait until the context event queue is empty or an error is thrown
*/
waitUntilObserversNotified(): Promise<void> {
return this.waitUntilPendingNotificationsDone(100);
return this.subscriptionManager.waitUntilPendingNotificationsDone(100);
}
}

Original file line number Diff line number Diff line change
@@ -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);
raymondfeng marked this conversation as resolved.
Show resolved Hide resolved
});

it('invokes static interceptors', async () => {
49 changes: 48 additions & 1 deletion packages/context/src/__tests__/unit/binding-filter.unit.ts
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@
// License text available at https://opensource.org/licenses/MIT

import {expect} from '@loopback/testlab';
import {Binding, filterByKey, filterByTag} from '../..';
import {Binding, filterByKey, filterByTag, isBindingTagFilter} from '../..';

const key = 'foo';

@@ -72,6 +72,53 @@ describe('BindingFilter', () => {
});
});

describe('BindingTagFilter', () => {
it('allows tag name as string', () => {
const filter = filterByTag('controller');
expect(filter.bindingTagPattern).to.eql('controller');
});

it('allows tag name wildcard as string', () => {
const filter = filterByTag('controllers.*');
expect(filter.bindingTagPattern).to.eql(/^controllers\.[^.:]*$/);
});

it('allows tag name as regexp', () => {
const filter = filterByTag(/controller/);
expect(filter.bindingTagPattern).to.eql(/controller/);
});

it('allows tag name as map', () => {
const filter = filterByTag({controller: 'controller', rest: true});
expect(filter.bindingTagPattern).to.eql({
controller: 'controller',
rest: true,
});
});
});

describe('isBindingTagFilter', () => {
it('returns true for binding tag filter functions', () => {
const filter = filterByTag('controller');
expect(isBindingTagFilter(filter)).to.be.true();
});

it('returns false for binding filter functions without tag', () => {
const filter = () => true;
expect(isBindingTagFilter(filter)).to.be.false();
});

it('returns false for undefined', () => {
expect(isBindingTagFilter(undefined)).to.be.false();
});

it('returns false if the bindingTagPattern with wrong type', () => {
const filter = () => true;
filter.bindingTagPattern = true; // wrong type
expect(isBindingTagFilter(filter)).to.be.false();
});
});

describe('filterByKey', () => {
it('accepts bindings MATCHING the provided key', () => {
const filter = filterByKey(key);
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@ import {
ContextEventType,
ContextObserver,
filterByTag,
ContextEventListener,
} from '../..';

const setImmediateAsync = promisify(setImmediate);
@@ -22,14 +21,15 @@ const setImmediateAsync = promisify(setImmediate);
* for assertions
*/
class TestContext extends Context {
// Make parentEventListener public for testing purpose
parentEventListener: ContextEventListener;
get parentEventListener() {
return this.subscriptionManager.parentContextEventListener;
}

/**
* Wait until the context event queue is empty or an error is thrown
*/
waitUntilObserversNotified(): Promise<void> {
return this.waitUntilPendingNotificationsDone(100);
return this.subscriptionManager.waitUntilPendingNotificationsDone(100);
}
}

144 changes: 144 additions & 0 deletions packages/context/src/__tests__/unit/context-tag-indexer.unit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Copyright IBM Corp. 2020. All Rights Reserved.
// Node module: @loopback/context
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {expect} from '@loopback/testlab';
import {Binding, BindingTag, Context, filterByTag} from '../..';

/**
* Create a subclass of context so that we can access parents and registry
* for assertions
*/
class TestContext extends Context {
constructor() {
super('app');
}

get parent() {
return this._parent;
}

get bindingMap() {
const map = new Map(this.registry);
return map;
}

get tagIndex() {
return this.tagIndexer.bindingsIndexedByTag;
}

findByTagInvoked = false;

_findByTagIndex(tag: BindingTag | RegExp) {
this.findByTagInvoked = true;
return super._findByTagIndex(tag);
}
}

describe('Context with tag indexer', () => {
let ctx: TestContext;
beforeEach('given a context', createContext);

describe('bind', () => {
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', () => {
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('unbind', () => {
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', () => {
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();
});
});

function createContext() {
ctx = new TestContext();
}

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();
}
}
});
22 changes: 20 additions & 2 deletions packages/context/src/__tests__/unit/context-view.unit.ts
Original file line number Diff line number Diff line change
@@ -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<unknown>[];
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')
39 changes: 35 additions & 4 deletions packages/context/src/__tests__/unit/context.unit.ts
Original file line number Diff line number Diff line change
@@ -11,8 +11,6 @@ import {
BindingScope,
BindingType,
Context,
ContextEventListener,
ContextEventObserver,
isPromiseLike,
Provider,
} from '../..';
@@ -22,14 +20,19 @@ import {
* for assertions
*/
class TestContext extends Context {
observers: Set<ContextEventObserver> | undefined;
get observers() {
return this.subscriptionManager.observers;
}

// Make parentEventListener public for testing purpose
parentEventListener: ContextEventListener;
get parentEventListener() {
return this.subscriptionManager.parentContextEventListener;
}

get parent() {
return this._parent;
}

get bindingMap() {
const map = new Map(this.registry);
return map;
@@ -318,6 +321,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');
Loading