From 24f771a6a30ad2c52df2c97376ab9c64d424b09a Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Thu, 4 Apr 2024 18:55:43 -0400 Subject: [PATCH] [DI] Make CategoryManager and Database injectable services Change-Id: I9fd6e7724dcf33be0b1feb19ec8eb448738cab09 --- extension.json | 26 +++++-- includes/ApiQueryLintErrors.php | 22 +++--- includes/ApiQueryLinterStats.php | 18 ++++- includes/CategoryManager.php | 12 ++-- includes/Database.php | 92 ++++++++++++++----------- includes/DatabaseFactory.php | 63 +++++++++++++++++ includes/Hooks.php | 40 ++++++----- includes/LintErrorsPager.php | 2 +- includes/RecordLintJob.php | 16 ++++- includes/ServiceWiring.php | 49 +++++++++++++ includes/SpecialLintErrors.php | 47 ++++++++----- includes/TotalsLookup.php | 4 +- maintenance/migrateNamespace.php | 5 +- maintenance/migrateTagTemplate.php | 5 +- tests/phpunit/CategoryMessagesTest.php | 44 +++++------- tests/phpunit/DatabaseTest.php | 10 ++- tests/phpunit/RecordLintJobTest.php | 25 ++++--- tests/phpunit/ServiceWiringTest.php | 21 ++++++ tests/phpunit/SpecialLintErrorsTest.php | 24 +++++-- 19 files changed, 374 insertions(+), 151 deletions(-) create mode 100644 includes/DatabaseFactory.php create mode 100644 includes/ServiceWiring.php create mode 100644 tests/phpunit/ServiceWiringTest.php diff --git a/extension.json b/extension.json index 7fcb353a..b41d01bd 100644 --- a/extension.json +++ b/extension.json @@ -30,7 +30,9 @@ "services": [ "LinkRenderer", "MainWANObjectCache", - "JobQueueGroup" + "JobQueueGroup", + "Linter.CategoryManager", + "Linter.DatabaseFactory" ] }, "schema": { @@ -47,13 +49,20 @@ "ParserLogLinterData": "main" }, "APIListModules": { - "linterrors": "MediaWiki\\Linter\\ApiQueryLintErrors" + "linterrors": { + "class": "MediaWiki\\Linter\\ApiQueryLintErrors", + "services": [ + "Linter.CategoryManager" + ] + } }, "APIMetaModules": { "linterstats": { "class": "MediaWiki\\Linter\\ApiQueryLinterStats", "services": [ - "MainWANObjectCache" + "MainWANObjectCache", + "Linter.CategoryManager", + "Linter.DatabaseFactory" ] } }, @@ -63,7 +72,9 @@ "services": [ "MainWANObjectCache", "NamespaceInfo", - "TitleParser" + "TitleParser", + "Linter.CategoryManager", + "Linter.DatabaseFactory" ] } }, @@ -71,7 +82,9 @@ "RecordLintJob": { "class": "MediaWiki\\Linter\\RecordLintJob", "services": [ - "MainWANObjectCache" + "MainWANObjectCache", + "Linter.CategoryManager", + "Linter.DatabaseFactory" ] } }, @@ -233,5 +246,8 @@ "value": false } }, + "ServiceWiringFiles": [ + "includes/ServiceWiring.php" + ], "manifest_version": 2 } diff --git a/includes/ApiQueryLintErrors.php b/includes/ApiQueryLintErrors.php index b5d2446f..8cc4a47b 100644 --- a/includes/ApiQueryLintErrors.php +++ b/includes/ApiQueryLintErrors.php @@ -29,28 +29,35 @@ use Wikimedia\ParamValidator\ParamValidator; use Wikimedia\ParamValidator\TypeDef\IntegerDef; class ApiQueryLintErrors extends ApiQueryBase { + private CategoryManager $categoryManager; + /** * @param ApiQuery $queryModule * @param string $moduleName + * @param CategoryManager $categoryManager */ - public function __construct( ApiQuery $queryModule, $moduleName ) { + public function __construct( + ApiQuery $queryModule, + string $moduleName, + CategoryManager $categoryManager + ) { parent::__construct( $queryModule, $moduleName, 'lnt' ); + $this->categoryManager = $categoryManager; } public function execute() { $params = $this->extractRequestParams(); - $categoryMgr = new CategoryManager(); $this->requireMaxOneParameter( $params, 'pageid', 'title' ); $this->requireMaxOneParameter( $params, 'namespace', 'title' ); $categories = $params['categories']; if ( !$categories ) { - $categories = $categoryMgr->getVisibleCategories(); + $categories = $this->categoryManager->getVisibleCategories(); } $this->addTables( 'linter' ); - $this->addWhereFld( 'linter_cat', array_values( $categoryMgr->getCategoryIds( + $this->addWhereFld( 'linter_cat', array_values( $this->categoryManager->getCategoryIds( $categories ) ) ); $db = $this->getDB(); @@ -84,7 +91,7 @@ class ApiQueryLintErrors extends ApiQueryBase { $result = $this->getResult(); $count = 0; foreach ( $rows as $row ) { - $lintError = Database::makeLintError( $row ); + $lintError = Database::makeLintError( $this->categoryManager, $row ); if ( !$lintError ) { continue; } @@ -119,9 +126,8 @@ class ApiQueryLintErrors extends ApiQueryBase { /** @inheritDoc */ public function getAllowedParams() { - $categoryMgr = new CategoryManager(); - $visibleCats = $categoryMgr->getVisibleCategories(); - $invisibleCats = $categoryMgr->getinvisibleCategories(); + $visibleCats = $this->categoryManager->getVisibleCategories(); + $invisibleCats = $this->categoryManager->getinvisibleCategories(); $categories = array_merge( $visibleCats, $invisibleCats ); return [ 'categories' => [ diff --git a/includes/ApiQueryLinterStats.php b/includes/ApiQueryLinterStats.php index d05737d1..01ee6cf0 100644 --- a/includes/ApiQueryLinterStats.php +++ b/includes/ApiQueryLinterStats.php @@ -27,15 +27,27 @@ use WANObjectCache; class ApiQueryLinterStats extends ApiQueryBase { private WANObjectCache $cache; + private CategoryManager $categoryManager; + private DatabaseFactory $databaseFactory; /** * @param ApiQuery $queryModule * @param string $moduleName * @param WANObjectCache $cache + * @param CategoryManager $categoryManager + * @param DatabaseFactory $databaseFactory */ - public function __construct( ApiQuery $queryModule, string $moduleName, WANObjectCache $cache ) { + public function __construct( + ApiQuery $queryModule, + string $moduleName, + WANObjectCache $cache, + CategoryManager $categoryManager, + DatabaseFactory $databaseFactory + ) { parent::__construct( $queryModule, $moduleName, 'lntrst' ); $this->cache = $cache; + $this->categoryManager = $categoryManager; + $this->databaseFactory = $databaseFactory; } /** @@ -43,9 +55,9 @@ class ApiQueryLinterStats extends ApiQueryBase { */ public function execute() { $totalsLookup = new TotalsLookup( - new CategoryManager(), + $this->categoryManager, $this->cache, - new Database( 0 ) + $this->databaseFactory->newDatabase( 0 ) ); $totals = $totalsLookup->getTotals(); ApiResult::setArrayType( $totals, 'assoc' ); diff --git a/includes/CategoryManager.php b/includes/CategoryManager.php index 102d069b..abbfb30f 100644 --- a/includes/CategoryManager.php +++ b/includes/CategoryManager.php @@ -21,10 +21,9 @@ namespace MediaWiki\Linter; use InvalidArgumentException; -use MediaWiki\MediaWikiServices; /** - * Functions for lint error categories + * CategoryManager services: functions for lint error categories. */ class CategoryManager { @@ -63,10 +62,11 @@ class CategoryManager { */ private $hasNoParams = []; - public function __construct() { - $mwServices = MediaWikiServices::getInstance(); - $linterCategories = $mwServices->getMainConfig()->get( 'LinterCategories' ); - + /** + * Do not instantiate directly: use MediaWikiServices to fetch. + * @param array $linterCategories + */ + public function __construct( array $linterCategories ) { foreach ( $linterCategories as $name => $info ) { if ( $info['enabled'] ) { $this->categories[$info['priority']][] = $name; diff --git a/includes/Database.php b/includes/Database.php index e12baa65..36f2d80c 100644 --- a/includes/Database.php +++ b/includes/Database.php @@ -22,10 +22,10 @@ namespace MediaWiki\Linter; use FormatJson; use MediaWiki\Logger\LoggerFactory; -use MediaWiki\MediaWikiServices; use stdClass; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\IReadableDatabase; +use Wikimedia\Rdbms\LBFactory; use Wikimedia\Rdbms\SelectQueryBuilder; /** @@ -47,10 +47,7 @@ class Database { public const MAX_TAG_LENGTH = 30; public const MAX_TEMPLATE_LENGTH = 250; - /** - * @var int - */ - private $pageId; + private int $pageId; /** * During the addition of this column to the table, the initial value of null allows the migrate stage code to @@ -59,38 +56,51 @@ class Database { * no nulls should exist in this field for any record, and if the migrate code times out during execution, * can be restarted and continue without duplicating work. The final code that enables the use of this field * during records search will depend on this fields index being valid for all records. - * @var int|null */ - private $namespaceID; + private ?int $namespaceID; /** - * @var CategoryManager + * Configuration options. */ - private $categoryManager; + private array $options = []; + + private CategoryManager $categoryManager; + private LBFactory $dbLoadBalancerFactory; /** * @param int $pageId * @param int|null $namespaceID + * @param array $options + * @param CategoryManager $categoryManager + * @param LBFactory $dbLoadBalancerFactory */ - public function __construct( $pageId, $namespaceID = null ) { + public function __construct( + int $pageId, + ?int $namespaceID, + array $options, + CategoryManager $categoryManager, + LBFactory $dbLoadBalancerFactory + ) { $this->pageId = $pageId; $this->namespaceID = $namespaceID; - $this->categoryManager = new CategoryManager(); + $this->options = $options; + $this->categoryManager = $categoryManager; + $this->dbLoadBalancerFactory = $dbLoadBalancerFactory; } /** * @param int $mode DB_PRIMARY or DB_REPLICA * @return IDatabase */ - public static function getDBConnectionRef( int $mode ): IDatabase { - return MediaWikiServices::getInstance()->getDBLoadBalancer()->getConnection( $mode ); + public function getDBConnectionRef( int $mode ): IDatabase { + return $this->dbLoadBalancerFactory->getMainLB()->getConnection( $mode ); } /** * @return IReadableDatabase */ - public static function getReplicaDBConnection(): IReadableDatabase { - return MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->getReplicaDatabase(); + public function getReplicaDBConnection(): IReadableDatabase { + return $this->dbLoadBalancerFactory->getReplicaDatabase(); } /** @@ -100,7 +110,7 @@ class Database { * @return bool|LintError */ public function getFromId( $id ) { - $row = self::getReplicaDBConnection()->newSelectQueryBuilder() + $row = $this->getReplicaDBConnection()->newSelectQueryBuilder() ->select( [ 'linter_cat', 'linter_params', 'linter_start', 'linter_end' ] ) ->from( 'linter' ) ->where( [ 'linter_id' => $id, 'linter_page' => $this->pageId ] ) @@ -109,7 +119,7 @@ class Database { if ( $row ) { $row->linter_id = $id; - return $this->makeLintError( $row ); + return self::makeLintError( $this->categoryManager, $row ); } else { return false; } @@ -118,12 +128,13 @@ class Database { /** * Turn a database row into a LintError object * + * @param CategoryManager $categoryManager * @param stdClass $row * @return LintError|bool false on error */ - public static function makeLintError( $row ) { + public static function makeLintError( CategoryManager $categoryManager, $row ) { try { - $name = ( new CategoryManager() )->getCategoryName( $row->linter_cat ); + $name = $categoryManager->getCategoryName( $row->linter_cat ); } catch ( MissingCategoryException $e ) { LoggerFactory::getInstance( 'Linter' )->error( 'Could not find name for id: {linter_cat}', @@ -146,7 +157,7 @@ class Database { * @return LintError[] */ public function getForPage() { - $rows = self::getReplicaDBConnection()->newSelectQueryBuilder() + $rows = $this->getReplicaDBConnection()->newSelectQueryBuilder() ->select( [ 'linter_id', 'linter_cat', 'linter_start', 'linter_end', 'linter_params' ] ) ->from( 'linter' ) ->where( [ 'linter_page' => $this->pageId ] ) @@ -155,7 +166,7 @@ class Database { $result = []; foreach ( $rows as $row ) { - $error = self::makeLintError( $row ); + $error = self::makeLintError( $this->categoryManager, $row ); if ( !$error ) { continue; } @@ -173,9 +184,6 @@ class Database { * @return array */ private function buildErrorRow( LintError $error ) { - $mwServices = MediaWikiServices::getInstance(); - $config = $mwServices->getMainConfig(); - $result = [ 'linter_page' => $this->pageId, 'linter_cat' => $this->categoryManager->getCategoryId( $error->category, $error->catId ), @@ -185,13 +193,15 @@ class Database { ]; // To enable 756101 - $enableWriteNamespaceColumn = $config->get( 'LinterWriteNamespaceColumnStage' ); + $enableWriteNamespaceColumn = + $this->options['writeNamespaceColumn'] ?? false; if ( $enableWriteNamespaceColumn && $this->namespaceID !== null ) { $result[ 'linter_namespace' ] = $this->namespaceID; } // To enable 720130 - $enableWriteTagAndTemplateColumns = $config->get( 'LinterWriteTagAndTemplateColumnsStage' ); + $enableWriteTagAndTemplateColumns = + $this->options['writeTagAndTemplateColumns'] ?? false; if ( $enableWriteTagAndTemplateColumns ) { $templateInfo = $error->templateInfo ?? ''; if ( is_array( $templateInfo ) ) { @@ -238,7 +248,7 @@ class Database { */ public function setForPage( $errors ) { $previous = $this->getForPage(); - $dbw = self::getDBConnectionRef( DB_PRIMARY ); + $dbw = $this->getDBConnectionRef( DB_PRIMARY ); if ( !$previous && !$errors ) { return [ 'deleted' => [], 'added' => [] ]; } elseif ( !$previous && $errors ) { @@ -307,7 +317,7 @@ class Database { * @return int */ private function getTotalsEstimate( $catId ) { - $dbr = self::getReplicaDBConnection(); + $dbr = $this->getReplicaDBConnection(); // First see if there are no rows, or a moderate number // within the limit specified by the MAX_ACCURATE_COUNT. // The distinction between 0, a few and a lot is important @@ -343,7 +353,7 @@ class Database { * @return int[] */ public function getTotalsForPage(): array { - $rows = self::getReplicaDBConnection()->newSelectQueryBuilder() + $rows = $this->getReplicaDBConnection()->newSelectQueryBuilder() ->select( [ 'linter_cat', 'COUNT(*) AS count' ] ) ->from( 'linter' ) ->where( [ 'linter_page' => $this->pageId ] ) @@ -395,15 +405,14 @@ class Database { * @param bool $bypassConfig * @return int number of pages updated, each with one or more linter records */ - public static function migrateNamespace( int $pageBatchSize, + public function migrateNamespace( int $pageBatchSize, int $linterBatchSize, int $sleep, bool $bypassConfig = false ): int { // code used by phpunit test, bypassed when run as a maintenance script if ( !$bypassConfig ) { - $mwServices = MediaWikiServices::getInstance(); - $config = $mwServices->getMainConfig(); - $enableMigrateNamespaceStage = $config->get( 'LinterWriteNamespaceColumnStage' ); + $enableMigrateNamespaceStage = + $this->options['writeNamespaceColumn'] ?? false; if ( !$enableMigrateNamespaceStage ) { return 0; } @@ -414,9 +423,9 @@ class Database { $logger = LoggerFactory::getInstance( 'MigrateNamespaceChannel' ); - $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); - $dbw = self::getDBConnectionRef( DB_PRIMARY ); - $dbread = self::getDBConnectionRef( DB_REPLICA ); + $lbFactory = $this->dbLoadBalancerFactory; + $dbw = $this->getDBConnectionRef( DB_PRIMARY ); + $dbread = $this->getDBConnectionRef( DB_REPLICA ); $logger->info( "Migrate namespace starting\n" ); @@ -504,15 +513,14 @@ class Database { * @param bool $bypassConfig * @return int */ - public static function migrateTemplateAndTagInfo( int $batchSize, + public function migrateTemplateAndTagInfo( int $batchSize, int $sleep, bool $bypassConfig = false ): int { // code used by phpunit test, bypassed when run as a maintenance script if ( !$bypassConfig ) { - $mwServices = MediaWikiServices::getInstance(); - $config = $mwServices->getMainConfig(); - $enableMigrateTagAndTemplateColumnsStage = $config->get( 'LinterWriteTagAndTemplateColumnsStage' ); + $enableMigrateTagAndTemplateColumnsStage = + $this->options['writeTagAndTemplateColumns'] ?? false; if ( !$enableMigrateTagAndTemplateColumnsStage ) { return 0; } @@ -523,8 +531,8 @@ class Database { $logger = LoggerFactory::getInstance( 'MigrateTagAndTemplateChannel' ); - $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); - $dbw = self::getDBConnectionRef( DB_PRIMARY ); + $lbFactory = $this->dbLoadBalancerFactory; + $dbw = $this->getDBConnectionRef( DB_PRIMARY ); $logger->info( "Migration of linter_params field to linter_tag and linter_template fields starting\n" ); diff --git a/includes/DatabaseFactory.php b/includes/DatabaseFactory.php new file mode 100644 index 00000000..e4747493 --- /dev/null +++ b/includes/DatabaseFactory.php @@ -0,0 +1,63 @@ +options = $options; + $this->categoryManager = $categoryManager; + $this->dbLoadBalancerFactory = $dbLoadBalancerFactory; + } + + /** + * Create a new Database helper. + * @param int $pageId + * @param ?int $namespaceId + * @return Database + */ + public function newDatabase( int $pageId, ?int $namespaceId = null ): Database { + return new Database( + $pageId, + $namespaceId, + $this->options, + $this->categoryManager, + $this->dbLoadBalancerFactory + ); + } +} diff --git a/includes/Hooks.php b/includes/Hooks.php index 086ba12d..0c17325d 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -53,20 +53,28 @@ class Hooks implements private LinkRenderer $linkRenderer; private WANObjectCache $cache; private JobQueueGroup $jobQueueGroup; + private CategoryManager $categoryManager; + private DatabaseFactory $databaseFactory; /** * @param LinkRenderer $linkRenderer * @param WANObjectCache $cache * @param JobQueueGroup $jobQueueGroup + * @param CategoryManager $categoryManager + * @param DatabaseFactory $databaseFactory */ public function __construct( LinkRenderer $linkRenderer, WANObjectCache $cache, - JobQueueGroup $jobQueueGroup + JobQueueGroup $jobQueueGroup, + CategoryManager $categoryManager, + DatabaseFactory $databaseFactory ) { $this->linkRenderer = $linkRenderer; $this->cache = $cache; $this->jobQueueGroup = $jobQueueGroup; + $this->categoryManager = $categoryManager; + $this->databaseFactory = $databaseFactory; } /** @@ -89,7 +97,8 @@ class Hooks implements return; } - $lintError = ( new Database( $title->getArticleID() ) )->getFromId( $lintId ); + $database = $this->databaseFactory->newDatabase( $title->getArticleID() ); + $lintError = $database->getFromId( $lintId ); if ( !$lintError ) { // Already fixed or bogus URL parameter? return; @@ -113,12 +122,11 @@ class Hooks implements */ public function onWikiPageDeletionUpdates( $wikiPage, $content, &$updates ) { $id = $wikiPage->getId(); - $cache = $this->cache; - $updates[] = new MWCallableUpdate( static function () use ( $id, $cache ) { - $database = new Database( $id ); + $updates[] = new MWCallableUpdate( function () use ( $id ) { + $database = $this->databaseFactory->newDatabase( $id ); $totalsLookup = new TotalsLookup( - new CategoryManager(), - $cache, + $this->categoryManager, + $this->cache, $database ); $totalsLookup->updateStats( $database->setForPage( [] ) ); @@ -155,9 +163,9 @@ class Hooks implements ( in_array( "mw-contentmodelchange", $tags ) && !in_array( $wikiPage->getContentModel(), self::LINTABLE_CONTENT_MODELS ) ) ) { - $database = new Database( $wikiPage->getId() ); + $database = $this->databaseFactory->newDatabase( $wikiPage->getId() ); $totalsLookup = new TotalsLookup( - new CategoryManager(), + $this->categoryManager, $this->cache, $database ); @@ -174,11 +182,10 @@ class Hooks implements * @param array &$data */ public function onAPIQuerySiteInfoGeneralInfo( $api, &$data ) { - $catManager = new CategoryManager(); $data['linter'] = [ - 'high' => $catManager->getHighPriority(), - 'medium' => $catManager->getMediumPriority(), - 'low' => $catManager->getLowPriority(), + 'high' => $this->categoryManager->getHighPriority(), + 'medium' => $this->categoryManager->getMediumPriority(), + 'low' => $this->categoryManager->getLowPriority(), ]; } @@ -196,7 +203,7 @@ class Hooks implements if ( !$pageId ) { return; } - $database = new Database( $pageId ); + $database = $this->databaseFactory->newDatabase( $pageId ); $totals = array_filter( $database->getTotalsForPage() ); if ( !$totals ) { // No errors, yay! @@ -243,10 +250,9 @@ class Hooks implements ) { return false; } - $categoryMgr = new CategoryManager(); $catCounts = []; foreach ( $data as $info ) { - if ( $categoryMgr->isKnownCategory( $info['type'] ) ) { + if ( $this->categoryManager->isKnownCategory( $info['type'] ) ) { $info[ 'dbid' ] = null; } elseif ( !isset( $info[ 'dbid' ] ) ) { continue; @@ -280,7 +286,7 @@ class Hooks implements $job = new RecordLintJob( $title, [ 'errors' => $errors, 'revision' => $revision, - ], $this->cache ); + ], $this->cache, $this->categoryManager, $this->databaseFactory ); $this->jobQueueGroup->push( $job ); return true; } diff --git a/includes/LintErrorsPager.php b/includes/LintErrorsPager.php index b6e3f3f8..9455ea77 100644 --- a/includes/LintErrorsPager.php +++ b/includes/LintErrorsPager.php @@ -203,7 +203,7 @@ class LintErrorsPager extends TablePager { $category = $this->category; $row->linter_cat = $this->categoryId; } - $lintError = Database::makeLintError( $row ); + $lintError = Database::makeLintError( $this->categoryManager, $row ); if ( !$lintError ) { return ''; diff --git a/includes/RecordLintJob.php b/includes/RecordLintJob.php index 4b4b9b29..fbf6f2c9 100644 --- a/includes/RecordLintJob.php +++ b/includes/RecordLintJob.php @@ -26,20 +26,28 @@ use WANObjectCache; class RecordLintJob extends Job { private WANObjectCache $cache; + private CategoryManager $categoryManager; + private DatabaseFactory $databaseFactory; /** * RecordLintJob constructor. * @param PageReference $page * @param array $params * @param WANObjectCache $cache + * @param CategoryManager $categoryManager + * @param DatabaseFactory $databaseFactory */ public function __construct( PageReference $page, array $params, - WANObjectCache $cache + WANObjectCache $cache, + CategoryManager $categoryManager, + DatabaseFactory $databaseFactory ) { parent::__construct( 'RecordLintJob', $page, $params ); $this->cache = $cache; + $this->categoryManager = $categoryManager; + $this->databaseFactory = $databaseFactory; } public function run() { @@ -66,9 +74,11 @@ class RecordLintJob extends Job { $errors[$error->id()] = $error; } - $lintDb = new Database( $this->title->getArticleID(), $this->title->getNamespace() ); + $lintDb = $this->databaseFactory->newDatabase( + $this->title->getArticleID(), $this->title->getNamespace() + ); $totalsLookup = new TotalsLookup( - new CategoryManager(), + $this->categoryManager, $this->cache, $lintDb ); diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php new file mode 100644 index 00000000..10087110 --- /dev/null +++ b/includes/ServiceWiring.php @@ -0,0 +1,49 @@ + static function ( MediaWikiServices $services ): CategoryManager { + return new CategoryManager( + $services->getMainConfig()->get( 'LinterCategories' ) + ); + }, + 'Linter.DatabaseFactory' => static function ( MediaWikiServices $services ): DatabaseFactory { + $config = $services->getMainConfig(); + return new DatabaseFactory( + [ + 'writeNamespaceColumn' => $config->get( 'LinterWriteNamespaceColumnStage' ), + 'writeTagAndTemplateColumns' => $config->get( 'LinterWriteTagAndTemplateColumnsStage' ), + ], + $services->get( 'Linter.CategoryManager' ), + $services->getDBLoadBalancerFactory() + ); + }, +]; +// @codeCoverageIgnoreEnd diff --git a/includes/SpecialLintErrors.php b/includes/SpecialLintErrors.php index 8f3e4a5f..ac01eff1 100644 --- a/includes/SpecialLintErrors.php +++ b/includes/SpecialLintErrors.php @@ -35,6 +35,8 @@ class SpecialLintErrors extends SpecialPage { private WANObjectCache $cache; private NamespaceInfo $namespaceInfo; private TitleParser $titleParser; + private CategoryManager $categoryManager; + private DatabaseFactory $databaseFactory; /** * @var string|null @@ -45,12 +47,22 @@ class SpecialLintErrors extends SpecialPage { * @param WANObjectCache $cache * @param NamespaceInfo $namespaceInfo * @param TitleParser $titleParser + * @param CategoryManager $categoryManager + * @param DatabaseFactory $databaseFactory */ - public function __construct( WANObjectCache $cache, NamespaceInfo $namespaceInfo, TitleParser $titleParser ) { + public function __construct( + WANObjectCache $cache, + NamespaceInfo $namespaceInfo, + TitleParser $titleParser, + CategoryManager $categoryManager, + DatabaseFactory $databaseFactory + ) { parent::__construct( 'LintErrors' ); $this->cache = $cache; $this->namespaceInfo = $namespaceInfo; $this->titleParser = $titleParser; + $this->categoryManager = $categoryManager; + $this->databaseFactory = $databaseFactory; } /** @@ -242,9 +254,10 @@ class SpecialLintErrors extends SpecialPage { if ( $titleSearch[ 'titlefield' ] !== null ) { $out->setPageTitleMsg( $this->msg( 'linter-prefix-search-subpage', $titleSearch[ 'titlefield' ] ) ); - $catManager = new CategoryManager(); $pager = new LintErrorsPager( - $this->getContext(), null, $this->getLinkRenderer(), $catManager, $namespaces, + $this->getContext(), null, + $this->getLinkRenderer(), $this->categoryManager, + $namespaces, $exactMatch, $titleSearch[ 'titlefield' ], $template, $tag ); $out->addParserOutput( $pager->getFullOutput() ); @@ -254,14 +267,13 @@ class SpecialLintErrors extends SpecialPage { return; } - $catManager = new CategoryManager(); if ( in_array( $par, $this->getSubpagesForPrefixSearch() ) ) { $this->category = $par; } if ( !$this->category ) { $this->addHelpLink( 'Help:Extension:Linter' ); - $this->showCategoryListings( $catManager ); + $this->showCategoryListings(); } else { $this->addHelpLink( "Help:Lint_errors/{$this->category}" ); $out->setPageTitleMsg( @@ -282,7 +294,9 @@ class SpecialLintErrors extends SpecialPage { if ( $titleCategorySearch[ 'titlefield' ] !== null ) { $this->showFilterForm( 'titlecategorysearch' ); $pager = new LintErrorsPager( - $this->getContext(), $this->category, $this->getLinkRenderer(), $catManager, $namespaces, + $this->getContext(), $this->category, + $this->getLinkRenderer(), $this->categoryManager, + $namespaces, $exactMatch, $titleCategorySearch[ 'titlefield' ], $template, $tag ); $out->addParserOutput( $pager->getFullOutput() ); @@ -313,21 +327,18 @@ class SpecialLintErrors extends SpecialPage { $this->showFilterForm( 'titlesearch' ); } - /** - * @param CategoryManager $catManager - */ - private function showCategoryListings( CategoryManager $catManager ) { + private function showCategoryListings() { $lookup = new TotalsLookup( - $catManager, + $this->categoryManager, $this->cache, - new Database( 0 ) + $this->databaseFactory->newDatabase( 0 ) ); $totals = $lookup->getTotals(); // Display lint issues by priority - $this->displayList( 'high', $totals, $catManager->getHighPriority() ); - $this->displayList( 'medium', $totals, $catManager->getMediumPriority() ); - $this->displayList( 'low', $totals, $catManager->getLowPriority() ); + $this->displayList( 'high', $totals, $this->categoryManager->getHighPriority() ); + $this->displayList( 'medium', $totals, $this->categoryManager->getMediumPriority() ); + $this->displayList( 'low', $totals, $this->categoryManager->getLowPriority() ); $this->displaySearchPage(); } @@ -362,8 +373,10 @@ class SpecialLintErrors extends SpecialPage { * @return string[] */ protected function getSubpagesForPrefixSearch() { - $categoryManager = new CategoryManager(); - return array_merge( $categoryManager->getVisibleCategories(), $categoryManager->getInvisibleCategories() ); + return array_merge( + $this->categoryManager->getVisibleCategories(), + $this->categoryManager->getInvisibleCategories() + ); } } diff --git a/includes/TotalsLookup.php b/includes/TotalsLookup.php index 78720297..93fc6cfe 100644 --- a/includes/TotalsLookup.php +++ b/includes/TotalsLookup.php @@ -81,9 +81,9 @@ class TotalsLookup { $totals[$cat] = $this->cache->getWithSetCallback( $this->makeKey( $cat ), WANObjectCache::TTL_INDEFINITE, - static function ( $oldValue, &$ttl, &$setOpts, $oldAsOf ) use ( $cat, $db, &$fetchedTotals ) { + function ( $oldValue, &$ttl, &$setOpts, $oldAsOf ) use ( $cat, $db, &$fetchedTotals ) { $setOpts += MWDatabase::getCacheSetOptions( - Database::getDBConnectionRef( DB_REPLICA ) + $this->db->getDBConnectionRef( DB_REPLICA ) ); if ( $fetchedTotals === false ) { $fetchedTotals = $db->getTotals(); diff --git a/maintenance/migrateNamespace.php b/maintenance/migrateNamespace.php index 3098478b..166be76b 100644 --- a/maintenance/migrateNamespace.php +++ b/maintenance/migrateNamespace.php @@ -8,7 +8,7 @@ * note: This code is based on migrateRevisionActorTemp.php and migrateLinksTable.php */ -use MediaWiki\Linter\Database; +use MediaWiki\MediaWikiServices; $IP = getenv( 'MW_INSTALL_PATH' ); if ( $IP === false ) { @@ -61,7 +61,8 @@ class MigrateNamespace extends LoggedUpdateMaintenance { $this->output( "Migrating the page table page_namespace field to the linter table...\n" ); - $updated = Database::migrateNamespace( $batchSize, $batchSize, $sleep, false ); + $database = MediaWikiServices::getInstance()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 ); + $updated = $database->migrateNamespace( $batchSize, $batchSize, $sleep, false ); $this->output( "Completed migration of page_namespace data to the linter table, $updated rows updated.\n" ); diff --git a/maintenance/migrateTagTemplate.php b/maintenance/migrateTagTemplate.php index a4cf9f4a..e9afa836 100644 --- a/maintenance/migrateTagTemplate.php +++ b/maintenance/migrateTagTemplate.php @@ -6,7 +6,7 @@ * recordLintJob has been enabled by setting LinterWriteTagAndTemplateColumnsStage true. */ -use MediaWiki\Linter\Database; +use MediaWiki\MediaWikiServices; $IP = getenv( 'MW_INSTALL_PATH' ); if ( $IP === false ) { @@ -59,7 +59,8 @@ class MigrateTagTemplate extends LoggedUpdateMaintenance { $this->output( "Migrating the linter_params field to the linter_tag and linter_template fields...\n" ); - $updated = Database::migrateTemplateAndTagInfo( $batchSize, $sleep, false ); + $database = MediaWikiServices::getInstance()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 ); + $updated = $database->migrateTemplateAndTagInfo( $batchSize, $sleep, false ); $this->output( "Completed migration of linter_params data in the linter table, $updated rows updated.\n" diff --git a/tests/phpunit/CategoryMessagesTest.php b/tests/phpunit/CategoryMessagesTest.php index 4362d4d5..a0158fad 100644 --- a/tests/phpunit/CategoryMessagesTest.php +++ b/tests/phpunit/CategoryMessagesTest.php @@ -20,7 +20,6 @@ namespace MediaWiki\Linter\Test; -use MediaWiki\Linter\CategoryManager; use MediaWikiLangTestCase; /** @@ -29,34 +28,27 @@ use MediaWikiLangTestCase; */ class CategoryMessagesTest extends MediaWikiLangTestCase { - public static function provideCategoryNames() { - $manager = new CategoryManager(); - $tests = []; - foreach ( $manager->getVisibleCategories() as $category ) { - $tests[] = [ $category ]; - } - - return $tests; - } - /** * @coversNothing - * @dataProvider provideCategoryNames */ - public function testMessagesExistence( $category ) { - $manager = new CategoryManager(); - $msgs = [ - "linter-category-$category", - "linter-category-$category-desc", - ]; - if ( !$manager->hasNoParams( $category ) ) { - $msgs[] = "linter-pager-$category-details"; - } - foreach ( $msgs as $msg ) { - $this->assertTrue( - wfMessage( $msg )->exists(), - "Missing '$msg' message" - ); + public function testMessagesExistence() { + $services = $this->getServiceContainer(); + $manager = $services->get( 'Linter.CategoryManager' ); + + foreach ( $manager->getVisibleCategories() as $category ) { + $msgs = [ + "linter-category-$category", + "linter-category-$category-desc", + ]; + if ( !$manager->hasNoParams( $category ) ) { + $msgs[] = "linter-pager-$category-details"; + } + foreach ( $msgs as $msg ) { + $this->assertTrue( + wfMessage( $msg )->exists(), + "Missing '$msg' message" + ); + } } } } diff --git a/tests/phpunit/DatabaseTest.php b/tests/phpunit/DatabaseTest.php index e7f695f9..1ab2f80d 100644 --- a/tests/phpunit/DatabaseTest.php +++ b/tests/phpunit/DatabaseTest.php @@ -29,8 +29,14 @@ use MediaWikiIntegrationTestCase; * @covers \MediaWiki\Linter\Database */ class DatabaseTest extends MediaWikiIntegrationTestCase { + private function newDatabase( int $pageId, ?int $namespaceId = null ) { + $services = $this->getServiceContainer(); + $databaseFactory = $services->get( 'Linter.DatabaseFactory' ); + return $databaseFactory->newDatabase( $pageId, $namespaceId ); + } + public function testConstructor() { - $this->assertInstanceOf( Database::class, new Database( 5 ) ); + $this->assertInstanceOf( Database::class, $this->newDatabase( 5 ) ); } private function getDummyLintErrors() { @@ -74,7 +80,7 @@ class DatabaseTest extends MediaWikiIntegrationTestCase { } public function testSetForPage() { - $lintDb = new Database( 5 ); + $lintDb = $this->newDatabase( 5 ); $dummyErrors = $this->getDummyLintErrors(); $result = $lintDb->setForPage( $dummyErrors ); $this->assertSetForPageResult( $result, [], [ 'fostered' => 1, 'obsolete-tag' => 1 ] ); diff --git a/tests/phpunit/RecordLintJobTest.php b/tests/phpunit/RecordLintJobTest.php index 0823a29c..92d1aff5 100644 --- a/tests/phpunit/RecordLintJobTest.php +++ b/tests/phpunit/RecordLintJobTest.php @@ -20,7 +20,6 @@ namespace MediaWiki\Linter\Test; -use MediaWiki\Linter\Database; use MediaWiki\Linter\LintError; use MediaWiki\Linter\RecordLintJob; use MediaWiki\Page\PageReference; @@ -34,12 +33,20 @@ use Wikimedia\Rdbms\SelectQueryBuilder; */ class RecordLintJobTest extends MediaWikiIntegrationTestCase { + private function newDatabase( int $pageId, ?int $namespaceId = null ) { + $services = $this->getServiceContainer(); + $databaseFactory = $services->get( 'Linter.DatabaseFactory' ); + return $databaseFactory->newDatabase( $pageId, $namespaceId ); + } + private function newRecordLintJob( PageReference $page, array $params ) { $services = $this->getServiceContainer(); return new RecordLintJob( $page, $params, - $services->getMainWANObjectCache() + $services->getMainWANObjectCache(), + $services->get( 'Linter.CategoryManager' ), + $services->get( 'Linter.DatabaseFactory' ) ); } @@ -120,7 +127,7 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { 'revision' => $titleAndPage[ 'revID' ] ] ); $this->assertTrue( $job->run() ); - $db = new Database( $titleAndPage[ 'pageID' ] ); + $db = $this->newDatabase( $titleAndPage[ 'pageID' ] ); $errorsFromDb = array_values( $db->getForPage() ); $this->assertCount( 1, $errorsFromDb ); $this->assertInstanceOf( LintError::class, $errorsFromDb[ 0 ] ); @@ -147,7 +154,7 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { ] ); $this->assertTrue( $job->run() ); $pageId = $titleAndPage[ 'pageID' ]; - $db = new Database( $pageId ); + $db = $this->newDatabase( $pageId ); $errorsFromDb = array_values( $db->getForPage() ); $this->assertCount( 1, $errorsFromDb ); $this->assertInstanceOf( LintError::class, $errorsFromDb[0] ); @@ -187,7 +194,7 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { ] ); $this->assertTrue( $job->run() ); $pageId = $titleAndPage[ 'pageID' ]; - $db = new Database( $pageId ); + $db = $this->newDatabase( $pageId ); $errorsFromDb = array_values( $db->getForPage() ); $this->assertCount( 1, $errorsFromDb ); $this->assertInstanceOf( LintError::class, $errorsFromDb[0] ); @@ -265,7 +272,8 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { $this->assertNull( $namespace ); // migrate unpopulated namespace_id(s) from the page table to linter table - Database::migrateNamespace( 2, 3, 0, true ); + $database = $this->newDatabase( 0 ); + $database->migrateNamespace( 2, 3, 0, true ); // Verify all linter records now have proper namespace IDs in the linter_namespace field $this->checkPagesNamespace( $titleAndPages, $namespaceIds ); @@ -373,7 +381,8 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { $this->assertSame( "", $template ); // Migrate unpopulated tag and template info from the params field - Database::migrateTemplateAndTagInfo( 3, 0, true ); + $database = $this->newDatabase( 0 ); + $database->migrateTemplateAndTagInfo( 3, 0, true ); // Verify all linter records have the proper tag and template field info migrated from the params field $this->checkPagesTagAndTemplate( $titleAndPages ); @@ -415,7 +424,7 @@ class RecordLintJobTest extends MediaWikiIntegrationTestCase { 'revision' => $titleAndPage[ 'revID' ] ] ); $this->assertTrue( $job->run() ); - $errorsFromDb = array_values( ( new Database( $titleAndPage['pageID'] ) )->getForPage() ); + $errorsFromDb = array_values( $this->newDatabase( $titleAndPage['pageID'] )->getForPage() ); $this->assertCount( 0, $errorsFromDb ); } } diff --git a/tests/phpunit/ServiceWiringTest.php b/tests/phpunit/ServiceWiringTest.php new file mode 100644 index 00000000..80261f65 --- /dev/null +++ b/tests/phpunit/ServiceWiringTest.php @@ -0,0 +1,21 @@ +getServiceContainer()->get( $name ); + $this->addToAssertionCount( 1 ); + } + + public static function provideService() { + $wiring = require __DIR__ . '/../../includes/ServiceWiring.php'; + foreach ( $wiring as $name => $_ ) { + yield $name => [ $name ]; + } + } +} diff --git a/tests/phpunit/SpecialLintErrorsTest.php b/tests/phpunit/SpecialLintErrorsTest.php index 925ac57f..2c27377b 100644 --- a/tests/phpunit/SpecialLintErrorsTest.php +++ b/tests/phpunit/SpecialLintErrorsTest.php @@ -22,8 +22,6 @@ namespace MediaWiki\Linter\Test; use ContentHandler; use Exception; -use MediaWiki\Linter\CategoryManager; -use MediaWiki\Linter\Database; use MediaWiki\Linter\RecordLintJob; use MediaWiki\Linter\SpecialLintErrors; use MediaWiki\Page\PageReference; @@ -38,12 +36,20 @@ use SpecialPageTestBase; */ class SpecialLintErrorsTest extends SpecialPageTestBase { + private function newDatabase( int $pageId, ?int $namespaceId = null ) { + $services = $this->getServiceContainer(); + $databaseFactory = $services->get( 'Linter.DatabaseFactory' ); + return $databaseFactory->newDatabase( $pageId, $namespaceId ); + } + private function newRecordLintJob( PageReference $page, array $params ) { $services = $this->getServiceContainer(); return new RecordLintJob( $page, $params, - $services->getMainWANObjectCache() + $services->getMainWANObjectCache(), + $services->get( 'Linter.CategoryManager' ), + $services->get( 'Linter.DatabaseFactory' ) ); } @@ -52,12 +58,16 @@ class SpecialLintErrorsTest extends SpecialPageTestBase { return new SpecialLintErrors( $services->getMainWANObjectCache(), $services->getNamespaceInfo(), - $services->getTitleParser() + $services->getTitleParser(), + $services->get( 'Linter.CategoryManager' ), + $services->get( 'Linter.DatabaseFactory' ) ); } public function testExecute() { - $category = ( new CategoryManager() )->getVisibleCategories()[0]; + $categoryManager = + $this->getServiceContainer()->get( 'Linter.CategoryManager' ); + $category = $categoryManager->getVisibleCategories()[0]; // Basic $html = $this->executeSpecialPage( '', null, 'qqx' )[0]; @@ -111,7 +121,7 @@ class SpecialLintErrorsTest extends SpecialPageTestBase { ] ); $this->assertTrue( $job->run() ); - $db = new Database( $titleAndPage['pageID'] ); + $db = $this->newDatabase( $titleAndPage['pageID'] ); $errorsFromDb = array_values( $db->getForPage() ); $this->assertCount( 1, $errorsFromDb ); @@ -147,7 +157,7 @@ class SpecialLintErrorsTest extends SpecialPageTestBase { ] ); $this->assertTrue( $job->run() ); - $db = new Database( $titleAndPage['pageID'] ); + $db = $this->newDatabase( $titleAndPage['pageID'] ); $errorsFromDb = array_values( $db->getForPage() ); $this->assertCount( 1, $errorsFromDb );