Move "wild" config types to new code

Summary:
Ref T12845. This is the last of the hard-coded types.

These are mostly used for values which users don't directly edit, so it's largely OK that they aren't carefully validated. In some cases, it would be good to introduce a separate validator eventually.

Test Plan: Edited, deleted and mangled these values via the web UI and CLI.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12845

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

Author: epriestley

src/__phutil_library_map__.php

@@ -3010,6 +3010,7 @@
     'PhabricatorIteratedMD5PasswordHasherTestCase' => 'infrastructure/util/password/__tests__/PhabricatorIteratedMD5PasswordHasherTestCase.php',
     'PhabricatorIteratorFileUploadSource' => 'applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php',
     'PhabricatorJIRAAuthProvider' => 'applications/auth/provider/PhabricatorJIRAAuthProvider.php',
+    'PhabricatorJSONConfigType' => 'applications/config/type/PhabricatorJSONConfigType.php',
     'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/PhabricatorJavelinLinter.php',
     'PhabricatorJiraIssueHasObjectEdgeType' => 'applications/doorkeeper/edge/PhabricatorJiraIssueHasObjectEdgeType.php',
     'PhabricatorJumpNavHandler' => 'applications/search/engine/PhabricatorJumpNavHandler.php',
@@ -4263,6 +4264,7 @@
     'PhabricatorWebContentSource' => 'infrastructure/contentsource/PhabricatorWebContentSource.php',
     'PhabricatorWebServerSetupCheck' => 'applications/config/check/PhabricatorWebServerSetupCheck.php',
     'PhabricatorWeekStartDaySetting' => 'applications/settings/setting/PhabricatorWeekStartDaySetting.php',
+    'PhabricatorWildConfigType' => 'applications/config/type/PhabricatorWildConfigType.php',
     'PhabricatorWordPressAuthProvider' => 'applications/auth/provider/PhabricatorWordPressAuthProvider.php',
     'PhabricatorWorker' => 'infrastructure/daemon/workers/PhabricatorWorker.php',
     'PhabricatorWorkerActiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php',
@@ -8348,6 +8350,7 @@
     'PhabricatorIteratedMD5PasswordHasherTestCase' => 'PhabricatorTestCase',
     'PhabricatorIteratorFileUploadSource' => 'PhabricatorFileUploadSource',
     'PhabricatorJIRAAuthProvider' => 'PhabricatorOAuth1AuthProvider',
+    'PhabricatorJSONConfigType' => 'PhabricatorTextConfigType',
     'PhabricatorJavelinLinter' => 'ArcanistLinter',
     'PhabricatorJiraIssueHasObjectEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorJumpNavHandler' => 'Phobject',
@@ -9839,6 +9842,7 @@
     'PhabricatorWebContentSource' => 'PhabricatorContentSource',
     'PhabricatorWebServerSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorWeekStartDaySetting' => 'PhabricatorSelectSetting',
+    'PhabricatorWildConfigType' => 'PhabricatorJSONConfigType',
     'PhabricatorWordPressAuthProvider' => 'PhabricatorOAuth2AuthProvider',
     'PhabricatorWorker' => 'Phobject',
     'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask',

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -325,40 +325,14 @@ private function readRequest(
     $e_value = null;
     $errors = array();
 
-
     if ($option->isCustomType()) {
       $info = $option->getCustomObject()->readRequest($option, $request);
       list($e_value, $errors, $set_value, $value) = $info;
     } else {
-      $value = $request->getStr('value');
-      if (!strlen($value)) {
-        $value = null;
-
-        $xaction->setNewValue(
-          array(
-            'deleted' => true,
-            'value'   => null,
-          ));
-
-        return array($e_value, $errors, $value, $xaction);
-      }
-
-      $type = $option->getType();
-      $set_value = null;
-
-      switch ($type) {
-        default:
-          $json = json_decode($value, true);
-          if ($json === null && strtolower($value) != 'null') {
-            $e_value = pht('Invalid');
-            $errors[] = pht(
-              'The given value must be valid JSON. This means, among '.
-              'other things, that you must wrap strings in double-quotes.');
-          } else {
-            $set_value = $json;
-          }
-          break;
-      }
+      throw new Exception(
+        pht(
+          'Unknown configuration option type "%s".',
+          $option->getType()));
     }
 
     if (!$errors) {
@@ -389,13 +363,12 @@ private function getDisplayValue(
         $option,
         $entry,
         $value);
-    } else {
-      $type = $option->getType();
-      switch ($type) {
-        default:
-          return PhabricatorConfigJSON::prettyPrintJSON($value);
-      }
     }
+
+    throw new Exception(
+      pht(
+        'Unknown configuration option type "%s".',
+        $option->getType()));
   }
 
   private function renderControls(
@@ -417,23 +390,10 @@ private function renderControls(
         $display_value,
         $e_value);
     } else {
-      $type = $option->getType();
-      switch ($type) {
-        default:
-          $control = id(new AphrontFormTextAreaControl())
-            ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
-            ->setCustomClass('PhabricatorMonospaced')
-            ->setCaption(pht('Enter value in JSON.'));
-          break;
-      }
-
-      $control
-        ->setLabel(pht('Database Value'))
-        ->setError($e_value)
-        ->setValue($display_value)
-        ->setName('value');
-
-      $controls = array($control);
+      throw new Exception(
+        pht(
+          'Unknown configuration option type "%s".',
+          $option->getType()));
     }
 
     return $controls;

