Begin moving Phabricator configuration into PHP

Summary: Ref T2255. Ref T2221. Lay the groundwork to move configuration into PHP, so we can show descriptions in the web UI, do typechecking, disable application options when an application is uninstalled, etc.

Test Plan:
{F28421}
{F28420}
{F28422}

Reviewers: codeblock, btrahan, vrana

Reviewed By: codeblock

CC: aran

Maniphest Tasks: T2221, T2255

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

src/__phutil_library_map__.php

@@ -581,6 +581,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationCalendar' => 'applications/calendar/application/PhabricatorApplicationCalendar.php',
     'PhabricatorApplicationConduit' => 'applications/conduit/application/PhabricatorApplicationConduit.php',
     'PhabricatorApplicationConfig' => 'applications/config/application/PhabricatorApplicationConfig.php',
+    'PhabricatorApplicationConfigOptions' => 'applications/config/option/PhabricatorApplicationConfigOptions.php',
     'PhabricatorApplicationCountdown' => 'applications/countdown/application/PhabricatorApplicationCountdown.php',
     'PhabricatorApplicationDaemons' => 'applications/daemon/application/PhabricatorApplicationDaemons.php',
     'PhabricatorApplicationDifferential' => 'applications/differential/application/PhabricatorApplicationDifferential.php',
