fix(context): inject nested properties (#587)

Fix the implementation of `@inject` resolver to correctly support
nested properties.

Simplify the implementation of nested properties to avoid edge cases
that are difficult to support:

 - All Context methods creating/returning full Binding instances require
   a key without property-path suffix.

 - Only `get` and `getSync`, which are (eventually) returning the bound
   value only, allow property-path suffix.

 - A new method `getValueOrPromise` is introduced, this is an internal
   method shared between `get`, `getSync` and
   `@inject` (`instantiateClass`). This method supports keys with
   property-path suffix too.

Author: bajtos

packages/context/src/binding.ts

@@ -97,31 +97,31 @@ export class Binding {
   static validateKey(key: string) {
     if (!key) throw new Error('Binding key must be provided.');
     if (key.includes(Binding.PROPERTY_SEPARATOR)) {
-      throw new Error(`Binding key ${key} cannot contain`
-        + ` '${Binding.PROPERTY_SEPARATOR}'.`);
+      throw new Error(
+        `Binding key ${key} cannot contain` +
+          ` '${Binding.PROPERTY_SEPARATOR}'.`,
+      );
     }
     return key;
   }
 
   /**
-   * Remove the segament that denotes a property path
-   * @param key Binding key, such as `a, a.b, a:b, a/b, a.b#x, a:b#x.y, a/b#x.y`
+   * Parse a string containing both the binding key and the path to the deeply
+   * nested property to retrieve.
+   *
+   * @param keyWithPath The key with an optional path,
+   *  e.g. "application.instance" or "config#rest.port".
    */
-  static normalizeKey(key: string) {
-    const index = key.indexOf(Binding.PROPERTY_SEPARATOR);
-    if (index !== -1) key = key.substr(0, index);
-    key = key.trim();
-    return key;
-  }
+  static parseKeyWithPath(keyWithPath: string) {
+    const index = keyWithPath.indexOf(Binding.PROPERTY_SEPARATOR);
+    if (index === -1) {
+      return {key: keyWithPath, path: undefined};
+    }
 
-  /**
-   * Get the property path separated by `#`
-   * @param key Binding key
-   */
-  static getKeyPath(key: string) {
-    const index = key.indexOf(Binding.PROPERTY_SEPARATOR);
-    if (index !== -1) return key.substr(index + 1);
-    return undefined;
+    return {
+      key: keyWithPath.substr(0, index).trim(),
+      path: keyWithPath.substr(index+1),
+    };
   }
 
   public readonly key: string;
@@ -145,8 +145,10 @@ export class Binding {
    * @param ctx The current context
    * @param result The calculated value for the binding
    */
-  private _cacheValue(ctx: Context, result: BoundValue | Promise<BoundValue>):
-    BoundValue | Promise<BoundValue> {
+  private _cacheValue(
+    ctx: Context,
+    result: BoundValue | Promise<BoundValue>,
+  ): BoundValue | Promise<BoundValue> {
     if (isPromise(result)) {
       if (this.scope === BindingScope.SINGLETON) {
         // Cache the value

packages/context/src/context.ts

@@ -3,7 +3,7 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {Binding, BoundValue} from './binding';
+import {Binding, BoundValue, ValueOrPromise} from './binding';
 import {inject} from './inject';
 import {isPromise} from './is-promise';
 
@@ -16,7 +16,6 @@ export class Context {
 
   bind(key: string): Binding {
     Binding.validateKey(key);
-    key = Binding.normalizeKey(key);
     const keyExists = this.registry.has(key);
     if (keyExists) {
       const existingBinding = this.registry.get(key);
@@ -31,15 +30,15 @@ export class Context {
   }
 
   contains(key: string): boolean {
-    key = Binding.normalizeKey(key);
+    Binding.validateKey(key);
     return this.registry.has(key);
   }
 
   find(pattern?: string): Binding[] {
     let bindings: Binding[] = [];
     if (pattern) {
       // TODO(@superkhau): swap with production grade glob to regex lib
-      pattern = Binding.normalizeKey(pattern);
+      Binding.validateKey(pattern);
       const glob = new RegExp('^' + pattern.split('*').join('.*') + '$');
       this.registry.forEach(binding => {
         const isMatch = glob.test(binding.key);
@@ -76,32 +75,71 @@ export class Context {
     return childList.concat(additions);
   }
 
+  /**
+   * Get the value bound to the given key, optionally return a (deep) property
+   * of the bound value.
+   *
+   * @example
+   *
+   * ```ts
+   * // get the value bound to "application.instance"
+   * const app = await ctx.get('application.instance');
+   *
+   * // get "rest" property from the value bound to "config"
+   * const config = await ctx.getValueOrPromise('config#rest');
+   *
+   * // get "a" property of "numbers" property from the value bound to "data"
+   * ctx.bind('data').to({numbers: {a: 1, b: 2}, port: 3000});
+   * const a = await ctx.get('data#numbers.a');
+   * ```
+   *
+   * @param keyWithPath The binding key, optionally suffixed with a path to the
+   *   (deeply) nested property to retrieve.
+   * @returns A promise of the bound value.
+   */
   get(key: string): Promise<BoundValue> {
     try {
-      const path = Binding.getKeyPath(key);
-      const binding = this.getBinding(key);
-      return Promise.resolve(binding.getValue(this)).then(
-        val => getValue(val, path));
+      return Promise.resolve(this.getValueOrPromise(key));
     } catch (err) {
       return Promise.reject(err);
     }
   }
 
+  /**
+   * Get the synchronous value bound to the given key, optionally
+   * return a (deep) property of the bound value.
+   *
+   * This method throws an error if the bound value requires async computation
+   * (returns a promise). You should never rely on sync bindings in production
+   * code.
+   *
+   * @example
+   *
+   * ```ts
+   * // get the value bound to "application.instance"
+   * const app = ctx.get('application.instance');
+   *
+   * // get "rest" property from the value bound to "config"
+   * const config = ctx.getValueOrPromise('config#rest');
+   * ```
+   *
+   * @param keyWithPath The binding key, optionally suffixed with a path to the
+   *   (deeply) nested property to retrieve.
+   * @returns A promise of the bound value.
+   */
   getSync(key: string): BoundValue {
-    const path = Binding.getKeyPath(key);
-    const binding = this.getBinding(key);
-    const valueOrPromise = binding.getValue(this);
+    const valueOrPromise = this.getValueOrPromise(key);
 
     if (isPromise(valueOrPromise)) {
       throw new Error(
         `Cannot get ${key} synchronously: the value is a promise`);
     }
 
-    return getValue(valueOrPromise, path);
+    return valueOrPromise;
   }
 
   getBinding(key: string): Binding {
-    key = Binding.normalizeKey(key);
+    Binding.validateKey(key);
     const binding = this.registry.get(key);
     if (binding) {
       return binding;
@@ -113,22 +151,55 @@ export class Context {
 
     throw new Error(`The key ${key} was not bound to any value.`);
   }
+
+  /**
+   * Get the value bound to the given key.
+   *
+   * This is an internal version that preserves the dual sync/async result
+   * of `Binding#getValue()`. Users should use `get()` or `getSync()` instead.
+   *
+   * @example
+   *
+   * ```ts
+   * // get the value bound to "application.instance"
+   * ctx.getValueOrPromise('application.instance');
+   *
+   * // get "rest" property from the value bound to "config"
+   * ctx.getValueOrPromise('config#rest');
+   *
+   * // get "a" property of "numbers" property from the value bound to "data"
+   * ctx.bind('data').to({numbers: {a: 1, b: 2}, port: 3000});
+   * ctx.getValueOrPromise('data#numbers.a');
+   * ```
+   *
+   * @param keyWithPath The binding key, optionally suffixed with a path to the
+   *   (deeply) nested property to retrieve.
+   * @returns The bound value or a promise of the bound value, depending
+   *   on how the binding was configured.
+   * @internal
+   */
+  getValueOrPromise(keyWithPath: string): ValueOrPromise<BoundValue> {
+    const {key, path} = Binding.parseKeyWithPath(keyWithPath);
+    const boundValue = this.getBinding(key).getValue(this);
+    if (path === undefined || path === '') {
+      return boundValue;
+    }
+
+    if (isPromise(boundValue)) {
+      return boundValue.then(v => getDeepProperty(v, path));
+    }
+
+    return getDeepProperty(boundValue, path);
+  }
 }
 
-/**
- * Get the value by `.` notation
- * @param obj The source value
- * @param path A path to the nested property, such as `x`, `x.y`, `x.length`,
- * or `x.0`
- */
-function getValue(obj: BoundValue, path?: string): BoundValue {
-  if (!path) return obj;
+function getDeepProperty(value: BoundValue, path: string) {
   const props = path.split('.');
-  let val = undefined;
   for (const p of props) {
-    val = obj[p];
-    if (val == null) return val;
-    obj = val;
+    value = value[p];
+    if (value === undefined || value === null) {
+      return value;
+    }
   }
-  return val;
+  return value;
 }

packages/context/src/resolver.ts

@@ -75,8 +75,7 @@ function resolve<T>(ctx: Context, injection: Injection): ValueOrPromise<T> {
     return injection.resolve(ctx, injection);
   }
   // Default to resolve the value from the context by binding key
-  const binding = ctx.getBinding(injection.bindingKey);
-  return binding.getValue(ctx);
+  return ctx.getValueOrPromise(injection.bindingKey);
 }
 
 /**

packages/context/test/acceptance/class-level-bindings.ts

@@ -171,6 +171,21 @@ describe('Context bindings - Injecting dependencies of classes', () => {
     expect(ctx.getSync('key')).to.equal('a-value');
   });
 
+  it('injects a nested property', async () => {
+    class TestComponent {
+      constructor(
+        @inject('config#test')
+        public config: string,
+      ) {}
+    }
+
+    ctx.bind('config').to({test: 'test-config'});
+    ctx.bind('component').toClass(TestComponent);
+
+    const resolved = await ctx.get('component');
+    expect(resolved.config).to.equal('test-config');
+  });
+
   function createContext() {
     ctx = new Context();
   }