From b1932f1f5660501dc2de44d396e2ed45068e1bd1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 6 Sep 2016 11:03:17 -0700 Subject: [PATCH] Split setup checks into "preflight" and "normal" checks Summary: Ref T11589. Currently, initialization order is a bit tangled: we load configuration from the database, then later test if we can connect to the database. Instead, I'm going to do: preflight checks ("PHP Version OK?", "Extensions installed?"), then configuration, then normal setup checks. To prepare for this, flag core checks as "preflight" and add a setup panel to visually confirm that I didn't miss anything. Test Plan: {F1803210} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11589 Differential Revision: https://secure.phabricator.com/D16499 --- src/__phutil_library_map__.php | 2 + .../check/PhabricatorDatabaseSetupCheck.php | 2 +- .../check/PhabricatorExtensionsSetupCheck.php | 4 +- .../check/PhabricatorPHPConfigSetupCheck.php | 4 +- .../config/check/PhabricatorSetupCheck.php | 20 +++++++- .../PhabricatorConfigSetupCheckModule.php | 49 +++++++++++++++++++ 6 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 src/applications/config/module/PhabricatorConfigSetupCheckModule.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 82dacf6c70..133ef9e5f4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2200,6 +2200,7 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaQuery' => 'applications/config/schema/PhabricatorConfigSchemaQuery.php', 'PhabricatorConfigSchemaSpec' => 'applications/config/schema/PhabricatorConfigSchemaSpec.php', 'PhabricatorConfigServerSchema' => 'applications/config/schema/PhabricatorConfigServerSchema.php', + 'PhabricatorConfigSetupCheckModule' => 'applications/config/module/PhabricatorConfigSetupCheckModule.php', 'PhabricatorConfigSiteModule' => 'applications/config/module/PhabricatorConfigSiteModule.php', 'PhabricatorConfigSiteSource' => 'infrastructure/env/PhabricatorConfigSiteSource.php', 'PhabricatorConfigSource' => 'infrastructure/env/PhabricatorConfigSource.php', @@ -6953,6 +6954,7 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaQuery' => 'Phobject', 'PhabricatorConfigSchemaSpec' => 'Phobject', 'PhabricatorConfigServerSchema' => 'PhabricatorConfigStorageSchema', + 'PhabricatorConfigSetupCheckModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigSource' => 'Phobject', diff --git a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php index dab9ad3b54..b99cd37adc 100644 --- a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php +++ b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php @@ -8,7 +8,7 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck { public function getExecutionOrder() { // This must run after basic PHP checks, but before most other checks. - return 0.5; + return 500; } protected function executeChecks() { diff --git a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php index fa723cc7da..0840e545e4 100644 --- a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php @@ -6,8 +6,8 @@ final class PhabricatorExtensionsSetupCheck extends PhabricatorSetupCheck { return self::GROUP_PHP; } - public function getExecutionOrder() { - return 0; + public function isPreflightCheck() { + return true; } protected function executeChecks() { diff --git a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php index ec61b4764d..86e885614a 100644 --- a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php @@ -6,8 +6,8 @@ final class PhabricatorPHPConfigSetupCheck extends PhabricatorSetupCheck { return self::GROUP_PHP; } - public function getExecutionOrder() { - return 0; + public function isPreflightCheck() { + return true; } protected function executeChecks() { diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php index a657c52799..5970b0ca81 100644 --- a/src/applications/config/check/PhabricatorSetupCheck.php +++ b/src/applications/config/check/PhabricatorSetupCheck.php @@ -12,7 +12,25 @@ abstract class PhabricatorSetupCheck extends Phobject { const GROUP_IMPORTANT = 'important'; public function getExecutionOrder() { - return 1; + if ($this->isPreflightCheck()) { + return 0; + } else { + return 1000; + } + } + + /** + * Should this check execute before we load configuration? + * + * The majority of checks (particularly, those checks which examine + * configuration) should run in the normal setup phase, after configuration + * loads. However, a small set of critical checks (mostly, tests for PHP + * setup and extensions) need to run before we can load configuration. + * + * @return bool True to execute before configuration is loaded. + */ + public function isPreflightCheck() { + return false; } final protected function newIssue($key) { diff --git a/src/applications/config/module/PhabricatorConfigSetupCheckModule.php b/src/applications/config/module/PhabricatorConfigSetupCheckModule.php new file mode 100644 index 0000000000..886a2e659f --- /dev/null +++ b/src/applications/config/module/PhabricatorConfigSetupCheckModule.php @@ -0,0 +1,49 @@ +getViewer(); + + $checks = PhabricatorSetupCheck::loadAllChecks(); + + $rows = array(); + foreach ($checks as $key => $check) { + if ($check->isPreflightCheck()) { + $icon = id(new PHUIIconView())->setIcon('fa-plane blue'); + } else { + $icon = id(new PHUIIconView())->setIcon('fa-times grey'); + } + + $rows[] = array( + $check->getExecutionOrder(), + $icon, + get_class($check), + ); + } + + return id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Order'), + pht('Preflight'), + pht('Class'), + )) + ->setColumnClasses( + array( + null, + null, + 'pri wide', + )); + } + +}