From: WMDE-Fisch <christoph.jauera@wikimedia.de>
Date: Tue, 30 Apr 2019 14:22:48 +0000 (+0200)
Subject: Let User::idFromName always return int or null
X-Git-Tag: 1.34.0-rc.0~1785^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/membres/fiche.php?a=commitdiff_plain;h=a91638bd387a320fb68e31f7bc0757a8c417c057;p=lhc%2Fweb%2Fwiklou.git

Let User::idFromName always return int or null

This patch makes sure, that idFromName always returns either an int
or null value. The $idCacheByName can contain string values so we cast
the values if necessary.

An alternative could have been to make sure that just int values go
into the cache. But since the cache is public the current approach
to seems to be better atm.

Change-Id: I3085d89b93db2b888c190ba193623b86dc93759a
---

diff --git a/includes/user/User.php b/includes/user/User.php
index ba6bb08a73..bcd444ee22 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -914,7 +914,7 @@ class User implements IDBAccessObject, UserIdentity {
 		}
 
 		if ( !( $flags & self::READ_LATEST ) && array_key_exists( $name, self::$idCacheByName ) ) {
-			return self::$idCacheByName[$name];
+			return is_null( self::$idCacheByName[$name] ) ? null : (int)self::$idCacheByName[$name];
 		}
 
 		list( $index, $options ) = DBAccessObjectUtils::getDBOptions( $flags );
diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index f84be3f1c4..d41a1f8f46 100644
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1506,4 +1506,38 @@ class UserTest extends MediaWikiTestCase {
 		$updater->setContent( 'main', $content );
 		return $updater->saveRevision( CommentStoreComment::newUnsavedComment( $comment ) );
 	}
+
+	/**
+	 * @covers User::idFromName
+	 */
+	public function testExistingIdFromName() {
+		$this->assertTrue(
+			array_key_exists( $this->user->getName(), User::$idCacheByName ),
+			'Test user should already be in the id cache.'
+		);
+		$this->assertSame(
+			$this->user->getId(), User::idFromName( $this->user->getName() ),
+			'Id is correctly retreived from the cache.'
+		);
+		$this->assertSame(
+			$this->user->getId(), User::idFromName( $this->user->getName(), User::READ_LATEST ),
+			'Id is correctly retreived from the database.'
+		);
+	}
+
+	/**
+	 * @covers User::idFromName
+	 */
+	public function testNonExistingIdFromName() {
+		$this->assertFalse(
+			array_key_exists( 'NotExisitngUser', User::$idCacheByName ),
+			'Non exisitng user should not be in the id cache.'
+		);
+		$this->assertSame( null, User::idFromName( 'NotExisitngUser' ) );
+		$this->assertTrue(
+			array_key_exists( 'NotExisitngUser', User::$idCacheByName ),
+			'Username will be cached when requested once.'
+		);
+		$this->assertSame( null, User::idFromName( 'NotExisitngUser' ) );
+	}
 }