diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php index 187236c174..4648965e4e 100644 --- a/src/applications/config/check/PhabricatorSetupCheck.php +++ b/src/applications/config/check/PhabricatorSetupCheck.php @@ -34,20 +34,41 @@ abstract class PhabricatorSetupCheck { $cache->setKey('phabricator.setup.issues', $count); } + final public static function getConfigNeedsRepair() { + $cache = PhabricatorCaches::getSetupCache(); + return $cache->getKey('phabricator.setup.needs-repair'); + } + + final public static function setConfigNeedsRepair($needs_repair) { + $cache = PhabricatorCaches::getSetupCache(); + $cache->setKey('phabricator.setup.needs-repair', $needs_repair); + } + + final public static function deleteSetupCheckCache() { + $cache = PhabricatorCaches::getSetupCache(); + $cache->deleteKeys( + array( + 'phabricator.setup.needs-repair', + 'phabricator.setup.issues', + )); + } + final public static function willProcessRequest() { $issue_count = self::getOpenSetupIssueCount(); - if ($issue_count !== null) { - // We've already run setup checks, didn't hit any fatals, and then set - // an issue count. This means we're good and don't need to do any extra - // work. - return null; + if ($issue_count === null) { + $issues = self::runAllChecks(); + self::setOpenSetupIssueCount(count($issues)); } - $issues = self::runAllChecks(); - - self::setOpenSetupIssueCount(count($issues)); - - return null; + // Try to repair configuration unless we have a clean bill of health on it. + // We need to keep doing this on every page load until all the problems + // are fixed, which is why it's separate from setup checks (which run + // once per restart). + $needs_repair = self::getConfigNeedsRepair(); + if ($needs_repair !== false) { + $needs_repair = self::repairConfig(); + self::setConfigNeedsRepair($needs_repair); + } } final public static function runAllChecks() { @@ -76,4 +97,22 @@ abstract class PhabricatorSetupCheck { return $issues; } + final public static function repairConfig() { + $needs_repair = false; + + $options = PhabricatorApplicationConfigOptions::loadAllOptions(); + foreach ($options as $option) { + try { + $option->getGroup()->validateOption( + $option, + PhabricatorEnv::getEnvConfig($option->getKey())); + } catch (PhabricatorConfigValidationException $ex) { + PhabricatorEnv::repairConfig($option->getKey(), $option->getDefault()); + $needs_repair = true; + } + } + + return $needs_repair; + } + } diff --git a/src/applications/config/check/PhabricatorSetupCheckInvalidConfig.php b/src/applications/config/check/PhabricatorSetupCheckInvalidConfig.php index 39b7d77cf3..d17c830ee0 100644 --- a/src/applications/config/check/PhabricatorSetupCheckInvalidConfig.php +++ b/src/applications/config/check/PhabricatorSetupCheckInvalidConfig.php @@ -10,14 +10,15 @@ final class PhabricatorSetupCheckInvalidConfig extends PhabricatorSetupCheck { try { $group->validateOption( $option, - PhabricatorEnv::getEnvConfig($option->getKey())); + PhabricatorEnv::getUnrepairedEnvConfig($option->getKey())); } catch (PhabricatorConfigValidationException $ex) { $this ->newIssue('config.invalid.'.$option->getKey()) ->setName(pht("Config '%s' Invalid", $option->getKey())) ->setMessage( pht( - "Configuration option '%s' has invalid value: %s", + "Configuration option '%s' has invalid value and ". + "was restored to the default: %s", $option->getKey(), $ex->getMessage())) ->addPhabricatorConfig($option->getKey()); diff --git a/src/applications/config/editor/PhabricatorConfigEditor.php b/src/applications/config/editor/PhabricatorConfigEditor.php index 673eff2307..a2ca330fdf 100644 --- a/src/applications/config/editor/PhabricatorConfigEditor.php +++ b/src/applications/config/editor/PhabricatorConfigEditor.php @@ -99,4 +99,9 @@ final class PhabricatorConfigEditor return parent::transactionHasEffect($object, $xaction); } + protected function didApplyTransactions(array $xactions) { + // Force all the setup checks to run on the next page load. + PhabricatorSetupCheck::deleteSetupCheckCache(); + } + } diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php index 2d8004ea00..3d33a7cd6a 100644 --- a/src/applications/config/view/PhabricatorSetupIssueView.php +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -125,7 +125,7 @@ final class PhabricatorSetupIssueView extends AphrontView { $table[] = ''; $table[] = ''.phutil_escape_html($key).''; - $value = PhabricatorEnv::getEnvConfig($key); + $value = PhabricatorEnv::getUnrepairedEnvConfig($key); if ($value === null) { $value = 'null'; } else if ($value === false) { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index c9510f636d..ec717468d3 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -287,9 +287,16 @@ abstract class PhabricatorApplicationTransactionEditor $mailed); } + $this->didApplyTransactions($xactions); + return $xactions; } + protected function didApplyTransactions(array $xactions) { + // Hook for subclasses. + return; + } + private function loadHandles(array $xactions) { $phids = array(); foreach ($xactions as $key => $xaction) { diff --git a/src/infrastructure/PhabricatorSetup.php b/src/infrastructure/PhabricatorSetup.php index 96d2f725fd..e7e6a30261 100644 --- a/src/infrastructure/PhabricatorSetup.php +++ b/src/infrastructure/PhabricatorSetup.php @@ -716,29 +716,6 @@ final class PhabricatorSetup { self::write("[OKAY] Mail configuration OKAY\n"); } - self::writeHeader('CONFIG CLASSES'); - foreach (PhabricatorEnv::getRequiredClasses() as $key => $instanceof) { - $config = PhabricatorEnv::getEnvConfig($key); - if (!$config) { - self::writeNote("'$key' is not set."); - } else { - try { - $r = new ReflectionClass($config); - if (!$r->isSubclassOf($instanceof)) { - throw new Exception( - "Config setting '$key' must be an instance of '$instanceof'."); - } else if (!$r->isInstantiable()) { - throw new Exception("Config setting '$key' must be instantiable."); - } - } catch (Exception $ex) { - self::writeFailure(); - self::write("Setup failure! ".$ex->getMessage()); - return; - } - } - } - self::write("[OKAY] Config classes OKAY\n"); - self::writeHeader('SUCCESS!'); self::write( "Congratulations! Your setup seems mostly correct, or at least fairly ". diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index 970114d9d7..afa90883c6 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -51,6 +51,7 @@ final class PhabricatorEnv { private static $sourceStack; + private static $repairSource; /** * @phutil-external-symbol class PhabricatorStartup @@ -155,6 +156,28 @@ final class PhabricatorEnv { ->setName(pht("Database"))); } + public static function repairConfig($key, $value) { + if (!self::$repairSource) { + self::$repairSource = id(new PhabricatorConfigDictionarySource(array())) + ->setName(pht("Repaired Config")); + self::$sourceStack->pushSource(self::$repairSource); + } + self::$repairSource->setKeys(array($key => $value)); + } + + public static function getUnrepairedEnvConfig($key, $default = null) { + foreach (self::$sourceStack->getStack() as $source) { + if ($source === self::$repairSource) { + continue; + } + $result = $source->getKeys(array($key)); + if ($result) { + return $result[$key]; + } + } + return $default; + } + public static function getSelectedEnvironmentName() { $env_var = 'PHABRICATOR_ENV'; @@ -259,13 +282,7 @@ final class PhabricatorEnv { */ public static function newObjectFromConfig($key, $args = array()) { $class = self::getEnvConfig($key); - $object = newv($class, $args); - $instanceof = idx(self::getRequiredClasses(), $key); - if (!($object instanceof $instanceof)) { - throw new Exception("Config setting '$key' must be an instance of ". - "'$instanceof', is '".get_class($object)."'."); - } - return $object; + return newv($class, $args); } @@ -386,33 +403,6 @@ final class PhabricatorEnv { /* -( Internals )---------------------------------------------------------- */ - /** - * @task internal - */ - public static function getRequiredClasses() { - return array( - 'translation.provider' => 'PhabricatorTranslation', - 'metamta.mail-adapter' => 'PhabricatorMailImplementationAdapter', - 'metamta.maniphest.reply-handler' => 'PhabricatorMailReplyHandler', - 'metamta.differential.reply-handler' => 'PhabricatorMailReplyHandler', - 'metamta.diffusion.reply-handler' => 'PhabricatorMailReplyHandler', - 'metamta.package.reply-handler' => 'PhabricatorMailReplyHandler', - 'storage.engine-selector' => 'PhabricatorFileStorageEngineSelector', - 'search.engine-selector' => 'PhabricatorSearchEngineSelector', - 'differential.field-selector' => 'DifferentialFieldSelector', - 'maniphest.custom-task-extensions-class' => 'ManiphestTaskExtensions', - 'aphront.default-application-configuration-class' => - 'AphrontApplicationConfiguration', - 'controller.oauth-registration' => - 'PhabricatorOAuthRegistrationController', - 'mysql.implementation' => 'AphrontMySQLDatabaseConnectionBase', - 'differential.attach-task-class' => 'DifferentialTasksAttacher', - 'mysql.configuration-provider' => 'DatabaseConfigurationProvider', - 'syntax-highlighter.engine' => 'PhutilSyntaxHighlighterEngine', - ); - } - - /** * @task internal */