From d9e00a3eadfe64df1a05eec67ae71809271dfcee Mon Sep 17 00:00:00 2001 From: Amir Sarabadani Date: Fri, 30 Sep 2022 16:03:56 +0200 Subject: [PATCH] RestrictionStore: Migrate Database::select usages to SelectQueryBuilder Bug: T311866 Change-Id: I420d3904cbda6997b63bd0b2fce47926d94811d8 --- includes/Permissions/RestrictionStore.php | 50 +++++++++---------- .../Permissions/RestrictionStoreTest.php | 11 +++- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/includes/Permissions/RestrictionStore.php b/includes/Permissions/RestrictionStore.php index 74412d678ca..8f5c0d8cf7d 100644 --- a/includes/Permissions/RestrictionStore.php +++ b/includes/Permissions/RestrictionStore.php @@ -374,12 +374,11 @@ class RestrictionStore { $fname = __METHOD__; $loadRestrictionsFromDb = static function ( IDatabase $dbr ) use ( $fname, $id ) { return iterator_to_array( - $dbr->select( - 'page_restrictions', - [ 'pr_type', 'pr_expiry', 'pr_level', 'pr_cascade' ], - [ 'pr_page' => $id ], - $fname - ) + $dbr->newSelectQueryBuilder() + ->select( [ 'pr_type', 'pr_expiry', 'pr_level', 'pr_cascade' ] ) + ->from( 'page_restrictions' ) + ->where( [ 'pr_page' => $id ] ) + ->caller( $fname )->fetchResultSet() ); }; @@ -576,36 +575,33 @@ class RestrictionStore { return $cacheEntry['has_cascading']; } + $dbr = $this->loadBalancer->getConnectionRef( DB_REPLICA ); + $queryBuilder = $dbr->newSelectQueryBuilder(); + $queryBuilder->select( [ 'pr_expiry' ] ) + ->from( 'page_restrictions' ) + ->where( [ 'pr_cascade' => 1 ] ); + if ( $page->getNamespace() === NS_FILE ) { // Files transclusion may receive cascading protection in the future // see https://phabricator.wikimedia.org/T241453 - $tables = [ 'imagelinks', 'page_restrictions' ]; - $where_clauses = [ - 'il_to' => $page->getDBkey(), - 'il_from=pr_page', - 'pr_cascade' => 1 - ]; + $queryBuilder->join( 'imagelinks', null, 'il_from=pr_page' ); + $queryBuilder->andWhere( [ 'il_to' => $page->getDBkey() ] ); } else { - $tables = [ 'templatelinks', 'page_restrictions' ]; - $where_clauses = $this->linksMigration->getLinksConditions( - 'templatelinks', - TitleValue::newFromPage( $page ) + $queryBuilder->join( 'templatelinks', null, 'tl_from=pr_page' ); + $queryBuilder->andWhere( + $this->linksMigration->getLinksConditions( + 'templatelinks', + TitleValue::newFromPage( $page ) + ) ); - $where_clauses[] = 'tl_from=pr_page'; - $where_clauses['pr_cascade'] = 1; } - if ( $shortCircuit ) { - $cols = [ 'pr_expiry' ]; - } else { - $cols = [ 'pr_page', 'page_namespace', 'page_title', - 'pr_expiry', 'pr_type', 'pr_level' ]; - $where_clauses[] = 'page_id=pr_page'; - $tables[] = 'page'; + if ( !$shortCircuit ) { + $queryBuilder->fields( [ 'pr_page', 'page_namespace', 'page_title', 'pr_type', 'pr_level' ] ); + $queryBuilder->join( 'page', null, 'page_id=pr_page' ); } - $dbr = $this->loadBalancer->getConnectionRef( DB_REPLICA ); - $res = $dbr->select( $tables, $cols, $where_clauses, __METHOD__ ); + $res = $queryBuilder->caller( __METHOD__ )->fetchResultSet(); $sources = []; $pageRestrictions = []; diff --git a/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php b/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php index 79012ce49dc..8fc20cb9ab3 100644 --- a/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php +++ b/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php @@ -23,6 +23,7 @@ use WANObjectCache; use Wikimedia\Assert\PreconditionException; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\ILoadBalancer; +use Wikimedia\Rdbms\SelectQueryBuilder; /** * @coversDefaultClass \MediaWiki\Permissions\RestrictionStore @@ -56,7 +57,10 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase { $dbs = []; foreach ( $expectedCalls as $index => $calls ) { - $dbs[$index] = $this->createNoOpMock( IDatabase::class, array_keys( $calls ) ); + $dbs[$index] = $this->createNoOpMock( + IDatabase::class, + array_merge( array_keys( $calls ), [ 'newSelectQueryBuilder' ] ) + ); foreach ( $calls as $method => $callback ) { $count = 1; if ( is_array( $callback ) ) { @@ -65,6 +69,11 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase { $dbs[$index]->expects( $count < 0 ? $this->any() : $this->exactly( $count ) ) ->method( $method )->willReturnCallback( $callback ); } + $dbs[$index] + ->method( 'newSelectQueryBuilder' ) + ->willReturnCallback( static function () use ( $dbs, $index ) { + return new SelectQueryBuilder( $dbs[$index] ); + } ); } $lb = $this->createMock( ILoadBalancer::class, [ 'getConnectionRef' ] );