diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index 6f785f403ac..a5003f12a3d 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -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' ]; diff --git a/includes/specials/SpecialRecentChanges.php b/includes/specials/SpecialRecentChanges.php index 38178d586ef..f68ad4a9e21 100644 --- a/includes/specials/SpecialRecentChanges.php +++ b/includes/specials/SpecialRecentChanges.php @@ -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; } diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index 0ed504fc611..b46d2e68845 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -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'] ) : []; diff --git a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php index 4f14f959f38..4caf7178af1 100644 --- a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php +++ b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php @@ -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" ); }