Migrate away from $db->makeList in favor of expression builder

Bug: T210206
Change-Id: I803a6130fff5ce6faaaa5221443a436195b03c75

includes/ResourceLoader/dependencystore/SqlModuleDependencyStore.php

@@ -24,6 +24,7 @@ use InvalidArgumentException;
 use Wikimedia\Rdbms\DBConnRef;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\ILoadBalancer;
+use Wikimedia\Rdbms\OrExpressionGroup;
 
 /**
  * Track per-module file dependencies in the core module_deps table
@@ -126,16 +127,15 @@ class SqlModuleDependencyStore extends DependencyStore {
 		$disjunctionConds = [];
 		foreach ( (array)$entities as $entity ) {
 			[ $module, $variant ] = $this->getEntityNameComponents( $entity );
-			$disjunctionConds[] = $dbw->makeList(
-				[ 'md_skin' => $variant, 'md_module' => $module ],
-				$dbw::LIST_AND
-			);
+			$disjunctionConds[] = $dbw
+				->expr( 'md_skin', '=', $variant )
+				->and( 'md_module', '=', $module );
 		}
 
 		if ( $disjunctionConds ) {
 			$dbw->newDeleteQueryBuilder()
 				->deleteFrom( 'module_deps' )
-				->where( $dbw->makeList( $disjunctionConds, $dbw::LIST_OR ) )
+				->where( new OrExpressionGroup( ...$disjunctionConds ) )
 				->caller( __METHOD__ )->execute();
 		}
 	}
@@ -154,10 +154,9 @@ class SqlModuleDependencyStore extends DependencyStore {
 
 		$disjunctionConds = [];
 		foreach ( $modulesByVariant as $variant => $modules ) {
-			$disjunctionConds[] = $db->makeList(
-				[ 'md_skin' => $variant, 'md_module' => $modules ],
-				$db::LIST_AND
-			);
+			$disjunctionConds[] = $db
+				->expr( 'md_skin', '=', $variant )
+				->and( 'md_module', '=', $modules );
 		}
 
 		$depsBlobByEntity = [];
@@ -166,7 +165,7 @@ class SqlModuleDependencyStore extends DependencyStore {
 			$res = $db->newSelectQueryBuilder()
 				->select( [ 'md_module', 'md_skin', 'md_deps' ] )
 				->from( 'module_deps' )
-				->where( $db->makeList( $disjunctionConds, $db::LIST_OR ) )
+				->where( new OrExpressionGroup( ...$disjunctionConds ) )
 				->caller( __METHOD__ )->fetchResultSet();
 
 			foreach ( $res as $row ) {

includes/filerepo/LocalRepo.php

@@ -32,6 +32,7 @@ use Wikimedia\Rdbms\IConnectionProvider;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\IReadableDatabase;
 use Wikimedia\Rdbms\IResultWrapper;
+use Wikimedia\Rdbms\OrExpressionGroup;
 
 /**
  * Local repository that stores files in the local filesystem and registers them
@@ -376,20 +377,16 @@ class LocalRepo extends FileRepo {
 		$oiConds = []; // WHERE clause array for each file
 		foreach ( $searchSet as $dbKey => $search ) {
 			if ( isset( $search['time'] ) ) {
-				$oiConds[] = $dbr->makeList(
-					[
-						'oi_name' => $this->getNameFromTitle( File::normalizeTitle( $dbKey ) ),
-						'oi_timestamp' => $dbr->timestamp( $search['time'] )
-					],
-					LIST_AND
-				);
+				$oiConds[] = $dbr
+					->expr( 'oi_name', '=', $this->getNameFromTitle( File::normalizeTitle( $dbKey ) ) )
+					->and( 'oi_timestamp', '=', $dbr->timestamp( $search['time'] ) );
 			}
 		}
 
 		if ( count( $oiConds ) ) {
 			$queryBuilder = FileSelectQueryBuilder::newForOldFile( $dbr );
 
-			$res = $queryBuilder->where( $dbr->makeList( $oiConds, LIST_OR ) )
+			$res = $queryBuilder->where( new OrExpressionGroup( ...$oiConds ) )
 				->caller( __METHOD__ )->fetchResultSet();
 			$applyMatchingFiles( $res, $searchSet, $finalFiles );
 		}

includes/specials/SpecialUncategorizedCategories.php

@@ -100,7 +100,7 @@ class SpecialUncategorizedCategories extends SpecialUncategorizedPages {
 		$exceptionList = $this->getExceptionList();
 		if ( $exceptionList ) {
 			$dbr = $this->getDatabaseProvider()->getReplicaDatabase();
-			$query['conds'][] = 'page_title not in ( ' . $dbr->makeList( $exceptionList ) . ' )';
+			$query['conds'][] = $dbr->expr( 'page_title', '!=', $exceptionList );
 		}
 
 		return $query;

includes/user/UserGroupManager.php

@@ -42,6 +42,7 @@ use Wikimedia\IPUtils;
 use Wikimedia\Rdbms\IConnectionProvider;
 use Wikimedia\Rdbms\ILBFactory;
 use Wikimedia\Rdbms\IReadableDatabase;
+use Wikimedia\Rdbms\OrExpressionGroup;
 use Wikimedia\Rdbms\ReadOnlyMode;
 use Wikimedia\Rdbms\SelectQueryBuilder;
 
@@ -1045,15 +1046,14 @@ class UserGroupManager implements IDBAccessObject {
 				$deleteCond = []; // array for deleting the rows that are to be moved around
 				foreach ( $res as $row ) {
 					$insertData[] = [ 'ufg_user' => $row->ug_user, 'ufg_group' => $row->ug_group ];
-					$deleteCond[] = $dbw->makeList(
-						[ 'ug_user' => $row->ug_user, 'ug_group' => $row->ug_group ],
-						$dbw::LIST_AND
-					);
+					$deleteCond[] = $dbw
+						->expr( 'ug_user', '=', $row->ug_user )
+						->and( 'ug_group', '=', $row->ug_group );
 				}
 				// Delete the rows we're about to move
 				$dbw->newDeleteQueryBuilder()
 					->deleteFrom( 'user_groups' )
-					->where( $dbw->makeList( $deleteCond, $dbw::LIST_OR ) )
+					->where( new OrExpressionGroup( ...$deleteCond ) )
 					->caller( __METHOD__ )->execute();
 				// Push the groups to user_former_groups
 				$dbw->newInsertQueryBuilder()

includes/watcheditem/WatchedItemQueryService.php

@@ -332,10 +332,7 @@ class WatchedItemQueryService {
 		if ( $this->expiryEnabled ) {
 			// If expiries are enabled, join with the watchlist_expiry table and exclude expired items.
 			$tables = [ 'watchlist', 'watchlist_expiry' ];
-			$conds[] = $db->makeList(
-				[ 'we_expiry' => null, $db->expr( 'we_expiry', '>', $db->timestamp() ) ],
-				$db::LIST_OR
-			);
+			$conds[] = $db->expr( 'we_expiry', '>', $db->timestamp() )->or( 'we_expiry', '=', null );
 			$joinConds['watchlist_expiry'] = [ 'LEFT JOIN', 'wl_id = we_item' ];
 		}
 		$res = $db->select(

maintenance/deleteOldRevisions.php

@@ -74,7 +74,7 @@ class DeleteOldRevisions extends Maintenance {
 		# Get all revisions that aren't in this set
 		$this->output( "Searching for inactive revisions..." );
 		if ( count( $latestRevs ) > 0 ) {
-			$revConds[] = 'rev_id NOT IN (' . $dbw->makeList( $latestRevs ) . ')';
+			$revConds[] = $dbw->expr( 'rev_id', '!=', $latestRevs );
 		}
 		$res = $dbw->newSelectQueryBuilder()
 			->select( 'rev_id' )

maintenance/findMissingActors.php

@@ -242,23 +242,18 @@ class FindMissingActors extends Maintenance {
 		LIMIT 1000;
 		*/
 
