Replace MediaWikiIntegrationTestCase::$tablesUsed with automatic query tracking

This patch deprecates the $tablesUsed property, and introduces a new
utility, ChangedTablesTracker, that automatically keeps track of all
tables changed in a test. Every table used is now guaranteed to be reset
after the test.

Note that tables changed in addDBDataOnce are only cleared at the end of
the test class, or the data would be lost.

Fix a test in SpecialBlockTest which would fail with this patch.

$tablesUsed is now a no-op and can be removed from all tests that
declare it.

Bug: T342301
Change-Id: Ie2f1809dac243ef06ba0c34f039ce4e62cbf99cf

RELEASE-NOTES-1.42

@@ -104,6 +104,8 @@ because of Phabricator reports.
 * OutputPage::addParserOutputText has been marked @internal since there are no
   known users of it. Its old behavior has been deprecated and will change
   in the future without further notice.
+* MediaWikiIntegrationTestCase::$tablesUsed has been deprecated. The framework
+  now detects these automatically.
 * …
 
 === Deprecations in 1.42 ===

autoload.php

@@ -3054,6 +3054,7 @@ $wgAutoloadLocalClasses = [
 	'Wikimedia\\Rdbms\\AndExpressionGroup' => __DIR__ . '/includes/libs/rdbms/expression/AndExpressionGroup.php',
 	'Wikimedia\\Rdbms\\AtomicSectionIdentifier' => __DIR__ . '/includes/libs/rdbms/database/utils/AtomicSectionIdentifier.php',
 	'Wikimedia\\Rdbms\\Blob' => __DIR__ . '/includes/libs/rdbms/encasing/Blob.php',
+	'Wikimedia\\Rdbms\\ChangedTablesTracker' => __DIR__ . '/includes/libs/rdbms/ChangedTablesTracker.php',
 	'Wikimedia\\Rdbms\\ChronologyProtector' => __DIR__ . '/includes/libs/rdbms/ChronologyProtector.php',
 	'Wikimedia\\Rdbms\\ConfiguredReadOnlyMode' => __DIR__ . '/includes/libs/rdbms/ConfiguredReadOnlyMode.php',
 	'Wikimedia\\Rdbms\\ConnectionManager' => __DIR__ . '/includes/libs/rdbms/connectionmanager/ConnectionManager.php',

includes/libs/rdbms/ChangedTablesTracker.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace Wikimedia\Rdbms;
+
+use RuntimeException;
+
+/**
+ * Utility class that keeps a list of DB tables that were (presumably) changed by write queries. This should only be
+ * used in PHPUnit tests.
+ * This class should remain a static util class, because it must never be replaced by a mock in tests.
+ *
+ * @ingroup Database
+ * @internal
+ */
+class ChangedTablesTracker {
+	private const TRACKED_VERBS = [ 'INSERT', 'UPDATE', 'REPLACE' ];
+	private static bool $trackingEnabled = false;
+	/** @var array<string,true> Map of [ table name => true ] */
+	private static array $tableMap = [];
+
+	/**
+	 * Enables query tracking and resets the list of changed tables.
+	 */
+	public static function startTracking(): void {
+		if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
+			throw new RuntimeException( "This class is internal and should only be used in tests." );
+		}
+		if ( self::$trackingEnabled ) {
+			throw new RuntimeException( "Tracking is already enabled" );
+		}
+		self::$trackingEnabled = true;
+		self::$tableMap = [];
+	}
+
+	/**
+	 * Returns a list of changed tables, resets the internal list and disables tracking.
+	 *
+	 * @return string[]
+	 */
+	public static function getTablesAndStop(): array {
+		if ( !self::$trackingEnabled ) {
+			throw new RuntimeException( "Tracking is not enabled" );
+		}
+		$ret = array_keys( self::$tableMap );
+		self::$tableMap = [];
+		self::$trackingEnabled = false;
+		return $ret;
+	}
+
+	/**
+	 * When tracking is enabled and a query alters tables, record the list of tables that are altered.
+	 * Any table that gets dropped gets removed from the list of altered tables.
+	 */
+	public static function recordQuery( Query $query ): void {
+		if ( !self::$trackingEnabled ) {
+			return;
+		}
+		if ( !$query->isWriteQuery() ) {
+			return;
+		}
+
+		$queryVerb = $query->getVerb();
+		if ( $queryVerb === 'DROP' && preg_match( '/^DROP( TEMPORARY)? TABLE/i', $query->getSQL() ) ) {
+			// Special case: if a table is being dropped, forget about it.
+			self::$tableMap = array_diff_key( self::$tableMap, array_fill_keys( $query->getTables(), true ) );
+			return;
+		}
+		if ( !in_array( $queryVerb, self::TRACKED_VERBS, true ) ) {
+			return;
+		}
+
+		foreach ( $query->getTables() as $table ) {
+			self::$tableMap[$table] = true;
+		}
+	}
+}

