Page MenuHomePhabricator
Files F15447727

D18155.diff
No OneTemporary
Actions

D18155.diff
View Options

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('/^list</', $type)) {
+ $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',
$key,
- '["a", "b", "c"]');
+ '{"value1": true, "value2": true}');
$message = sprintf(
"%s\n\n %s\n",
@@ -125,18 +109,36 @@
$key,
$type),
$command);
- } else {
- $message = pht(
- 'Config key "%s" is of type "%s". Specify it in JSON.',
- $key,
- $type);
- }
- break;
+ break;
+ default:
+ if (preg_match('/^list</', $type)) {
+ $command = csprintf(
+ './bin/config set %R %s',
+ $key,
+ '["a", "b", "c"]');
+
+ $message = sprintf(
+ "%s\n\n %s\n",
+ pht(
+ 'Config key "%s" is of type "%s". Specify it in JSON. '.
+ 'For example:',
+ $key,
+ $type),
+ $command);
+ } else {
+ $message = pht(
+ 'Config key "%s" is of type "%s". Specify it in JSON.',
+ $key,
+ $type);
+ }
+ break;
+ }
+ throw new PhutilArgumentUsageException($message);
}
- throw new PhutilArgumentUsageException($message);
- }
- break;
+ break;
+ }
}
+
$use_database = $args->getArg('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 @@
+<?php
+
+abstract class PhabricatorConfigType extends Phobject {
+
+ final public function getTypeKey() {
+ return $this->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 @@
+<?php
+
+final class PhabricatorIntConfigType
+ extends PhabricatorTextConfigType {
+
+ const TYPEKEY = 'int';
+
+ protected function newCanonicalValue(
+ PhabricatorConfigOption $option,
+ $value) {
+
+ if (!preg_match('/^-?[0-9]+\z/', $value)) {
+ throw $this->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 @@
+<?php
+
+abstract class PhabricatorTextConfigType
+ extends PhabricatorConfigType {
+
+ public function isValuePresentInRequest(
+ PhabricatorConfigOption $option,
+ AphrontRequest $request) {
+ $value = parent::readValueFromRequest($option, $request);
+ return (bool)strlen($value);
+ }
+
+ protected function newHTTPParameterType() {
+ return new AphrontStringHTTPParameterType();
+ }
+
+ protected function newControl(PhabricatorConfigOption $option) {
+ return new AphrontFormTextControl();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Mar 29 2025, 1:18 AM (5 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7715218
Default Alt Text
D18155.diff (19 KB)

Event Timeline

Phabricator · Built by Phacility