-		$conds = $type == 'missing'
-			? [ 'actor_id' => null ]
-			: [ 'actor_name' => '' ];
+		$queryBuilder = $dbr->newSelectQueryBuilder()
+			->select( [ $actorField, $idField ] )
+			->from( $table )
+			->leftJoin( 'actor', null, [ "$actorField = actor_id" ] )
+			->where( $type == 'missing' ? [ 'actor_id' => null ] : [ 'actor_name' => '' ] )
+			->limit( $this->getBatchSize() );
 
 		if ( $skip ) {
-			$conds[] = $actorField . ' NOT IN ( ' . $dbr->makeList( $skip ) . ' ) ';
+			$queryBuilder->andWhere( $dbr->expr( $actorField, '!=', $skip ) );
 		}
 
-		$queryBuilder = $dbr->newSelectQueryBuilder();
-		$queryBuilder->table( $table )
-			->fields( [ $actorField, $idField ] )
-			->conds( $conds )
-			->leftJoin( 'actor', null, [ "$actorField = actor_id" ] )
-			->limit( $this->getBatchSize() )
-			->caller( __METHOD__ );
-
-		$res = $queryBuilder->fetchResultSet();
+		$res = $queryBuilder->caller( __METHOD__ )->fetchResultSet();
 		$count = $res->numRows();
 
 		$bad = [];

maintenance/findOrphanedFiles.php

@@ -19,6 +19,7 @@
  */
 
 use MediaWiki\Title\Title;
+use Wikimedia\Rdbms\OrExpressionGroup;
 
 require_once __DIR__ . '/Maintenance.php';
 
