feat(context): enable detection of circular dependencies

Author: raymondfeng

packages/context/src/binding.ts

@@ -4,7 +4,7 @@
 // License text available at https://opensource.org/licenses/MIT
 
 import {Context} from './context';
-import {Constructor, instantiateClass} from './resolver';
+import {Constructor, instantiateClass, ResolutionSession} from './resolver';
 import {isPromise} from './is-promise';
 import {Provider} from './provider';
 
@@ -147,7 +147,10 @@ export class Binding {
   public type: BindingType;
 
   private _cache: BoundValue;
-  private _getValue: (ctx?: Context) => BoundValue | Promise<BoundValue>;
+  private _getValue: (
+    ctx?: Context,
+    session?: ResolutionSession,
+  ) => BoundValue | Promise<BoundValue>;
 
   // For bindings bound via toClass, this property contains the constructor
   // function
@@ -224,8 +227,14 @@ export class Binding {
    *   doSomething(result);
    * }
    * ```
+   *
+   * @param ctx Context for the resolution
+   * @param session Optional session for binding and dependency resolution
    */
-  getValue(ctx: Context): BoundValue | Promise<BoundValue> {
+  getValue(
+    ctx: Context,
+    session?: ResolutionSession,
+  ): BoundValue | Promise<BoundValue> {
     // First check cached value for non-transient
     if (this._cache !== undefined) {
       if (this.scope === BindingScope.SINGLETON) {
@@ -237,7 +246,22 @@ export class Binding {
       }
     }
     if (this._getValue) {
-      const result = this._getValue(ctx);
+      const resolutionSession = ResolutionSession.enterBinding(this, session);
+      const result = this._getValue(ctx, resolutionSession);
+      if (isPromise(result)) {
+        if (result instanceof Promise) {
+          result.catch(err => {
+            resolutionSession.exit();
+            return Promise.reject(err);
+          });
+        }
+        result.then(val => {
+          resolutionSession.exit();
+          return val;
+        });
+      } else {
+        resolutionSession.exit();
+      }
       return this._cacheValue(ctx, result);
     }
     return Promise.reject(
@@ -347,10 +371,11 @@ export class Binding {
    */
   public toProvider<T>(providerClass: Constructor<Provider<T>>): this {
     this.type = BindingType.PROVIDER;
-    this._getValue = ctx => {
+    this._getValue = (ctx, session) => {
       const providerOrPromise = instantiateClass<Provider<T>>(
         providerClass,
         ctx!,
+        session,
       );
       if (isPromise(providerOrPromise)) {
         return providerOrPromise.then(p => p.value());
@@ -370,7 +395,7 @@ export class Binding {
    */
   toClass<T>(ctor: Constructor<T>): this {
     this.type = BindingType.CLASS;
-    this._getValue = ctx => instantiateClass(ctor, ctx!);
+    this._getValue = (ctx, session) => instantiateClass(ctor, ctx!, session);
     this.valueConstructor = ctor;
     return this;
   }

packages/context/src/context.ts

@@ -5,6 +5,7 @@
 
 import {Binding, BoundValue, ValueOrPromise} from './binding';
 import {isPromise} from './is-promise';
+import {ResolutionSession} from './resolver';
 
 /**
  * Context provides an implementation of Inversion of Control (IoC) container
@@ -143,9 +144,9 @@ export class Context {
    *   (deeply) nested property to retrieve.
    * @returns A promise of the bound value.
    */
-  get(key: string): Promise<BoundValue> {
+  get(key: string, session?: ResolutionSession): Promise<BoundValue> {
     try {
-      return Promise.resolve(this.getValueOrPromise(key));
+      return Promise.resolve(this.getValueOrPromise(key, session));
     } catch (err) {
       return Promise.reject(err);
     }
@@ -173,8 +174,8 @@ export class Context {
    *   (deeply) nested property to retrieve.
    * @returns A promise of the bound value.
    */
-  getSync(key: string): BoundValue {
-    const valueOrPromise = this.getValueOrPromise(key);
+  getSync(key: string, session?: ResolutionSession): BoundValue {
+    const valueOrPromise = this.getValueOrPromise(key, session);
 
     if (isPromise(valueOrPromise)) {
       throw new Error(
@@ -227,13 +228,17 @@ export class Context {
    *
    * @param keyWithPath The binding key, optionally suffixed with a path to the
    *   (deeply) nested property to retrieve.
+   * @param session An object to keep states of the resolution
    * @returns The bound value or a promise of the bound value, depending
    *   on how the binding was configured.
    * @internal
    */
-  getValueOrPromise(keyWithPath: string): ValueOrPromise<BoundValue> {
+  getValueOrPromise(
+    keyWithPath: string,
+    session?: ResolutionSession,
+  ): ValueOrPromise<BoundValue> {
     const {key, path} = Binding.parseKeyWithPath(keyWithPath);
-    const boundValue = this.getBinding(key).getValue(this);
+    const boundValue = this.getBinding(key).getValue(this, session);
     if (path === undefined || path === '') {
       return boundValue;
     }

packages/context/src/inject.ts

@@ -11,6 +11,8 @@ import {
 } from '@loopback/metadata';
 import {BoundValue, ValueOrPromise} from './binding';
 import {Context} from './context';
+import {isPromise} from './is-promise';
+import {ResolutionSession} from './resolver';
 
 const PARAMETERS_KEY = 'inject:parameters';
 const PROPERTIES_KEY = 'inject:properties';
@@ -19,7 +21,11 @@ const PROPERTIES_KEY = 'inject:properties';
  * A function to provide resolution of injected values
  */
 export interface ResolverFunction {
-  (ctx: Context, injection: Injection): ValueOrPromise<BoundValue>;
+  (
+    ctx: Context,
+    injection: Injection,
+    session?: ResolutionSession,
+  ): ValueOrPromise<BoundValue>;
 }
 
 /**
@@ -168,12 +174,14 @@ export namespace inject {
 }
 
 function resolveAsGetter(ctx: Context, injection: Injection) {
+  // No resolution session should be propagated into the getter
   return function getter() {
     return ctx.get(injection.bindingKey);
   };
 }
 
 function resolveAsSetter(ctx: Context, injection: Injection) {
+  // No resolution session should be propagated into the setter
   return function setter(value: BoundValue) {
     ctx.bind(injection.bindingKey).to(value);
   };

packages/context/src/resolver.ts

@@ -4,7 +4,7 @@
 // License text available at https://opensource.org/licenses/MIT
 
 import {Context} from './context';
-import {BoundValue, ValueOrPromise} from './binding';
+import {BoundValue, ValueOrPromise, Binding} from './binding';
 import {isPromise} from './is-promise';
 import {
   describeInjectedArguments,
@@ -20,6 +20,68 @@ export type Constructor<T> =
   // tslint:disable-next-line:no-any
   new (...args: any[]) => T;
 
+/**
+ * Object to keep states for a session to resolve bindings and their
+ * dependencies within a context
+ */
+export class ResolutionSession {
+  /**
+   * A stack of bindings for the current resolution session. It's used to track
+   * the path of dependency resolution and detect circular dependencies.
+   */
+  readonly bindings: Binding[] = [];
+
+  /**
+   * Start to resolve a binding within the session
+   * @param binding Binding
+   * @param session Resolution session
+   */
+  static enterBinding(
+    binding: Binding,
+    session?: ResolutionSession,
+  ): ResolutionSession {
+    session = session || new ResolutionSession();
+    session.enter(binding);
+    return session;
+  }
+
+  /**
+   * Getter for the current binding
+   */
+  get binding() {
+    return this.bindings[this.bindings.length - 1];
+  }
+
+  /**
+   * Enter the resolution of the given binding. If
+   * @param binding Binding
+   */
+  enter(binding: Binding) {
+    if (this.bindings.indexOf(binding) !== -1) {
+      throw new Error(
+        `Circular dependency detected for '${
+          binding.key
+        }' on path '${this.getBindingPath()}'`,
+      );
+    }
+    this.bindings.push(binding);
+  }
+
+  /**
+   * Exit the resolution of a binding
+   */
+  exit() {
+    return this.bindings.pop();
+  }
+
+  /**
+   * Get the binding path as `bindingA->bindingB->bindingC`.
+   */
+  getBindingPath() {
+    return this.bindings.map(b => b.key).join('->');
+  }
+}
+
 /**
  * Create an instance of a class which constructor has arguments
  * decorated with `@inject`.
@@ -29,16 +91,19 @@ export type Constructor<T> =
  *
  * @param ctor The class constructor to call.
  * @param ctx The context containing values for `@inject` resolution
+ * @param session Optional session for binding and dependency resolution
  * @param nonInjectedArgs Optional array of args for non-injected parameters
  */
 export function instantiateClass<T>(
   ctor: Constructor<T>,
   ctx: Context,
+  session?: ResolutionSession,
   // tslint:disable-next-line:no-any
   nonInjectedArgs?: any[],
 ): T | Promise<T> {
-  const argsOrPromise = resolveInjectedArguments(ctor, ctx, '');
-  const propertiesOrPromise = resolveInjectedProperties(ctor, ctx);
+  session = session || new ResolutionSession();
+  const argsOrPromise = resolveInjectedArguments(ctor, '', ctx, session);
+  const propertiesOrPromise = resolveInjectedProperties(ctor, ctx, session);
   let inst: T | Promise<T>;
   if (isPromise(argsOrPromise)) {
     // Instantiate the class asynchronously
@@ -72,14 +137,19 @@ export function instantiateClass<T>(
  * Resolve the value or promise for a given injection
  * @param ctx Context
  * @param injection Descriptor of the injection
+ * @param session Optional session for binding and dependency resolution
  */
-function resolve<T>(ctx: Context, injection: Injection): ValueOrPromise<T> {
+function resolve<T>(
+  ctx: Context,
+  injection: Injection,
+  session?: ResolutionSession,
+): ValueOrPromise<T> {
   if (injection.resolve) {
     // A custom resolve function is provided
-    return injection.resolve(ctx, injection);
+    return injection.resolve(ctx, injection, session);
   }
   // Default to resolve the value from the context by binding key
-  return ctx.getValueOrPromise(injection.bindingKey);
+  return ctx.getValueOrPromise(injection.bindingKey, session);
 }
 
 /**
@@ -92,16 +162,18 @@ function resolve<T>(ctx: Context, injection: Injection): ValueOrPromise<T> {
  *
  * @param target The class for constructor injection or prototype for method
  * injection
- * @param ctx The context containing values for `@inject` resolution
  * @param method The method name. If set to '', the constructor will
  * be used.
+ * @param ctx The context containing values for `@inject` resolution
+ * @param session Optional session for binding and dependency resolution
  * @param nonInjectedArgs Optional array of args for non-injected parameters
  */
 export function resolveInjectedArguments(
   // tslint:disable-next-line:no-any
   target: any,
-  ctx: Context,
   method: string,
+  ctx: Context,
+  session?: ResolutionSession,
   // tslint:disable-next-line:no-any
   nonInjectedArgs?: any[],
 ): BoundValue[] | Promise<BoundValue[]> {
@@ -137,7 +209,7 @@ export function resolveInjectedArguments(
       }
     }
 
-    const valueOrPromise = resolve(ctx, injection);
+    const valueOrPromise = resolve(ctx, injection, session);
     if (isPromise(valueOrPromise)) {
       if (!asyncResolvers) asyncResolvers = [];
       asyncResolvers.push(
@@ -173,8 +245,9 @@ export function invokeMethod(
 ): ValueOrPromise<BoundValue> {
   const argsOrPromise = resolveInjectedArguments(
     target,
-    ctx,
     method,
+    ctx,
+    undefined,
     nonInjectedArgs,
   );
   assert(typeof target[method] === 'function', `Method ${method} not found`);
@@ -189,11 +262,24 @@ export function invokeMethod(
 
 export type KV = {[p: string]: BoundValue};
 
+/**
+ * Given a class with properties decorated with `@inject`,
+ * return the map of properties resolved using the values
+ * bound in `ctx`.
+
+ * The function returns an argument array when all dependencies were
+ * resolved synchronously, or a Promise otherwise.
+ *
+ * @param constructor The class for which properties should be resolved.
+ * @param ctx The context containing values for `@inject` resolution
+ * @param session Optional session for binding and dependency resolution
+ */
 export function resolveInjectedProperties(
-  fn: Function,
+  constructor: Function,
   ctx: Context,
+  session?: ResolutionSession,
 ): KV | Promise<KV> {
-  const injectedProperties = describeInjectedProperties(fn.prototype);
+  const injectedProperties = describeInjectedProperties(constructor.prototype);
 
   const properties: KV = {};
   let asyncResolvers: Promise<void>[] | undefined = undefined;
@@ -205,11 +291,12 @@ export function resolveInjectedProperties(
     const injection = injectedProperties[p];
     if (!injection.bindingKey && !injection.resolve) {
       throw new Error(
-        `Cannot resolve injected property for class ${fn.name}: ` +
+        `Cannot resolve injected property for class ${constructor.name}: ` +
           `The property ${p} was not decorated for dependency injection.`,
       );
     }
-    const valueOrPromise = resolve(ctx, injection);
+
+    const valueOrPromise = resolve(ctx, injection, session);
     if (isPromise(valueOrPromise)) {
       if (!asyncResolvers) asyncResolvers = [];
       asyncResolvers.push(valueOrPromise.then(propertyResolver(p)));