Make CliInstaller control the processing logic of the error

Previously, if there was an error during CLI installation,
CliInstaller::showStatusMessage() exited the script directly. The exit
timing of the script should be given to the caller, not the callee.
So, I coding:
[1] Remove `exit()` from CliInstaller::showStatusMessage()
[2] Make the callee to return Status, the caller determine how to handle these Status
[3] Strictly check the key database type instead of just outputting message

Bug: T46511
Change-Id: I72ffd33fe5c592b9ea78f37bae5a9c081295c624

diff --git a/autoload.php b/autoload.php
index e6e6504..c417986 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -884,6 +884,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php',
        'MediaWiki\\HeaderCallback' => __DIR__ . '/includes/HeaderCallback.php',
        'MediaWiki\\Http\\HttpRequestFactory' => __DIR__ . '/includes/http/HttpRequestFactory.php',
+       'MediaWiki\\Installer\\InstallException' => __DIR__ . '/includes/installer/InstallException.php',
        'MediaWiki\\Interwiki\\ClassicInterwikiLookup' => __DIR__ . '/includes/interwiki/ClassicInterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookup' => __DIR__ . '/includes/interwiki/InterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookupAdapter' => __DIR__ . '/includes/interwiki/InterwikiLookupAdapter.php',

diff --git a/includes/installer/CliInstaller.php b/includes/installer/CliInstaller.php
index 567fb10..99d594d 100644 (file)
--- a/includes/installer/CliInstaller.php
+++ b/includes/installer/CliInstaller.php
@@ -21,6 +21,7 @@
  * @ingroup Deployment
  */
 