@@ -91,10 +92,7 @@ class FindOrphanedFiles extends Maintenance {
 
 				$oldNames[] = $name;
 				[ , $base ] = explode( '!', $name, 2 ); // <TS_MW>!<img_name>
-				$oiWheres[] = $dbr->makeList(
-					[ 'oi_name' => $base, 'oi_archive_name' => $name ],
-					LIST_AND
-				);
+				$oiWheres[]  = $dbr->expr( 'oi_name', '=', $base )->and( 'oi_archive_name', '=', $name );
 			} else {
 				if ( $verbose ) {
 					$this->output( "Checking current file $name\n" );
@@ -115,7 +113,7 @@ class FindOrphanedFiles extends Maintenance {
 			$dbr->newSelectQueryBuilder()
 				->select( [ 'name' => 'oi_archive_name', 'old' => '1' ] )
 				->from( 'oldimage' )
-				->where( $oiWheres ? $dbr->makeList( $oiWheres, LIST_OR ) : '1=0' )
+				->where( $oiWheres ? new OrExpressionGroup( ...$oiWheres ) : '1=0' )
 		);
 
 		$res = $uqb->all()->caller( __METHOD__ )->fetchResultSet();

maintenance/includes/Maintenance.php

@@ -1007,7 +1007,7 @@ abstract class Maintenance {
 			->select( 'old_id' )
 			->distinct()
 			->from( 'text' )
-			->where( [ 'old_id NOT IN ( ' . $dbw->makeList( $cur ) . ' )' ] )
+			->where( $dbw->expr( 'old_id', '!=', $cur ) )
 			->caller( __METHOD__ )->fetchResultSet();
 		$old = [];
 		foreach ( $res as $row ) {

maintenance/populateRevisionLength.php

@@ -21,8 +21,6 @@
  * @ingroup Maintenance
  */
 
-use Wikimedia\Rdbms\IDatabase;
-
 require_once __DIR__ . '/Maintenance.php';
 
 /**
@@ -116,14 +114,12 @@ class PopulateRevisionLength extends LoggedUpdateMaintenance {
 				->where( [
 					"$idCol >= $blockStart",
 					"$idCol <= $blockEnd",
-					$dbr->makeList( [
-						"{$prefix}_len" => null,
-						$dbr->makeList( [
-							"{$prefix}_len" => 0,
-							// sha1( "" )
-							"{$prefix}_sha1 != " . $dbr->addQuotes( 'phoiac9h4m842xq45sp7s6u21eteeq1' ),
-						], IDatabase::LIST_AND )
-					], IDatabase::LIST_OR )
+					$dbr->expr( "{$prefix}_len", '=', null )
+						->orExpr(
+							$dbr->expr( "{$prefix}_len", '=', 0 )
+								// sha1( "" )
+								->and( "{$prefix}_sha1", '!=', 'phoiac9h4m842xq45sp7s6u21eteeq1' )
+						),
 				] )
 				->caller( __METHOD__ )->fetchResultSet();
 

tests/phpunit/includes/specials/SpecialUncategorizedCategoriesTest.php

@@ -2,6 +2,7 @@
 
 use MediaWiki\Language\RawMessage;
 use MediaWiki\Specials\SpecialUncategorizedCategories;
+use Wikimedia\Rdbms\Expression;
 
 /**
  * Tests for Special:UncategorizedCategories
@@ -52,22 +53,22 @@ class SpecialUncategorizedCategoriesTest extends MediaWikiIntegrationTestCase {
 		return [
 			[
 				"* Stubs\n* Test\n* *\n* * test123",
-				[ 0 => "page_title not in ( 'Stubs','Test','*','*_test123' )" ]
+				[ 0 => new Expression( 'page_title', '!=', [ 'Stubs', 'Test', '*', '*_test123' ] ) ]
 			],
 			[
 				"Stubs\n* Test\n* *\n* * test123",
-				[ 0 => "page_title not in ( 'Test','*','*_test123' )" ]
+				[ 0 => new Expression( 'page_title', '!=', [ 'Test', '*', '*_test123' ] ) ],
 			],
 			[
 				"* StubsTest\n* *\n* * test123",
-				[ 0 => "page_title not in ( 'StubsTest','*','*_test123' )" ]
+				[ 0 => new Expression( 'page_title', '!=', [ 'StubsTest', '*', '*_test123' ] ) ],
 			],
 			[ "", [] ],
 			[ "\n\n\n", [] ],
 			[ "\n", [] ],
-			[ "Test\n*Test2", [ 0 => "page_title not in ( 'Test2' )" ] ],
+			[ "Test\n*Test2", [ 0 => new Expression( 'page_title', '!=', [ 'Test2' ] ) ] ],
 			[ "Test", [] ],
-			[ "*Test\nTest2", [ 0 => "page_title not in ( 'Test' )" ] ],
+			[ "*Test\nTest2", [ 0 => new Expression( 'page_title', '!=', [ 'Test' ] ) ] ],
 			[ "Test\nTest2", [] ],
 		];
 	}