diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2448,6 +2448,7 @@ 'PhabricatorConfigTableSchema' => 'applications/config/schema/PhabricatorConfigTableSchema.php', 'PhabricatorConfigTransaction' => 'applications/config/storage/PhabricatorConfigTransaction.php', 'PhabricatorConfigTransactionQuery' => 'applications/config/query/PhabricatorConfigTransactionQuery.php', + 'PhabricatorConfigType' => 'applications/config/type/PhabricatorConfigType.php', 'PhabricatorConfigValidationException' => 'applications/config/exception/PhabricatorConfigValidationException.php', 'PhabricatorConfigVersionController' => 'applications/config/controller/PhabricatorConfigVersionController.php', 'PhabricatorConpherenceApplication' => 'applications/conpherence/application/PhabricatorConpherenceApplication.php', @@ -2997,6 +2998,7 @@ 'PhabricatorInlineCommentPreviewController' => 'infrastructure/diff/PhabricatorInlineCommentPreviewController.php', 'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php', 'PhabricatorInstructionsEditField' => 'applications/transactions/editfield/PhabricatorInstructionsEditField.php', + 'PhabricatorIntConfigType' => 'applications/config/type/PhabricatorIntConfigType.php', 'PhabricatorInternalSetting' => 'applications/settings/setting/PhabricatorInternalSetting.php', 'PhabricatorInternationalizationManagementExtractWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php', 'PhabricatorInternationalizationManagementWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementWorkflow.php', @@ -4127,6 +4129,7 @@ 'PhabricatorTestStorageEngine' => 'applications/files/engine/PhabricatorTestStorageEngine.php', 'PhabricatorTestWorker' => 'infrastructure/daemon/workers/__tests__/PhabricatorTestWorker.php', 'PhabricatorTextAreaEditField' => 'applications/transactions/editfield/PhabricatorTextAreaEditField.php', + 'PhabricatorTextConfigType' => 'applications/config/type/PhabricatorTextConfigType.php', 'PhabricatorTextEditField' => 'applications/transactions/editfield/PhabricatorTextEditField.php', 'PhabricatorTime' => 'infrastructure/time/PhabricatorTime.php', 'PhabricatorTimeFormatSetting' => 'applications/settings/setting/PhabricatorTimeFormatSetting.php', @@ -7697,6 +7700,7 @@ 'PhabricatorConfigTableSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorConfigType' => 'Phobject', 'PhabricatorConfigValidationException' => 'Exception', 'PhabricatorConfigVersionController' => 'PhabricatorConfigController', 'PhabricatorConpherenceApplication' => 'PhabricatorApplication', @@ -8324,6 +8328,7 @@ 'PhabricatorInlineCommentPreviewController' => 'PhabricatorController', 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInstructionsEditField' => 'PhabricatorEditField', + 'PhabricatorIntConfigType' => 'PhabricatorTextConfigType', 'PhabricatorInternalSetting' => 'PhabricatorSetting', 'PhabricatorInternationalizationManagementExtractWorkflow' => 'PhabricatorInternationalizationManagementWorkflow', 'PhabricatorInternationalizationManagementWorkflow' => 'PhabricatorManagementWorkflow', @@ -9659,6 +9664,7 @@ 'PhabricatorTestStorageEngine' => 'PhabricatorFileStorageEngine', 'PhabricatorTestWorker' => 'PhabricatorWorker', 'PhabricatorTextAreaEditField' => 'PhabricatorEditField', + 'PhabricatorTextConfigType' => 'PhabricatorConfigType', 'PhabricatorTextEditField' => 'PhabricatorEditField', 'PhabricatorTime' => 'Phobject', 'PhabricatorTimeFormatSetting' => 'PhabricatorSelectSetting', diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -274,12 +274,58 @@ PhabricatorConfigOption $option, AphrontRequest $request) { + $type = $option->newOptionType(); + if ($type) { + $is_set = $type->isValuePresentInRequest($option, $request); + if ($is_set) { + $value = $type->readValueFromRequest($option, $request); + + $errors = array(); + try { + $canonical_value = $type->newValueFromRequestValue( + $option, + $value); + $type->validateStoredValue($canonical_value); + $xaction = $type->newTransaction($option, $canonical_value); + } catch (PhabricatorConfigValidationException $ex) { + $errors[] = $ex->getMessage(); + $xaction = null; + } + + return array( + $errors ? pht('Invalid') : null, + $errors, + $value, + $xaction, + ); + } else { + $delete_xaction = id(new PhabricatorConfigTransaction()) + ->setTransactionType(PhabricatorConfigTransaction::TYPE_EDIT) + ->setNewValue( + array( + 'deleted' => true, + 'value' => null, + )); + + return array( + null, + array(), + null, + $delete_xaction, + ); + } + } + + // TODO: If we missed on the new `PhabricatorConfigType` map, fall back + // to the old semi-modular, semi-hacky way of doing things. + $xaction = new PhabricatorConfigTransaction(); $xaction->setTransactionType(PhabricatorConfigTransaction::TYPE_EDIT); $e_value = null; $errors = array(); + if ($option->isCustomType()) { $info = $option->getCustomObject()->readRequest($option, $request); list($e_value, $errors, $set_value, $value) = $info; @@ -301,14 +347,6 @@ $set_value = null; switch ($type) { - case 'int': - if (preg_match('/^-?[0-9]+$/', trim($value))) { - $set_value = (int)$value; - } else { - $e_value = pht('Invalid'); - $errors[] = pht('Value must be an integer.'); - } - break; case 'string': case 'enum': $set_value = (string)$value; @@ -390,6 +428,11 @@ PhabricatorConfigEntry $entry, $value) { + $type = $option->newOptionType(); + if ($type) { + return $type->newDisplayValue($option, $value); + } + if ($option->isCustomType()) { return $option->getCustomObject()->getDisplayValue( $option, @@ -398,7 +441,6 @@ } else { $type = $option->getType(); switch ($type) { - case 'int': case 'string': case 'enum': case 'class': @@ -421,6 +463,14 @@ $display_value, $e_value) { + $type = $option->newOptionType(); + if ($type) { + return $type->newControls( + $option, + $display_value, + $e_value); + } + if ($option->isCustomType()) { $controls = $option->getCustomObject()->renderControls( $option, @@ -429,7 +479,6 @@ } else { $type = $option->getType(); switch ($type) { - case 'int': case 'string': $control = id(new AphrontFormTextControl()); break; diff --git a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php --- a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php +++ b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php @@ -59,63 +59,47 @@ $option = $options[$key]; - $type = $option->getType(); - switch ($type) { - case 'string': - case 'class': - case 'enum': - $value = (string)$value; - break; - case 'int': - if (!ctype_digit($value)) { - throw new PhutilArgumentUsageException( - pht( - "Config key '%s' is of type '%s'. Specify an integer.", - $key, - $type)); - } - $value = (int)$value; - break; - case 'bool': - if ($value == 'true') { - $value = true; - } else if ($value == 'false') { - $value = false; - } else { - throw new PhutilArgumentUsageException( - pht( - "Config key '%s' is of type '%s'. Specify '%s' or '%s'.", - $key, - $type, - 'true', - 'false')); - } - break; - default: - $value = json_decode($value, true); - if (!is_array($value)) { - switch ($type) { - case 'set': - $command = csprintf( - './bin/config set %R %s', + $type = $option->newOptionType(); + if ($type) { + try { + $value = $type->newValueFromCommandLineValue( + $option, + $value); + } catch (PhabricatorConfigValidationException $ex) { + throw new PhutilArgumentUsageException($ex->getMessage()); + } + } else { + $type = $option->getType(); + switch ($type) { + case 'string': + case 'class': + case 'enum': + $value = (string)$value; + break; + case 'bool': + if ($value == 'true') { + $value = true; + } else if ($value == 'false') { + $value = false; + } else { + throw new PhutilArgumentUsageException( + pht( + "Config key '%s' is of type '%s'. Specify '%s' or '%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: - if (preg_match('/^listgetArg('database'); if ($option->getLocked() && $use_database) { throw new PhutilArgumentUsageException( diff --git a/src/applications/config/option/PhabricatorApplicationConfigOptions.php b/src/applications/config/option/PhabricatorApplicationConfigOptions.php --- a/src/applications/config/option/PhabricatorApplicationConfigOptions.php +++ b/src/applications/config/option/PhabricatorApplicationConfigOptions.php @@ -20,13 +20,24 @@ return; } + $type = $option->newOptionType(); + if ($type) { + try { + $type->validateStoredValue($option, $value); + } catch (PhabricatorConfigValidationException $ex) { + throw $ex; + } catch (Exception $ex) { + // If custom validators threw exceptions other than validation + // exceptions, convert them to validation exceptions so we repair the + // configuration and raise an error. + throw new PhabricatorConfigValidationException($ex->getMessage()); + } + } + if ($option->isCustomType()) { try { return $option->getCustomObject()->validateOption($option, $value); } catch (Exception $ex) { - // If custom validators threw exceptions, convert them to configuation - // validation exceptions so we repair the configuration and raise - // an error. throw new PhabricatorConfigValidationException($ex->getMessage()); } } @@ -41,14 +52,6 @@ $option->getKey())); } break; - case 'int': - if (!is_int($value)) { - throw new PhabricatorConfigValidationException( - pht( - "Option '%s' is of type int, but value is not an integer.", - $option->getKey())); - } - break; case 'string': if (!is_string($value)) { throw new PhabricatorConfigValidationException( diff --git a/src/applications/config/option/PhabricatorConfigOption.php b/src/applications/config/option/PhabricatorConfigOption.php --- a/src/applications/config/option/PhabricatorConfigOption.php +++ b/src/applications/config/option/PhabricatorConfigOption.php @@ -175,6 +175,12 @@ return $this->type; } + public function newOptionType() { + $type_key = $this->getType(); + $type_map = PhabricatorConfigType::getAllTypes(); + return idx($type_map, $type_key); + } + public function isCustomType() { return !strncmp($this->getType(), 'custom:', 7); } diff --git a/src/applications/config/type/PhabricatorConfigType.php b/src/applications/config/type/PhabricatorConfigType.php new file mode 100644 --- /dev/null +++ b/src/applications/config/type/PhabricatorConfigType.php @@ -0,0 +1,115 @@ +getPhobjectClassConstant('TYPEKEY'); + } + + final public static function getAllTypes() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getTypeKey') + ->execute(); + } + + public function isValuePresentInRequest( + PhabricatorConfigOption $option, + AphrontRequest $request) { + $http_type = $this->newHTTPParameterType(); + return $http_type->getExists($request, 'value'); + } + + public function readValueFromRequest( + PhabricatorConfigOption $option, + AphrontRequest $request) { + $http_type = $this->newHTTPParameterType(); + return $http_type->getValue($request, 'value'); + } + + abstract protected function newHTTPParameterType(); + + public function validateValue(PhabricatorConfigOption $option, $value) { + return array(); + } + + public function newTransaction( + PhabricatorConfigOption $option, + $value) { + + $xaction_value = $this->newTransactionValue($option, $value); + + return id(new PhabricatorConfigTransaction()) + ->setTransactionType(PhabricatorConfigTransaction::TYPE_EDIT) + ->setNewValue( + array( + 'deleted' => false, + 'value' => $xaction_value, + )); + } + + protected function newTransactionValue( + PhabricatorConfigOption $option, + $value) { + return $value; + } + + public function newDisplayValue( + PhabricatorConfigOption $option, + $value) { + return $value; + } + + public function newControls( + PhabricatorConfigOption $option, + $value, + $error) { + + $control = $this->newControl($option) + ->setError($error) + ->setLabel(pht('Database Value')) + ->setName('value'); + + $value = $this->newControlValue($option, $value); + $control->setValue($value); + + return array( + $control, + ); + } + + abstract protected function newControl(PhabricatorConfigOption $option); + + protected function newControlValue( + PhabricatorConfigOption $option, + $value) { + return $value; + } + + protected function newException($message) { + return new PhabricatorConfigValidationException($message); + } + + public function newValueFromRequestValue( + PhabricatorConfigOption $option, + $value) { + return $this->newCanonicalValue($option, $value); + } + + public function newValueFromCommandLineValue( + PhabricatorConfigOption $option, + $value) { + return $this->newCanonicalValue($option, $value); + } + + protected function newCanonicalValue( + PhabricatorConfigOption $option, + $value) { + return $value; + } + + abstract public function validateStoredValue( + PhabricatorConfigOption $option, + $value); + +} diff --git a/src/applications/config/type/PhabricatorIntConfigType.php b/src/applications/config/type/PhabricatorIntConfigType.php new file mode 100644 --- /dev/null +++ b/src/applications/config/type/PhabricatorIntConfigType.php @@ -0,0 +1,36 @@ +newException( + pht( + 'Value for option "%s" must be an integer.', + $option->getKey())); + } + + return (int)$value; + } + + public function validateStoredValue( + PhabricatorConfigOption $option, + $value) { + + if (!is_int($value)) { + throw $this->newException( + pht( + 'Option "%s" is of type "%s", but the configured value is not '. + 'an integer.', + $option->getKey(), + $this->getTypeKey())); + } + } + +} diff --git a/src/applications/config/type/PhabricatorTextConfigType.php b/src/applications/config/type/PhabricatorTextConfigType.php new file mode 100644 --- /dev/null +++ b/src/applications/config/type/PhabricatorTextConfigType.php @@ -0,0 +1,21 @@ +