includes/libs/rdbms/database/Database.php

@@ -703,6 +703,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
 		$isPermWrite = false;
 		if ( $sql->isWriteQuery() ) {
+			ChangedTablesTracker::recordQuery( $sql );
 			$pseudoPermanent = $this->flagsHolder::contains( $sql->getFlags(), self::QUERY_PSEUDO_PERMANENT );
 			$tempTableChanges = $this->getTempTableWrites( $sql, $pseudoPermanent );
 			$isPermWrite = !$tempTableChanges;

tests/phpunit/MediaWikiIntegrationTestCase.php

@@ -29,6 +29,7 @@ use MediaWiki\User\User;
 use MediaWiki\User\UserIdentityValue;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
+use Wikimedia\Rdbms\ChangedTablesTracker;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\IMaintainableDatabase;
@@ -96,6 +97,7 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 	/**
 	 * @var array
 	 * @since 1.19
+	 * @deprecated since 1.41 Tables used are now detected automatically.
 	 */
 	protected $tablesUsed = []; // tables with data
 
@@ -188,6 +190,12 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 	 */
 	private static bool $settingsLazyLoaded = false;
 
+	/**
+	 * @var string[] Used to store tables changed in the subclass addDBDataOnce method. These are only cleared in the
+	 * tearDownAfterClass method.
+	 */
+	private static array $dbDataOnceTables = [];
+
 	/**
 	 * @stable to call
 	 * @param string|null $name
@@ -230,6 +238,17 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 		}
 	}
 
+	/**
+	 * The annotation causes this to be called immediately after tearDownAfterClass()
+	 * @afterClass
+	 */
+	final public static function mediaWikiTearDownAfterClass(): void {
+		if ( static::$dbDataOnceTables ) {
+			$db = MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->getPrimaryDatabase();
+			self::resetDB( $db, self::$dbDataOnceTables );
+		}
+	}
+
 	/**
 	 * Get a DB_PRIMARY database connection reference on the current testing domain
 	 *
@@ -671,10 +690,12 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 		// This would also remove the need for the HACK that is oncePerClass().
 		if ( self::oncePerClass( $this->db ) ) {
 			$this->setUpSchema( $this->db );
-			self::resetDB( $this->db, $this->tablesUsed );
+			ChangedTablesTracker::startTracking();
 			$this->addDBDataOnce();
+			static::$dbDataOnceTables = ChangedTablesTracker::getTablesAndStop();
 		}
 
+		ChangedTablesTracker::startTracking();
 		$this->addDBData();
 	}
 
@@ -755,7 +776,10 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 		$this->temporaryHookHandlers = [];
 
 		if ( self::needsDB() ) {
-			self::resetDB( $this->db, $this->tablesUsed );
+			$tablesUsed = ChangedTablesTracker::getTablesAndStop();
+			// Do not clear tables written by addDBDataOnce, otherwise the data would need to be added again every time.
+			$tablesUsed = array_diff( $tablesUsed, static::$dbDataOnceTables );
+			self::resetDB( $this->db, $tablesUsed );
 		}
 
 		self::restoreMwServices();
@@ -1620,8 +1644,7 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 	 * implement this method and do so. This method is called once per test suite
 	 * (once per class).
 	 *
-	 * Note data added by this method may be removed by resetDB() depending on
-	 * the contents of $tablesUsed.
+	 * All tables touched by this method will not be cleared until the end of the test class.
 	 *
 	 * To add additional data between test function runs, override addDBData().
 	 *
@@ -2084,29 +2107,6 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 		if ( !$tablesUsed ) {
 			return;
 		}
-		// some groups of tables are connected such that if any is used, all should be cleared
-		$extraTables = [
-			'user' => [ 'user', 'user_groups', 'user_properties', 'actor' ],
-			'page' => [ 'page', 'revision', 'ip_changes', 'comment', 'archive',
-				'slots', 'content', 'content_models', 'slot_roles', 'redirect', 'change_tag' ],
-			'logging' => [ 'logging', 'log_search', 'change_tag' ],
-		];
-
-		foreach ( $extraTables as $i => $group ) {
-			if ( !array_intersect( $tablesUsed, $group ) ) {
-				unset( $extraTables[$i] );
-			}
-		}
-		$extraTables = array_values( $extraTables );
-		$tablesUsed = array_unique( array_merge( $tablesUsed, ...$extraTables ) );
-		// special case: if revision/logging is used, clear the recentchanges table
-		// (but not necessarily logging/revision, unless that is also used)
-		if (
-			array_intersect( $tablesUsed, [ 'revision', 'logging' ] ) &&
-			!in_array( 'recentchanges', $tablesUsed, true )
-		) {
-			$tablesUsed[] = 'recentchanges';
-		}
 
 		if ( in_array( 'user', $tablesUsed ) ) {
 			TestUserRegistry::clear();

tests/phpunit/includes/specials/SpecialBlockTest.php

@@ -9,6 +9,7 @@ use MediaWiki\MainConfigNames;
 use MediaWiki\Request\FauxRequest;
 use MediaWiki\Specials\SpecialBlock;
 use MediaWiki\Status\Status;
+use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
 use Wikimedia\Rdbms\IConnectionProvider;
 use Wikimedia\TestingAccessWrapper;
 
@@ -18,6 +19,8 @@ use Wikimedia\TestingAccessWrapper;
  * @coversDefaultClass \MediaWiki\Specials\SpecialBlock
  */
 class SpecialBlockTest extends SpecialPageTestBase {
+	use MockAuthorityTrait;
+
 	private $blockStore;
 
 	/**
@@ -43,11 +46,6 @@ class SpecialBlockTest extends SpecialPageTestBase {
 		$this->blockStore = $this->getServiceContainer()->getDatabaseBlockStore();
 	}
 
-	protected function tearDown(): void {
-		$this->resetTables();
-		parent::tearDown();
-	}
-
 	/**
 	 * @covers ::getFormFields
 	 */
@@ -766,9 +764,7 @@ class SpecialBlockTest extends SpecialPageTestBase {
 	public function testProcessFormErrorsHideUserProlific() {
 		$this->overrideConfigValue( MainConfigNames::HideUserContribLimit, 0 );
 
-		$performer = $this->getTestSysop()->getUser();
-		$this->overrideUserPermissions( $performer, [ 'block', 'hideuser' ] );
-
+		$performer = $this->mockRegisteredUltimateAuthority();
 		$userToBlock = $this->getTestUser()->getUser();
 		$pageSaturn = $this->getExistingTestPage( 'Saturn' );
 		$pageSaturn->doUserEditContent(
@@ -778,7 +774,7 @@ class SpecialBlockTest extends SpecialPageTestBase {
 		);
 
 		$context = new DerivativeContext( RequestContext::getMain() );
-		$context->setUser( $performer );
+		$context->setAuthority( $performer );
 
 		$result = $this->newSpecialPage()->processForm(
 			[
@@ -884,11 +880,6 @@ class SpecialBlockTest extends SpecialPageTestBase {
 		return $block;
 	}
 
-	protected function resetTables() {
-		$this->db->delete( 'ipblocks', '*', __METHOD__ );
-		$this->db->delete( 'ipblocks_restrictions', '*', __METHOD__ );
-	}
-
 	/**
 	 * Get a BlockRestrictionStore instance
 	 *

tests/phpunit/integration/includes/db/DatabaseMysqlTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\MediaWikiServices;
+use Wikimedia\Rdbms\ChangedTablesTracker;
 use Wikimedia\Rdbms\DatabaseMySQL;
 use Wikimedia\Rdbms\DBQueryDisconnectedError;
 use Wikimedia\Rdbms\DBQueryError;
@@ -30,11 +31,14 @@ class DatabaseMysqlTest extends \MediaWikiIntegrationTestCase {
 		}
 
 		$this->conn = $this->newConnection();
+		// FIXME: Tables used by this test aren't parsed correctly, see T344510.
+		ChangedTablesTracker::getTablesAndStop();
 	}
 
 	protected function tearDown(): void {
 		if ( $this->conn ) {
 			$this->conn->close( __METHOD__ );
+			ChangedTablesTracker::startTracking();
 		}
 
 		parent::tearDown();