ActorNormalization should require a DB connection.

ActorNormalization should only be used in contexts where a DB connection
is already being used, or at least easily available.

Bug: T276986
Change-Id: Ie8aadaffa4a30e63fcad0502d45b8ec364fc79ef

5 files changed, 61 insertions, 26 deletions

diff --git a/tests/phpunit/includes/ActorMigrationTest.php b/tests/phpunit/includes/ActorMigrationTest.php
index 84e5e526fbda..15445548198c 100644
--- a/tests/phpunit/includes/ActorMigrationTest.php
+++ b/tests/phpunit/includes/ActorMigrationTest.php
@@ -515,7 +515,8 @@ class ActorMigrationTest extends MediaWikiLangTestCase {
 	 */
 	public function testInsertRoundTrip( $table, $key, $pk, $usesTemp ) {
 		$u = $this->getTestUser()->getUser();
-		$actorId = $this->getServiceContainer()->getActorNormalization()->acquireActorId( $u );
+		$actorId =
+			$this->getServiceContainer()->getActorNormalization()->acquireActorId( $u, $this->db );
 		$user = new UserIdentityValue( $u->getId(), $u->getName(), $actorId );
 
 		$stageNames = [
diff --git a/tests/phpunit/includes/page/PageArchiveTest.php b/tests/phpunit/includes/page/PageArchiveTest.php
index ad98afd11dc7..043c8ea2bed4 100644
--- a/tests/phpunit/includes/page/PageArchiveTest.php
+++ b/tests/phpunit/includes/page/PageArchiveTest.php
@@ -170,7 +170,7 @@ class PageArchiveTest extends MediaWikiIntegrationTestCase {
 				'ar_user' => null,
 				'ar_user_text' => $this->ipEditor,
 				'ar_actor' => (string)$this->getServiceContainer()->getActorNormalization()
-					->acquireActorId( new UserIdentityValue( 0, $this->ipEditor, 0 ) ),
+					->acquireActorId( new UserIdentityValue( 0, $this->ipEditor, 0 ), $this->db ),
 				'ar_len' => '11',
 				'ar_deleted' => '0',
 				'ar_rev_id' => strval( $this->ipRev->getId() ),
diff --git a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
index f363dc59538c..5c8a9ef7438a 100644
--- a/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
+++ b/tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
@@ -262,7 +262,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 	public function testRcHidemyselfFilter() {
 		$actorNormalization = $this->getServiceContainer()->getActorNormalization();
 		$user = $this->getTestUser()->getUser();
-		$actorId = $actorNormalization->acquireActorId( $user );
+		$actorId = $actorNormalization->acquireActorId( $user, $this->db );
 		$this->assertConditions(
 			[ # expected
 				"NOT((rc_actor = {$actorId}))",
@@ -275,7 +275,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 		);
 
 		$user = User::newFromName( '10.11.12.13', false );
-		$actorId = $actorNormalization->acquireActorId( $user );
+		$actorId = $actorNormalization->acquireActorId( $user, $this->db );
 		$this->assertConditions(
 			[ # expected
 				"NOT((rc_actor = {$actorId}))",
@@ -291,7 +291,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 	public function testRcHidebyothersFilter() {
 		$actorNormalization = $this->getServiceContainer()->getActorNormalization();
 		$user = $this->getTestUser()->getUser();
-		$actorId = $actorNormalization->acquireActorId( $user );
+		$actorId = $actorNormalization->acquireActorId( $user, $this->db );
 		$this->assertConditions(
 			[ # expected
 				"(rc_actor = {$actorId})",
@@ -304,7 +304,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
 		);
 
 		$user = User::newFromName( '10.11.12.13', false );
-		$actorId = $actorNormalization->acquireActorId( $user );
+		$actorId = $actorNormalization->acquireActorId( $user, $this->db );
 		$this->assertConditions(
 			[ # expected
 				"(rc_actor = {$actorId})",
diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 096b24c1dcff..2022281376c8 100644
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1120,7 +1120,7 @@ class UserTest extends MediaWikiIntegrationTestCase {
 		// Anon user. Can't load by only user ID when that's 0.
 		$user = User::newFromName( '192.168.12.34', false );
 		// Make sure an actor ID exists
-		MediaWikiServices::getInstance()->getActorNormalization()->acquireActorId( $user );
+		MediaWikiServices::getInstance()->getActorNormalization()->acquireActorId( $user, $this->db );
 
 		$test = User::newFromAnyId( null, '192.168.12.34', null );
 		$this->assertSame( $user->getId(), $test->getId() );
diff --git a/tests/phpunit/integration/includes/user/ActorStoreTest.php b/tests/phpunit/integration/includes/user/ActorStoreTest.php
index 76a753e0503e..419d7d0f331e 100644
--- a/tests/phpunit/integration/includes/user/ActorStoreTest.php
+++ b/tests/phpunit/integration/includes/user/ActorStoreTest.php
@@ -11,6 +11,7 @@ use MediaWiki\User\ActorStore;
 use MediaWiki\User\UserIdentity;
 use MediaWiki\User\UserIdentityValue;
 use MediaWiki\User\UserNameUtils;
+use MediaWiki\User\UserSelectQueryBuilder;
 use stdClass;
 use Wikimedia\Assert\PreconditionException;
 
@@ -21,27 +22,46 @@ use Wikimedia\Assert\PreconditionException;
  */
 class ActorStoreTest extends ActorStoreTestBase {
 
-	public function provideGetActorByMethods() {
+	public function provideGetActorById() {
 		yield 'getActorById, registered' => [
-			'getActorById', // $method
 			42, // $argument
 			new UserIdentityValue( 24, 'TestUser', 42 ), // $expected
 		];
 		yield 'getActorById, anon' => [
-			'getActorById', // $method
 			43, // $argument
 			new UserIdentityValue( 0, self::IP, 43 ), // $expected
 		];
 		yield 'getActorById, non-existent' => [
-			'getActorById', // $method
 			4321231, // $argument
 			null, // $expected
 		];
 		yield 'getActorById, zero' => [
-			'getActorById', // $method
 			0, // $argument
 			null, // $expected
 		];
+	}
+
+	/**
+	 * @dataProvider provideGetActorById
+	 * @covers ::getActorById
+	 * @covers ::getUserIdentityByName
+	 * @covers ::getUserIdentityByUserId
+	 */
+	public function testGetActorById( $argument, ?UserIdentity $expected ) {
+		$actor = $this->getStore()->getActorById( $argument, $this->db );
+		if ( $expected ) {
+			$this->assertNotNull( $actor );
+			$this->assertSameActors( $expected, $actor );
+
+			// test caching
+			$cachedActor = $this->getStore()->getActorById( $argument, $this->db );
+			$this->assertSame( $actor, $cachedActor );
+		} else {
+			$this->assertNull( $actor );
+		}
+	}
+
+	public function provideGetActorByMethods() {
 		yield 'getUserIdentityByName, registered' => [
 			'getUserIdentityByName', // $method
 			'TestUser', // $argument
@@ -115,16 +135,16 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 */
 	public function testSequentialCacheRetrieval( UserIdentity $expected ) {
 		// ensure UserIdentity is cached
-		$actorId = $this->getStore()->findActorId( $expected );
+		$actorId = $this->getStore()->findActorId( $expected, $this->db );
 		$this->assertSame( $expected->getActorId(), $actorId );
 
-		$cachedActorId = $this->getStore()->findActorId( $expected );
+		$cachedActorId = $this->getStore()->findActorId( $expected, $this->db );
 		$this->assertSame( $actorId, $cachedActorId );
 
-		$cachedActorId = $this->getStore()->acquireActorId( $expected );
+		$cachedActorId = $this->getStore()->acquireActorId( $expected, $this->db );
 		$this->assertSame( $actorId, $cachedActorId );
 
-		$cached = $this->getStore()->getActorById( $actorId );
+		$cached = $this->getStore()->getActorById( $actorId, $this->db );
 		$this->assertNotNull( $cached );
 		$this->assertSameActors( $expected, $cached );
 
@@ -145,7 +165,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 */
 	public function testGetActorByIdRealUser() {
 		$user = $this->getTestUser()->getUser();
-		$actor = $this->getStore()->getActorById( $user->getActorId() );
+		$actor = $this->getStore()->getActorById( $user->getActorId(), $this->db );
 		$this->assertSameActors( $user, $actor );
 	}
 
@@ -400,12 +420,12 @@ class ActorStoreTest extends ActorStoreTestBase {
 			$this->executeWithForeignStore(
 				$wikiId,
 				function ( ActorStore $store ) use ( $expected, $actor ) {
-					$this->assertSame( $expected, $store->findActorId( $actor ) );
+					$this->assertSame( $expected, $store->findActorId( $actor, $this->db ) );
 					$this->assertSame( $expected ?: 0, $actor->getActorId( $actor->getWikiId() ) );
 				}
 			);
 		} else {
-			$this->assertSame( $expected, $this->getStore()->findActorId( $actor ) );
+			$this->assertSame( $expected, $this->getStore()->findActorId( $actor, $this->db ) );
 			$this->assertSame( $expected ?: 0, $actor->getActorId( $actor->getWikiId() ) );
 		}
 	}
@@ -417,7 +437,8 @@ class ActorStoreTest extends ActorStoreTestBase {
 		$this->markTestSkipped();
 		$this->expectException( PreconditionException::class );
 		$this->getStore()->findActorId(
-			new UserIdentityValue( 0, self::IP, 0, 'acmewiki' )
+			new UserIdentityValue( 0, self::IP, 0, 'acmewiki' ),
+			$this->db
 		);
 	}
 
@@ -461,7 +482,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 * @covers ::findActorId
 	 */
 	public function testFindActorIdByName( $name, $expected ) {
-		$this->assertSame( $expected, $this->getStore()->findActorIdByName( $name ) );
+		$this->assertSame( $expected, $this->getStore()->findActorIdByName( $name, $this->db ) );
 	}
 
 	public function provideAcquireActorId() {
@@ -482,7 +503,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 */
 	public function testAcquireActorId( callable $userCallback ) {
 		$user = $userCallback( $this->getServiceContainer() );
-		$actorId = $this->getStore()->acquireActorId( $user );
+		$actorId = $this->getStore()->acquireActorId( $user, $this->db );
 		$this->assertTrue( $actorId > 0 );
 		$this->assertSame( $actorId, $user->getActorId() );
 	}
@@ -505,7 +526,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 		$this->executeWithForeignStore(
 			'acmewiki',
 			function ( ActorStore $store ) use ( $user ) {
-				$actorId = $store->acquireActorId( $user );
+				$actorId = $store->acquireActorId( $user, $this->db );
 				$this->assertTrue( $actorId > 0 );
 				$this->assertSame( $actorId, $user->getActorId( $user->getWikiId() ) );
 			}
@@ -539,7 +560,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 */
 	public function testAcquireActorId_canNotCreate( UserIdentityValue $actor ) {
 		$this->expectException( CannotCreateActorException::class );
-		$this->getStore()->acquireActorId( $actor );
+		$this->getStore()->acquireActorId( $actor, $this->db );
 	}
 
 	public function provideAcquireActorId_existing() {
@@ -558,7 +579,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	 * @covers ::acquireActorId
 	 */
 	public function testAcquireActorId_existing( UserIdentityValue $actor, int $expected ) {
-		$this->assertSame( $expected, $this->getStore()->acquireActorId( $actor ) );
+		$this->assertSame( $expected, $this->getStore()->acquireActorId( $actor, $this->db ) );
 	}
 
 	public function testAcquireActorId_domain_mismatch() {
@@ -576,7 +597,8 @@ class ActorStoreTest extends ActorStoreTestBase {
 		$this->markTestSkipped();
 		$this->expectException( PreconditionException::class );
 		$this->getStore()->acquireActorId(
-			new UserIdentityValue( 0, self::IP, 0, 'acmewiki' )
+			new UserIdentityValue( 0, self::IP, 0, 'acmewiki' ),
+			$this->db
 		);
 	}
 
@@ -610,4 +632,16 @@ class ActorStoreTest extends ActorStoreTestBase {
 		$this->assertSame( $expected, $store->normalizeUserName( $name, $rigor ) );
 	}
 
+	public function testNewSelectQueryBuilderWithoutDB() {
+		$store = $this->getStore();
+		$queryBuilder = $store->newSelectQueryBuilder();
+		$this->assertInstanceOf( UserSelectQueryBuilder::class, $queryBuilder );
+	}
+
+	public function testNewSelectQueryBuilderWithDB() {
+		$store = $this->getStore();
+		$queryBuilder = $store->newSelectQueryBuilder( $this->db );
+		$this->assertInstanceOf( UserSelectQueryBuilder::class, $queryBuilder );
+	}
+
 }