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 ba6bb08..bcd444e 100644 (file)
--- 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 f84be3f..d41a1f8 100644 (file)
--- 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' ) );
+       }
 }