diff --git a/RELEASE-NOTES-1.42 b/RELEASE-NOTES-1.42 index 0a6c993ddb2..34ec09ea5c4 100644 --- a/RELEASE-NOTES-1.42 +++ b/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 === diff --git a/autoload.php b/autoload.php index f67f794f02b..25d42d107c9 100644 --- a/autoload.php +++ b/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', diff --git a/includes/libs/rdbms/ChangedTablesTracker.php b/includes/libs/rdbms/ChangedTablesTracker.php new file mode 100644 index 00000000000..b87d81b7e0d --- /dev/null +++ b/includes/libs/rdbms/ChangedTablesTracker.php @@ -0,0 +1,76 @@ + 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; + } + } +} diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index bdba056c923..f38fd025a42 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/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; diff --git a/tests/phpunit/MediaWikiIntegrationTestCase.php b/tests/phpunit/MediaWikiIntegrationTestCase.php index ab8573533e8..2ac2a4f155b 100644 --- a/tests/phpunit/MediaWikiIntegrationTestCase.php +++ b/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(); diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php index 28d93b87183..feb9ea59137 100644 --- a/tests/phpunit/includes/specials/SpecialBlockTest.php +++ b/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 * diff --git a/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php b/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php index ff81309ca7b..be2dd185ad7 100644 --- a/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php +++ b/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php @@ -1,6 +1,7 @@ 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();