specials: Replace ISQLPlatform::makeList on ChangesListSpecialPages

Use expression builder instead

Change-Id: Ic68fde5a2da37aa3d8004b303c43500076617e6c
This commit is contained in:
Umherirrender 2024-07-04 22:42:38 +02:00
parent bc6438949b
commit 7512eb300b
4 changed files with 70 additions and 94 deletions

View file

@ -41,11 +41,14 @@ use MediaWiki\User\UserIdentityUtils;
use MWExceptionHandler;
use OOUI\IconWidget;
use RecentChange;
use Wikimedia\Rdbms\AndExpressionGroup;
use Wikimedia\Rdbms\DBQueryTimeoutError;
use Wikimedia\Rdbms\FakeResultWrapper;
use Wikimedia\Rdbms\IExpression;
use Wikimedia\Rdbms\IReadableDatabase;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\OrExpressionGroup;
use Wikimedia\Rdbms\RawSQLValue;
/**
* Special page which uses a ChangesList to show query results.
@ -382,13 +385,8 @@ abstract class ChangesListSpecialPage extends SpecialPage {
'queryCallable' => static function ( string $specialClassName, IContextSource $ctx,
IReadableDatabase $dbr, &$tables, &$fields, &$conds, &$query_options, &$join_conds
) use ( $nonRevisionTypes ) {
$conds[] = $dbr->makeList(
[
'rc_this_oldid <> page_latest',
'rc_type' => $nonRevisionTypes,
],
LIST_OR
);
$conds[] = $dbr->expr( 'rc_this_oldid', '!=', new RawSQLValue( 'page_latest' ) )
->or( 'rc_type', '=', $nonRevisionTypes );
},
'cssClassSuffix' => 'last',
'isRowApplicableCallable' => static function ( IContextSource $ctx, RecentChange $rc ) {
@ -403,13 +401,8 @@ abstract class ChangesListSpecialPage extends SpecialPage {
'queryCallable' => static function ( string $specialClassName, IContextSource $ctx,
IReadableDatabase $dbr, &$tables, &$fields, &$conds, &$query_options, &$join_conds
) use ( $nonRevisionTypes ) {
$conds[] = $dbr->makeList(
[
'rc_this_oldid = page_latest',
'rc_type' => $nonRevisionTypes,
],
LIST_OR
);
$conds[] = $dbr->expr( 'rc_this_oldid', '=', new RawSQLValue( 'page_latest' ) )
->or( 'rc_type', '=', $nonRevisionTypes );
},
'cssClassSuffix' => 'previous',
'isRowApplicableCallable' => static function ( IContextSource $ctx, RecentChange $rc ) {
@ -486,13 +479,8 @@ abstract class ChangesListSpecialPage extends SpecialPage {
'queryCallable' => static function ( string $specialClassName, IContextSource $ctx,
IReadableDatabase $dbr, &$tables, &$fields, &$conds, &$query_options, &$join_conds
) {
$conds[] = $dbr->makeList(
[
$dbr->expr( 'rc_log_type', '!=', 'newusers' ),
'rc_log_type' => null
],
IReadableDatabase::LIST_OR
);
$conds[] = $dbr->expr( 'rc_log_type', '!=', 'newusers' )
->or( 'rc_log_type', '=', null );
},
'cssClassSuffix' => 'src-mw-newuserlog',
'isRowApplicableCallable' => static function ( IContextSource $ctx, RecentChange $rc ) {
@ -1527,15 +1515,9 @@ abstract class ChangesListSpecialPage extends SpecialPage {
$namespaces = array_unique( array_merge( $namespaces, $associatedNamespaces ) );
}
if ( count( $namespaces ) === 1 ) {
$operator = $opts[ 'invert' ] ? '!=' : '=';
$value = $dbr->addQuotes( reset( $namespaces ) );
} else {
$operator = $opts[ 'invert' ] ? 'NOT IN' : 'IN';
sort( $namespaces );
$value = '(' . $dbr->makeList( $namespaces ) . ')';
}
$conds[] = "rc_namespace $operator $value";
$operator = $opts[ 'invert' ] ? '!=' : '=';
sort( $namespaces );
$conds[] = $dbr->expr( 'rc_namespace', $operator, $namespaces );
}
}
@ -1831,9 +1813,10 @@ abstract class ChangesListSpecialPage extends SpecialPage {
* @param string $level 'learner' or 'experienced'
* @param int $now Current time as UNIX timestamp (if 0, uses actual time)
* @param IReadableDatabase $dbr
* @param bool $asNotCondition
* @return IExpression
*/
private function getExperienceExpr( $level, $now, IReadableDatabase $dbr ): IExpression {
private function getExperienceExpr( $level, $now, IReadableDatabase $dbr, $asNotCondition = false ): IExpression {
$config = $this->getConfig();
$configSince = [
@ -1851,6 +1834,10 @@ abstract class ChangesListSpecialPage extends SpecialPage {
'experienced' => $config->get( MainConfigNames::ExperiencedUserEdits ),
][$level];
if ( $asNotCondition ) {
return $dbr->expr( 'user_editcount', '<', intval( $editCutoff ) )
->or( 'user_registration', '>', $dbr->timestamp( $timeCutoff ) );
}
return $dbr->expr( 'user_editcount', '>=', intval( $editCutoff ) )->andExpr(
// Users who don't have user_registration set are very old, so we assume they're above any cutoff
$dbr->expr( 'user_registration', '=', null )
@ -1878,10 +1865,12 @@ abstract class ChangesListSpecialPage extends SpecialPage {
) {
$selected = array_fill_keys( $selectedExpLevels, true );
$isUnregistered = $this->getRegisteredExpr( false, $dbr )->toSql( $dbr );
$isRegistered = $this->getRegisteredExpr( true, $dbr )->toSql( $dbr );
$aboveNewcomer = $this->getExperienceExpr( 'learner', $now, $dbr )->toSql( $dbr );
$aboveLearner = $this->getExperienceExpr( 'experienced', $now, $dbr )->toSql( $dbr );
$isUnregistered = $this->getRegisteredExpr( false, $dbr );
$isRegistered = $this->getRegisteredExpr( true, $dbr );
$aboveNewcomer = $this->getExperienceExpr( 'learner', $now, $dbr );
$notAboveNewcomer = $this->getExperienceExpr( 'learner', $now, $dbr, true );
$aboveLearner = $this->getExperienceExpr( 'experienced', $now, $dbr );
$notAboveLearner = $this->getExperienceExpr( 'experienced', $now, $dbr, true );
// We need to select some range of user experience levels, from the following table:
// | Unregistered | --------- Registered --------- |
@ -1890,15 +1879,12 @@ abstract class ChangesListSpecialPage extends SpecialPage {
// We just need to define a condition for each of the columns, figure out which are selected,
// and then OR them together.
$columnConds = [
'unregistered' => [ $isUnregistered ],
'registered' => [ $isRegistered ],
'newcomer' => [ $isRegistered, "NOT ( $aboveNewcomer )" ],
'learner' => [ $isRegistered, $aboveNewcomer, "NOT ( $aboveLearner )" ],
'experienced' => [ $isRegistered, $aboveLearner ],
'unregistered' => $isUnregistered,
'registered' => $isRegistered,
'newcomer' => new AndExpressionGroup( $isRegistered, $notAboveNewcomer ),
'learner' => new AndExpressionGroup( $isRegistered, $aboveNewcomer, $notAboveLearner ),
'experienced' => new AndExpressionGroup( $isRegistered, $aboveLearner ),
];
foreach ( $columnConds as &$nestedConds ) {
$nestedConds = $dbr->makeList( $nestedConds, IReadableDatabase::LIST_AND );
}
// There are some cases where we can easily optimize away some queries:
// | Unregistered | --------- Registered --------- |
@ -1924,7 +1910,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
return;
}
$selectedColumnConds = array_values( array_intersect_key( $columnConds, $selected ) );
$conds[] = $dbr->makeList( $selectedColumnConds, IReadableDatabase::LIST_OR );
$conds[] = new OrExpressionGroup( ...$selectedColumnConds );
// Add necessary tables to the queries.
$join_conds['recentchanges_actor'] = [ 'JOIN', 'actor_id=rc_actor' ];

View file

@ -45,6 +45,7 @@ use OOUI\HtmlSnippet;
use RecentChange;
use Wikimedia\Rdbms\IReadableDatabase;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\RawSQLExpression;
/**
* List of the last changes made to the wiki
@ -136,16 +137,18 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
IReadableDatabase $dbr, &$tables, &$fields, &$conds, &$query_options, &$join_conds, $selectedValues
) {
sort( $selectedValues );
$notwatchedCond = 'wl_user IS NULL';
$watchedCond = 'wl_user IS NOT NULL';
$notwatchedCond = $dbr->expr( 'wl_user', '=', null );
$watchedCond = $dbr->expr( 'wl_user', '!=', null );
if ( $this->getConfig()->get( MainConfigNames::WatchlistExpiry ) ) {
// Expired watchlist items stay in the DB after their expiry time until they're purged,
// so it's not enough to only check for wl_user.
$quotedNow = $dbr->addQuotes( $dbr->timestamp() );
$notwatchedCond = "wl_user IS NULL OR ( we_expiry IS NOT NULL AND we_expiry < $quotedNow )";
$watchedCond = "wl_user IS NOT NULL AND ( we_expiry IS NULL OR we_expiry >= $quotedNow )";
$dbNow = $dbr->timestamp();
$notwatchedCond = $notwatchedCond
->orExpr( $dbr->expr( 'we_expiry', '!=', null )->and( 'we_expiry', '<', $dbNow ) );
$watchedCond = $watchedCond
->andExpr( $dbr->expr( 'we_expiry', '=', null )->or( 'we_expiry', '>=', $dbNow ) );
}
$newCond = 'rc_timestamp >= wl_notificationtimestamp';
$newCond = new RawSQLExpression( 'rc_timestamp >= wl_notificationtimestamp' );
if ( $selectedValues === [ 'notwatched' ] ) {
$conds[] = $notwatchedCond;
@ -158,10 +161,8 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
}
if ( $selectedValues === [ 'watchednew' ] ) {
$conds[] = $dbr->makeList( [
$watchedCond,
$newCond
], LIST_AND );
$conds[] = $watchedCond
->andExpr( $newCond );
return;
}
@ -171,13 +172,11 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
}
if ( $selectedValues === [ 'notwatched', 'watchednew' ] ) {
$conds[] = $dbr->makeList( [
$notwatchedCond,
$dbr->makeList( [
$watchedCond,
$newCond
], LIST_AND )
], LIST_OR );
$conds[] = $notwatchedCond
->orExpr(
$watchedCond
->andExpr( $newCond )
);
return;
}

View file

@ -48,6 +48,8 @@ use RecentChange;
use UserNotLoggedIn;
use Wikimedia\Rdbms\IReadableDatabase;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\RawSQLExpression;
use Wikimedia\Rdbms\RawSQLValue;
/**
* @defgroup Watchlist Users watchlist handling
@ -229,13 +231,8 @@ class SpecialWatchlist extends ChangesListSpecialPage {
$nonRevisionTypes = [ RC_LOG ];
$this->getHookRunner()->onSpecialWatchlistGetNonRevisionTypes( $nonRevisionTypes );
if ( $nonRevisionTypes ) {
$conds[] = $dbr->makeList(
[
'rc_this_oldid=page_latest',
'rc_type' => $nonRevisionTypes,
],
LIST_OR
);
$conds[] = $dbr->expr( 'rc_this_oldid', '=', new RawSQLValue( 'page_latest' ) )
->or( 'rc_type', '=', $nonRevisionTypes );
}
},
]
@ -288,15 +285,11 @@ class SpecialWatchlist extends ChangesListSpecialPage {
$selectedValues
) {
if ( $selectedValues === [ 'seen' ] ) {
$conds[] = $dbr->makeList( [
'wl_notificationtimestamp' => null,
'rc_timestamp < wl_notificationtimestamp'
], LIST_OR );
$conds[] = $dbr->expr( 'wl_notificationtimestamp', '=', null )
->orExpr( new RawSQLExpression( 'rc_timestamp < wl_notificationtimestamp' ) );
} elseif ( $selectedValues === [ 'unseen' ] ) {
$conds[] = $dbr->makeList( [
'wl_notificationtimestamp IS NOT NULL',
'rc_timestamp >= wl_notificationtimestamp'
], LIST_AND );
$conds[] = $dbr->expr( 'wl_notificationtimestamp', '!=', null )
->andExpr( new RawSQLExpression( 'rc_timestamp >= wl_notificationtimestamp' ) );
}
}
] ) );
@ -461,10 +454,8 @@ class SpecialWatchlist extends ChangesListSpecialPage {
$bitmask = 0;
}
if ( $bitmask ) {
$conds[] = $dbr->makeList( [
'rc_type != ' . RC_LOG,
$dbr->bitAnd( 'rc_deleted', $bitmask ) . " != $bitmask",
], LIST_OR );
$conds[] = $dbr->expr( 'rc_type', '!=', RC_LOG )
->orExpr( new RawSQLExpression( $dbr->bitAnd( 'rc_deleted', $bitmask ) . " != $bitmask" ) );
}
$tagFilter = $opts['tagfilter'] !== '' ? explode( '|', $opts['tagfilter'] ) : [];

View file

@ -563,7 +563,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
'((actor_user IS NOT NULL))',
'(actor_user IS NOT NULL)',
],
[
'userExpLevel' => 'newcomer;learner;experienced',
@ -577,7 +577,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
'((actor_user IS NOT NULL))',
'(actor_user IS NOT NULL)',
],
[
'userExpLevel' => 'registered',
@ -594,7 +594,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
"(((actor_user IS NOT NULL AND $tempUserMatchPattern)))",
"((actor_user IS NOT NULL AND $tempUserMatchPattern))",
],
[
'userExpLevel' => 'registered',
@ -608,7 +608,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
'((actor_user IS NULL))'
'(actor_user IS NULL)'
],
[
'userExpLevel' => 'unregistered',
@ -625,7 +625,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
"(((actor_user IS NULL OR $tempUserMatchPattern)))",
"((actor_user IS NULL OR $tempUserMatchPattern))",
],
[
'userExpLevel' => 'unregistered',
@ -639,7 +639,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
$this->assertConditions(
[
# expected
'((actor_user IS NOT NULL))',
'(actor_user IS NOT NULL)',
],
[
'userExpLevel' => 'registered;learner',
@ -650,21 +650,21 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
public function testFilterUserExpLevelUnregisteredOrExperienced() {
$this->disableAutoCreateTempUser();
$conds = $this->buildQuery( [ 'userExpLevel' => 'unregistered;experienced' ] );
[ $cond ] = $this->buildQuery( [ 'userExpLevel' => 'unregistered;experienced' ] );
$this->assertMatchesRegularExpression(
'/\(\(actor_user IS NULL\)\) OR '
. '\(\(actor_user IS NOT NULL\) AND \(\('
'/\(actor_user IS NULL OR '
. '\(actor_user IS NOT NULL AND \('
. 'user_editcount >= 500 AND \(user_registration IS NULL OR '
. 'user_registration <= \'[^\']+\'\)'
. '\)\)\)/',
reset( $conds ),
$cond->toSql( $this->getDb() ),
"rc conditions: userExpLevel=unregistered;experienced"
);
}
public function testFilterUserExpLevelUnregisteredOrExperiencedWhenTemporaryAccountsEnabled() {
$this->enableAutoCreateTempUser();
$conds = $this->buildQuery( [ 'userExpLevel' => 'unregistered;experienced' ] );
[ $cond ] = $this->buildQuery( [ 'userExpLevel' => 'unregistered;experienced' ] );
$notLikeTempUserMatchExpression = $this->getServiceContainer()->getTempUserConfig()
->getMatchCondition( $this->getDb(), 'actor_name', IExpression::NOT_LIKE )
->toSql( $this->getDb() );
@ -674,12 +674,12 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
->toSql( $this->getDb() );
$likeTempUserMatchExpression = preg_quote( $likeTempUserMatchExpression );
$this->assertMatchesRegularExpression(
"/\(\(\(actor_user IS NULL OR $likeTempUserMatchExpression\)\)\) OR "
. "\(\(\(actor_user IS NOT NULL AND $notLikeTempUserMatchExpression\)\) AND \(\("
"/\(\(actor_user IS NULL OR $likeTempUserMatchExpression\) OR "
. "\(\(actor_user IS NOT NULL AND $notLikeTempUserMatchExpression\) AND \("
. 'user_editcount >= 500 AND \(user_registration IS NULL OR '
. 'user_registration <= \'[^\']+\'\)'
. '\)\)\)/',
reset( $conds ),
$cond->toSql( $this->getDb() ),
"rc conditions: userExpLevel=unregistered;experienced"
);
}