@@ -686,12 +687,14 @@ phutil_register_library_map(array(
     'PhabricatorConfigEntry' => 'applications/config/storage/PhabricatorConfigEntry.php',
     'PhabricatorConfigEntryDAO' => 'applications/config/storage/PhabricatorConfigEntryDAO.php',
     'PhabricatorConfigFileSource' => 'infrastructure/env/PhabricatorConfigFileSource.php',
+    'PhabricatorConfigGroupController' => 'applications/config/controller/PhabricatorConfigGroupController.php',
     'PhabricatorConfigIssueListController' => 'applications/config/controller/PhabricatorConfigIssueListController.php',
     'PhabricatorConfigIssueViewController' => 'applications/config/controller/PhabricatorConfigIssueViewController.php',
     'PhabricatorConfigListController' => 'applications/config/controller/PhabricatorConfigListController.php',
     'PhabricatorConfigLocalSource' => 'infrastructure/env/PhabricatorConfigLocalSource.php',
     'PhabricatorConfigManagementSetWorkflow' => 'infrastructure/env/management/PhabricatorConfigManagementSetWorkflow.php',
     'PhabricatorConfigManagementWorkflow' => 'infrastructure/env/management/PhabricatorConfigManagementWorkflow.php',
+    'PhabricatorConfigOption' => 'applications/config/option/PhabricatorConfigOption.php',
     'PhabricatorConfigProxySource' => 'infrastructure/env/PhabricatorConfigProxySource.php',
     'PhabricatorConfigSource' => 'infrastructure/env/PhabricatorConfigSource.php',
     'PhabricatorConfigStackSource' => 'infrastructure/env/PhabricatorConfigStackSource.php',
@@ -725,6 +728,7 @@ phutil_register_library_map(array(
     'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php',
     'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php',
     'PhabricatorDifferenceEngine' => 'infrastructure/diff/PhabricatorDifferenceEngine.php',
+    'PhabricatorDifferentialConfigOptions' => 'applications/differential/config/PhabricatorDifferentialConfigOptions.php',
     'PhabricatorDirectoryController' => 'applications/directory/controller/PhabricatorDirectoryController.php',
     'PhabricatorDirectoryMainController' => 'applications/directory/controller/PhabricatorDirectoryMainController.php',
     'PhabricatorDisabledUserController' => 'applications/auth/controller/PhabricatorDisabledUserController.php',
@@ -1889,6 +1893,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationCalendar' => 'PhabricatorApplication',
     'PhabricatorApplicationConduit' => 'PhabricatorApplication',
     'PhabricatorApplicationConfig' => 'PhabricatorApplication',
+    'PhabricatorApplicationConfigOptions' => 'Phobject',
     'PhabricatorApplicationCountdown' => 'PhabricatorApplication',
     'PhabricatorApplicationDaemons' => 'PhabricatorApplication',
     'PhabricatorApplicationDifferential' => 'PhabricatorApplication',
@@ -2008,12 +2013,14 @@ phutil_register_library_map(array(
     'PhabricatorConfigEntry' => 'PhabricatorConfigEntryDAO',
     'PhabricatorConfigEntryDAO' => 'PhabricatorLiskDAO',
     'PhabricatorConfigFileSource' => 'PhabricatorConfigProxySource',
+    'PhabricatorConfigGroupController' => 'PhabricatorConfigController',
     'PhabricatorConfigIssueListController' => 'PhabricatorConfigController',
     'PhabricatorConfigIssueViewController' => 'PhabricatorConfigController',
     'PhabricatorConfigListController' => 'PhabricatorConfigController',
     'PhabricatorConfigLocalSource' => 'PhabricatorConfigProxySource',
     'PhabricatorConfigManagementSetWorkflow' => 'PhabricatorConfigManagementWorkflow',
     'PhabricatorConfigManagementWorkflow' => 'PhutilArgumentWorkflow',
+    'PhabricatorConfigOption' => 'Phobject',
     'PhabricatorConfigProxySource' => 'PhabricatorConfigSource',
     'PhabricatorConfigStackSource' => 'PhabricatorConfigSource',
     'PhabricatorContentSourceView' => 'AphrontView',
@@ -2042,6 +2049,7 @@ phutil_register_library_map(array(
     'PhabricatorDaemonTimelineEventController' => 'PhabricatorDaemonController',
     'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector',
     'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector',
+    'PhabricatorDifferentialConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorDirectoryController' => 'PhabricatorController',
     'PhabricatorDirectoryMainController' => 'PhabricatorDirectoryController',
     'PhabricatorDisabledUserController' => 'PhabricatorAuthController',

src/applications/config/application/PhabricatorApplicationConfig.php

@@ -7,7 +7,7 @@ final class PhabricatorApplicationConfig extends PhabricatorApplication {
   }
 
   public function getIconName() {
-    return 'config';
+    return 'setup';
   }
 
   public function getTitleGlyph() {
@@ -27,6 +27,7 @@ final class PhabricatorApplicationConfig extends PhabricatorApplication {
       '/config/' => array(
         ''                          => 'PhabricatorConfigListController',
         'edit/(?P<key>[\w\.\-]+)/'  => 'PhabricatorConfigEditController',
+        'group/(?P<key>[^/]+)/'     => 'PhabricatorConfigGroupController',
         'issue/' => array(
           '' => 'PhabricatorConfigIssueListController',
           '(?P<key>[^/]+)/' => 'PhabricatorConfigIssueViewController',

src/applications/config/controller/PhabricatorConfigController.php

@@ -10,7 +10,9 @@ abstract class PhabricatorConfigController extends PhabricatorController {
     $user = $this->getRequest()->getUser();
 
     $nav = new AphrontSideNavFilterView();
-    $nav->setBaseURI(new PhutilURI($this->getApplicationURI('filter/')));
+    $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
+    $nav->addFilter('/', pht('Configuration'));
+    $nav->addFilter('issue/', pht('Setup Issues'));
 
     return $nav;
   }
@@ -19,11 +21,6 @@ abstract class PhabricatorConfigController extends PhabricatorController {
     return $this->buildSideNavView(null, true)->getMenu();
   }
 
-  public function buildApplicationCrumbs() {
-    $crumbs = parent::buildApplicationCrumbs();
-    return $crumbs;
-  }
-
   /**
    * Properly format a JSON value.
    *

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -6,27 +6,35 @@ final class PhabricatorConfigEditController
   private $key;
 
   public function willProcessRequest(array $data) {
-    $this->key = idx($data, 'key');
+    $this->key = $data['key'];
   }
 
   public function processRequest() {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $config = id(new PhabricatorConfigFileSource('default'))
+
-      ->getAllKeys();
+    $options = PhabricatorApplicationConfigOptions::loadAllOptions();
-    if (!$this->key || !array_key_exists($this->key, $config)) {
+    if (empty($options[$this->key])) {
       return new Aphront404Response();
     }
+    $option = $options[$this->key];
+    $group = $option->getGroup();
+    $group_uri = $this->getApplicationURI('group/'.$group->getKey().'/');
 
-    $default = $this->prettyPrintJSON($config[$this->key]);
+    // TODO: This isn't quite correct -- we should read from the entire
+    // configuration stack, ignoring database configuration. For now, though,
+    // it's a reasonable approximation.
+    $default_value = $option->getDefault();
+
+    $default = $this->prettyPrintJSON($default_value);
 
     // Check if the config key is already stored in the database.
     // Grab the value if it is.
     $value = null;
     $config_entry = id(new PhabricatorConfigEntry())
       ->loadOneWhere(
-        'configKey = %s AND namespace=%s',
+        'configKey = %s AND namespace = %s',
         $this->key,
         'default');
     if ($config_entry) {
@@ -39,6 +47,7 @@ final class PhabricatorConfigEditController
     $e_value = null;
     $errors = array();
     if ($request->isFormPost()) {
+
       $new_value = $request->getStr('value');
       if (strlen($new_value)) {
         $json = json_decode($new_value, true);
@@ -53,8 +62,7 @@ final class PhabricatorConfigEditController
       } else {
         // TODO: When we do Transactions, make this just set isDeleted = 1
         $config_entry->delete();
-        return id(new AphrontRedirectResponse())
+        return id(new AphrontRedirectResponse())->setURI($group_uri);
-          ->setURI($config_entry->getURI());
       }
 
       $config_entry->setValue($value);
@@ -62,8 +70,7 @@ final class PhabricatorConfigEditController
 
       if (!$errors) {
         $config_entry->save();
-        return id(new AphrontRedirectResponse())
+        return id(new AphrontRedirectResponse())->setURI($group_uri);
-          ->setURI($config_entry->getURI());
       }
     }
 
@@ -91,7 +98,7 @@ final class PhabricatorConfigEditController
         ->setName('value'))
       ->appendChild(
         id(new AphrontFormSubmitControl())
-          ->addCancelButton($config_entry->getURI())
+          ->addCancelButton($group_uri)
           ->setValue(pht('Save Config Entry')))
       ->appendChild(
         phutil_render_tag(
@@ -112,13 +119,20 @@ final class PhabricatorConfigEditController
       $title = pht('Edit %s', $this->key);
       $short = pht('Edit');
 
-    $crumbs = $this->buildApplicationCrumbs($this->buildSideNavView());
+    $crumbs = $this->buildApplicationCrumbs();
-    $crumbs->addCrumb(
+    $crumbs
-      id(new PhabricatorCrumbView())
+      ->addCrumb(
-        ->setName($this->key)
+        id(new PhabricatorCrumbView())
-        ->setHref('/config/edit/'.$this->key));
+          ->setName(pht('Config'))
-    $crumbs->addCrumb(
+          ->setHref($this->getApplicationURI()))
-      id(new PhabricatorCrumbView())->setName($short));
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName($group->getName())
+          ->setHref($group_uri))
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName($this->key)
+          ->setHref('/config/edit/'.$this->key));
 
     return $this->buildApplicationPage(
       array(

src/applications/config/controller/PhabricatorConfigGroupController.php

@@ -0,0 +1,68 @@
+<?php
+
+final class PhabricatorConfigGroupController
+  extends PhabricatorConfigController {
+
+  private $groupKey;
+
+  public function willProcessRequest(array $data) {
+    $this->groupKey = $data['key'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $groups = PhabricatorApplicationConfigOptions::loadAll();
+    $options = idx($groups, $this->groupKey);
+    if (!$options) {
+      return new Aphront404Response();
+    }
+
+    $title = pht('%s Configuration', $options->getName());
+
+    $header = id(new PhabricatorHeaderView())
+      ->setHeader($title);
+
+    $list = $this->buildOptionList($options->getOptions());
+
+    $crumbs = $this
+      ->buildApplicationCrumbs()
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName(pht('Config'))
+          ->setHref($this->getApplicationURI()))
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName($options->getName())
+          ->setHref($this->getApplicationURI()));
+
+    return $this->buildApplicationPage(
+      array(
+        $crumbs,
+        $header,
+        $list,
+      ),
+      array(
+        'title' => $title,
+        'device' => true,
+      )
+    );
+  }
+
+  private function buildOptionList(array $options) {
+    assert_instances_of($options, 'PhabricatorConfigOption');
+
+    $list = new PhabricatorObjectItemListView();
+    foreach ($options as $option) {
+      $item = id(new PhabricatorObjectItemView())
+        ->setHeader($option->getKey())
+        ->setHref('/config/edit/'.$option->getKey().'/')
+        ->addAttribute(phutil_escape_html($option->getSummary()));
+      $list->addItem($item);
+    }
+
+    return $list;
+  }
+
+}

src/applications/config/controller/PhabricatorConfigIssueListController.php

@@ -8,6 +8,7 @@ final class PhabricatorConfigIssueListController
     $user = $request->getUser();
 
     $nav = $this->buildSideNavView();
+    $nav->selectFilter('issue/');
 
     $issues = PhabricatorSetupCheck::runAllChecks();
     PhabricatorSetupCheck::setOpenSetupIssueCount(count($issues));
@@ -30,7 +31,7 @@ final class PhabricatorConfigIssueListController
       ->buildApplicationCrumbs($nav)
       ->addCrumb(
         id(new PhabricatorCrumbView())
-          ->setName($title)
+          ->setName(pht('Setup'))
           ->setHref($this->getApplicationURI('issue/')));
 
     $nav->setCrumbs($crumbs);

src/applications/config/controller/PhabricatorConfigListController.php

@@ -8,18 +8,15 @@ final class PhabricatorConfigListController
     $user = $request->getUser();
 
     $nav = $this->buildSideNavView();
+    $nav->selectFilter('/');
 
-    $pager = new AphrontCursorPagerView();
+    $groups = PhabricatorApplicationConfigOptions::loadAll();
-    $pager->readFromRequest($request);
+    $list = $this->buildConfigOptionsList($groups);
 
-    $config = new PhabricatorConfigFileSource('default');
+    $title = pht('Phabricator Configuration');
-    $list = $this->buildConfigList(array_keys($config->getAllKeys()));
-    $list->setPager($pager);
-    $list->setNoDataString(
-      'No data. Something probably went wrong in reading the default config.');
 
     $header = id(new PhabricatorHeaderView())
-      ->setHeader(pht('Configuration'));
+      ->setHeader($title);
 
     $nav->appendChild(
       array(
@@ -28,31 +25,32 @@ final class PhabricatorConfigListController
       ));
 
     $crumbs = $this
-      ->buildApplicationCrumbs($nav)
+      ->buildApplicationCrumbs()
       ->addCrumb(
         id(new PhabricatorCrumbView())
-          ->setName(pht('Configuration'))
+          ->setName(pht('Config'))
-          ->setHref($this->getApplicationURI('filter/')));
+          ->setHref($this->getApplicationURI()));
 
     $nav->setCrumbs($crumbs);
 
     return $this->buildApplicationPage(
       $nav,
       array(
-        'title' => pht('Configuration'),
+        'title' => $title,
         'device' => true,
       )
     );
   }
 
-  private function buildConfigList(array $keys) {
+  private function buildConfigOptionsList(array $groups) {
-    $list = new PhabricatorObjectItemListView();
+    assert_instances_of($groups, 'PhabricatorApplicationConfigOptions');
 
-    foreach ($keys as $key) {
+    $list = new PhabricatorObjectItemListView();
+    foreach ($groups as $group) {
       $item = id(new PhabricatorObjectItemView())
-        ->setHeader($key)
+        ->setHeader($group->getName())
-        ->setHref('/config/edit/'.$key)
+        ->setHref('/config/group/'.$group->getKey().'/')
-        ->setObject($key);
+        ->addAttribute(phutil_escape_html($group->getDescription()));
       $list->addItem($item);
     }
 

src/applications/config/option/PhabricatorApplicationConfigOptions.php

@@ -0,0 +1,70 @@
+<?php
+
+abstract class PhabricatorApplicationConfigOptions extends Phobject {
+
+  abstract public function getName();
+  abstract public function getDescription();
+  abstract public function getOptions();
+
+  public function getKey() {
+    $class = get_class($this);
+    $matches = null;
+    if (preg_match('/^Phabricator(.*)ConfigOptions$/', $class, $matches)) {
+      return strtolower($matches[1]);
+    }
+    return strtolower(get_class($this));
+  }
+
+  final protected function newOption($key, $type, $default) {
+    return id(new PhabricatorConfigOption())
+      ->setKey($key)
+      ->setType($type)
+      ->setDefault($default)
+      ->setGroup($this);
+  }
+
+  final public static function loadAll() {
+    $symbols = id(new PhutilSymbolLoader())
+      ->setAncestorClass('PhabricatorApplicationConfigOptions')
+      ->setConcreteOnly(true)
+      ->selectAndLoadSymbols();
+
+    $groups = array();
+    foreach ($symbols as $symbol) {
+      $obj = newv($symbol['name'], array());
+      $key = $obj->getKey();
+      if (isset($groups[$key])) {
+        $pclass = get_class($groups[$key]);
+        $nclass = $symbol['name'];
+
+        throw new Exception(
+          "Multiple PhabricatorApplicationConfigOptions subclasses have the ".
+          "same key ('{$key}'): {$pclass}, {$nclass}.");
+      }
+      $groups[$key] = $obj;
+    }
+
+    return $groups;
+  }
+
+  final public static function loadAllOptions() {
+    $groups = self::loadAll();
+
+    $options = array();
+    foreach ($groups as $group) {
+      foreach ($group->getOptions() as $option) {
+        $key = $option->getKey();
+        if (isset($options[$key])) {
+          throw new Exception(
+            "Mulitple PhabricatorApplicationConfigOptions subclasses contain ".
+            "an option named '{$key}'!");
+        }
+        $options[$key] = $option;
+      }
+    }
+
+    return $options;
+  }
+
+
+}

src/applications/config/option/PhabricatorConfigOption.php

@@ -0,0 +1,77 @@
+<?php
+
+final class PhabricatorConfigOption extends Phobject {
+
+  private $key;
+  private $default;
+  private $summary;
+  private $description;
+  private $type;
+  private $options;
+  private $group;
+
+
+  public function setGroup(PhabricatorApplicationConfigOptions $group) {
+    $this->group = $group;
+    return $this;
+  }
+
+  public function getGroup() {
+    return $this->group;
+  }
+
+  public function setOptions(array $options) {
+    $this->options = $options;
+    return $this;
+  }
+
+  public function getOptions() {
+    return $this->options;
+  }
+
+  public function setKey($key) {
+    $this->key = $key;
+    return $this;
+  }
+
+  public function getKey() {
+    return $this->key;
+  }
+
+  public function setDefault($default) {
+    $this->default = $default;
+    return $this;
+  }
+
+  public function getDefault() {
+    return $this->default;
+  }
+
+  public function setSummary($summary) {
+    $this->summary = $summary;
+    return $this;
+  }
+
+  public function getSummary() {
+    return $this->summary;
+  }
+
+  public function setDescription($description) {
+    $this->description = $description;
+    return $this;
+  }
+
+  public function getDescription() {
+    return $this->description;
+  }
+
+  public function setType($type) {
+    $this->type = $type;
+    return $this;
+  }
+
+  public function getType() {
+    return $this->type;
+  }
+
+}

src/applications/differential/config/PhabricatorDifferentialConfigOptions.php

@@ -0,0 +1,67 @@
+<?php
+
+final class PhabricatorDifferentialConfigOptions
+  extends PhabricatorApplicationConfigOptions {
+
+  public function getName() {
+    return pht('Differential');
+  }
+
+  public function getDescription() {
+    return pht('Configure Differential code review.');
+  }
+
+  public function getOptions() {
+    return array(
+      $this->newOption('differential.show-host-field', 'bool', false)
+        ->setOptions(
+          array(
+            pht('Disable "Host" Fields'),
+            pht('Show "Host" Fields'),
+          ))
+        ->setSummary(pht('Show or hide the "Host" and "Path" fields.'))
+        ->setDescription(
+          pht(
+            'Differential can show "Host" and "Path" fields on revisions, '.
+            'with information about the machine and working directory where '.
+            'the change came from. These fields are disabled by default '.
+            'because they may occasionally have sensitive information, but '.
+            'they can be useful if you work in an environment with shared '.
+            'development machines. You can set this option to true to enable '.
+            'these fields.')),
+      $this->newOption('differential.show-test-plan-field', 'bool', true)
+        ->setOptions(
+          array(
+            pht('Hide "Test Plan" Field'),
+            pht('Show "Test Plan" Field'),
+          ))
+        ->setSummary(pht('Show or hide the "Test Plan" field.'))
+        ->setDescription(
+          pht(
+            'Differential has a required "Test Plan" field by default, which '.
+            'requires authors to fill out information about how they verified '.
+            'the correctness of their changes when they send code for review. '.
+            'If you would prefer not to use this field, you can disable it '.
+            'here. You can also make it optional (instead of required) by '.
+            'setting {{differential.require-test-plan-field}}.')),
+      $this->newOption('differential.enable-email-accept', 'bool', false)
+        ->setOptions(
+          array(
+            pht('Disable Email "!accept" Action'),
+            pht('Enable Email "!accept" Action'),
+          ))
+        ->setSummary(pht('Enable or disable "!accept" action via email.'))
+        ->setDescription(
+          pht(
+            'If inbound email is configured, users can interact with '.
+            'revisions by using "!actions" in email replies (for example, '.
+            '"!resign" or "!rethink"). However, by default, users may not '.
+            '"!accept" revisions via email: email authentication can be '.
+            'configured to be very weak, and email "!accept" is kind of '.
+            'sketchy and implies the revision may not actually be receiving '.
+            'thorough review. You can enable "!accept" by setting this '.
+            'option to true.')),
+    );
+  }
+
+}