+use MediaWiki\Installer\InstallException;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -51,6 +52,7 @@ class CliInstaller extends Installer {
         * @param string $siteName
         * @param string|null $admin
         * @param array $options
+        * @throws InstallException
         */
        function __construct( $siteName, $admin = null, array $options = [] ) {
                global $wgContLang;
@@ -114,7 +116,7 @@ class CliInstaller extends Installer {
                        $status = $this->validateExtensions(
                                'extension', 'extensions', $options['extensions'] );
                        if ( !$status->isOK() ) {
-                               $this->showStatusMessage( $status );
+                               throw new InstallException( $status );
                        }
                        $this->setVar( '_Extensions', $status->value );
                } elseif ( isset( $options['with-extensions'] ) ) {
@@ -125,7 +127,7 @@ class CliInstaller extends Installer {
                if ( isset( $options['skins'] ) ) {
                        $status = $this->validateExtensions( 'skin', 'skins', $options['skins'] );
                        if ( !$status->isOK() ) {
-                               $this->showStatusMessage( $status );
+                               throw new InstallException( $status );
                        }
                        $skins = $status->value;
                } else {
@@ -176,15 +178,23 @@ class CliInstaller extends Installer {
 
                $vars = Installer::getExistingLocalSettings();
                if ( $vars ) {
-                       $this->showStatusMessage(
-                               Status::newFatal( "config-localsettings-cli-upgrade" )
-                       );
+                       $status = Status::newFatal( "config-localsettings-cli-upgrade" );
+                       $this->showStatusMessage( $status );
+                       return $status;
                }
 
-               $this->performInstallation(
+               $result = $this->performInstallation(
                        [ $this, 'startStage' ],
                        [ $this, 'endStage' ]
                );
+               // PerformInstallation bails on a fatal, so make sure the last item
+               // completed before giving 'next.' Likewise, only provide back on failure
+               $lastStepStatus = end( $result );
+               if ( $lastStepStatus->isOk() ) {
+                       return Status::newGood();
+               } else {
+                       return $lastStepStatus;
+               }
        }
 
        /**
@@ -248,11 +258,6 @@ class CliInstaller extends Installer {
                                $this->showMessage( ...$w );
                        }
                }
-
-               if ( !$status->isOK() ) {
-                       echo "\n";
-                       exit( 1 );
-               }
        }
 
        public function envCheckPath() {

diff --git a/includes/installer/InstallException.php b/includes/installer/InstallException.php
new file mode 100644 (file)
index 0000000..a03a5aa
--- /dev/null
+++ b/includes/installer/InstallException.php
@@ -0,0 +1,51 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Installer;
+
+use Throwable;
+
+/**
+ * Exception thrown if an error occur which installation
+ * @ingroup Exception
+ */
+class InstallException extends \MWException {
+       /**
+        * @var \Status State when an exception occurs
+        */
+       private $status;
+
+       /**
+        * InstallException constructor.
+        * @param \Status $status State when an exception occurs
+        * @param string $message The Exception message to throw
+        * @param int $code The Exception code
+        * @param Throwable|null $previous The previous throwable used for the exception chaining
+        */
+       public function __construct( \Status $status, $message = '', $code = 0,
+               Throwable $previous = null ) {
+               parent::__construct( $message, $code, $previous );
+               $this->status = $status;
+       }
+
+       public function getStatus() : \Status {
+               return $this->status;
+       }
+}

diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php
index f6a5c41..7452b7c 100644 (file)
--- a/includes/installer/Installer.php
+++ b/includes/installer/Installer.php
@@ -756,6 +756,8 @@ abstract class Installer {
         */
        protected function envCheckDB() {
                global $wgLang;
+               /** @var string|null $dbType The user-specified database type */
+               $dbType = $this->getVar( 'wgDBtype' );
 
                $allNames = [];
 
@@ -768,25 +770,27 @@ abstract class Installer {
                $databases = $this->getCompiledDBs();
 
                $databases = array_flip( $databases );
+               $ok = true;
                foreach ( array_keys( $databases ) as $db ) {
                        $installer = $this->getDBInstaller( $db );
                        $status = $installer->checkPrerequisites();
                        if ( !$status->isGood() ) {
+                               if ( !$this instanceof WebInstaller && $db === $dbType ) {
+                                       // Strictly check the key database type instead of just outputting message
+                                       // Note: No perform this check run from the web installer, since this method always called by
+                                       // the welcome page under web installation, so $dbType will always be 'mysql'
+                                       $ok = false;
+                               }
                                $this->showStatusMessage( $status );
-                       }
-                       if ( !$status->isOK() ) {
                                unset( $databases[$db] );
                        }
                }
                $databases = array_flip( $databases );
                if ( !$databases ) {
                        $this->showError( 'config-no-db', $wgLang->commaList( $allNames ), count( $allNames ) );
-
-                       // @todo FIXME: This only works for the web installer!
                        return false;
                }
-
-               return true;
+               return $ok;
        }
 
        /**
@@ -1586,7 +1590,7 @@ abstract class Installer {
         * @param callable $startCB A callback array for the beginning of each step
         * @param callable $endCB A callback array for the end of each step
         *
-        * @return array Array of Status objects
+        * @return Status[] Array of Status objects
         */
        public function performInstallation( $startCB, $endCB ) {
                $installResults = [];

diff --git a/maintenance/install.php b/maintenance/install.php
index 1dd1909..a71bb74 100644 (file)
--- a/maintenance/install.php
+++ b/maintenance/install.php
@@ -115,7 +115,12 @@ class CommandLineInstaller extends Maintenance {
                        $this->setPassOption();
                }
 
-               $installer = InstallerOverrides::getCliInstaller( $siteName, $adminName, $this->mOptions );
+               try {
+                       $installer = InstallerOverrides::getCliInstaller( $siteName, $adminName, $this->mOptions );
+               } catch ( \MediaWiki\Installer\InstallException $e ) {
+                       $this->output( $e->getStatus()->getMessage()->parse() . "\n" );
+                       return false;
+               }
 
                $status = $installer->doEnvironmentChecks();
                if ( $status->isGood() ) {
@@ -123,17 +128,21 @@ class CommandLineInstaller extends Maintenance {
                } else {
                        $installer->showStatusMessage( $status );
 
-                       return;
+                       return false;
                }
                if ( !$envChecksOnly ) {
-                       $installer->execute();
+                       $status = $installer->execute();
+                       if ( !$status->isGood() ) {
+                               return false;
+                       }
                        $installer->writeConfigurationFile( $this->getOption( 'confpath', $IP ) );
+                       $installer->showMessage(
+                               'config-install-success',
+                               $installer->getVar( 'wgServer' ),
+                               $installer->getVar( 'wgScriptPath' )
+                       );
                }
-               $installer->showMessage(
-                       'config-install-success',
-                       $installer->getVar( 'wgServer' ),
-                       $installer->getVar( 'wgScriptPath' )
-               );
+               return true;
        }
 
        private function setDbPassOption() {