From 7518475bb1e711d119fd1595d81b00ac827901d1 Mon Sep 17 00:00:00 2001
From: Eddie Greiner-Petter <git@eddie-sh.de>
Date: Sun, 7 May 2017 01:59:58 +0200
Subject: [PATCH] Add DatabaseUpdater::modifyTable

This introduces the function modifyTable to the DatabaseUpdater class
which will allow to alter multiple columns on the same table in a single
sql patch file. In favor of this new function, usage of
MssqlUpdater::updateSchema for mssql patches is discouraged for the
future.

Also fix T70786, which is about update rows being inserted regardless
whether applying the patch was successful or not (no retry on failure).
Now update rows only get inserted if applying the patch was successful.

Bug: T70786
Change-Id: Id2a1583cd9efa5ef271ae67238119bc24b76a3c6
---
 includes/installer/DatabaseUpdater.php | 34 ++++++++++++++++++++++++--
 includes/installer/MssqlUpdater.php    | 12 ++++++---
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php
index a4cb6959f0..e5cbb7ce71 100644
--- a/includes/installer/DatabaseUpdater.php
+++ b/includes/installer/DatabaseUpdater.php
@@ -926,11 +926,41 @@ abstract class DatabaseUpdater {
 		} elseif ( $this->updateRowExists( $updateKey ) ) {
 			$this->output( "...$field in table $table already modified by patch $patch.\n" );
 		} else {
-			$this->insertUpdateRow( $updateKey );
+			$apply = $this->applyPatch( $patch, $fullpath, "Modifying $field field of table $table" );
+			if ( $apply ) {
+				$this->insertUpdateRow( $updateKey );
+			}
+			return $apply;
+		}
+		return true;
+	}
 
-			return $this->applyPatch( $patch, $fullpath, "Modifying $field field of table $table" );
+	/**
+	 * Modify an existing table, similar to modifyField. Intended for changes that
+	 *  touch more than one column on a table.
+	 *
+	 * @param string $table Name of the table to modify
+	 * @param string $patch Name of the patch file to apply
+	 * @param string $fullpath Whether to treat $patch path as relative or not, defaults to false
+	 * @return bool False if this was skipped because of schema changes being skipped
+	 */
+	public function modifyTable( $table, $patch,  $fullpath = false ) {
+		if ( !$this->doTable( $table ) ) {
+			return true;
 		}
 
+		$updateKey = "$table-$patch";
+		if ( !$this->db->tableExists( $table, __METHOD__ ) ) {
+			$this->output( "...$table table does not exist, skipping modify table patch.\n" );
+		} elseif ( $this->updateRowExists( $updateKey ) ) {
+			$this->output( "...table $table already modified by patch $patch.\n" );
+		} else {
+			$apply = $this->applyPatch( $patch, $fullpath, "Modifying table $table" );
+			if ( $apply ) {
+				$this->insertUpdateRow( $updateKey );
+			}
+			return $apply;
+		}
 		return true;
 	}
 
diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php
index dfe595e92d..1a9915da66 100644
--- a/includes/installer/MssqlUpdater.php
+++ b/includes/installer/MssqlUpdater.php
@@ -114,7 +114,9 @@ class MssqlUpdater extends DatabaseUpdater {
 
 	/**
 	 * General schema update for a table that touches more than one field or requires
-	 * destructive actions (such as dropping and recreating the table).
+	 * destructive actions (such as dropping and recreating the table). NOTE: Usage of
+	 * this function is highly discouraged, use it's successor DatabaseUpdater::modifyTable
+	 * instead.
 	 *
 	 * @param string $table
 	 * @param string $updatekey
@@ -127,9 +129,11 @@ class MssqlUpdater extends DatabaseUpdater {
 		} elseif ( $this->updateRowExists( $updatekey ) ) {
 			$this->output( "...$table already had schema updated by $patch.\n" );
 		} else {
-			$this->insertUpdateRow( $updatekey );
-
-			return $this->applyPatch( $patch, $fullpath, "Updating schema of table $table" );
+			$apply = $this->applyPatch( $patch, $fullpath, "Updating schema of table $table" );
+			if ( $apply ) {
+				$this->insertUpdateRow( $updatekey );
+			}
+			return $apply;
 		}
 
 		return true;
-- 
2.20.1