From bb355422f6fd0f3ba6e49991f13a2c675841093b Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 7 May 2019 09:07:42 -0700 Subject: [PATCH 1/2] feat(context): add binding comparator to sort bindings - add ability to sort bindings by tag values - allow binding sorter on ContextView and @inject.* --- docs/site/Context.md | 3 +- docs/site/Decorators_inject.md | 17 ++ .../inject-multiple-values.acceptance.ts | 81 ++++++++- .../src/__tests__/unit/binding-sorter.unit.ts | 155 ++++++++++++++++++ .../src/__tests__/unit/context-view.unit.ts | 30 +++- packages/context/src/binding-sorter.ts | 124 ++++++++++++++ packages/context/src/context-view.ts | 46 +++++- packages/context/src/context.ts | 6 +- packages/context/src/index.ts | 1 + packages/context/src/inject.ts | 29 +++- packages/core/src/extension-point.ts | 7 +- 11 files changed, 485 insertions(+), 14 deletions(-) create mode 100644 packages/context/src/__tests__/unit/binding-sorter.unit.ts create mode 100644 packages/context/src/binding-sorter.ts diff --git a/docs/site/Context.md b/docs/site/Context.md index 141c3a6a4206..94f9d88b51af 100644 --- a/docs/site/Context.md +++ b/docs/site/Context.md @@ -435,7 +435,8 @@ should be able to pick up these new routes without restarting. To support the dynamic tracking of such artifacts registered within a context chain, we introduce `ContextObserver` interface and `ContextView` class that can be used to watch a list of bindings matching certain criteria depicted by a -`BindingFilter` function. +`BindingFilter` function and an optional `BindingComparator` function to sort +matched bindings. ```ts import {Context, ContextView} from '@loopback/context'; diff --git a/docs/site/Decorators_inject.md b/docs/site/Decorators_inject.md index 5f8e5115ace2..9b2e801d6d51 100644 --- a/docs/site/Decorators_inject.md +++ b/docs/site/Decorators_inject.md @@ -83,6 +83,23 @@ class MyControllerWithValues { } ``` +To sort matched bindings found by the binding filter function, `@inject` honors +`bindingComparator` in `metadata`: + +```ts +class MyControllerWithValues { + constructor( + @inject(binding => binding.tagNames.includes('foo'), { + bindingComparator: (a, b) => { + // Sort by value of `foo` tag + return a.tagMap.foo.localeCompare(b.tagMap.foo); + }, + }) + public values: string[], + ) {} +} +``` + A few variants of `@inject` are provided to declare special forms of dependencies. diff --git a/packages/context/src/__tests__/acceptance/inject-multiple-values.acceptance.ts b/packages/context/src/__tests__/acceptance/inject-multiple-values.acceptance.ts index 474d81bfb48a..1ec046f22d5e 100644 --- a/packages/context/src/__tests__/acceptance/inject-multiple-values.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/inject-multiple-values.acceptance.ts @@ -4,7 +4,14 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {Context, ContextView, filterByTag, Getter, inject} from '../..'; +import { + compareBindingsByTag, + Context, + ContextView, + filterByTag, + Getter, + inject, +} from '../..'; let app: Context; let server: Context; @@ -29,6 +36,26 @@ describe('@inject.* to receive multiple values matching a filter', async () => { expect(await getter()).to.eql([3, 7, 5]); }); + it('injects as getter with bindingComparator', async () => { + class MyControllerWithGetter { + @inject.getter(workloadMonitorFilter, { + bindingComparator: compareBindingsByTag('name'), + }) + getter: Getter; + } + + server.bind('my-controller').toClass(MyControllerWithGetter); + const inst = await server.get('my-controller'); + const getter = inst.getter; + // app-reporter, server-reporter + expect(await getter()).to.eql([5, 3]); + // Add a new binding that matches the filter + givenWorkloadMonitor(server, 'server-reporter-2', 7); + // The getter picks up the new binding by order + // // app-reporter, server-reporter, server-reporter-2 + expect(await getter()).to.eql([5, 3, 7]); + }); + describe('@inject', () => { class MyControllerWithValues { constructor( @@ -48,6 +75,37 @@ describe('@inject.* to receive multiple values matching a filter', async () => { const inst = server.getSync('my-controller'); expect(inst.values).to.eql([3, 5]); }); + + it('injects as values with bindingComparator', async () => { + class MyControllerWithBindingSorter { + constructor( + @inject(workloadMonitorFilter, { + bindingComparator: compareBindingsByTag('name'), + }) + public values: number[], + ) {} + } + server.bind('my-controller').toClass(MyControllerWithBindingSorter); + const inst = await server.get( + 'my-controller', + ); + // app-reporter, server-reporter + expect(inst.values).to.eql([5, 3]); + }); + + it('throws error if bindingComparator is provided without a filter', () => { + expect(() => { + // tslint:disable-next-line:no-unused + class ControllerWithInvalidInject { + constructor( + @inject('my-key', { + bindingComparator: compareBindingsByTag('name'), + }) + public values: number[], + ) {} + } + }).to.throw('Binding sorter is only allowed with a binding filter'); + }); }); it('injects as a view', async () => { @@ -68,6 +126,24 @@ describe('@inject.* to receive multiple values matching a filter', async () => { expect(await view.values()).to.eql([3, 5]); }); + it('injects as a view with bindingComparator', async () => { + class MyControllerWithView { + @inject.view(workloadMonitorFilter, { + bindingComparator: compareBindingsByTag('name'), + }) + view: ContextView; + } + + server.bind('my-controller').toClass(MyControllerWithView); + const inst = await server.get('my-controller'); + const view = inst.view; + expect(view.bindings.map(b => b.tagMap.name)).to.eql([ + 'app-reporter', + 'server-reporter', + ]); + expect(await view.values()).to.eql([5, 3]); + }); + function givenWorkloadMonitors() { givenServerWithinAnApp(); givenWorkloadMonitor(server, 'server-reporter', 3); @@ -84,7 +160,8 @@ describe('@inject.* to receive multiple values matching a filter', async () => { return ctx .bind(`workloadMonitors.${name}`) .to(workload) - .tag('workloadMonitor'); + .tag('workloadMonitor') + .tag({name}); } }); diff --git a/packages/context/src/__tests__/unit/binding-sorter.unit.ts b/packages/context/src/__tests__/unit/binding-sorter.unit.ts new file mode 100644 index 000000000000..5f5a329da651 --- /dev/null +++ b/packages/context/src/__tests__/unit/binding-sorter.unit.ts @@ -0,0 +1,155 @@ +// Copyright IBM Corp. 2019. 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, compareByOrder, sortBindingsByPhase} from '../..'; + +describe('BindingComparator', () => { + const FINAL = Symbol('final'); + const orderOfPhases = ['log', 'auth', FINAL]; + const phaseTagName = 'phase'; + let bindings: Binding[]; + let sortedBindingKeys: string[]; + + beforeEach(givenBindings); + beforeEach(sortBindings); + + it('sorts by phase', () => { + /** + * Phases + * - 'log': logger1, logger2 + * - 'auth': auth1, auth2 + */ + assertOrder('logger1', 'logger2', 'auth1', 'auth2'); + }); + + it('sorts by phase - unknown phase comes before known ones', () => { + /** + * Phases + * - 'metrics': metrics // not part of ['log', 'auth'] + * - 'log': logger1 + */ + assertOrder('metrics', 'logger1'); + }); + + it('sorts by phase alphabetically without orderOf phase', () => { + /** + * Phases + * - 'metrics': metrics // not part of ['log', 'auth'] + * - 'rateLimit': rateLimit // not part of ['log', 'auth'] + */ + assertOrder('metrics', 'rateLimit'); + }); + + it('sorts by binding order without phase tags', () => { + /** + * Phases + * - '': validator1, validator2 // not part of ['log', 'auth'] + * - 'metrics': metrics // not part of ['log', 'auth'] + * - 'log': logger1 + */ + assertOrder('validator1', 'validator2', 'metrics', 'logger1'); + }); + + it('sorts by binding order without phase tags', () => { + /** + * Phases + * - '': validator1 // not part of ['log', 'auth'] + * - 'metrics': metrics // not part of ['log', 'auth'] + * - 'log': logger1 + * - 'final': Symbol('final') + */ + assertOrder('validator1', 'metrics', 'logger1', 'final'); + }); + + /** + * The sorted bindings by phase: + * - '': validator1, validator2 // not part of ['log', 'auth'] + * - 'metrics': metrics // not part of ['log', 'auth'] + * - 'rateLimit': rateLimit // not part of ['log', 'auth'] + * - 'log': logger1, logger2 + * - 'auth': auth1, auth2 + */ + function givenBindings() { + bindings = [ + Binding.bind('logger1').tag({[phaseTagName]: 'log'}), + Binding.bind('auth1').tag({[phaseTagName]: 'auth'}), + Binding.bind('auth2').tag({[phaseTagName]: 'auth'}), + Binding.bind('logger2').tag({[phaseTagName]: 'log'}), + Binding.bind('metrics').tag({[phaseTagName]: 'metrics'}), + Binding.bind('rateLimit').tag({[phaseTagName]: 'rateLimit'}), + Binding.bind('validator1'), + Binding.bind('validator2'), + Binding.bind('final').tag({[phaseTagName]: FINAL}), + ]; + } + + function sortBindings() { + sortBindingsByPhase(bindings, phaseTagName, orderOfPhases); + sortedBindingKeys = bindings.map(b => b.key); + } + + function assertOrder(...keys: string[]) { + let prev: number = -1; + let prevKey: string = ''; + for (const key of keys) { + const current = sortedBindingKeys.indexOf(key); + expect(current).to.greaterThan( + prev, + `Binding ${key} should come after ${prevKey}`, + ); + prev = current; + prevKey = key; + } + } +}); + +describe('compareByOrder', () => { + it('honors order', () => { + expect(compareByOrder('a', 'b', ['b', 'a'])).to.greaterThan(0); + }); + + it('value not included in order comes first', () => { + expect(compareByOrder('a', 'c', ['a', 'b'])).to.greaterThan(0); + }); + + it('values not included are compared alphabetically', () => { + expect(compareByOrder('a', 'c', [])).to.lessThan(0); + }); + + it('null/undefined/"" values are treated as ""', () => { + expect(compareByOrder('', 'c')).to.lessThan(0); + expect(compareByOrder(null, 'c')).to.lessThan(0); + expect(compareByOrder(undefined, 'c')).to.lessThan(0); + }); + + it('returns 0 for equal values', () => { + expect(compareByOrder('c', 'c')).to.equal(0); + expect(compareByOrder(null, '')).to.equal(0); + expect(compareByOrder('', undefined)).to.equal(0); + }); + + it('allows symbols', () => { + const a = Symbol('a'); + const b = Symbol('b'); + expect(compareByOrder(a, b)).to.lessThan(0); + expect(compareByOrder(a, b, [b, a])).to.greaterThan(0); + expect(compareByOrder(a, 'b', [b, a])).to.greaterThan(0); + }); + + it('list symbols before strings', () => { + const a = 'a'; + const b = Symbol('a'); + expect(compareByOrder(a, b)).to.greaterThan(0); + expect(compareByOrder(b, a)).to.lessThan(0); + }); + + it('compare symbols by description', () => { + const a = Symbol('a'); + const b = Symbol('b'); + expect(compareByOrder(a, b)).to.lessThan(0); + expect(compareByOrder(b, a)).to.greaterThan(0); + }); +}); diff --git a/packages/context/src/__tests__/unit/context-view.unit.ts b/packages/context/src/__tests__/unit/context-view.unit.ts index 0ae0cf458c9b..0264c751a578 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, + compareBindingsByTag, Context, ContextView, createViewGetter, @@ -26,6 +27,15 @@ describe('ContextView', () => { expect(taggedAsFoo.bindings).to.eql(bindings); }); + it('sorts matched bindings', () => { + const view = new ContextView( + server, + filterByTag('foo'), + compareBindingsByTag('phase', ['b', 'a']), + ); + expect(view.bindings).to.eql([bindings[1], bindings[0]]); + }); + it('resolves bindings', async () => { expect(await taggedAsFoo.resolve()).to.eql(['BAR', 'FOO']); expect(await taggedAsFoo.values()).to.eql(['BAR', 'FOO']); @@ -154,6 +164,22 @@ describe('ContextView', () => { .tag('foo'); expect(await getter()).to.eql(['BAR', 'XYZ', 'FOO']); }); + + it('creates a getter function for the binding filter and sorter', async () => { + const getter = createViewGetter(server, filterByTag('foo'), (a, b) => { + return a.key.localeCompare(b.key); + }); + expect(await getter()).to.eql(['BAR', 'FOO']); + server + .bind('abc') + .to('ABC') + .tag('abc'); + server + .bind('xyz') + .to('XYZ') + .tag('foo'); + expect(await getter()).to.eql(['BAR', 'FOO', 'XYZ']); + }); }); function givenContextView() { @@ -169,14 +195,14 @@ describe('ContextView', () => { server .bind('bar') .toDynamicValue(() => Promise.resolve('BAR')) - .tag('foo', 'bar') + .tag('foo', 'bar', {phase: 'a'}) .inScope(BindingScope.SINGLETON), ); bindings.push( app .bind('foo') .to('FOO') - .tag('foo', 'bar'), + .tag('foo', 'bar', {phase: 'b'}), ); } }); diff --git a/packages/context/src/binding-sorter.ts b/packages/context/src/binding-sorter.ts new file mode 100644 index 000000000000..9c1c11e19a9b --- /dev/null +++ b/packages/context/src/binding-sorter.ts @@ -0,0 +1,124 @@ +// Copyright IBM Corp. 2019. 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 {Binding} from './binding'; + +/** + * Compare function to sort an array of bindings. + * It is used by `Array.prototype.sort()`. + * + * @example + * ```ts + * const compareByKey: BindingComparator = (a, b) => a.key.localeCompare(b.key); + * ``` + */ +export interface BindingComparator { + /** + * Compare two bindings + * @param bindingA First binding + * @param bindingB Second binding + * @returns A number to determine order of bindingA and bindingB + * - 0 leaves bindingA and bindingB unchanged + * - <0 bindingA comes before bindingB + * - >0 bindingA comes after bindingB + */ + ( + bindingA: Readonly>, + bindingB: Readonly>, + ): number; +} + +/** + * Creates a binding compare function to sort bindings by tagged phase name. + * + * @remarks + * Two bindings are compared as follows: + * + * 1. Get values for the given tag as `phase` for bindings, if the tag is not + * present, default `phase` to `''`. + * 2. If both bindings have `phase` value in `orderOfPhases`, honor the order + * specified by `orderOfPhases`. + * 3. If a binding's `phase` does not exist in `orderOfPhases`, it comes before + * the one with `phase` exists in `orderOfPhases`. + * 4. If both bindings have `phase` value outside of `orderOfPhases`, they are + * ordered by phase names alphabetically and symbol values come before string + * values. + * + * @param phaseTagName Name of the binding tag for phase + * @param orderOfPhases An array of phase names as the predefined order + */ +export function compareBindingsByTag( + phaseTagName: string = 'phase', + orderOfPhases: (string | symbol)[] = [], +): BindingComparator { + return (a: Readonly>, b: Readonly>) => { + return compareByOrder( + a.tagMap[phaseTagName], + b.tagMap[phaseTagName], + orderOfPhases, + ); + }; +} + +/** + * Compare two values by the predefined order + * + * @remarks + * + * The comparison is performed as follows: + * + * 1. If both values are included in `order`, they are sorted by their indexes in + * `order`. + * 2. The value included in `order` comes after the value not included in `order`. + * 3. If neither values are included in `order`, they are sorted: + * - symbol values come before string values + * - alphabetical order for two symbols or two strings + * + * @param a First value + * @param b Second value + * @param order An array of values as the predefined order + */ +export function compareByOrder( + a: string | symbol | undefined | null, + b: string | symbol | undefined | null, + order: (string | symbol)[] = [], +) { + a = a || ''; + b = b || ''; + const i1 = order.indexOf(a); + const i2 = order.indexOf(b); + if (i1 !== -1 || i2 !== -1) { + // Honor the order + return i1 - i2; + } else { + // Neither value is in the pre-defined order + + // symbol comes before string + if (typeof a === 'symbol' && typeof b === 'string') return -1; + if (typeof a === 'string' && typeof b === 'symbol') return 1; + + // both a and b are symbols or both a and b are strings + if (typeof a === 'symbol') a = a.toString(); + if (typeof b === 'symbol') b = b.toString(); + return a < b ? -1 : a > b ? 1 : 0; + } +} + +/** + * Sort bindings by phase names denoted by a tag and the predefined order + * + * @param bindings An array of bindings + * @param phaseTagName Tag name for phase, for example, we can use the value + * `'a'` of tag `order` as the phase name for `binding.tag({order: 'a'})`. + * + * @param orderOfPhases An array of phase names as the predefined order + */ +export function sortBindingsByPhase( + bindings: Readonly>[], + phaseTagName?: string, + orderOfPhases?: (string | symbol)[], +) { + return bindings.sort(compareBindingsByTag(phaseTagName, orderOfPhases)); +} diff --git a/packages/context/src/context-view.ts b/packages/context/src/context-view.ts index 1b643c5db521..6e00c1e80f36 100644 --- a/packages/context/src/context-view.ts +++ b/packages/context/src/context-view.ts @@ -8,6 +8,7 @@ import {EventEmitter} from 'events'; import {promisify} from 'util'; import {Binding} from './binding'; import {BindingFilter} from './binding-filter'; +import {BindingComparator} from './binding-sorter'; import {Context} from './context'; import { ContextEventType, @@ -43,6 +44,7 @@ export class ContextView extends EventEmitter constructor( protected readonly context: Context, public readonly filter: BindingFilter, + public readonly sorter?: BindingComparator, ) { super(); } @@ -88,6 +90,9 @@ export class ContextView extends EventEmitter protected findBindings(): Readonly>[] { debug('Finding matching bindings'); const found = this.context.find(this.filter); + if (typeof this.sorter === 'function') { + found.sort(this.sorter); + } this._cachedBindings = found; return found; } @@ -155,17 +160,54 @@ export class ContextView extends EventEmitter } /** - * Create a context view as a getter + * Create a context view as a getter with the given filter + * @param ctx Context object + * @param bindingFilter A function to match bindings + * @param session Resolution session + */ +export function createViewGetter( + ctx: Context, + bindingFilter: BindingFilter, + session?: ResolutionSession, +): Getter; + +/** + * Create a context view as a getter with the given filter and sort matched + * bindings by the comparator. * @param ctx Context object * @param bindingFilter A function to match bindings + * @param bindingComparator A function to compare two bindings * @param session Resolution session */ export function createViewGetter( ctx: Context, bindingFilter: BindingFilter, + bindingComparator?: BindingComparator, + session?: ResolutionSession, +): Getter; + +/** + * Create a context view as a getter + * @param ctx Context object + * @param bindingFilter A function to match bindings + * @param bindingComparatorOrSession A function to sort matched bindings or + * resolution session if the comparator is not needed + * @param session Resolution session if the comparator is provided + */ +export function createViewGetter( + ctx: Context, + bindingFilter: BindingFilter, + bindingComparatorOrSession?: BindingComparator | ResolutionSession, session?: ResolutionSession, ): Getter { - const view = new ContextView(ctx, bindingFilter); + let bindingComparator: BindingComparator | undefined = undefined; + if (typeof bindingComparatorOrSession === 'function') { + bindingComparator = bindingComparatorOrSession; + } else if (bindingComparatorOrSession instanceof ResolutionSession) { + session = bindingComparatorOrSession; + } + + const view = new ContextView(ctx, bindingFilter, bindingComparator); view.open(); return view.asGetter(session); } diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts index ff158753e952..5d746acd5115 100644 --- a/packages/context/src/context.ts +++ b/packages/context/src/context.ts @@ -9,6 +9,7 @@ import {v1 as uuidv1} from 'uuid'; import {Binding, BindingTag} from './binding'; import {BindingFilter, filterByKey, filterByTag} from './binding-filter'; import {BindingAddress, BindingKey} from './binding-key'; +import {BindingComparator} from './binding-sorter'; import { ContextEventObserver, ContextEventType, @@ -441,9 +442,10 @@ export class Context extends EventEmitter { /** * Create a view of the context chain with the given binding filter * @param filter A function to match bindings + * @param sorter A function to sort matched bindings */ - createView(filter: BindingFilter) { - const view = new ContextView(this, filter); + createView(filter: BindingFilter, sorter?: BindingComparator) { + const view = new ContextView(this, filter, sorter); view.open(); return view; } diff --git a/packages/context/src/index.ts b/packages/context/src/index.ts index 70eb7613aab3..47d902fb9b1e 100644 --- a/packages/context/src/index.ts +++ b/packages/context/src/index.ts @@ -9,6 +9,7 @@ export * from './binding-decorator'; export * from './binding-filter'; export * from './binding-inspector'; export * from './binding-key'; +export * from './binding-sorter'; export * from './context'; export * from './context-observer'; export * from './context-view'; diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts index 10c540549048..0a33beada3ea 100644 --- a/packages/context/src/inject.ts +++ b/packages/context/src/inject.ts @@ -20,6 +20,7 @@ import { isBindingAddress, } from './binding-filter'; import {BindingAddress} from './binding-key'; +import {BindingComparator} from './binding-sorter'; import {BindingCreationPolicy, Context} from './context'; import {ContextView, createViewGetter} from './context-view'; import {ResolutionOptions, ResolutionSession} from './resolution-session'; @@ -52,6 +53,10 @@ export interface InjectionMetadata extends ResolutionOptions { * It's usually set by the decorator implementation. */ decorator?: string; + /** + * Optional sorter for matched bindings + */ + bindingComparator?: BindingComparator; /** * Other attributes */ @@ -109,6 +114,9 @@ export function inject( resolve = resolveValuesByFilter; } const injectionMetadata = Object.assign({decorator: '@inject'}, metadata); + if (injectionMetadata.bindingComparator && !resolve) { + throw new Error('Binding sorter is only allowed with a binding filter'); + } return function markParameterOrPropertyAsInjected( target: Object, member: string, @@ -343,7 +351,7 @@ export namespace inject { * @param bindingFilter A binding filter function * @param metadata */ - export const view = function injectByFilter( + export const view = function injectContextView( bindingFilter: BindingFilter, metadata?: InjectionMetadata, ) { @@ -530,7 +538,11 @@ function resolveValuesByFilter( ) { assertTargetType(injection, Array); const bindingFilter = injection.bindingSelector as BindingFilter; - const view = new ContextView(ctx, bindingFilter); + const view = new ContextView( + ctx, + bindingFilter, + injection.metadata.bindingComparator, + ); return view.resolve(session); } @@ -549,7 +561,12 @@ function resolveAsGetterByFilter( ) { assertTargetType(injection, Function, 'Getter function'); const bindingFilter = injection.bindingSelector as BindingFilter; - return createViewGetter(ctx, bindingFilter, session); + return createViewGetter( + ctx, + bindingFilter, + injection.metadata.bindingComparator, + session, + ); } /** @@ -562,7 +579,11 @@ function resolveAsContextView(ctx: Context, injection: Readonly) { assertTargetType(injection, ContextView); const bindingFilter = injection.bindingSelector as BindingFilter; - const view = new ContextView(ctx, bindingFilter); + const view = new ContextView( + ctx, + bindingFilter, + injection.metadata.bindingComparator, + ); view.open(); return view; } diff --git a/packages/core/src/extension-point.ts b/packages/core/src/extension-point.ts index 7d970297f6e9..0796197bb2c1 100644 --- a/packages/core/src/extension-point.ts +++ b/packages/core/src/extension-point.ts @@ -71,7 +71,12 @@ export function extensions(extensionPointName?: string) { inferExtensionPointName(injection.target, session.currentBinding); const bindingFilter = extensionFilter(extensionPointName); - return createViewGetter(ctx, bindingFilter, session); + return createViewGetter( + ctx, + bindingFilter, + injection.metadata.bindingComparator, + session, + ); }); } From 428325ffd2904a7893bd3c3cf9bef78360d4182a Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 9 May 2019 13:37:37 -0700 Subject: [PATCH 2/2] chore: refactor code to use binding sorter --- packages/context/src/interceptor.ts | 21 +++++---------- packages/core/src/lifecycle-registry.ts | 27 +++++++++---------- packages/rest/src/body-parsers/body-parser.ts | 12 ++++----- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/packages/context/src/interceptor.ts b/packages/context/src/interceptor.ts index 48fa29618c83..f941aa630ba8 100644 --- a/packages/context/src/interceptor.ts +++ b/packages/context/src/interceptor.ts @@ -16,6 +16,7 @@ import * as debugFactory from 'debug'; import {Binding, BindingTemplate} from './binding'; import {filterByTag} from './binding-filter'; import {BindingAddress} from './binding-key'; +import {sortBindingsByPhase} from './binding-sorter'; import {Context} from './context'; import {ContextBindings, ContextTags} from './keys'; import { @@ -91,21 +92,11 @@ export class InvocationContext extends Context { this.getSync(ContextBindings.GLOBAL_INTERCEPTOR_ORDERED_GROUPS, { optional: true, }) || []; - bindings.sort((a, b) => { - const g1: string = a.tagMap[ContextTags.GLOBAL_INTERCEPTOR_GROUP] || ''; - const g2: string = b.tagMap[ContextTags.GLOBAL_INTERCEPTOR_GROUP] || ''; - const i1 = orderedGroups.indexOf(g1); - const i2 = orderedGroups.indexOf(g2); - if (i1 !== -1 || i2 !== -1) { - // Honor the group order - return i1 - i2; - } else { - // Neither group is in the pre-defined order - // Use alphabetical order instead so that `1-group` is invoked before - // `2-group` - return g1 < g2 ? -1 : g1 > g2 ? 1 : 0; - } - }); + return sortBindingsByPhase( + bindings, + ContextTags.GLOBAL_INTERCEPTOR_GROUP, + orderedGroups, + ); } /** diff --git a/packages/core/src/lifecycle-registry.ts b/packages/core/src/lifecycle-registry.ts index 9e89ee928bd8..cfb5e1148c96 100644 --- a/packages/core/src/lifecycle-registry.ts +++ b/packages/core/src/lifecycle-registry.ts @@ -3,7 +3,12 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Binding, ContextView, inject} from '@loopback/context'; +import { + Binding, + ContextView, + inject, + sortBindingsByPhase, +} from '@loopback/context'; import {CoreBindings, CoreTags} from './keys'; import {LifeCycleObserver, lifeCycleObserverFilter} from './lifecycle'; import debugFactory = require('debug'); @@ -111,6 +116,11 @@ export class LifeCycleObserverRegistry implements LifeCycleObserver { string, Readonly>[] > = new Map(); + sortBindingsByPhase( + bindings, + CoreTags.LIFE_CYCLE_OBSERVER_GROUP, + this.options.orderedGroups, + ); for (const binding of bindings) { const group = this.getObserverGroup(binding); let bindingsInGroup = groupMap.get(group); @@ -125,20 +135,7 @@ export class LifeCycleObserverRegistry implements LifeCycleObserver { for (const [group, bindingsInGroup] of groupMap) { groups.push({group, bindings: bindingsInGroup}); } - // Sort the groups - return groups.sort((g1, g2) => { - const i1 = this.options.orderedGroups.indexOf(g1.group); - const i2 = this.options.orderedGroups.indexOf(g2.group); - if (i1 !== -1 || i2 !== -1) { - // Honor the group order - return i1 - i2; - } else { - // Neither group is in the pre-defined order - // Use alphabetical order instead so that `1-group` is invoked before - // `2-group` - return g1.group < g2.group ? -1 : g1.group > g2.group ? 1 : 0; - } - }); + return groups; } /** diff --git a/packages/rest/src/body-parsers/body-parser.ts b/packages/rest/src/body-parsers/body-parser.ts index e6d7cc6e5205..95b47448fe10 100644 --- a/packages/rest/src/body-parsers/body-parser.ts +++ b/packages/rest/src/body-parsers/body-parser.ts @@ -4,8 +4,10 @@ // License text available at https://opensource.org/licenses/MIT import { + compareByOrder, Constructor, Context, + filterByTag, inject, instantiateClass, } from '@loopback/context'; @@ -32,7 +34,7 @@ export class RequestBodyParser { readonly parsers: BodyParser[]; constructor( - @inject.tag(REQUEST_BODY_PARSER_TAG, {optional: true}) + @inject(filterByTag(REQUEST_BODY_PARSER_TAG), {optional: true}) parsers?: BodyParser[], @inject.context() private readonly ctx?: Context, ) { @@ -197,9 +199,7 @@ function isBodyParserClass( * @param parsers */ function sortParsers(parsers: BodyParser[]) { - return parsers.sort((a, b) => { - const i1 = builtinParsers.names.indexOf(a.name); - const i2 = builtinParsers.names.indexOf(b.name); - return i1 - i2; - }); + return parsers.sort((a, b) => + compareByOrder(a.name, b.name, builtinParsers.names), + ); }