src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php

@@ -70,6 +70,7 @@ public function execute(PhutilArgumentParser $args) {
         throw new PhutilArgumentUsageException($ex->getMessage());
       }
     } else {
+      // NOTE: For now, this handles both "wild" values and custom types.
       $type = $option->getType();
       switch ($type) {
         default:

src/applications/config/option/PhabricatorApplicationConfigOptions.php

@@ -24,6 +24,7 @@ public function validateOption(PhabricatorConfigOption $option, $value) {
     if ($type) {
       try {
         $type->validateStoredValue($option, $value);
+        $this->didValidateOption($option, $value);
       } catch (PhabricatorConfigValidationException $ex) {
         throw $ex;
       } catch (Exception $ex) {
@@ -32,6 +33,8 @@ public function validateOption(PhabricatorConfigOption $option, $value) {
         // configuration and raise an error.
         throw new PhabricatorConfigValidationException($ex->getMessage());
       }
+
+      return;
     }
 
     if ($option->isCustomType()) {
@@ -40,12 +43,11 @@ public function validateOption(PhabricatorConfigOption $option, $value) {
       } catch (Exception $ex) {
         throw new PhabricatorConfigValidationException($ex->getMessage());
       }
-    }
-
-    switch ($option->getType()) {
-      case 'wild':
-      default:
-        break;
+    } else {
+      throw new Exception(
+        pht(
+          'Unknown configuration option type "%s".',
+          $option->getType()));
     }
 
     $this->didValidateOption($option, $value);

src/applications/config/type/PhabricatorJSONConfigType.php

@@ -0,0 +1,38 @@
+<?php
+
+abstract class PhabricatorJSONConfigType
+  extends PhabricatorTextConfigType {
+
+  protected function newCanonicalValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    try {
+      $value = phutil_json_decode($value);
+    } catch (Exception $ex) {
+      throw $this->newException(
+        pht(
+          'Value for option "%s" (of type "%s") must be specified in JSON, '.
+          'but input could not be decoded: %s',
+          $option->getKey(),
+          $this->getTypeKey(),
+          $ex->getMessage()));
+    }
+
+    return $value;
+  }
+
+  protected function newControl(PhabricatorConfigOption $option) {
+    return id(new AphrontFormTextAreaControl())
+      ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL)
+      ->setCustomClass('PhabricatorMonospaced')
+      ->setCaption(pht('Enter value in JSON.'));
+  }
+
+  public function newDisplayValue(
+    PhabricatorConfigOption $option,
+    $value) {
+    return PhabricatorConfigJSON::prettyPrintJSON($value);
+  }
+
+}

src/applications/config/type/PhabricatorWildConfigType.php

@@ -0,0 +1,39 @@
+<?php
+
+final class PhabricatorWildConfigType
+  extends PhabricatorJSONConfigType {
+
+  const TYPEKEY = 'wild';
+
+  protected function newCanonicalValue(
+    PhabricatorConfigOption $option,
+    $value) {
+
+    $raw_value = $value;
+
+    // NOTE: We're significantly more liberal about canonicalizing "wild"
+    // values than "JSON" values because they're used to deal with some
+    // unusual edge cases, including situations where old config has been left
+    // in the database and we aren't sure what type it's supposed to be.
+    // Accept anything we can decode.
+
+    $value = json_decode($raw_value, true);
+    if ($value === null && $raw_value != 'null') {
+      throw $this->newException(
+        pht(
+          'Value for option "%s" (of type "%s") must be specified in JSON, '.
+          'but input could not be decoded. (Did you forget to quote a string?)',
+          $option->getKey(),
+          $this->getTypeKey()));
+    }
+
+    return $value;
+  }
+
+  public function validateStoredValue(
+    PhabricatorConfigOption $option,
+    $value) {
+    return;
+  }
+
+}

src/applications/people/config/PhabricatorUserConfigOptions.php

@@ -43,7 +43,7 @@ public function getOptions() {
       $this->newOption('user.fields', $custom_field_type, $default)
         ->setCustomData(id(new PhabricatorUser())->getCustomFieldBaseClass())
         ->setDescription(pht('Select and reorder user profile fields.')),
-      $this->newOption('user.custom-field-definitions', 'map', array())
+      $this->newOption('user.custom-field-definitions', 'wild', array())
         ->setDescription(pht('Add new simple fields to user profiles.')),
       $this->newOption('user.require-real-name', 'bool', true)
         ->setDescription(pht('Always require real name for user profiles.'))