From 2a1ccb409a889d8b30b03ddf3284c9e9d5554e27 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 19 Nov 2019 17:10:51 -0800 Subject: [PATCH] feat(context): make it possible to set source information for interceptions Some interceptors want to check invocationContext.source to decide if its logic should be applied. For example, an http access logger only cares about invocations from the rest layer to the first controller. This is also useful for metrics, tracing and logging. --- docs/site/Interceptors.md | 44 +++++++++++++++++++ .../interception-proxy.acceptance.ts | 19 +++++--- .../acceptance/interceptor.acceptance.ts | 32 +++++++++++++- .../__tests__/unit/resolution-session.unit.ts | 4 ++ packages/context/src/binding.ts | 8 +++- packages/context/src/interception-proxy.ts | 28 ++++++++++-- packages/context/src/interceptor.ts | 1 + packages/context/src/invocation.ts | 23 +++++++++- packages/context/src/resolution-session.ts | 4 ++ .../caching-interceptor.ts | 8 +++- .../unit/router/controller-route.unit.ts | 15 +++++++ .../unit/router/handler-route.unit.ts | 11 ++++- packages/rest/src/router/base-route.ts | 14 +++++- packages/rest/src/router/controller-route.ts | 6 ++- packages/rest/src/router/handler-route.ts | 10 ++++- 15 files changed, 207 insertions(+), 20 deletions(-) diff --git a/docs/site/Interceptors.md b/docs/site/Interceptors.md index 4cd4606f978b..589f44d304a7 100644 --- a/docs/site/Interceptors.md +++ b/docs/site/Interceptors.md @@ -514,6 +514,7 @@ bindings. It extends `Context` with additional properties as follows: (for instance methods) - `methodName` (`string`): Method name - `args` (`InvocationArgs`, i.e., `any[]`): An array of arguments +- `source`: Source information about the invoker of the invocation ```ts /** @@ -533,6 +534,7 @@ export class InvocationContext extends Context { public readonly target: object, public readonly methodName: string, public readonly args: InvocationArgs, // any[] + public readonly source?: InvocationSource, ) { super(parent); } @@ -542,6 +544,48 @@ export class InvocationContext extends Context { It's possible for an interceptor to mutate items in the `args` array to pass in transformed input to downstream interceptors and the target method. +### Source for an invocation + +The `source` property of `InvocationContext` is defined as `InvocationSource`: + +```ts +/** + * An interface to represent the caller of the invocation + */ +export interface InvocationSource { + /** + * Type of the invoker, such as `proxy` and `route` + */ + readonly type: string; + /** + * Metadata for the source, such as `ResolutionSession` + */ + readonly value: T; +} +``` + +The `source` describes the caller that invokes a method with interceptors. +Interceptors can be invoked in the following cases: + +1. A route to a controller method + + - The source describes the REST Route + +2. A controller to a repository/service with injected proxy + + - The source describes a ResolutionSession that tracks a stack of bindings + and injections + +3. A controller/repository/service method invoked explicitly with + `invokeMethodWithInterceptors()` or `invokeMethod` + + - The source can be set by the caller of `invokeMethodWithInterceptors()` or + `invokeMethod` + +The implementation of an interceptor can check `source` to decide if its logic +should apply. For example, a global interceptor that provides caching for REST +APIs should only run if the source is from a REST Route. + ### Logic around `next` An interceptor will receive the `next` parameter, which is a function to execute diff --git a/packages/context/src/__tests__/acceptance/interception-proxy.acceptance.ts b/packages/context/src/__tests__/acceptance/interception-proxy.acceptance.ts index 1659fa933ea3..707e9979ac40 100644 --- a/packages/context/src/__tests__/acceptance/interception-proxy.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/interception-proxy.acceptance.ts @@ -11,6 +11,7 @@ import { inject, intercept, Interceptor, + ResolutionSession, ValueOrPromise, } from '../..'; @@ -156,8 +157,8 @@ describe('Interception proxy', () => { expect(msg).to.equal('Hello, JOHN'); expect(events).to.eql([ 'convertName: before-greet', - 'log: before-greet', - 'log: after-greet', + 'log: [my-controller] before-greet', + 'log: [my-controller] after-greet', 'convertName: after-greet', ]); }); @@ -198,8 +199,8 @@ describe('Interception proxy', () => { expect(msg).to.equal('Hello, JOHN'); expect(events).to.eql([ 'convertName: before-greet', - 'log: before-greet', - 'log: after-greet', + 'log: [dummy-controller --> my-controller] before-greet', + 'log: [dummy-controller --> my-controller] after-greet', 'convertName: after-greet', ]); }); @@ -207,9 +208,15 @@ describe('Interception proxy', () => { let events: string[]; const log: Interceptor = async (invocationCtx, next) => { - events.push('log: before-' + invocationCtx.methodName); + let source: string; + if (invocationCtx.source instanceof ResolutionSession) { + source = `[${invocationCtx.source.getBindingPath()}] `; + } else { + source = invocationCtx.source ? `[${invocationCtx.source}] ` : ''; + } + events.push(`log: ${source}before-${invocationCtx.methodName}`); const result = await next(); - events.push('log: after-' + invocationCtx.methodName); + events.push(`log: ${source}after-${invocationCtx.methodName}`); return result; }; diff --git a/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts b/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts index 685d8759bf53..d39b8b56b018 100644 --- a/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/interceptor.acceptance.ts @@ -419,6 +419,33 @@ describe('Interceptor', () => { }).to.throw(/skipInterceptors is not allowed/); }); + it('can set source information', async () => { + const controller = givenController(); + ctx.bind('name').to('Jane'); + const source = { + type: 'path', + value: 'rest', + toString: () => 'path:rest', + }; + const msg = await invokeMethodWithInterceptors( + ctx, + controller, + 'greetWithDI', + // No name is passed in here as it will be provided by the injection + [], + { + source, + skipParameterInjection: false, + }, + ); + // `Jane` is bound to `name` in the current context + expect(msg).to.equal('Hello, Jane'); + expect(events).to.eql([ + 'log: [path:rest] before-greetWithDI', + 'log: [path:rest] after-greetWithDI', + ]); + }); + function givenController() { class MyController { // Apply `log` to an async instance method with parameter injection @@ -682,9 +709,10 @@ describe('Interceptor', () => { }; const log: Interceptor = async (invocationCtx, next) => { - events.push('log: before-' + invocationCtx.methodName); + const source = invocationCtx.source ? `[${invocationCtx.source}] ` : ''; + events.push(`log: ${source}before-${invocationCtx.methodName}`); const result = await next(); - events.push('log: after-' + invocationCtx.methodName); + events.push(`log: ${source}after-${invocationCtx.methodName}`); return result; }; diff --git a/packages/context/src/__tests__/unit/resolution-session.unit.ts b/packages/context/src/__tests__/unit/resolution-session.unit.ts index 19e95a7ccdf8..2aec9775e52f 100644 --- a/packages/context/src/__tests__/unit/resolution-session.unit.ts +++ b/packages/context/src/__tests__/unit/resolution-session.unit.ts @@ -78,6 +78,10 @@ describe('ResolutionSession', () => { 'a --> @MyController.constructor[0] --> b', ); + expect(session.toString()).to.eql( + 'a --> @MyController.constructor[0] --> b', + ); + expect(session.popBinding()).to.be.exactly(bindingB); expect(session.popInjection()).to.be.exactly(injection); expect(session.popBinding()).to.be.exactly(bindingA); diff --git a/packages/context/src/binding.ts b/packages/context/src/binding.ts index c18f1c7af5fd..a5a478f00178 100644 --- a/packages/context/src/binding.ts +++ b/packages/context/src/binding.ts @@ -532,7 +532,11 @@ export class Binding { this._setValueGetter((ctx, options) => { const instOrPromise = instantiateClass(ctor, ctx, options.session); if (!options.asProxyWithInterceptors) return instOrPromise; - return createInterceptionProxyFromInstance(instOrPromise, ctx); + return createInterceptionProxyFromInstance( + instOrPromise, + ctx, + options.session, + ); }); this._valueConstructor = ctor; return this; @@ -640,6 +644,7 @@ export class Binding { function createInterceptionProxyFromInstance( instOrPromise: ValueOrPromise, context: Context, + session?: ResolutionSession, ) { return transformValueOrPromise(instOrPromise, inst => { if (typeof inst !== 'object') return inst; @@ -647,6 +652,7 @@ function createInterceptionProxyFromInstance( // Cast inst from `T` to `object` (inst as unknown) as object, context, + session, ) as unknown) as T; }); } diff --git a/packages/context/src/interception-proxy.ts b/packages/context/src/interception-proxy.ts index 4d6eea8dc603..c6a11f633dd9 100644 --- a/packages/context/src/interception-proxy.ts +++ b/packages/context/src/interception-proxy.ts @@ -5,7 +5,8 @@ import {Context} from './context'; import {invokeMethodWithInterceptors} from './interceptor'; -import {InvocationArgs} from './invocation'; +import {InvocationArgs, InvocationSource} from './invocation'; +import {ResolutionSession} from './resolution-session'; import {ValueOrPromise} from './value-promise'; /** @@ -57,13 +58,29 @@ export type AsInterceptedFunction = T extends ( */ export type AsyncProxy = {[P in keyof T]: AsInterceptedFunction}; +/** + * Invocation source for injected proxies. It wraps a snapshot of the + * `ResolutionSession` that tracks the binding/injection stack. + */ +export class ProxySource implements InvocationSource { + type = 'proxy'; + constructor(readonly value: ResolutionSession) {} + + toString() { + return this.value.getBindingPath(); + } +} + /** * A proxy handler that applies interceptors * * See https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Proxy */ export class InterceptionHandler implements ProxyHandler { - constructor(private context = new Context()) {} + constructor( + private context = new Context(), + private session?: ResolutionSession, + ) {} get(target: T, propertyName: PropertyKey, receiver: unknown) { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -77,6 +94,7 @@ export class InterceptionHandler implements ProxyHandler { target, propertyName, args, + {source: this.session && new ProxySource(this.session)}, ); }; } else { @@ -93,6 +111,10 @@ export class InterceptionHandler implements ProxyHandler { export function createProxyWithInterceptors( target: T, context?: Context, + session?: ResolutionSession, ): AsyncProxy { - return new Proxy(target, new InterceptionHandler(context)) as AsyncProxy; + return new Proxy( + target, + new InterceptionHandler(context, ResolutionSession.fork(session)), + ) as AsyncProxy; } diff --git a/packages/context/src/interceptor.ts b/packages/context/src/interceptor.ts index 0d2a945b9dc4..94f37dec4a48 100644 --- a/packages/context/src/interceptor.ts +++ b/packages/context/src/interceptor.ts @@ -305,6 +305,7 @@ export function invokeMethodWithInterceptors( target, methodName, args, + options.source, ); invocationCtx.assertMethodExists(); diff --git a/packages/context/src/invocation.ts b/packages/context/src/invocation.ts index 9fbe7c461214..93ca105d91b9 100644 --- a/packages/context/src/invocation.ts +++ b/packages/context/src/invocation.ts @@ -26,6 +26,20 @@ export type InvocationResult = any; // eslint-disable-next-line @typescript-eslint/no-explicit-any export type InvocationArgs = any[]; +/** + * An interface to represent the caller of the invocation + */ +export interface InvocationSource { + /** + * Type of the invoker, such as `proxy` and `route` + */ + readonly type: string; + /** + * Metadata for the source, such as `ResolutionSession` + */ + readonly value: T; +} + /** * InvocationContext represents the context to invoke interceptors for a method. * The context can be used to access metadata about the invocation as well as @@ -47,6 +61,7 @@ export class InvocationContext extends Context { public readonly target: object, public readonly methodName: string, public readonly args: InvocationArgs, + public readonly source?: InvocationSource, ) { super(parent); } @@ -71,7 +86,8 @@ export class InvocationContext extends Context { * Description of the invocation */ get description() { - return `InvocationContext(${this.name}): ${this.targetName}`; + const source = this.source == null ? '' : `${this.source} => `; + return `InvocationContext(${this.name}): ${source}${this.targetName}`; } toString() { @@ -129,6 +145,11 @@ export type InvocationOptions = { * Skip invocation of interceptors */ skipInterceptors?: boolean; + /** + * Information about the source object that makes the invocation. For REST, + * it's a `Route`. For injected proxies, it's a `Binding`. + */ + source?: InvocationSource; }; /** diff --git a/packages/context/src/resolution-session.ts b/packages/context/src/resolution-session.ts index a5a29fae1cc8..79fef0a3c2b9 100644 --- a/packages/context/src/resolution-session.ts +++ b/packages/context/src/resolution-session.ts @@ -321,6 +321,10 @@ export class ResolutionSession { getResolutionPath() { return this.stack.map(i => ResolutionSession.describe(i)).join(' --> '); } + + toString() { + return this.getResolutionPath(); + } } /** diff --git a/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts b/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts index 2fcbe90ac427..9270db8fdb1c 100644 --- a/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts +++ b/packages/rest/src/__tests__/acceptance/caching-interceptor/caching-interceptor.ts @@ -10,7 +10,7 @@ import { Provider, ValueOrPromise, } from '@loopback/context'; -import {Request, RestBindings} from '../../..'; +import {Request, RestBindings, RouteSource} from '../../..'; /** * Execution status @@ -61,6 +61,12 @@ export async function cache( next: () => ValueOrPromise, ) { status.returnFromCache = false; + if ( + invocationCtx.source == null || + !(invocationCtx.source instanceof RouteSource) + ) { + return next(); + } const req = await invocationCtx.get(RestBindings.Http.REQUEST, { optional: true, }); diff --git a/packages/rest/src/__tests__/unit/router/controller-route.unit.ts b/packages/rest/src/__tests__/unit/router/controller-route.unit.ts index f595c6906a48..846e0990a3d0 100644 --- a/packages/rest/src/__tests__/unit/router/controller-route.unit.ts +++ b/packages/rest/src/__tests__/unit/router/controller-route.unit.ts @@ -12,6 +12,7 @@ import { createControllerFactoryForBinding, createControllerFactoryForClass, RestBindings, + RouteSource, } from '../../..'; describe('ControllerRoute', () => { @@ -87,6 +88,20 @@ describe('ControllerRoute', () => { expect(route._controllerName).to.eql('my-controller'); }); + it('implements toString', () => { + const spec = anOperationSpec().build(); + const route = new MyRoute( + 'get', + '/greet', + spec, + MyController, + myControllerFactory, + 'greet', + ); + expect(route.toString()).to.equal('MyRoute - get /greet'); + expect(new RouteSource(route).toString()).to.equal('get /greet'); + }); + describe('updateBindings()', () => { let appCtx: Context; let requestCtx: Context; diff --git a/packages/rest/src/__tests__/unit/router/handler-route.unit.ts b/packages/rest/src/__tests__/unit/router/handler-route.unit.ts index b1eeb70d129f..a46f17b1a7be 100644 --- a/packages/rest/src/__tests__/unit/router/handler-route.unit.ts +++ b/packages/rest/src/__tests__/unit/router/handler-route.unit.ts @@ -6,7 +6,7 @@ import {Context} from '@loopback/core'; import {anOperationSpec} from '@loopback/openapi-spec-builder'; import {expect} from '@loopback/testlab'; -import {RestBindings, Route} from '../../..'; +import {RestBindings, Route, RouteSource} from '../../..'; describe('HandlerRoute', () => { describe('updateBindings()', () => { @@ -31,4 +31,13 @@ describe('HandlerRoute', () => { route.updateBindings(requestCtx); } }); + + describe('toString', () => { + it('implements toString', () => { + const spec = anOperationSpec().build(); + const route = new Route('get', '/greet', spec, () => {}); + expect(route.toString()).to.equal('Route - get /greet'); + expect(new RouteSource(route).toString()).to.equal('get /greet'); + }); + }); }); diff --git a/packages/rest/src/router/base-route.ts b/packages/rest/src/router/base-route.ts index cc2a8175e2f2..8618f0ca5845 100644 --- a/packages/rest/src/router/base-route.ts +++ b/packages/rest/src/router/base-route.ts @@ -3,7 +3,7 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {Context} from '@loopback/context'; +import {Context, InvocationSource} from '@loopback/context'; import {OperationObject} from '@loopback/openapi-v3'; import {OperationArgs, OperationRetval} from '../types'; import {RouteEntry} from './route-entry'; @@ -38,4 +38,16 @@ export abstract class BaseRoute implements RouteEntry { describe(): string { return `"${this.verb} ${this.path}"`; } + + toString() { + return `${this.constructor.name} - ${this.verb} ${this.path}`; + } +} + +export class RouteSource implements InvocationSource { + type = 'route'; + constructor(readonly value: RouteEntry) {} + toString() { + return `${this.value.verb} ${this.value.path}`; + } } diff --git a/packages/rest/src/router/controller-route.ts b/packages/rest/src/router/controller-route.ts index 49375052da5c..ef1f1fadeb4b 100644 --- a/packages/rest/src/router/controller-route.ts +++ b/packages/rest/src/router/controller-route.ts @@ -16,7 +16,7 @@ import {OperationObject} from '@loopback/openapi-v3'; import * as HttpErrors from 'http-errors'; import {RestBindings} from '../keys'; import {OperationArgs, OperationRetval} from '../types'; -import {BaseRoute} from './base-route'; +import {BaseRoute, RouteSource} from './base-route'; /* * A controller instance with open properties/methods @@ -138,7 +138,9 @@ export class ControllerRoute extends BaseRoute { ); } // Invoke the method with dependency injection - return invokeMethod(controller, this._methodName, requestContext, args); + return invokeMethod(controller, this._methodName, requestContext, args, { + source: new RouteSource(this), + }); } } diff --git a/packages/rest/src/router/handler-route.ts b/packages/rest/src/router/handler-route.ts index b13c7f87e69a..d4e23a923332 100644 --- a/packages/rest/src/router/handler-route.ts +++ b/packages/rest/src/router/handler-route.ts @@ -7,7 +7,7 @@ import {Context, invokeMethodWithInterceptors} from '@loopback/context'; import {OperationObject} from '@loopback/openapi-v3'; import {RestBindings} from '../keys'; import {OperationArgs, OperationRetval} from '../types'; -import {BaseRoute} from './base-route'; +import {BaseRoute, RouteSource} from './base-route'; export class Route extends BaseRoute { constructor( @@ -33,6 +33,12 @@ export class Route extends BaseRoute { ): Promise { // Use `invokeMethodWithInterceptors` to invoke the handler function so // that global interceptors are applied - return invokeMethodWithInterceptors(requestContext, this, '_handler', args); + return invokeMethodWithInterceptors( + requestContext, + this, + '_handler', + args, + {source: new RouteSource(this)}, + ); } }