Move 'set' config option type to new structure

Summary: Ref T12845. This move 'set' options (a set of values).

Test Plan: Set, deleted and mangled 'set' options from CLI and web UI.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12845

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

src/__phutil_library_map__.php

@@ -3943,6 +3943,7 @@ phutil_register_library_map(array(
     'PhabricatorSelectSetting' => 'applications/settings/setting/PhabricatorSelectSetting.php',
     'PhabricatorSendGridConfigOptions' => 'applications/config/option/PhabricatorSendGridConfigOptions.php',
     'PhabricatorSessionsSettingsPanel' => 'applications/settings/panel/PhabricatorSessionsSettingsPanel.php',
+    'PhabricatorSetConfigType' => 'applications/config/type/PhabricatorSetConfigType.php',
     'PhabricatorSetting' => 'applications/settings/setting/PhabricatorSetting.php',
     'PhabricatorSettingsAccountPanelGroup' => 'applications/settings/panelgroup/PhabricatorSettingsAccountPanelGroup.php',
     'PhabricatorSettingsAddEmailAction' => 'applications/settings/action/PhabricatorSettingsAddEmailAction.php',
@@ -9469,6 +9470,7 @@ phutil_register_library_map(array(
     'PhabricatorSelectSetting' => 'PhabricatorSetting',
     'PhabricatorSendGridConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorSessionsSettingsPanel' => 'PhabricatorSettingsPanel',
+    'PhabricatorSetConfigType' => 'PhabricatorTextConfigType',
     'PhabricatorSetting' => 'Phobject',
     'PhabricatorSettingsAccountPanelGroup' => 'PhabricatorSettingsPanelGroup',
     'PhabricatorSettingsAddEmailAction' => 'PhabricatorSystemAction',

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -347,9 +347,6 @@ final class PhabricatorConfigEditController
       $set_value = null;
 
       switch ($type) {
-        case 'set':
-          $set_value = array_fill_keys($request->getStrList('value'), true);
-          break;
         default:
           $json = json_decode($value, true);
           if ($json === null && strtolower($value) != 'null') {
@@ -395,8 +392,6 @@ final class PhabricatorConfigEditController
     } else {
       $type = $option->getType();
       switch ($type) {
-        case 'set':
-          return implode("\n", nonempty(array_keys($value), array()));
         default:
           return PhabricatorConfigJSON::prettyPrintJSON($value);
       }
@@ -424,10 +419,6 @@ final class PhabricatorConfigEditController
     } else {
       $type = $option->getType();
       switch ($type) {
-        case 'set':
-          $control = id(new AphrontFormTextAreaControl())
-            ->setCaption(pht('Separate values with newlines or commas.'));
-          break;
         default:
           $control = id(new AphrontFormTextAreaControl())
             ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)

src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php

@@ -76,21 +76,6 @@ final class PhabricatorConfigManagementSetWorkflow
           $value = json_decode($value, true);
           if (!is_array($value)) {
             switch ($type) {
-              case 'set':
-                $command = csprintf(
-                  './bin/config set %R %s',
-                  $key,
-                  '{"value1": true, "value2": true}');
-
-                $message = sprintf(
-                  "%s\n\n    %s\n",
-                  pht(
-                    'Config key "%s" is of type "%s". Specify it in JSON. '.
-                    'For example:',
-                    $key,
-                    $type),
-                  $command);
-                break;
               default:
                 $message = pht(
                   'Config key "%s" is of type "%s". Specify it in JSON.',

src/applications/config/option/PhabricatorApplicationConfigOptions.php

@@ -43,24 +43,6 @@ abstract class PhabricatorApplicationConfigOptions extends Phobject {
     }
 
     switch ($option->getType()) {
-      case 'set':
-        $valid = true;
-        if (!is_array($value)) {
-          throw new PhabricatorConfigValidationException(
-            pht(
-              "Option '%s' must be a set, but value is not an array.",
-              $option->getKey()));
-        }
-        foreach ($value as $v) {
-          if ($v !== true) {
-            throw new PhabricatorConfigValidationException(
-              pht(
-                "Option '%s' must be a set, but array contains values other ".
-                "than 'true'.",
-                $option->getKey()));
-          }
-        }
-        break;
       case 'wild':
       default:
         break;

src/applications/config/type/PhabricatorSetConfigType.php

@@ -0,0 +1,92 @@
+<?php
+
+final class PhabricatorSetConfigType
+  extends PhabricatorTextConfigType {
+
+  const TYPEKEY = 'set';
+
+  protected function newControl(PhabricatorConfigOption $option) {
+    return id(new AphrontFormTextAreaControl())
+      ->setCaption(pht('Separate values with newlines or commas.'));
+  }
+
+  protected function newCanonicalValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    $value = preg_split('/[\n,]+/', $value);
+    foreach ($value as $k => $v) {
+      if (!strlen($v)) {
+        unset($value[$k]);
+      }
+      $value[$k] = trim($v);
+    }
+
+    return array_fill_keys($value, true);
+  }
+
+  public function newValueFromCommandLineValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    try {
+      $value = phutil_json_decode($value);
+    } catch (Exception $ex) {
+      throw $this->newException(
+        pht(
+          'Option "%s" is of type "%s", but the value you provided is not a '.
+          'valid JSON list: when providing a set from the command line, '.
+          'specify it as a list of values in JSON. You may need to quote the '.
+          'value for your shell (for example: \'["a", "b", ...]\').',
+          $option->getKey(),
+          $this->getTypeKey()));
+    }
+
+    if ($value) {
+      if (array_keys($value) !== range(0, count($value) - 1)) {
+        throw $this->newException(
+          pht(
+            'Option "%s" is of type "%s", and should be specified on the '.
+            'command line as a JSON list of values. You may need to quote '.
+            'the value for your shell (for example: \'["a", "b", ...]\').',
+            $option->getKey(),
+            $this->getTypeKey()));
+      }
+    }
+
+    return array_fill_keys($value, true);
+  }
+
+  public function newDisplayValue(
+    PhabricatorConfigOption $option,
+    $value) {
+    return implode("\n", array_keys($value));
+  }
+
+  public function validateStoredValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    if (!is_array($value)) {
+      throw $this->newException(
+        pht(
+          'Option "%s" is of type "%s", but the configured value is not '.
+          'a list.',
+          $option->getKey(),
+          $this->getTypeKey()));
+    }
+
+    foreach ($value as $k => $v) {
+      if ($v !== true) {
+        throw $this->newException(
+          pht(
+            'Option "%s" is of type "%s", but the value at index "%s" of the '.
+            'list is not "true".',
+            $option->getKey(),
+            $this->getTypeKey(),
+            $k));
+      }
+    }
+  }
+
+}