From c03f5c2ad4ec4b34a0eb0b1ce9fdc4aaf08d0927 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sat, 26 Sep 2015 11:03:42 -0400 Subject: [PATCH] Massively simplifying the BC and deprecated-throwing code thanks to suggestions by stof in #15870 --- .../Authorization/Voter/AbstractVoter.php | 27 +++++-------- .../Authorization/Voter/AbstractVoterTest.php | 38 +++++++++++++++---- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php index 26e4e55f936dd..12b54db529c2b 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php @@ -70,12 +70,6 @@ public function vote(TokenInterface $token, $object, array $attributes) $vote = self::ACCESS_ABSTAIN; $class = get_class($object); - $reflector = new \ReflectionMethod($this, 'voteOnAttribute'); - $isNewOverwritten = $reflector->getDeclaringClass()->getName() !== 'Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter'; - if (!$isNewOverwritten) { - @trigger_error(sprintf("The AbstractVoter::isGranted method is deprecated since 2.8 and won't be called anymore in 3.0. Override voteOnAttribute() instead.", $reflector->class), E_USER_DEPRECATED); - } - foreach ($attributes as $attribute) { if (!$this->supports($attribute, $class)) { continue; @@ -84,16 +78,9 @@ public function vote(TokenInterface $token, $object, array $attributes) // as soon as at least one attribute is supported, default is to deny access $vote = self::ACCESS_DENIED; - if ($isNewOverwritten) { - if ($this->voteOnAttribute($attribute, $object, $token)) { - // grant access as soon as at least one voter returns a positive response - return self::ACCESS_GRANTED; - } - } else { - if ($this->isGranted($attribute, $object, $token->getUser())) { - // grant access as soon as at least one voter returns a positive response - return self::ACCESS_GRANTED; - } + if ($this->voteOnAttribute($attribute, $object, $token)) { + // grant access as soon as at least one voter returns a positive response + return self::ACCESS_GRANTED; } } @@ -191,7 +178,8 @@ protected function getSupportedAttributes() */ protected function isGranted($attribute, $object, $user = null) { - return false; + // forces isGranted() or voteOnAttribute() to be overridden + throw new \BadMethodCallException(sprintf('You must override the voteOnAttribute() method in "%s".', get_class($this))); } /** @@ -211,6 +199,9 @@ protected function isGranted($attribute, $object, $user = null) */ protected function voteOnAttribute($attribute, $object, TokenInterface $token) { - return false; + // the user should override this method, and not rely on the deprecated isGranted() + @trigger_error(sprintf("The AbstractVoter::isGranted() method is deprecated since 2.8 and won't be called anymore in 3.0. Override voteOnAttribute() in %s instead.", get_class($this)), E_USER_DEPRECATED); + + return $this->isGranted($attribute, $object, $token->getUser()); } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AbstractVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AbstractVoterTest.php index 44da147ea9334..ea72e75954492 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AbstractVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AbstractVoterTest.php @@ -19,17 +19,10 @@ */ class AbstractVoterTest extends \PHPUnit_Framework_TestCase { - /** - * @var AbstractVoter - */ - private $voter; - private $token; protected function setUp() { - $this->voter = new VoterFixture(); - $tokenMock = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); $tokenMock ->expects($this->any()) @@ -44,7 +37,9 @@ protected function setUp() */ public function testVote($expectedVote, $object, $attributes, $message) { - $this->assertEquals($expectedVote, $this->voter->vote($this->token, $object, $attributes), $message); + $voter = new VoterFixture(); + + $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); } /** @@ -58,6 +53,16 @@ public function testVoteUsingDeprecatedIsGranted($expectedVote, $object, $attrib $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); } + /** + * @group legacy + * @expectedException \BadMethodCallException + */ + public function testNoOverriddenMethodsThrowsException() + { + $voter = new DeprecatedVoterNothingImplementedFixture(); + $voter->vote($this->token, new ObjectFixture(), array('foo')); + } + public function getData() { return array( @@ -113,6 +118,23 @@ protected function isGranted($attribute, $object, $user = null) } } +class DeprecatedVoterNothingImplementedFixture extends AbstractVoter +{ + protected function getSupportedClasses() + { + return array( + 'Symfony\Component\Security\Core\Tests\Authorization\Voter\ObjectFixture', + ); + } + + protected function getSupportedAttributes() + { + return array('foo', 'bar', 'baz'); + } + + // this is a bad voter that hasn't overridden isGranted or voteOnAttribute +} + class ObjectFixture { }