From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 21 Feb 2018 03:16:51 +0000 (-0800)
Subject: rdbms: make sure non-native replace() uses one transaction
X-Git-Tag: 1.31.0-rc.0~556
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/ajouter.php?a=commitdiff_plain;h=a9af460c3db88c8e0c6642c5981f0201e0638dae;p=lhc%2Fweb%2Fwiklou.git

rdbms: make sure non-native replace() uses one transaction

This is similar to what upsert() already does

Change-Id: Ide83eefe0d937fb2cdc20aa3c7dc9654c4d34beb
---

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 9a8996c7b3..572a798c06 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2244,37 +2244,51 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 			$rows = [ $rows ];
 		}
 
-		$affectedRowCount = 0;
-		foreach ( $rows as $row ) {
-			// Delete rows which collide with this one
-			$indexWhereClauses = [];
-			foreach ( $uniqueIndexes as $index ) {
-				$indexColumns = (array)$index;
-				$indexRowValues = array_intersect_key( $row, array_flip( $indexColumns ) );
-				if ( count( $indexRowValues ) != count( $indexColumns ) ) {
-					throw new DBUnexpectedError(
-						$this,
-						'New record does not provide all values for unique key (' .
+		$useTrx = !$this->trxLevel;
+		if ( $useTrx ) {
+			$this->begin( $fname, self::TRANSACTION_INTERNAL );
+		}
+		try {
+			$affectedRowCount = 0;
+			foreach ( $rows as $row ) {
+				// Delete rows which collide with this one
+				$indexWhereClauses = [];
+				foreach ( $uniqueIndexes as $index ) {
+					$indexColumns = (array)$index;
+					$indexRowValues = array_intersect_key( $row, array_flip( $indexColumns ) );
+					if ( count( $indexRowValues ) != count( $indexColumns ) ) {
+						throw new DBUnexpectedError(
+							$this,
+							'New record does not provide all values for unique key (' .
 							implode( ', ', $indexColumns ) . ')'
-					);
-				} elseif ( in_array( null, $indexRowValues, true ) ) {
-					throw new DBUnexpectedError(
-						$this,
-						'New record has a null value for unique key (' .
+						);
+					} elseif ( in_array( null, $indexRowValues, true ) ) {
+						throw new DBUnexpectedError(
+							$this,
+							'New record has a null value for unique key (' .
 							implode( ', ', $indexColumns ) . ')'
-					);
+						);
+					}
+					$indexWhereClauses[] = $this->makeList( $indexRowValues, LIST_AND );
 				}
-				$indexWhereClauses[] = $this->makeList( $indexRowValues, LIST_AND );
-			}
 
-			if ( $indexWhereClauses ) {
-				$this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
+				if ( $indexWhereClauses ) {
+					$this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
+					$affectedRowCount += $this->affectedRows();
+				}
+
+				// Now insert the row
+				$this->insert( $table, $row, $fname );
 				$affectedRowCount += $this->affectedRows();
 			}
-
-			// Now insert the row
-			$this->insert( $table, $row, $fname );
-			$affectedRowCount += $this->affectedRows();
+		} catch ( Exception $e ) {
+			if ( $useTrx ) {
+				$this->rollback( $fname, self::FLUSHING_INTERNAL );
+			}
+			throw $e;
+		}
+		if ( $useTrx ) {
+			$this->commit( $fname, self::FLUSHING_INTERNAL );
 		}
 
 		$this->affectedRowCount = $affectedRowCount;
diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 184d6260f8..ebf6e45199 100644
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -559,11 +559,11 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 					'uniqueIndexes' => [ 'field' ],
 					'rows' => [ 'field' => 'text', 'field2' => 'text2' ],
 				],
-				"DELETE FROM replace_table " .
+				"BEGIN; DELETE FROM replace_table " .
 					"WHERE (field = 'text'); " .
 					"INSERT INTO replace_table " .
 					"(field,field2) " .
-					"VALUES ('text','text2')"
+					"VALUES ('text','text2'); COMMIT"
 			],
 			[
 				[
@@ -575,11 +575,11 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 						'md_deps' => 'deps',
 					],
 				],
-				"DELETE FROM module_deps " .
+				"BEGIN; DELETE FROM module_deps " .
 					"WHERE (md_module = 'module' AND md_skin = 'skin'); " .
 					"INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
-					"VALUES ('module','skin','deps')"
+					"VALUES ('module','skin','deps'); COMMIT"
 			],
 			[
 				[
@@ -597,7 +597,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 						],
 					],
 				],
-				"DELETE FROM module_deps " .
+				"BEGIN; DELETE FROM module_deps " .
 					"WHERE (md_module = 'module' AND md_skin = 'skin'); " .
 					"INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
@@ -606,7 +606,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 					"WHERE (md_module = 'module2' AND md_skin = 'skin2'); " .
 					"INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
-					"VALUES ('module2','skin2','deps2')"
+					"VALUES ('module2','skin2','deps2'); COMMIT"
 			],
 			[
 				[
@@ -624,7 +624,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 						],
 					],
 				],
-				"DELETE FROM module_deps " .
+				"BEGIN; DELETE FROM module_deps " .
 					"WHERE (md_module = 'module') OR (md_skin = 'skin'); " .
 					"INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
@@ -633,7 +633,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 					"WHERE (md_module = 'module2') OR (md_skin = 'skin2'); " .
 					"INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
-					"VALUES ('module2','skin2','deps2')"
+					"VALUES ('module2','skin2','deps2'); COMMIT"
 			],
 			[
 				[
@@ -645,9 +645,9 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 						'md_deps' => 'deps',
 					],
 				],
-				"INSERT INTO module_deps " .
+				"BEGIN; INSERT INTO module_deps " .
 					"(md_module,md_skin,md_deps) " .
-					"VALUES ('module','skin','deps')"
+					"VALUES ('module','skin','deps'); COMMIT"
 			],
 		];
 	}