[BC Break][Security] Moved user comparsion logic out of UserInterface

As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.

BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: feedback needed

src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php

@@ -14,6 +14,8 @@ namespace Symfony\Component\Security\Core\Authentication\Token;
 use Symfony\Component\Security\Core\Role\RoleInterface;
 use Symfony\Component\Security\Core\Role\Role;
 use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Security\Core\User\AdvancedUserInterface;
+use Symfony\Component\Security\Core\User\ComparableInterface;
 
 /**
  * Base class for Token instances.
@@ -87,7 +89,7 @@ abstract class AbstractToken implements TokenInterface
             if (!$user instanceof UserInterface) {
                 $changed = true;
             } else {
-                $changed = !$this->user->equals($user);
+                $changed = !$this->compareUser($user);
             }
         } elseif ($user instanceof UserInterface) {
             $changed = true;
@@ -220,4 +222,49 @@ abstract class AbstractToken implements TokenInterface
 
         return sprintf('%s(user="%s", authenticated=%s, roles="%s")', $class, $this->getUsername(), json_encode($this->authenticated), implode(', ', $roles));
     }
+
+    private function compareUser(UserInterface $user)
+    {
+        if (!($this->user instanceof UserInterface)) {
+            throw new \BadMethodCallException('Method "compareUser" should be called when current user class is instance of "UserInterface".');
+        }
+
+        if ($this->user instanceof ComparableInterface) {
+            return $this->user->compareTo($user);
+        }
+
+        if ($this->user->getPassword() !== $user->getPassword()) {
+            return false;
+        }
+
+        if ($this->user->getSalt() !== $user->getSalt()) {
+            return false;
+        }
+
+        if ($this->user->getUsername() !== $user->getUsername()) {
+            return false;
+        }
+
+        if ($this->user instanceof AdvancedUserInterface && $user instanceof AdvancedUserInterface) {
+            if ($this->user->isAccountNonExpired() !== $user->isAccountNonExpired()) {
+                return false;
+            }
+
+            if ($this->user->isAccountNonLocked() !== $user->isAccountNonLocked()) {
+                return false;
+            }
+
+            if ($this->user->isCredentialsNonExpired() !== $user->isCredentialsNonExpired()) {
+                return false;
+            }
+
+            if ($this->user->isEnabled() !== $user->isEnabled()) {
+                return false;
+            }
+        } elseif ($this->user instanceof AdvancedUserInterface xor $user instanceof AdvancedUserInterface) {
+            return false;
+        }
+
+        return true;
+    }
 }

src/Symfony/Component/Security/Core/User/ComparableInterface.php

@@ -0,0 +1,32 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\User;
+
+/**
+ * ComparatorInterface used to test if two object are equal in security
+ * and re-authentication context.
+ *
+ * @author Dariusz Górecki <darek.krk@gmail.com>
+ */
+interface ComparableInterface
+{
+    /**
+     * The equality comparison should neither be done by referential equality
+     * nor by comparing identities (i.e. getId() === getId()).
+     *
+     * However, you do not need to compare every attribute, but only those that
+     * are relevant for assessing whether re-authentication is required.
+     *
+     * @return boolean
+     */
+    public function compareTo($object);
+}

src/Symfony/Component/Security/Core/User/User.php

@@ -112,44 +112,4 @@ final class User implements AdvancedUserInterface
     public function eraseCredentials()
     {
     }
-
-    /**
-     * {@inheritDoc}
-     */
-    public function equals(UserInterface $user)
-    {
-        if (!$user instanceof User) {
-            return false;
-        }
-
-        if ($this->password !== $user->getPassword()) {
-            return false;
-        }
-
-        if ($this->getSalt() !== $user->getSalt()) {
-            return false;
-        }
-
-        if ($this->username !== $user->getUsername()) {
-            return false;
-        }
-
-        if ($this->accountNonExpired !== $user->isAccountNonExpired()) {
-            return false;
-        }
-
-        if ($this->accountNonLocked !== $user->isAccountNonLocked()) {
-            return false;
-        }
-
-        if ($this->credentialsNonExpired !== $user->isCredentialsNonExpired()) {
-            return false;
-        }
-
-        if ($this->enabled !== $user->isEnabled()) {
-            return false;
-        }
-
-        return true;
-    }
 }

src/Symfony/Component/Security/Core/User/UserInterface.php

@@ -84,19 +84,4 @@ interface UserInterface
      * @return void
      */
     function eraseCredentials();
-
-    /**
-     * Returns whether or not the given user is equivalent to *this* user.
-     *
-     * The equality comparison should neither be done by referential equality
-     * nor by comparing identities (i.e. getId() === getId()).
-     *
-     * However, you do not need to compare every attribute, but only those that
-     * are relevant for assessing whether re-authentication is required.
-     *
-     * @param UserInterface $user
-     *
-     * @return Boolean
-     */
-    function equals(UserInterface $user);
 }

tests/Symfony/Tests/Component/Security/Core/Authentication/Token/AbstractTokenTest.php

@@ -144,11 +144,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
     public function getUsers()
     {
         $user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
-        $user
-            ->expects($this->any())
-            ->method('equals')
-            ->will($this->returnValue(true))
-        ;
 
         return array(
             array($user),
@@ -176,11 +171,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
     public function getUserChanges()
     {
         $user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
-        $user
-            ->expects($this->any())
-            ->method('equals')
-            ->will($this->returnValue(false))
-        ;
 
         return array(
             array(
@@ -192,9 +182,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
             array(
                 'foo', $user,
             ),
-            array(
-                $user, $user,
-            ),
             array(
                 $user, 'foo'
             ),

tests/Symfony/Tests/Component/Security/Core/User/UserTest.php

@@ -123,22 +123,4 @@ class UserTest extends \PHPUnit_Framework_TestCase
         $user->eraseCredentials();
         $this->assertEquals('superpass', $user->getPassword());
     }
-
-    public function testUsersAreEqual()
-    {
-        $user1 = new User('fabien', 'superpass', array('ROLE_USER'));
-        $user2 = clone $user1;
-
-        $this->assertTrue($user1->equals($user2));
-        $this->assertTrue($user2->equals($user1));
-    }
-
-    public function testUsersAreNotEqual()
-    {
-        $user1 = new User('fabien', 'superpass', array('ROLE_USER'));
-        $user2 = new User('fabien', 'superpass', array('ROLE_USER'), false);
-
-        $this->assertFalse($user1->equals($user2));
-        $this->assertFalse($user2->equals($user1));
-    }
 }