Add User::equals

Seems stupid omission. Title has one. Why do I need to think how
to determine how to users objects point to the same user. Allows
more expressive code.

Also fixes a bug in multiple places where users "0" and "00" were
considered equal.

Change-Id: I682392e564b332b77ab489f2ad394fa2d28098a5

diff --git a/includes/User.php b/includes/User.php
index 7ca7d80..ad4ce60 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -4739,7 +4739,7 @@ class User implements IDBAccessObject {
                if ( $action === true ) {
                        $action = 'byemail';
                } elseif ( $action === false ) {
-                       if ( $this->getName() == $wgUser->getName() ) {
+                       if ( $this->equals( $wgUser ) ) {
                                $action = 'create';
                        } else {
                                $action = 'create2';
@@ -5024,4 +5024,15 @@ class User implements IDBAccessObject {
                        return Status::newFatal( 'badaccess-group0' );
                }
        }
+
+       /**
+        * Checks if two user objects point to the same user.
+        *
+        * @since 1.25
+        * @param User $user
+        * @return bool
+        */
+       public function equals( User $user ) {
+               return $this->getName() === $user->getName();
+       }
 }

diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php
index c719d8d..86cd1d7 100644 (file)
--- a/includes/changes/RecentChange.php
+++ b/includes/changes/RecentChange.php
@@ -450,7 +450,7 @@ class RecentChange {
                }
                // Users without the 'autopatrol' right can't patrol their
                // own revisions
-               if ( $user->getName() == $this->getAttribute( 'rc_user_text' )
+               if ( $user->getName() === $this->getAttribute( 'rc_user_text' )
                        && !$user->isAllowed( 'autopatrol' )
                ) {
                        $errors[] = array( 'markedaspatrollederror-noautopatrol' );

diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index 90a2785..47967e4 100644 (file)
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -487,7 +487,7 @@ class DifferenceEngine extends ContextSource {
                                        array( 'USE INDEX' => 'rc_timestamp' )
                                );
 
-                               if ( $change && $change->getPerformer()->getName() !== $user->getName() ) {
+                               if ( $change && !$change->getPerformer()->equals( $user ) ) {
                                        $rcid = $change->getAttribute( 'rc_id' );
                                } else {
                                        // None found or the page has been created by the current user.

diff --git a/includes/page/Article.php b/includes/page/Article.php
index 438a17c..9ce3854 100644 (file)
--- a/includes/page/Article.php
+++ b/includes/page/Article.php
@@ -1129,7 +1129,7 @@ class Article implements Page {
                        return false;
                }
 
-               if ( $rc->getPerformer()->getName() == $user->getName() ) {
+               if ( $rc->getPerformer()->equals( $user ) ) {
                        // Don't show a patrol link for own creations. If the user could
                        // patrol them, they already would be patrolled
                        return false;

diff --git a/includes/specials/SpecialUserrights.php b/includes/specials/SpecialUserrights.php
index 892ff5b..36a31bd 100644 (file)
--- a/includes/specials/SpecialUserrights.php
+++ b/includes/specials/SpecialUserrights.php
@@ -106,7 +106,7 @@ class UserrightsPage extends SpecialPage {
                        }
                }
 
-               if ( User::getCanonicalName( $this->mTarget ) == $user->getName() ) {
+               if ( User::getCanonicalName( $this->mTarget ) === $user->getName() ) {
                        $this->isself = true;
                }
 
@@ -228,7 +228,7 @@ class UserrightsPage extends SpecialPage {
                global $wgAuth;
 
                // Validate input set...
-               $isself = ( $user->getName() == $this->getUser()->getName() );
+               $isself = $user->equals( $this->getUser() );
                $groups = $user->getGroups();
                $changeable = $this->changeableGroups();
                $addable = array_merge( $changeable['add'], $isself ? $changeable['add-self'] : array() );

diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php
index 8cc2a8e..f5cd1fc 100644 (file)
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -355,4 +355,31 @@ class UserTest extends MediaWikiTestCase {
                                'false' => 'With / slash' ), 'With slash' ),
                );
        }
+
+       public function testEquals() {
+               $first = User::newFromName( 'EqualUser' );
+               $second = User::newFromName( 'EqualUser' );
+
+               $this->assertTrue( $first->equals( $first ) );
+               $this->assertTrue( $first->equals( $second ) );
+               $this->assertTrue( $second->equals( $first ) );
+
+               $third = User::newFromName( '0' );
+               $fourth = User::newFromName( '000' );
+
+               $this->assertFalse( $third->equals( $fourth ) );
+               $this->assertFalse( $fourth->equals( $third ) );
+
+               // Test users loaded from db with id
+               $user = User::newFromName( 'EqualUnitTestUser' );
+               if ( !$user->getId() ) {
+                       $user->addToDatabase();
+               }
+
+               $id = $user->getId();
+
+               $fifth = User::newFromId( $id );
+               $sixth = User::newFromName( 'EqualUnitTestUser' );
+               $this->assertTrue( $fifth->equals( $sixth ) );
+       }
 }