Formalize configuration sources and source stacks

Summary: Currently, we have a configuration stack for unit tests, but they're built in to `PhabricatorEnv`. Pull them out and formalize them, so we can add more configuration sources (e.g., database).

Test Plan: Ran unit tests, web requests, scripts. This code had fairly good existing test coverage.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2223, T2221

Differential Revision: https://secure.phabricator.com/D4284

src/__phutil_library_map__.php

@@ -679,6 +679,11 @@ phutil_register_library_map(array(
     'PhabricatorConduitLogController' => 'applications/conduit/controller/PhabricatorConduitLogController.php',
     'PhabricatorConduitMethodCallLog' => 'applications/conduit/storage/PhabricatorConduitMethodCallLog.php',
     'PhabricatorConduitTokenController' => 'applications/conduit/controller/PhabricatorConduitTokenController.php',
+    'PhabricatorConfigDictionarySource' => 'infrastructure/env/PhabricatorConfigDictionarySource.php',
+    'PhabricatorConfigFileSource' => 'infrastructure/env/PhabricatorConfigFileSource.php',
+    'PhabricatorConfigProxySource' => 'infrastructure/env/PhabricatorConfigProxySource.php',
+    'PhabricatorConfigSource' => 'infrastructure/env/PhabricatorConfigSource.php',
+    'PhabricatorConfigStackSource' => 'infrastructure/env/PhabricatorConfigStackSource.php',
     'PhabricatorContentSource' => 'applications/metamta/contentsource/PhabricatorContentSource.php',
     'PhabricatorContentSourceView' => 'applications/metamta/contentsource/PhabricatorContentSourceView.php',
     'PhabricatorController' => 'applications/base/controller/PhabricatorController.php',
@@ -726,8 +731,8 @@ phutil_register_library_map(array(
     'PhabricatorEmailTokenController' => 'applications/auth/controller/PhabricatorEmailTokenController.php',
     'PhabricatorEmailVerificationController' => 'applications/people/controller/PhabricatorEmailVerificationController.php',
     'PhabricatorEnglishTranslation' => 'infrastructure/internationalization/PhabricatorEnglishTranslation.php',
-    'PhabricatorEnv' => 'infrastructure/PhabricatorEnv.php',
+    'PhabricatorEnv' => 'infrastructure/env/PhabricatorEnv.php',
-    'PhabricatorEnvTestCase' => 'infrastructure/__tests__/PhabricatorEnvTestCase.php',
+    'PhabricatorEnvTestCase' => 'infrastructure/env/__tests__/PhabricatorEnvTestCase.php',
     'PhabricatorErrorExample' => 'applications/uiexample/examples/PhabricatorErrorExample.php',
     'PhabricatorEvent' => 'infrastructure/events/PhabricatorEvent.php',
     'PhabricatorEventEngine' => 'infrastructure/events/PhabricatorEventEngine.php',
@@ -1083,7 +1088,7 @@ phutil_register_library_map(array(
     'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
     'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php',
     'PhabricatorSSHWorkflow' => 'infrastructure/ssh/PhabricatorSSHWorkflow.php',
-    'PhabricatorScopedEnv' => 'infrastructure/PhabricatorScopedEnv.php',
+    'PhabricatorScopedEnv' => 'infrastructure/env/PhabricatorScopedEnv.php',
     'PhabricatorSearchAbstractDocument' => 'applications/search/index/PhabricatorSearchAbstractDocument.php',
     'PhabricatorSearchAttachController' => 'applications/search/controller/PhabricatorSearchAttachController.php',
     'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php',
@@ -1981,6 +1986,10 @@ phutil_register_library_map(array(
     'PhabricatorConduitLogController' => 'PhabricatorConduitController',
     'PhabricatorConduitMethodCallLog' => 'PhabricatorConduitDAO',
     'PhabricatorConduitTokenController' => 'PhabricatorConduitController',
+    'PhabricatorConfigDictionarySource' => 'PhabricatorConfigSource',
+    'PhabricatorConfigFileSource' => 'PhabricatorConfigProxySource',
+    'PhabricatorConfigProxySource' => 'PhabricatorConfigSource',
+    'PhabricatorConfigStackSource' => 'PhabricatorConfigSource',
     'PhabricatorContentSourceView' => 'AphrontView',
     'PhabricatorController' => 'AphrontController',
     'PhabricatorCountdownController' => 'PhabricatorController',

src/infrastructure/env/PhabricatorConfigDictionarySource.php

@@ -0,0 +1,36 @@
+<?php
+
+final class PhabricatorConfigDictionarySource
+  extends PhabricatorConfigSource {
+
+  private $dictionary;
+
+  public function __construct(array $dictionary) {
+    $this->dictionary = $dictionary;
+  }
+
+  public function getAllKeys() {
+    return $this->dictionary;
+  }
+
+  public function getKeys(array $keys) {
+    return array_select_keys($this->dictionary, $keys);
+  }
+
+  public function canWrite() {
+    return true;
+  }
+
+  public function setKeys(array $keys) {
+    $this->dictionary = $keys + $this->dictionary;
+    return $this;
+  }
+
+  public function deleteKeys(array $keys) {
+    foreach ($keys as $key) {
+      unset($this->dictionary[$key]);
+    }
+    return $keys;
+  }
+
+}

src/infrastructure/env/PhabricatorConfigFileSource.php

@@ -0,0 +1,23 @@
+<?php
+
+/**
+ * Configuration source which reads from a configuration file on disk (a
+ * PHP file in the conf/ directory). This source
+ */
+final class PhabricatorConfigFileSource
+  extends PhabricatorConfigProxySource {
+
+  /**
+   * @phutil-external-symbol function phabricator_read_config_file
+   */
+  public function __construct($config) {
+    $root = dirname(phutil_get_library_root('phabricator'));
+    require_once $root.'/conf/__init_conf__.php';
+
+    $dictionary = phabricator_read_config_file($config);
+    $dictionary['phabricator.env'] = $config;
+
+    $this->setSource(new PhabricatorConfigDictionarySource($dictionary));
+  }
+
+}

src/infrastructure/env/PhabricatorConfigProxySource.php

@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * Configuration source which proxies some other configuration source.
+ */
+abstract class PhabricatorConfigProxySource
+  extends PhabricatorConfigSource {
+
+  private $source;
+
+  final protected function getSource() {
+    if (!$this->source) {
+      throw new Exception("No configuration source set!");
+    }
+    return $this->source;
+  }
+
+  final protected function setSource(PhabricatorConfigSource $source) {
+    $this->source = $source;
+    return $this;
+  }
+
+  public function getAllKeys() {
+    return $this->getSource()->getAllKeys();
+  }
+
+  public function getKeys(array $keys) {
+    return $this->getSource()->getKeys($keys);
+  }
+
+  public function canWrite() {
+    return $this->getSource->canWrite();
+  }
+
+  public function setKeys(array $keys) {
+    return $this->getSource->setKeys();
+  }
+
+  public function deleteKeys(array $keys) {
+    return $this->getSource->deleteKeys();
+  }
+
+}

src/infrastructure/env/PhabricatorConfigSource.php

@@ -0,0 +1,20 @@
+<?php
+
+abstract class PhabricatorConfigSource {
+
+  abstract public function getKeys(array $keys);
+  abstract public function getAllKeys();
+
+  public function canWrite() {
+    return false;
+  }
+
+  public function setKeys(array $keys) {
+    throw new Exception("This configuration source does not support writes.");
+  }
+
+  public function deleteKeys(array $keys) {
+    throw new Exception("This configuration source does not support writes.");
+  }
+
+}

src/infrastructure/env/PhabricatorConfigStackSource.php

@@ -0,0 +1,76 @@
+<?php
+
+/**
+ * Configuration source which reads from a stack of other configuration
+ * sources.
+ *
+ * This source is writable if any source in the stack is writable. Writes happen
+ * to the first writable source only.
+ */
+final class PhabricatorConfigStackSource
+  extends PhabricatorConfigSource {
+
+  private $stack = array();
+
+  public function pushSource(PhabricatorConfigSource $source) {
+    array_unshift($this->stack, $source);
+    return $this;
+  }
+
+  public function popSource() {
+    if (empty($this->stack)) {
+      throw new Exception("Popping an empty config stack!");
+    }
+    return array_shift($this->stack);
+  }
+
+  public function getKeys(array $keys) {
+    $result = array();
+    foreach ($this->stack as $source) {
+      $result = $result + $source->getKeys($keys);
+    }
+    return $result;
+  }
+
+  public function getAllKeys() {
+    $result = array();
+    foreach ($this->stack as $source) {
+      $result = $result + $source->getAllKeys();
+    }
+    return $result;
+  }
+
+  public function canWrite() {
+    foreach ($this->stack as $source) {
+      if ($source->canWrite()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  public function setKeys(array $keys) {
+    foreach ($this->stack as $source) {
+      if ($source->canWrite()) {
+        $source->setKeys($keys);
+        return;
+      }
+    }
+
+    // We can't write; this will throw an appropriate exception.
+    parent::setKeys($keys);
+  }
+
+  public function deleteKeys(array $keys) {
+    foreach ($this->stack as $source) {
+      if ($source->canWrite()) {
+        $source->deleteKeys($keys);
+        return;
+      }
+    }
+
+    // We can't write; this will throw an appropriate exception.
+    parent::deleteKeys($keys);
+  }
+
+}

src/infrastructure/env/PhabricatorEnv.php

@@ -50,8 +50,7 @@
  */
 final class PhabricatorEnv {
 
-  private static $env;
+  private static $sourceStack;
-  private static $stack = array();
 
   /**
    * @phutil-external-symbol class PhabricatorStartup
@@ -93,18 +92,12 @@ final class PhabricatorEnv {
     AphrontWriteGuard::allowDangerousUnguardedWrites(true);
   }
 
-  /**
+
-   * @phutil-external-symbol function phabricator_read_config_file
-   */
   private static function initializeCommonEnvironment() {
     $env = self::getSelectedEnvironmentName();
 
-    $root = dirname(phutil_get_library_root('phabricator'));
+    self::$sourceStack = new PhabricatorConfigStackSource();
-    require_once $root.'/conf/__init_conf__.php';
+    self::$sourceStack->pushSource(new PhabricatorConfigFileSource($env));
-    $conf = phabricator_read_config_file($env);
-    $conf['phabricator.env'] = $env;
-
-    PhabricatorEnv::setEnvConfig($conf);
 
     PhutilErrorHandler::initialize();
 
@@ -159,18 +152,8 @@ final class PhabricatorEnv {
    * @task read
    */
   public static function getEnvConfig($key, $default = null) {
-
+    $result = self::$sourceStack->getKeys(array($key));
-    // If we have environment overrides via beginScopedEnv(), check them for
+    return idx($result, $key, $default);
-    // the key first.
-    if (self::$stack) {
-      foreach (array_reverse(self::$stack) as $override) {
-        if (array_key_exists($key, $override)) {
-          return $override[$key];
-        }
-      }
-    }
-
-    return idx(self::$env, $key, $default);
   }
 
 
@@ -256,27 +239,26 @@ final class PhabricatorEnv {
    * @task test
    */
   public static function beginScopedEnv() {
-    return new PhabricatorScopedEnv(self::pushEnvironment());
+    return new PhabricatorScopedEnv(self::pushTestEnvironment());
   }
 
 
   /**
    * @task test
    */
-  private static function pushEnvironment() {
+  private static function pushTestEnvironment() {
-    self::$stack[] = array();
+    $source = new PhabricatorConfigDictionarySource(array());
-    return last_key(self::$stack);
+    self::$sourceStack->pushSource($source);
+    return spl_object_hash($source);
   }
 
 
   /**
    * @task test
    */
-  public static function popEnvironment($key) {
+  public static function popTestEnvironment($key) {
-    $stack_key = last_key(self::$stack);
+    $source = self::$sourceStack->popSource();
-
+    $stack_key = spl_object_hash($source);
-    array_pop(self::$stack);
-
     if ($stack_key !== $key) {
       throw new Exception(
         "Scoped environments were destroyed in a diffent order than they ".
@@ -366,14 +348,6 @@ final class PhabricatorEnv {
 /* -(  Internals  )---------------------------------------------------------- */
 
 
-  /**
-   * @task internal
-   */
-  public static function setEnvConfig(array $config) {
-    self::$env = $config;
-  }
-
-
   /**
    * @task internal
    */
@@ -405,7 +379,7 @@ final class PhabricatorEnv {
    * @task internal
    */
   public static function envConfigExists($key) {
-    return array_key_exists($key, self::$env);
+    return array_key_exists($key, self::$sourceStack->getKeys(array($key)));
   }
 
 
@@ -413,15 +387,30 @@ final class PhabricatorEnv {
    * @task internal
    */
   public static function getAllConfigKeys() {
-    return self::$env;
+    return self::$sourceStack->getAllKeys();
   }
 
 
   /**
    * @task internal
    */
-  public static function overrideEnvConfig($stack_key, $key, $value) {
+  public static function overrideTestEnvConfig($stack_key, $key, $value) {
-    self::$stack[$stack_key][$key] = $value;
+    $tmp = array();
+
+    // If we don't have the right key, we'll throw when popping the last
+    // source off the stack.
+    do {
+      $source = self::$sourceStack->popSource();
+      array_unshift($tmp, $source);
+      if (spl_object_hash($source) == $stack_key) {
+        $source->setKeys(array($key => $value));
+        break;
+      }
+    } while (true);
+
+    foreach ($tmp as $source) {
+      self::$sourceStack->pushSource($source);
+    }
   }
 
 }

src/infrastructure/env/PhabricatorScopedEnv.php

@@ -24,7 +24,7 @@ final class PhabricatorScopedEnv {
    * @task override
    */
   public function overrideEnvConfig($key, $value) {
-    PhabricatorEnv::overrideEnvConfig(
+    PhabricatorEnv::overrideTestEnvConfig(
       $this->key,
       $key,
       $value);
@@ -36,7 +36,6 @@ final class PhabricatorScopedEnv {
 
 
   /**
-   *
    * @task internal
    */
   public function __construct($stack_key) {
@@ -52,7 +51,7 @@ final class PhabricatorScopedEnv {
    */
   public function __destruct() {
     if (!$this->isPopped) {
-      PhabricatorEnv::popEnvironment($this->key);
+      PhabricatorEnv::popTestEnvironment($this->key);
       $this->isPopped = true;
     }
   }

src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php

@@ -38,8 +38,78 @@ final class PhabricatorEnvTestCase extends PhabricatorTestCase {
     }
   }
 
+  public function testDictionarySource() {
+    $source = new PhabricatorConfigDictionarySource(array('x' => 1));
+
+    $this->assertEqual(
+      array(
+        'x' => 1,
+      ),
+      $source->getKeys(array('x', 'z')));
+
+    $source->setKeys(array('z' => 2));
+
+    $this->assertEqual(
+      array(
+        'x' => 1,
+        'z' => 2,
+      ),
+      $source->getKeys(array('x', 'z')));
+
+    $source->setKeys(array('x' => 3));
+
+    $this->assertEqual(
+      array(
+        'x' => 3,
+        'z' => 2,
+      ),
+      $source->getKeys(array('x', 'z')));
+
+    $source->deleteKeys(array('x'));
+
+    $this->assertEqual(
+      array(
+        'z' => 2,
+      ),
+      $source->getKeys(array('x', 'z')));
+  }
+
+  public function testStackSource() {
+    $s1 = new PhabricatorConfigDictionarySource(array('x' => 1));
+    $s2 = new PhabricatorConfigDictionarySource(array('x' => 2));
+
+    $stack = new PhabricatorConfigStackSource();
+
+    $this->assertEqual(array(), $stack->getKeys(array('x')));
+
+    $stack->pushSource($s1);
+    $this->assertEqual(array('x' => 1), $stack->getKeys(array('x')));
+
+    $stack->pushSource($s2);
+    $this->assertEqual(array('x' => 2), $stack->getKeys(array('x')));
+
+    $stack->setKeys(array('x' => 3));
+    $this->assertEqual(array('x' => 3), $stack->getKeys(array('x')));
+
+    $stack->popSource();
+    $this->assertEqual(array('x' => 1), $stack->getKeys(array('x')));
+
+    $stack->popSource();
+    $this->assertEqual(array(), $stack->getKeys(array('x')));
+
+    $caught = null;
+    try {
+      $stack->popSource();
+    } catch (Exception $ex) {
+      $caught = $ex;
+    }
+
+    $this->assertEqual(true, ($caught instanceof Exception));
+  }
+
   public function testOverrides() {
     $outer = PhabricatorEnv::beginScopedEnv();
+
       $outer->overrideEnvConfig('test.value', 1);
       $this->assertEqual(1, PhabricatorEnv::getEnvConfig('test.value'));