Add GlobalNameUtils class

This is just a temporary location for these two methods. Since they're
used a lot, having them in the AbuseFilter class means that the
dependency graph is unnecessarily complicated. Thus, since these methods
aren't doing much, they were moved to a dedicated class. Future todo is
finding an appropriate location, that might be either as part of another
service, or keep them in a Utilities class, perhaps a single class with
all util methods, rather than a specific class.

Change-Id: I52cc47a6b9a387cd1e68c5127f6598a4c43ca428
This commit is contained in:
Daimona Eaytoy 2020-12-02 23:47:40 +01:00
parent 04f6c29fb5
commit 5e609eb537
16 changed files with 98 additions and 70 deletions

View file

@ -158,6 +158,7 @@
"AbuseFilterChangesList": "includes/AbuseFilterChangesList.php",
"TableDiffFormatterFullContext": "includes/TableDiffFormatterFullContext.php",
"AbuseFilterVariableHolder": "includes/AbuseFilterVariableHolder.php",
"MediaWiki\\Extension\\AbuseFilter\\GlobalNameUtils": "includes/GlobalNameUtils.php",
"MediaWiki\\Extension\\AbuseFilter\\FilterLookup": "includes/FilterLookup.php",
"MediaWiki\\Extension\\AbuseFilter\\KeywordsManager": "includes/KeywordsManager.php",
"MediaWiki\\Extension\\AbuseFilter\\AbuseFilterPermissionManager": "includes/AbuseFilterPermissionManager.php",

View file

