chore(context): remove the hierarchical configuration resolution

loopbackio · Jan 26, 2019 · c388192 · c388192
1 parent fa71b6c
commit c388192
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 299 deletions.
diff --git a/packages/context/src/binding-config.ts b/packages/context/src/binding-config.ts
@@ -3,17 +3,10 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import * as debugFactory from 'debug';
 import {BindingAddress, BindingKey} from './binding-key';
 import {Context} from './context';
 import {ResolutionOptions} from './resolution-session';
-import {
-  resolveUntil,
-  transformValueOrPromise,
-  ValueOrPromise,
-} from './value-promise';
-
-const debug = debugFactory('loopback:context:binding-config');
+import {ValueOrPromise} from './value-promise';
 
 /**
  * Interface for configuration resolver
@@ -36,7 +29,7 @@ export interface ConfigurationResolver {
 /**
  * Resolver for configurations of bindings
  */
-export class SimpleConfigurationResolver implements ConfigurationResolver {
+export class DefaultConfigurationResolver implements ConfigurationResolver {
   constructor(public readonly context: Context) {}
 
   /**
@@ -62,132 +55,10 @@ export class SimpleConfigurationResolver implements ConfigurationResolver {
       configPath,
     );
 
-    return this.context.getValueOrPromise<ConfigValueType>(
-      configKey,
+    const options: ResolutionOptions = Object.assign(
+      {optional: true},
       resolutionOptions,
     );
-  }
-}
-
-/**
- * Resolver for configurations of bindings
- */
-export class HierarchicalConfigurationResolver
-  implements ConfigurationResolver {
-  constructor(public readonly context: Context) {}
-
-  /**
-   * Resolve config from the binding key hierarchy using namespaces
-   * separated by `.`
-   *
-   * For example, if the binding key is `servers.rest.server1`, we'll try the
-   * following entries:
-   * 1. servers.rest.server1:$config#host (namespace: server1)
-   * 2. servers.rest:$config#server1.host (namespace: rest)
-   * 3. servers.$config#rest.server1.host` (namespace: server)
-   * 4. $config#servers.rest.server1.host (namespace: '' - root)
-   *
-   * @param key Binding key with namespaces separated by `.`
-   * @param configPath Property path for the option. For example, `x.y`
-   * requests for `config.x.y`. If not set, the `config` object will be
-   * returned.
-   * @param resolutionOptions Options for the resolution.
-   * - localConfigOnly: if set to `true`, no parent namespaces will be checked
-   * - optional: if not set or set to `true`, `undefined` will be returned if
-   * no corresponding value is found. Otherwise, an error will be thrown.
-   */
-  getConfigAsValueOrPromise<ConfigValueType>(
-    key: BindingAddress<unknown>,
-    configPath?: string,
-    resolutionOptions?: ResolutionOptions,
-  ): ValueOrPromise<ConfigValueType | undefined> {
-    const env = resolutionOptions && resolutionOptions.environment;
-    configPath = configPath || '';
-    const configKey = BindingKey.create<ConfigValueType>(
-      BindingKey.buildKeyForConfig(key, env),
-      configPath,
-    );
-
-    const localConfigOnly =
-      resolutionOptions && resolutionOptions.localConfigOnly;
-
-    /**
-     * Set up possible keys to resolve the config value
-     */
-    key = key.toString();
-    const keys = [];
-    while (true) {
-      const configKeyAndPath = BindingKey.create<ConfigValueType>(
-        BindingKey.buildKeyForConfig(key, env),
-        configPath,
-      );
-      keys.push(configKeyAndPath);
-      if (env) {
-        // The `environment` is set, let's try the non env specific binding too
-        keys.push(
-          BindingKey.create<ConfigValueType>(
-            BindingKey.buildKeyForConfig(key),
-            configPath,
-          ),
-        );
-      }
-      if (!key || localConfigOnly) {
-        // No more keys
-        break;
-      }
-      // Shift last part of the key into the path as we'll try the parent
-      // namespace in the next iteration
-      const index = key.lastIndexOf('.');
-      configPath = configPath
-        ? `${key.substring(index + 1)}.${configPath}`
-        : `${key.substring(index + 1)}`;
-      key = key.substring(0, index);
-    }
-    /* istanbul ignore if */
-    if (debug.enabled) {
-      debug('Configuration keyWithPaths: %j', keys);
-    }
-
-    const resolveConfig = (keyWithPath: string) => {
-      // Set `optional` to `true` to resolve config locally
-      const options = Object.assign(
-        {}, // Make sure resolutionOptions is copied
-        resolutionOptions,
-        {optional: true}, // Force optional to be true
-      );
-      return this.context.getValueOrPromise<ConfigValueType>(
-        keyWithPath,
-        options,
-      );
-    };
-
-    const evaluateConfig = (keyWithPath: string, val: ConfigValueType) => {
-      /* istanbul ignore if */
-      if (debug.enabled) {
-        debug('Configuration keyWithPath: %s => value: %j', keyWithPath, val);
-      }
-      // Found the corresponding config
-      if (val !== undefined) return true;
-
-      if (localConfigOnly) {
-        return true;
-      }
-      return false;
-    };
-
-    const required = resolutionOptions && resolutionOptions.optional === false;
-    const valueOrPromise = resolveUntil<
-      BindingAddress<ConfigValueType>,
-      ConfigValueType
-    >(keys[Symbol.iterator](), resolveConfig, evaluateConfig);
-    return transformValueOrPromise<
-      ConfigValueType | undefined,
-      ConfigValueType | undefined
-    >(valueOrPromise, val => {
-      if (val === undefined && required) {
-        throw Error(`Configuration '${configKey}' cannot be resolved`);
-      }
-      return val;
-    });
+    return this.context.getValueOrPromise<ConfigValueType>(configKey, options);
   }
 }
diff --git a/packages/context/src/binding-key.ts b/packages/context/src/binding-key.ts
@@ -107,18 +107,19 @@ export class BindingKey<ValueType> {
   static CONFIG_NAMESPACE = '$config';
   /**
    * Build a binding key for the configuration of the given binding and env.
-   * The format is `$config.<env>:<key>`
+   * The format is `<key>:$config.<env>`
    *
    * @param key The binding key that accepts the configuration
    * @param env The environment such as `dev`, `test`, and `prod`
    */
-  static buildKeyForConfig<T>(key: BindingAddress<T> = '', env: string = '') {
-    const namespace = env
+  static buildKeyForConfig(
+    key: BindingAddress<unknown> = '',
+    env: string = '',
+  ) {
+    const suffix = env
       ? `${BindingKey.CONFIG_NAMESPACE}.${env}`
       : BindingKey.CONFIG_NAMESPACE;
-    const bindingKey = key
-      ? `${namespace}.${key}`
-      : BindingKey.CONFIG_NAMESPACE;
+    const bindingKey = key ? `${key}:${suffix}` : suffix;
     return bindingKey;
   }
 }
diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts
@@ -8,7 +8,7 @@ import {v1 as uuidv1} from 'uuid';
 import {Binding, BindingTag} from './binding';
 import {
   ConfigurationResolver,
-  HierarchicalConfigurationResolver,
+  DefaultConfigurationResolver,
 } from './binding-config';
 import {BindingFilter, filterByKey, filterByTag} from './binding-filter';
 import {BindingAddress, BindingKey} from './binding-key';
@@ -100,11 +100,11 @@ export class Context {
    * For example, `ctx.configure('controllers.MyController').to({x: 1})` will
    * create binding `controllers.MyController:$config` with value `{x: 1}`.
    *
-   * @param key The key for the binding that accepts the config
+   * @param key The key for the binding to be configured
    * @param env The env (such as `dev`, `test`, and `prod`) for the config
    */
   configure<ConfigValueType = BoundValue>(
-    key: BindingAddress<BoundValue> = '',
+    key: BindingAddress<unknown> = '',
     env: string = '',
   ): Binding<ConfigValueType> {
     const keyForConfig = BindingKey.buildKeyForConfig(key, env);
@@ -115,27 +115,18 @@ export class Context {
   }
 
   /**
-   * Resolve config from the binding key hierarchy using namespaces
-   * separated by `.`
+   * Get the value or promise of configuration for a given binding by key
    *
-   * For example, if the binding key is `servers.rest.server1`, we'll try the
-   * following entries:
-   * 1. servers.rest.server1:$config#host (namespace: server1)
-   * 2. servers.rest:$config#server1.host (namespace: rest)
-   * 3. servers.$config#rest.server1.host` (namespace: server)
-   * 4. $config#servers.rest.server1.host (namespace: '' - root)
-   *
-   * @param key Binding key with namespaces separated by `.`
+   * @param key Binding key
    * @param configPath Property path for the option. For example, `x.y`
-   * requests for `config.x.y`. If not set, the `config` object will be
+   * requests for `<config>.x.y`. If not set, the `<config>` object will be
    * returned.
    * @param resolutionOptions Options for the resolution.
-   * - localConfigOnly: if set to `true`, no parent namespaces will be checked
    * - optional: if not set or set to `true`, `undefined` will be returned if
    * no corresponding value is found. Otherwise, an error will be thrown.
    */
   getConfigAsValueOrPromise<ConfigValueType>(
-    key: BindingAddress<BoundValue>,
+    key: BindingAddress<unknown>,
     configPath?: string,
     resolutionOptions?: ResolutionOptions,
   ): ValueOrPromise<ConfigValueType | undefined> {
@@ -149,31 +140,22 @@ export class Context {
   private getConfigResolver() {
     if (!this.configResolver) {
       // TODO: Check bound ConfigurationResolver
-      this.configResolver = new HierarchicalConfigurationResolver(this);
+      this.configResolver = new DefaultConfigurationResolver(this);
     }
     return this.configResolver;
   }
 
   /**
-   * Resolve config from the binding key hierarchy using namespaces
-   * separated by `.`
+   * Resolve configuration for the binding by key
    *
-   * For example, if the binding key is `servers.rest.server1`, we'll try the
-   * following entries:
-   * 1. servers.rest.server1:$config#host (namespace: server1)
-   * 2. servers.rest:$config#server1.host (namespace: rest)
-   * 3. servers.$config#rest.server1.host` (namespace: server)
-   * 4. $config#servers.rest.server1.host (namespace: '' - root)
-   *
-   * @param key Binding key with namespaces separated by `.`
+   * @param key Binding key
    * @param configPath Property path for the option. For example, `x.y`
-   * requests for `config.x.y`. If not set, the `config` object will be
+   * requests for `<config>.x.y`. If not set, the `<config>` object will be
    * returned.
-   * @param resolutionOptions Options for the resolution. If `localConfigOnly` is
-   * set to true, no parent namespaces will be looked up.
+   * @param resolutionOptions Options for the resolution.
    */
   async getConfig<ConfigValueType>(
-    key: BindingAddress<BoundValue>,
+    key: BindingAddress<unknown>,
     configPath?: string,
     resolutionOptions?: ResolutionOptions,
   ): Promise<ConfigValueType | undefined> {
@@ -185,25 +167,17 @@ export class Context {
   }
 
   /**
-   * Resolve config synchronously from the binding key hierarchy using
-   * namespaces separated by `.`
+   * Resolve configuration synchronously for the binding by key
    *
-   * For example, if the binding key is `servers.rest.server1`, we'll try the
-   * following entries:
-   * 1. servers.rest.server1:$config#host (namespace: server1)
-   * 2. servers.rest:$config#server1.host (namespace: rest)
-   * 3. servers.$config#rest.server1.host` (namespace: server)
-   * 4. $config#servers.rest.server1.host (namespace: '' - root)
-   *
-   * @param key Binding key with namespaces separated by `.`
+   * @param key Binding key
    * @param configPath Property path for the option. For example, `x.y`
    * requests for `config.x.y`. If not set, the `config` object will be
    * returned.
    * @param resolutionOptions Options for the resolution. If `localConfigOnly`
    * is set to `true`, no parent namespaces will be looked up.
    */
   getConfigSync<ConfigValueType>(
-    key: BindingAddress<BoundValue>,
+    key: BindingAddress<unknown>,
     configPath?: string,
     resolutionOptions?: ResolutionOptions,
   ): ConfigValueType | undefined {

diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts
@@ -377,7 +377,6 @@ function resolveFromConfig(
   return ctx.getConfigAsValueOrPromise(binding.key, meta.configPath, {
     session,
     optional: meta.optional,
-    localConfigOnly: meta.localConfigOnly,
     environment: env,
   });
 }

diff --git a/packages/context/src/resolution-session.ts b/packages/context/src/resolution-session.ts
@@ -343,22 +343,6 @@ export interface ResolutionOptions {
    */
   optional?: boolean;
 
-  /**
-   * A boolean flag to control if the resolution only looks up properties from
-   * the local configuration of the target binding itself. If not set to `true`,
-   * all namespaces of a binding key will be checked.
-   *
-   * For example, if the binding key is `servers.rest.server1`, we'll try the
-   * following entries:
-   * 1. servers.rest.server1:$config#host (namespace: server1)
-   * 2. servers.rest:$config#server1.host (namespace: rest)
-   * 3. servers.$config#rest.server1.host` (namespace: server)
-   * 4. $config#servers.rest.server1.host (namespace: '' - root)
-   *
-   * The default value is `false`.
-   */
-  localConfigOnly?: boolean;
-
   /**
    * Environment for resolution, such as `dev`, `test`, `staging`, and `prod`
    */

diff --git a/packages/context/test/acceptance/config.ts → ...t/acceptance/binding-config.acceptance.ts b/packages/context/test/acceptance/config.ts → ...t/acceptance/binding-config.acceptance.ts
@@ -56,21 +56,6 @@ describe('Context bindings - injecting configuration for bound artifacts', () =>
     expect(server1.config).to.eql({port: 3000});
   });
 
-  it('configure values at parent level(s)', async () => {
-    const ctx = new Context();
-
-    // Bind configuration
-    ctx.configure('servers.rest').to({server1: {port: 3000}});
-
-    // Bind RestServer
-    ctx.bind('servers.rest.server1').toClass(RestServer);
-
-    // Resolve an instance of RestServer
-    // Expect server1.config to be `{port: 3000}
-    const server1 = await ctx.get<RestServer>('servers.rest.server1');
-    expect(server1.config).to.eql({port: 3000});
-  });
-
   it('binds configuration for environments', async () => {
     const ctx = new Context();
 
@@ -94,9 +79,12 @@ describe('Context bindings - injecting configuration for bound artifacts', () =>
 
     ctx.bind(ENVIRONMENT_KEY).to('dev');
     // Bind configuration
-    ctx.configure('servers.rest.server1', 'dev').to({port: 4000});
-    ctx.configure('servers.rest.server1', 'test').to({port: 3000});
-    ctx.configure('servers.rest.server1').to({host: 'localhost'});
+    ctx
+      .configure('servers.rest.server1', 'dev')
+      .to({host: 'localhost', port: 4000});
+    ctx
+      .configure('servers.rest.server1', 'test')
+      .to({host: 'localhost', port: 3000});
 
     // Bind RestServer
     ctx.bind('servers.rest.server1').toClass(RestServer2);