Construct services with ServiceOptions

And addresses some other cleanup from review comments.

Follows-Up: I9fd6e7724dcf33be0b1feb19ec8eb448738cab09
Change-Id: If87b0bf91930f0f8d89ed046d18aadb8f346f9aa
This commit is contained in:
Arlo Breault 2024-04-09 19:37:56 -04:00
parent 4f991b5d0c
commit 1c53684200
7 changed files with 63 additions and 42 deletions

View file

@ -21,6 +21,7 @@
namespace MediaWiki\Linter; namespace MediaWiki\Linter;
use FormatJson; use FormatJson;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Logger\LoggerFactory; use MediaWiki\Logger\LoggerFactory;
use stdClass; use stdClass;
use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\IDatabase;
@ -32,6 +33,10 @@ use Wikimedia\Rdbms\SelectQueryBuilder;
* Database logic * Database logic
*/ */
class Database { class Database {
public const CONSTRUCTOR_OPTIONS = [
'LinterWriteNamespaceColumnStage',
'LinterWriteTagAndTemplateColumnsStage',
];
/** /**
* Maximum number of errors to save per category, * Maximum number of errors to save per category,
@ -59,30 +64,27 @@ class Database {
*/ */
private ?int $namespaceID; private ?int $namespaceID;
/** private ServiceOptions $options;
* Configuration options.
*/
private array $options = [];
private CategoryManager $categoryManager; private CategoryManager $categoryManager;
private LBFactory $dbLoadBalancerFactory; private LBFactory $dbLoadBalancerFactory;
/** /**
* @param int $pageId * @param int $pageId
* @param int|null $namespaceID * @param int|null $namespaceID
* @param array $options * @param ServiceOptions $options
* @param CategoryManager $categoryManager * @param CategoryManager $categoryManager
* @param LBFactory $dbLoadBalancerFactory * @param LBFactory $dbLoadBalancerFactory
*/ */
public function __construct( public function __construct(
int $pageId, int $pageId,
?int $namespaceID, ?int $namespaceID,
array $options, ServiceOptions $options,
CategoryManager $categoryManager, CategoryManager $categoryManager,
LBFactory $dbLoadBalancerFactory LBFactory $dbLoadBalancerFactory
) { ) {
$this->pageId = $pageId; $this->pageId = $pageId;
$this->namespaceID = $namespaceID; $this->namespaceID = $namespaceID;
$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
$this->options = $options; $this->options = $options;
$this->categoryManager = $categoryManager; $this->categoryManager = $categoryManager;
$this->dbLoadBalancerFactory = $dbLoadBalancerFactory; $this->dbLoadBalancerFactory = $dbLoadBalancerFactory;
@ -193,16 +195,15 @@ class Database {
]; ];
// To enable 756101 // To enable 756101
$enableWriteNamespaceColumn = if (
$this->options['writeNamespaceColumn'] ?? false; $this->options->get( 'LinterWriteNamespaceColumnStage' ) &&
if ( $enableWriteNamespaceColumn && $this->namespaceID !== null ) { $this->namespaceID !== null
) {
$result[ 'linter_namespace' ] = $this->namespaceID; $result[ 'linter_namespace' ] = $this->namespaceID;
} }
// To enable 720130 // To enable 720130
$enableWriteTagAndTemplateColumns = if ( $this->options->get( 'LinterWriteTagAndTemplateColumnsStage' ) ) {
$this->options['writeTagAndTemplateColumns'] ?? false;
if ( $enableWriteTagAndTemplateColumns ) {
$templateInfo = $error->templateInfo ?? ''; $templateInfo = $error->templateInfo ?? '';
if ( is_array( $templateInfo ) ) { if ( is_array( $templateInfo ) ) {
if ( isset( $templateInfo[ 'multiPartTemplateBlock' ] ) ) { if ( isset( $templateInfo[ 'multiPartTemplateBlock' ] ) ) {
@ -411,9 +412,7 @@ class Database {
bool $bypassConfig = false ): int { bool $bypassConfig = false ): int {
// code used by phpunit test, bypassed when run as a maintenance script // code used by phpunit test, bypassed when run as a maintenance script
if ( !$bypassConfig ) { if ( !$bypassConfig ) {
$enableMigrateNamespaceStage = if ( !$this->options->get( 'LinterWriteNamespaceColumnStage' ) ) {
$this->options['writeNamespaceColumn'] ?? false;
if ( !$enableMigrateNamespaceStage ) {
return 0; return 0;
} }
} }
@ -519,9 +518,7 @@ class Database {
): int { ): int {
// code used by phpunit test, bypassed when run as a maintenance script // code used by phpunit test, bypassed when run as a maintenance script
if ( !$bypassConfig ) { if ( !$bypassConfig ) {
$enableMigrateTagAndTemplateColumnsStage = if ( !$this->options->get( 'LinterWriteTagAndTemplateColumnsStage' ) ) {
$this->options['writeTagAndTemplateColumns'] ?? false;
if ( !$enableMigrateTagAndTemplateColumnsStage ) {
return 0; return 0;
} }
} }

View file

@ -20,23 +20,24 @@
namespace MediaWiki\Linter; namespace MediaWiki\Linter;
use MediaWiki\Config\ServiceOptions;
use Wikimedia\Rdbms\LBFactory; use Wikimedia\Rdbms\LBFactory;
/** /**
* Create a Database helper specialized to a particular page id and namespace. * Create a Database helper specialized to a particular page id and namespace.
*/ */
class DatabaseFactory { class DatabaseFactory {
private array $options; private ServiceOptions $options;
private CategoryManager $categoryManager; private CategoryManager $categoryManager;
private LBFactory $dbLoadBalancerFactory; private LBFactory $dbLoadBalancerFactory;
/** /**
* @param array $options * @param ServiceOptions $options
* @param CategoryManager $categoryManager * @param CategoryManager $categoryManager
* @param LBFactory $dbLoadBalancerFactory * @param LBFactory $dbLoadBalancerFactory
*/ */
public function __construct( public function __construct(
array $options, ServiceOptions $options,
CategoryManager $categoryManager, CategoryManager $categoryManager,
LBFactory $dbLoadBalancerFactory LBFactory $dbLoadBalancerFactory
) { ) {

View file

@ -19,11 +19,12 @@
*/ */
namespace MediaWiki\Linter; namespace MediaWiki\Linter;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\MediaWikiServices; use MediaWiki\MediaWikiServices;
// PHP unit does not understand code coverage for this file // PHP unit does not understand code coverage for this file
// as the @covers annotation cannot cover a specific file // as the @covers annotation cannot cover a specific file
// This is fully tested in CategoryManagerServiceWiringTest.php // This is fully tested in ServiceWiringTest.php
// @codeCoverageIgnoreStart // @codeCoverageIgnoreStart
/** /**
* Linter wiring for MediaWiki services. * Linter wiring for MediaWiki services.
@ -35,22 +36,21 @@ return [
); );
}, },
'Linter.DatabaseFactory' => static function ( MediaWikiServices $services ): DatabaseFactory { 'Linter.DatabaseFactory' => static function ( MediaWikiServices $services ): DatabaseFactory {
$config = $services->getMainConfig();
return new DatabaseFactory( return new DatabaseFactory(
[ new ServiceOptions(
'writeNamespaceColumn' => $config->get( 'LinterWriteNamespaceColumnStage' ), Database::CONSTRUCTOR_OPTIONS,
'writeTagAndTemplateColumns' => $config->get( 'LinterWriteTagAndTemplateColumnsStage' ), $services->getMainConfig()
], ),
$services->get( 'Linter.CategoryManager' ), $services->get( 'Linter.CategoryManager' ),
$services->getDBLoadBalancerFactory() $services->getDBLoadBalancerFactory()
); );
}, },
'Linter.TotalsLookup' => static function ( MediaWikiServices $services ): TotalsLookup { 'Linter.TotalsLookup' => static function ( MediaWikiServices $services ): TotalsLookup {
$config = $services->getMainConfig();
return new TotalsLookup( return new TotalsLookup(
[ new ServiceOptions(
'sampleFactor' => $config->get( 'LinterStatsdSampleFactor' ), TotalsLookup::CONSTRUCTOR_OPTIONS,
], $services->getMainConfig()
),
$services->getMainWANObjectCache(), $services->getMainWANObjectCache(),
$services->getStatsdDataFactory(), $services->getStatsdDataFactory(),
$services->get( 'Linter.CategoryManager' ) $services->get( 'Linter.CategoryManager' )

View file

@ -21,6 +21,7 @@
namespace MediaWiki\Linter; namespace MediaWiki\Linter;
use IBufferingStatsdDataFactory; use IBufferingStatsdDataFactory;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\WikiMap\WikiMap; use MediaWiki\WikiMap\WikiMap;
use WANObjectCache; use WANObjectCache;
use Wikimedia\Rdbms\Database as MWDatabase; use Wikimedia\Rdbms\Database as MWDatabase;
@ -30,24 +31,28 @@ use Wikimedia\Rdbms\Database as MWDatabase;
* lint errors in each category * lint errors in each category
*/ */
class TotalsLookup { class TotalsLookup {
public const CONSTRUCTOR_OPTIONS = [
'LinterStatsdSampleFactor',
];
private array $options; private ServiceOptions $options;
private WANObjectCache $cache; private WANObjectCache $cache;
private IBufferingStatsdDataFactory $statsdDataFactory; private IBufferingStatsdDataFactory $statsdDataFactory;
private CategoryManager $categoryManager; private CategoryManager $categoryManager;
/** /**
* @param array $options * @param ServiceOptions $options
* @param WANObjectCache $cache * @param WANObjectCache $cache
* @param IBufferingStatsdDataFactory $statsdDataFactory * @param IBufferingStatsdDataFactory $statsdDataFactory
* @param CategoryManager $categoryManager * @param CategoryManager $categoryManager
*/ */
public function __construct( public function __construct(
array $options, ServiceOptions $options,
WANObjectCache $cache, WANObjectCache $cache,
IBufferingStatsdDataFactory $statsdDataFactory, IBufferingStatsdDataFactory $statsdDataFactory,
CategoryManager $categoryManager CategoryManager $categoryManager
) { ) {
$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
$this->options = $options; $this->options = $options;
$this->cache = $cache; $this->cache = $cache;
$this->statsdDataFactory = $statsdDataFactory; $this->statsdDataFactory = $statsdDataFactory;
@ -105,7 +110,7 @@ class TotalsLookup {
* @param array $changes * @param array $changes
*/ */
public function updateStats( Database $db, array $changes ) { public function updateStats( Database $db, array $changes ) {
$linterStatsdSampleFactor = $this->options['sampleFactor'] ?? false; $linterStatsdSampleFactor = $this->options->get( 'LinterStatsdSampleFactor' );
if ( $linterStatsdSampleFactor === false ) { if ( $linterStatsdSampleFactor === false ) {
// Don't send to statsd, but update cache with $changes // Don't send to statsd, but update cache with $changes

View file

@ -8,8 +8,6 @@
* note: This code is based on migrateRevisionActorTemp.php and migrateLinksTable.php * note: This code is based on migrateRevisionActorTemp.php and migrateLinksTable.php
*/ */
use MediaWiki\MediaWikiServices;
$IP = getenv( 'MW_INSTALL_PATH' ); $IP = getenv( 'MW_INSTALL_PATH' );
if ( $IP === false ) { if ( $IP === false ) {
$IP = __DIR__ . '/../../..'; $IP = __DIR__ . '/../../..';
@ -61,7 +59,7 @@ class MigrateNamespace extends LoggedUpdateMaintenance {
$this->output( "Migrating the page table page_namespace field to the linter table...\n" ); $this->output( "Migrating the page table page_namespace field to the linter table...\n" );
$database = MediaWikiServices::getInstance()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 ); $database = $this->getServiceContainer()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 );
$updated = $database->migrateNamespace( $batchSize, $batchSize, $sleep, false ); $updated = $database->migrateNamespace( $batchSize, $batchSize, $sleep, false );
$this->output( "Completed migration of page_namespace data to the linter table, $updated rows updated.\n" ); $this->output( "Completed migration of page_namespace data to the linter table, $updated rows updated.\n" );

View file

@ -6,8 +6,6 @@
* recordLintJob has been enabled by setting LinterWriteTagAndTemplateColumnsStage true. * recordLintJob has been enabled by setting LinterWriteTagAndTemplateColumnsStage true.
*/ */
use MediaWiki\MediaWikiServices;
$IP = getenv( 'MW_INSTALL_PATH' ); $IP = getenv( 'MW_INSTALL_PATH' );
if ( $IP === false ) { if ( $IP === false ) {
$IP = __DIR__ . '/../../..'; $IP = __DIR__ . '/../../..';
@ -59,7 +57,7 @@ class MigrateTagTemplate extends LoggedUpdateMaintenance {
$this->output( "Migrating the linter_params field to the linter_tag and linter_template fields...\n" ); $this->output( "Migrating the linter_params field to the linter_tag and linter_template fields...\n" );
$database = MediaWikiServices::getInstance()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 ); $database = $this->getServiceContainer()->get( 'Linter.DatabaseFactory' )->getDatabase( 0 );
$updated = $database->migrateTemplateAndTagInfo( $batchSize, $sleep, false ); $updated = $database->migrateTemplateAndTagInfo( $batchSize, $sleep, false );
$this->output( $this->output(

View file

@ -1,4 +1,26 @@
<?php <?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
*/
namespace MediaWiki\Linter\Test;
use MediaWikiIntegrationTestCase;
/** /**
* @coversNothing * @coversNothing