@ -12,9 +12,6 @@ use MediaWiki\Revision\RevisionRecord;
*/
class AbuseFilter {
/** @var string The prefix to use for global filters */
public const GLOBAL_FILTER_PREFIX = 'global-';
/**
* @var array IDs of logged filters like [ page title => [ 'local' => [ids], 'global' => [ids] ] ].
* @fixme avoid global state
@ -99,42 +96,14 @@ class AbuseFilter {
}
/**
* Utility function to split "<GLOBAL_FILTER_PREFIX>$index" to an array [ $id, $global ], where
* $id is $index casted to int, and $global is a boolean: true if the filter is global,
* false otherwise (i.e. if the $filter === $index). Note that the $index
* is always casted to int. Passing anything which isn't an integer-like value or a string
* in the shape "<GLOBAL_FILTER_PREFIX>integer" will throw.
* This reverses self::buildGlobalName
*
* @deprecated Use GlobalNameUtils
* @param string|int $filter
* @return array
* @phan-return array{0:int,1:bool}
* @throws InvalidArgumentException
*/
public static function splitGlobalName( $filter ) {
if ( preg_match( '/^' . self::GLOBAL_FILTER_PREFIX . '\d+$/', $filter ) === 1 ) {
$id = intval( substr( $filter, strlen( self::GLOBAL_FILTER_PREFIX ) ) );
return [ $id, true ];
} elseif ( is_numeric( $filter ) ) {
return [ (int)$filter, false ];
} else {
throw new InvalidArgumentException( "Invalid filter name: $filter" );
}
}
/**
* Given a filter ID and a boolean indicating whether it's global, build a string like
* "<GLOBAL_FILTER_PREFIX>$ID". Note that, with global = false, $id is casted to string.
* This reverses self::splitGlobalName.
*
* @param int $id The filter ID
* @param bool $global Whether the filter is global
* @return string
* @todo Calling this method should be avoided wherever possible
*/
public static function buildGlobalName( $id, $global = true ) {
$prefix = $global ? self::GLOBAL_FILTER_PREFIX : '';
return "$prefix$id";
return MediaWiki\Extension\AbuseFilter\GlobalNameUtils::splitGlobalName( $filter );
}
/**

View file

@ -10,6 +10,7 @@ use MediaWiki\Extension\AbuseFilter\Consequence\Parameters;
use MediaWiki\Extension\AbuseFilter\Filter\Filter;
use MediaWiki\Extension\AbuseFilter\FilterLookup;
use MediaWiki\Extension\AbuseFilter\FilterProfiler;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Extension\AbuseFilter\Hooks\AbuseFilterHookRunner;
use MediaWiki\Extension\AbuseFilter\Parser\AbuseFilterParser;
use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGenerator;
@ -390,7 +391,7 @@ class AbuseFilterRunner {
if ( $wgAbuseFilterCentralDB && !$wgAbuseFilterIsCentral ) {
foreach ( $this->filterLookup->getAllActiveFiltersInGroup( $this->group, true ) as $filter ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$matchedFilters[ AbuseFilter::buildGlobalName( $filter->getID() ) ] =
$matchedFilters[GlobalNameUtils::buildGlobalName( $filter->getID() )] =
$this->checkFilter( $filter, true );
}
}
@ -410,7 +411,7 @@ class AbuseFilterRunner {
*/
protected function checkFilter( Filter $filter, $global = false ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$filterName = AbuseFilter::buildGlobalName( $filter->getID(), $global );
$filterName = GlobalNameUtils::buildGlobalName( $filter->getID(), $global );
$startConds = $this->parser->getCondCount();
$startTime = microtime( true );
@ -506,7 +507,7 @@ class AbuseFilterRunner {
$dangerousActions = AbuseFilterServices::getConsequencesRegistry()->getDangerousActionNames();
foreach ( $actionsByFilter as $filter => &$actions ) {
$isGlobalFilter = AbuseFilter::splitGlobalName( $filter )[1];
$isGlobalFilter = GlobalNameUtils::splitGlobalName( $filter )[1];
if ( $isGlobalFilter ) {
$actions = array_diff_key( $actions, array_filter( $wgAbuseFilterLocallyDisabledGlobalActions ) );
@ -594,7 +595,7 @@ class AbuseFilterRunner {
*/
private function actionsParamsToConsequence( string $actionName, array $rawParams, $filter ) : ?Consequence {
global $wgAbuseFilterBlockAutopromoteDuration, $wgAbuseFilterCustomActionsHandlers;
[ $filterID, $isGlobalFilter ] = AbuseFilter::splitGlobalName( $filter );
[ $filterID, $isGlobalFilter ] = GlobalNameUtils::splitGlobalName( $filter );
$filterObj = $this->filterLookup->getFilter( $filterID, $isGlobalFilter );
$consFactory = AbuseFilterServices::getConsequencesFactory();

View file

@ -106,7 +106,7 @@ class AbuseLogger {
$writeNewSchema = $wgAbuseFilterAflFilterMigrationStage & SCHEMA_COMPAT_WRITE_NEW;
$writeOldSchema = $wgAbuseFilterAflFilterMigrationStage & SCHEMA_COMPAT_WRITE_OLD;
foreach ( $actionsTaken as $filter => $actions ) {
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $filter );
$thisLog = $logTemplate;
if ( $writeOldSchema ) {
$thisLog['afl_filter'] = $filter;
@ -215,7 +215,7 @@ class AbuseLogger {
$entry->setPerformer( $user );
$entry->setTarget( $this->title );
if ( $writeNewSchema ) {
$filterName = AbuseFilter::buildGlobalName(
$filterName = GlobalNameUtils::buildGlobalName(
$data['afl_filter_id'],
$data['afl_global'] === 1
);
@ -248,7 +248,7 @@ class AbuseLogger {
$global = $data['afl_global'];
} else {
// SCHEMA_COMPAT_WRITE_OLD
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $data['afl_filter'] );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $data['afl_filter'] );
}
if (
!$this->options->get( 'AbuseFilterNotificationsPrivate' ) &&

View file

@ -32,6 +32,7 @@ use ApiQueryBase;
use InvalidArgumentException;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\MediaWikiServices;
use MWTimestamp;
use SpecialAbuseLog;
@ -99,7 +100,7 @@ class QueryAbuseLog extends ApiQueryBase {
$foundInvalid = false;
foreach ( $params['filter'] as $filter ) {
try {
$searchFilters[] = AbuseFilter::splitGlobalName( $filter );
$searchFilters[] = GlobalNameUtils::splitGlobalName( $filter );
} catch ( InvalidArgumentException $e ) {
$foundInvalid = true;
continue;
@ -222,7 +223,7 @@ class QueryAbuseLog extends ApiQueryBase {
// SCHEMA_COMPAT_READ_OLD
$names = [];
foreach ( $searchFilters as $filter ) {
$names[] = AbuseFilter::buildGlobalName( ...$filter );
$names[] = GlobalNameUtils::buildGlobalName( ...$filter );
}
$conds = [ 'afl_filter' => $names ];
}
@ -269,10 +270,10 @@ class QueryAbuseLog extends ApiQueryBase {
if ( $aflFilterMigrationStage & SCHEMA_COMPAT_READ_NEW ) {
$filterID = $row->afl_filter_id;
$global = $row->afl_global;
$fullName = AbuseFilter::buildGlobalName( $filterID, $global );
$fullName = GlobalNameUtils::buildGlobalName( $filterID, $global );
} else {
// SCHEMA_COMPAT_READ_OLD
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $row->afl_filter );
$fullName = $row->afl_filter;
}
$isHidden = $lookup->getFilter( $filterID, $global )->isHidden();
@ -367,7 +368,7 @@ class QueryAbuseLog extends ApiQueryBase {
ApiBase::PARAM_ISMULTI => true,
ApiBase::PARAM_HELP_MSG => [
'apihelp-query+abuselog-param-filter',
AbuseFilter::GLOBAL_FILTER_PREFIX
GlobalNameUtils::GLOBAL_FILTER_PREFIX
]
],
'limit' => [

View file

@ -2,7 +2,7 @@
namespace MediaWiki\Extension\AbuseFilter\Consequence;
use AbuseFilter;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
/**
* Consequence that simply disallows the ongoing action.
@ -36,7 +36,7 @@ class Disallow extends Consequence implements HookAborterConsequence {
$this->message,
$filter->getName(),
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
AbuseFilter::buildGlobalName( $filter->getID(), $this->parameters->getIsGlobalFilter() )
GlobalNameUtils::buildGlobalName( $filter->getID(), $this->parameters->getIsGlobalFilter() )
];
}
}

View file

@ -2,6 +2,7 @@
namespace MediaWiki\Extension\AbuseFilter\Consequence;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Session\Session;
use Title;
@ -63,7 +64,7 @@ class Warn extends Consequence implements HookAborterConsequence, ConsequencesDi
$this->message,
$filter->getName(),
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
\AbuseFilter::buildGlobalName( $filter->getID(), $this->parameters->getIsGlobalFilter() )
GlobalNameUtils::buildGlobalName( $filter->getID(), $this->parameters->getIsGlobalFilter() )
];
}
@ -91,8 +92,8 @@ class Warn extends Consequence implements HookAborterConsequence, ConsequencesDi
* @return string
*/
private function getWarnKey() : string {
$globalFilterName = \AbuseFilter::buildGlobalName(
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$globalFilterName = GlobalNameUtils::buildGlobalName(
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$this->parameters->getFilter()->getID(),
$this->parameters->getIsGlobalFilter()
);

View file

@ -2,7 +2,6 @@
namespace MediaWiki\Extension\AbuseFilter;
use AbuseFilter;
use Psr\Log\LoggerInterface;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILoadBalancer;
@ -49,7 +48,7 @@ class ConsequencesLookup {
$localFilters = [];
foreach ( $filters as $filter ) {
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $filter );
if ( $global ) {
$globalFilters[] = $filterID;
@ -71,7 +70,7 @@ class ConsequencesLookup {
$consequences += $this->loadConsequencesFromDB(
$this->centralDBManager->getConnection( DB_REPLICA ),
$globalFilters,
AbuseFilter::GLOBAL_FILTER_PREFIX
GlobalNameUtils::GLOBAL_FILTER_PREFIX
);
}

View file

@ -374,6 +374,6 @@ class FilterLookup implements IDBAccessObject {
* @return string
*/
private function getCacheKey( int $filterID, bool $global ) : string {
return AbuseFilter::buildGlobalName( $filterID, $global );
return GlobalNameUtils::buildGlobalName( $filterID, $global );
}
}

View file

@ -2,7 +2,6 @@
namespace MediaWiki\Extension\AbuseFilter;
use AbuseFilter;
use BagOStuff;
use DeferredUpdates;
use IBufferingStatsdDataFactory;
@ -174,7 +173,7 @@ class FilterProfiler {
$profileKey = $this->filterProfileGroupKey( $group );
$this->objectStash->delete( $profileKey );
foreach ( $allFilters as $filter ) {
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $filter );
if ( $global === false ) {
$this->resetFilterProfile( $filterID );
}
@ -247,7 +246,7 @@ class FilterProfiler {
*/
public function recordPerFilterProfiling( Title $title, array $data ) : void {
foreach ( $data as $filterName => $params ) {
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filterName );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $filterName );
if ( !$global ) {
// @todo Maybe add a parameter to recordProfilingResult to record global filters
// data separately (in the foreign wiki)

View file

@ -0,0 +1,51 @@
<?php
namespace MediaWiki\Extension\AbuseFilter;
use InvalidArgumentException;
/**
* @internal
*/
class GlobalNameUtils {
/** @var string The prefix to use for global filters */
public const GLOBAL_FILTER_PREFIX = 'global-';
/**
* Given a filter ID and a boolean indicating whether it's global, build a string like
* "<GLOBAL_FILTER_PREFIX>$ID". Note that, with global = false, $id is casted to string.
* This reverses self::splitGlobalName.
*
* @param int $id The filter ID
* @param bool $global Whether the filter is global
* @return string
* @todo Calling this method should be avoided wherever possible
*/
public static function buildGlobalName( $id, $global = true ) {
$prefix = $global ? self::GLOBAL_FILTER_PREFIX : '';
return "$prefix$id";
}
/**
* Utility function to split "<GLOBAL_FILTER_PREFIX>$index" to an array [ $id, $global ], where
* $id is $index casted to int, and $global is a boolean: true if the filter is global,
* false otherwise (i.e. if the $filter === $index). Note that the $index
* is always casted to int. Passing anything which isn't an integer-like value or a string
* in the shape "<GLOBAL_FILTER_PREFIX>integer" will throw.
* This reverses self::buildGlobalName
*
* @param string|int $filter
* @return array
* @throws InvalidArgumentException
*/
public static function splitGlobalName( $filter ) {
if ( preg_match( '/^' . self::GLOBAL_FILTER_PREFIX . '\d+$/', $filter ) === 1 ) {
$id = intval( substr( $filter, strlen( self::GLOBAL_FILTER_PREFIX ) ) );
return [ $id, true ];
} elseif ( is_numeric( $filter ) ) {
return [ (int)$filter, false ];
} else {
throw new InvalidArgumentException( "Invalid filter name: $filter" );
}
}
}

View file

@ -10,6 +10,7 @@ use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Linker\LinkRenderer;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Permissions\PermissionManager;
@ -169,7 +170,7 @@ class AbuseLogPager extends ReverseChronologicalPager {
$global = $row->afl_global;
} else {
// SCHEMA_COMPAT_READ_OLD
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $row->afl_filter );
}
if ( $global ) {

View file

@ -11,6 +11,7 @@ use IContextSource;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\EditBoxBuilderFactory;
use MediaWiki\Extension\AbuseFilter\FilterLookup;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Extension\AbuseFilter\Pager\AbuseFilterExaminePager;
use MediaWiki\Extension\AbuseFilter\VariableGenerator\RCVariableGenerator;
use MediaWiki\Linker\LinkRenderer;
@ -228,7 +229,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
$isGlobal = $row->afl_global;
} else {
// SCHEMA_COMPAT_READ_OLD
[ $filterID, $isGlobal ] = AbuseFilter::splitGlobalName( $row->afl_filter );
[ $filterID, $isGlobal ] = GlobalNameUtils::splitGlobalName( $row->afl_filter );
}
$isHidden = $this->filterLookup->getFilter( $filterID, $isGlobal )->isHidden();
if ( !$this->afPermManager->canSeeLogDetailsForFilter( $user, $isHidden ) ) {

View file

@ -4,6 +4,7 @@ use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\ConsequencesRegistry;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Extension\AbuseFilter\Pager\AbuseLogPager;
use MediaWiki\Extension\AbuseFilter\View\HideAbuseLog;
use MediaWiki\Logger\LoggerFactory;
@ -309,7 +310,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$helpmsg = $this->getConfig()->get( 'AbuseFilterIsCentral' )
? $this->msg( 'abusefilter-log-search-filter-help-central' )->escaped()
: $this->msg( 'abusefilter-log-search-filter-help' )
->params( AbuseFilter::GLOBAL_FILTER_PREFIX )->escaped();
->params( GlobalNameUtils::GLOBAL_FILTER_PREFIX )->escaped();
$formDescriptor['SearchFilter'] = [
'label-message' => 'abusefilter-log-search-filter',
'type' => 'text',
@ -408,7 +409,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$foundInvalid = false;
foreach ( $rawFilters as $filter ) {
try {
$filtersList[] = AbuseFilter::splitGlobalName( $filter );
$filtersList[] = GlobalNameUtils::splitGlobalName( $filter );
} catch ( InvalidArgumentException $e ) {
$foundInvalid = true;
continue;
@ -443,7 +444,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
}
foreach ( $filtersList as $filterData ) {
$searchFilters[] = AbuseFilter::buildGlobalName( ...$filterData );
$searchFilters[] = GlobalNameUtils::buildGlobalName( ...$filterData );
}
}
@ -465,7 +466,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
if ( $aflFilterMigrationStage & SCHEMA_COMPAT_READ_NEW ) {
$filterConds = [ 'local' => [], 'global' => [] ];
foreach ( $searchIDs as $filter ) {
list( $filterID, $isGlobal ) = AbuseFilter::splitGlobalName( $filter );
list( $filterID, $isGlobal ) = GlobalNameUtils::splitGlobalName( $filter );
$key = $isGlobal ? 'global' : 'local';
$filterConds[$key][] = $filterID;
}
@ -614,7 +615,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$global = $row->afl_global;
} else {
// SCHEMA_COMPAT_READ_OLD
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $row->afl_filter );
}
if ( $global ) {
@ -758,7 +759,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$global = $row->afl_global;
} else {
// SCHEMA_COMPAT_READ_OLD
list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
list( $filterID, $global ) = GlobalNameUtils::splitGlobalName( $row->afl_filter );
}
if ( $global ) {

View file

@ -1,5 +1,6 @@
<?php
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\MediaWikiServices;
if ( getenv( 'MW_INSTALL_PATH' ) ) {
@ -48,7 +49,7 @@ class MigrateAflFilter extends LoggedUpdateMaintenance {
$dryRun = $this->hasOption( 'dry-run' );
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
$globalPrefix = AbuseFilter::GLOBAL_FILTER_PREFIX;
$globalPrefix = GlobalNameUtils::GLOBAL_FILTER_PREFIX;
$batchSize = $this->getBatchSize();
$updated = 0;
@ -92,7 +93,7 @@ class MigrateAflFilter extends LoggedUpdateMaintenance {
}
if ( !$dryRun ) {
// Use native SQL functions instead of AbuseFilter::splitGlobalName so that we can update
// Use native SQL functions instead of GlobalNameUtils::splitGlobalName so that we can update
// all rows at the same time.
$newIdSQL = $dbw->buildIntegerCast( $dbw->strreplace(
'afl_filter',

View file

@ -23,6 +23,8 @@
* @license GPL-2.0-or-later
*/
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
/**
* @group Test
* @group AbuseFilter
@ -33,16 +35,16 @@ class AbuseFilterTest extends MediaWikiUnitTestCase {
* @param string $name The name of a filter
* @param array|null $expected If array, the expected result like [ id, isGlobal ].
* If null it means that we're expecting an exception.
* @covers AbuseFilter::splitGlobalName
* @covers \MediaWiki\Extension\AbuseFilter\GlobalNameUtils::splitGlobalName
* @dataProvider provideGlobalNames
*/
public function testSplitGlobalName( $name, $expected ) {
if ( $expected !== null ) {
$actual = AbuseFilter::splitGlobalName( $name );
$actual = GlobalNameUtils::splitGlobalName( $name );
$this->assertSame( $expected, $actual );
} else {
$this->expectException( InvalidArgumentException::class );
AbuseFilter::splitGlobalName( $name );
GlobalNameUtils::splitGlobalName( $name );
}
}