Convert 'class' config options to new validation

Summary: Ref T12845. These options prompt the user to select from among concrete subclasses of some base class.

Test Plan: Set, deleted and mangled these values from the web UI and CLI.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12845

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

Author: epriestley

src/__phutil_library_map__.php

@@ -2329,6 +2329,7 @@
     'PhabricatorChatLogEvent' => 'applications/chatlog/storage/PhabricatorChatLogEvent.php',
     'PhabricatorChatLogQuery' => 'applications/chatlog/query/PhabricatorChatLogQuery.php',
     'PhabricatorChunkedFileStorageEngine' => 'applications/files/engine/PhabricatorChunkedFileStorageEngine.php',
+    'PhabricatorClassConfigType' => 'applications/config/type/PhabricatorClassConfigType.php',
     'PhabricatorClusterConfigOptions' => 'applications/config/option/PhabricatorClusterConfigOptions.php',
     'PhabricatorClusterDatabasesConfigOptionType' => 'infrastructure/cluster/config/PhabricatorClusterDatabasesConfigOptionType.php',
     'PhabricatorClusterException' => 'infrastructure/cluster/exception/PhabricatorClusterException.php',
@@ -7575,6 +7576,7 @@
     ),
     'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorChunkedFileStorageEngine' => 'PhabricatorFileStorageEngine',
+    'PhabricatorClassConfigType' => 'PhabricatorTextConfigType',
     'PhabricatorClusterConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorClusterDatabasesConfigOptionType' => 'PhabricatorConfigJSONOptionType',
     'PhabricatorClusterException' => 'Exception',

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -350,20 +350,6 @@ private function readRequest(
         case 'set':
           $set_value = array_fill_keys($request->getStrList('value'), true);
           break;
-        case 'class':
-          if (!class_exists($value)) {
-            $e_value = pht('Invalid');
-            $errors[] = pht('Class does not exist.');
-          } else {
-            $base = $option->getBaseClass();
-            if (!is_subclass_of($value, $base)) {
-              $e_value = pht('Invalid');
-              $errors[] = pht('Class is not of valid type.');
-            } else {
-              $set_value = $value;
-            }
-          }
-          break;
         default:
           $json = json_decode($value, true);
           if ($json === null && strtolower($value) != 'null') {
@@ -409,8 +395,6 @@ private function getDisplayValue(
     } else {
       $type = $option->getType();
       switch ($type) {
-        case 'class':
-          return $value;
         case 'set':
           return implode("\n", nonempty(array_keys($value), array()));
         default:
@@ -440,21 +424,6 @@ private function renderControls(
     } else {
       $type = $option->getType();
       switch ($type) {
-        case 'class':
-          $symbols = id(new PhutilSymbolLoader())
-            ->setType('class')
-            ->setAncestorClass($option->getBaseClass())
-            ->setConcreteOnly(true)
-            ->selectSymbolsWithoutLoading();
-          $names = ipull($symbols, 'name', 'name');
-          asort($names);
-          $names = array(
-            '' => pht('(Use Default)'),
-          ) + $names;
-
-          $control = id(new AphrontFormSelectControl())
-            ->setOptions($names);
-          break;
         case 'set':
           $control = id(new AphrontFormTextAreaControl())
             ->setCaption(pht('Separate values with newlines or commas.'));

src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php

@@ -72,9 +72,6 @@ public function execute(PhutilArgumentParser $args) {
     } else {
       $type = $option->getType();
       switch ($type) {
-        case 'class':
-          $value = (string)$value;
-          break;
         default:
           $value = json_decode($value, true);
           if (!is_array($value)) {

src/applications/config/option/PhabricatorApplicationConfigOptions.php

@@ -43,21 +43,6 @@ public function validateOption(PhabricatorConfigOption $option, $value) {
     }
 
     switch ($option->getType()) {
-      case 'class':
-        $symbols = id(new PhutilSymbolLoader())
-          ->setType('class')
-          ->setAncestorClass($option->getBaseClass())
-          ->setConcreteOnly(true)
-          ->selectSymbolsWithoutLoading();
-        $names = ipull($symbols, 'name', 'name');
-        if (empty($names[$value])) {
-          throw new PhabricatorConfigValidationException(
-            pht(
-              "Option '%s' value must name a class extending '%s'.",
-              $option->getKey(),
-              $option->getBaseClass()));
-        }
-        break;
       case 'set':
         $valid = true;
         if (!is_array($value)) {

src/applications/config/type/PhabricatorClassConfigType.php

@@ -0,0 +1,76 @@
+<?php
+
+final class PhabricatorClassConfigType
+  extends PhabricatorTextConfigType {
+
+  const TYPEKEY = 'class';
+
+  public function validateStoredValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    if (!is_string($value)) {
+      throw $this->newException(
+        pht(
+          'Option "%s" is of type "%s", but the configured value is not '.
+          'a string.',
+          $option->getKey(),
+          $this->getTypeKey()));
+    }
+
+    $base = $option->getBaseClass();
+    $map = $this->getClassOptions($option);
+
+    try {
+      $ok = class_exists($value);
+    } catch (Exception $ex) {
+      $ok = false;
+    }
+
+    if (!$ok) {
+      throw $this->newException(
+        pht(
+          'Option "%s" is of type "%s", but the configured value is not the '.
+          'name of a known class. Valid selections are: %s.',
+          $option->getKey(),
+          $this->getTypeKey(),
+          implode(', ', array_keys($map))));
+    }
+
+    if (!isset($map[$value])) {
+      throw $this->newException(
+        pht(
+          'Option "%s" is of type "%s", but the current value ("%s") is not '.
+          'a known, concrete subclass of base class "%s". Valid selections '.
+          'are: %s.',
+          $option->getKey(),
+          $this->getTypeKey(),
+          $value,
+          $base,
+          implode(', ', array_keys($map))));
+    }
+  }
+
+  protected function newControl(PhabricatorConfigOption $option) {
+    $map = array(
+      '' => pht('(Use Default)'),
+    ) + $this->getClassOptions($option);
+
+    return id(new AphrontFormSelectControl())
+      ->setOptions($map);
+  }
+
+  private function getClassOptions(PhabricatorConfigOption $option) {
+    $symbols = id(new PhutilSymbolLoader())
+      ->setType('class')
+      ->setAncestorClass($option->getBaseClass())
+      ->setConcreteOnly(true)
+      ->selectSymbolsWithoutLoading();
+
+    $map = ipull($symbols, 'name', 'name');
+    asort($map);
+
+    return $map;
+  }
+
+}