From 4d9cb9d669e15c8c548a1d8b5552d4e5a6dd9aac Mon Sep 17 00:00:00 2001 From: full Date: Thu, 26 Apr 2018 17:21:57 +0200 Subject: [PATCH 1/7] Don't get repository at construction time, it could be not loaded yet! --- Entity/TokenManager.php | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index 4a87c17f..f99b638f 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -25,11 +25,6 @@ class TokenManager extends BaseTokenManager */ protected $em; - /** - * @var EntityRepository - */ - protected $repository; - /** * @var string */ @@ -37,15 +32,23 @@ class TokenManager extends BaseTokenManager public function __construct(EntityManagerInterface $em, $class) { - // NOTE: bug in Doctrine, hinting EntityRepository|ObjectRepository when only EntityRepository is expected - /** @var EntityRepository $repository */ - $repository = $em->getRepository($class); - $this->em = $em; - $this->repository = $repository; $this->class = $class; } + /** + * retrocompatibility with old $repository property + * @param $name + * @return mixed + */ + public function __get($name) + { + if('repository' === $name){ + return $this->getRepository(); + } + + return $this->$name; + } /** * {@inheritdoc} */ @@ -59,7 +62,7 @@ public function getClass() */ public function findTokenBy(array $criteria) { - return $this->repository->findOneBy($criteria); + return $this->getRepository()->findOneBy($criteria); } /** @@ -85,7 +88,7 @@ public function deleteToken(TokenInterface $token) */ public function deleteExpired() { - $qb = $this->repository->createQueryBuilder('t'); + $qb = $this->getRepository()->createQueryBuilder('t'); $qb ->delete() ->where('t.expiresAt < ?1') @@ -94,4 +97,12 @@ public function deleteExpired() return $qb->getQuery()->execute(); } + + /** + * @return EntityRepository + */ + protected function getRepository(): EntityRepository + { + return $this->em->getRepository($this->class); + } } From eedfce1426291d4c8989ee2d045257c0c4dee3bc Mon Sep 17 00:00:00 2001 From: full Date: Thu, 26 Apr 2018 18:10:55 +0200 Subject: [PATCH 2/7] codestyle --- Entity/TokenManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index f99b638f..f4fdbf14 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -37,13 +37,13 @@ public function __construct(EntityManagerInterface $em, $class) } /** - * retrocompatibility with old $repository property + * retrocompatibility with old $repository property. * @param $name * @return mixed */ public function __get($name) { - if('repository' === $name){ + if ('repository' === $name) { return $this->getRepository(); } From c4a4e2a1acaa0e03847d793dd17350fad3a728d1 Mon Sep 17 00:00:00 2001 From: full Date: Fri, 27 Apr 2018 09:47:49 +0200 Subject: [PATCH 3/7] we can't have retrocompatibility, __get is allowed only to be public --- Entity/TokenManager.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index f4fdbf14..64ba3d94 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -36,19 +36,6 @@ public function __construct(EntityManagerInterface $em, $class) $this->class = $class; } - /** - * retrocompatibility with old $repository property. - * @param $name - * @return mixed - */ - public function __get($name) - { - if ('repository' === $name) { - return $this->getRepository(); - } - - return $this->$name; - } /** * {@inheritdoc} */ From ea4735156e37866d708da3b758b6213338d01781 Mon Sep 17 00:00:00 2001 From: full Date: Fri, 27 Apr 2018 10:14:54 +0200 Subject: [PATCH 4/7] force EntityRepository --- Entity/TokenManager.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index 64ba3d94..8287280f 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -90,6 +90,11 @@ public function deleteExpired() */ protected function getRepository(): EntityRepository { - return $this->em->getRepository($this->class); + $repository = $this->em->getRepository($this->class); + if(!($repository instanceof EntityRepository)){ + throw new \RuntimeException('EntityRepository needed'); + } + + return $repository; } } From 37571cfd4370629631eb3c786d0afa2ad23da6da Mon Sep 17 00:00:00 2001 From: full Date: Fri, 27 Apr 2018 10:49:31 +0200 Subject: [PATCH 5/7] fix tests --- Entity/TokenManager.php | 2 +- Tests/Entity/TokenManagerTest.php | 28 ++++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index 8287280f..a5487d17 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -91,7 +91,7 @@ public function deleteExpired() protected function getRepository(): EntityRepository { $repository = $this->em->getRepository($this->class); - if(!($repository instanceof EntityRepository)){ + if (!($repository instanceof EntityRepository)) { throw new \RuntimeException('EntityRepository needed'); } diff --git a/Tests/Entity/TokenManagerTest.php b/Tests/Entity/TokenManagerTest.php index acdeca16..b5dea6de 100644 --- a/Tests/Entity/TokenManagerTest.php +++ b/Tests/Entity/TokenManagerTest.php @@ -13,6 +13,7 @@ namespace FOS\OAuthServerBundle\Tests\Entity; +use Doctrine\Common\Persistence\ObjectRepository; use Doctrine\ORM\AbstractQuery; use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityManagerInterface; @@ -64,7 +65,6 @@ public function setUp() ; $this->entityManager - ->expects($this->once()) ->method('getRepository') ->with($this->className) ->willReturn($this->repository) @@ -76,7 +76,6 @@ public function setUp() public function testConstructWillSetParameters() { $this->assertAttributeSame($this->entityManager, 'em', $this->instance); - $this->assertAttributeSame($this->repository, 'repository', $this->instance); $this->assertAttributeSame($this->className, 'class', $this->instance); } @@ -228,4 +227,29 @@ public function testDeleteExpired() $this->assertSame($randomResult, $this->instance->deleteExpired()); } + + public function testExceptionWithObjectRepository() + { + $this->repository = $this->getMockBuilder(ObjectRepository::class) + ->disableOriginalConstructor() + ->getMock() + ; + + $this->entityManager = $this->getMockBuilder(EntityManager::class) + ->disableOriginalConstructor() + ->getMock() + ; + + $this->entityManager + ->expects($this->once()) + ->method('getRepository') + ->with($this->className) + ->willReturn($this->repository) + ; + + $this->instance = new TokenManager($this->entityManager, $this->className); + + $this->expectException(\RuntimeException::class); + $this->instance->findTokenBy([]); + } } From b1271e118d567ac5f9146732cc1d5a259ca0bd58 Mon Sep 17 00:00:00 2001 From: full Date: Fri, 27 Apr 2018 10:58:46 +0200 Subject: [PATCH 6/7] phpstan --- Tests/Entity/TokenManagerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Entity/TokenManagerTest.php b/Tests/Entity/TokenManagerTest.php index b5dea6de..05891dd6 100644 --- a/Tests/Entity/TokenManagerTest.php +++ b/Tests/Entity/TokenManagerTest.php @@ -16,12 +16,12 @@ use Doctrine\Common\Persistence\ObjectRepository; use Doctrine\ORM\AbstractQuery; use Doctrine\ORM\EntityManager; -use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; use FOS\OAuthServerBundle\Entity\AccessToken; use FOS\OAuthServerBundle\Entity\TokenManager; use FOS\OAuthServerBundle\Model\TokenInterface; +use PHPUnit\Framework\MockObject\MockObject; /** * @group time-sensitive @@ -33,12 +33,12 @@ class TokenManagerTest extends \PHPUnit\Framework\TestCase { /** - * @var \PHPUnit_Framework_MockObject_MockObject|EntityManagerInterface + * @var MockObject */ protected $entityManager; /** - * @var \PHPUnit_Framework_MockObject_MockObject|EntityRepository + * @var MockObject */ protected $repository; From 1d895fb9d576f06d3296354dd55accae3db810ee Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Fri, 14 Sep 2018 16:44:58 +0200 Subject: [PATCH 7/7] Update Client Manager --- Entity/ClientManager.php | 19 ++++++++----------- Entity/TokenManager.php | 8 +------- Tests/Entity/ClientManagerTest.php | 2 -- Tests/Entity/TokenManagerTest.php | 26 -------------------------- 4 files changed, 9 insertions(+), 46 deletions(-) diff --git a/Entity/ClientManager.php b/Entity/ClientManager.php index 346c9732..615b7a82 100644 --- a/Entity/ClientManager.php +++ b/Entity/ClientManager.php @@ -25,11 +25,6 @@ class ClientManager extends BaseClientManager */ protected $em; - /** - * @var EntityRepository - */ - protected $repository; - /** * @var string */ @@ -37,12 +32,7 @@ class ClientManager extends BaseClientManager public function __construct(EntityManagerInterface $em, $class) { - // NOTE: bug in Doctrine, hinting EntityRepository|ObjectRepository when only EntityRepository is expected - /** @var EntityRepository $repository */ - $repository = $em->getRepository($class); - $this->em = $em; - $this->repository = $repository; $this->class = $class; } @@ -59,7 +49,7 @@ public function getClass() */ public function findClientBy(array $criteria) { - return $this->repository->findOneBy($criteria); + return $this->getRepository()->findOneBy($criteria); } /** @@ -79,4 +69,11 @@ public function deleteClient(ClientInterface $client) $this->em->remove($client); $this->em->flush(); } + + private function getRepository(): EntityRepository + { + $repository = $this->em->getRepository($this->class); + + return $repository; + } } diff --git a/Entity/TokenManager.php b/Entity/TokenManager.php index a5487d17..e4d46ad0 100644 --- a/Entity/TokenManager.php +++ b/Entity/TokenManager.php @@ -85,15 +85,9 @@ public function deleteExpired() return $qb->getQuery()->execute(); } - /** - * @return EntityRepository - */ - protected function getRepository(): EntityRepository + private function getRepository(): EntityRepository { $repository = $this->em->getRepository($this->class); - if (!($repository instanceof EntityRepository)) { - throw new \RuntimeException('EntityRepository needed'); - } return $repository; } diff --git a/Tests/Entity/ClientManagerTest.php b/Tests/Entity/ClientManagerTest.php index d575d320..86f342ba 100644 --- a/Tests/Entity/ClientManagerTest.php +++ b/Tests/Entity/ClientManagerTest.php @@ -58,7 +58,6 @@ public function setUp() $this->className = 'RandomClassName'.\random_bytes(5); $this->entityManager - ->expects($this->once()) ->method('getRepository') ->with($this->className) ->willReturn($this->repository) @@ -72,7 +71,6 @@ public function setUp() public function testConstructWillSetParameters() { $this->assertAttributeSame($this->entityManager, 'em', $this->instance); - $this->assertAttributeSame($this->repository, 'repository', $this->instance); $this->assertAttributeSame($this->className, 'class', $this->instance); } diff --git a/Tests/Entity/TokenManagerTest.php b/Tests/Entity/TokenManagerTest.php index 05891dd6..a5d94294 100644 --- a/Tests/Entity/TokenManagerTest.php +++ b/Tests/Entity/TokenManagerTest.php @@ -13,7 +13,6 @@ namespace FOS\OAuthServerBundle\Tests\Entity; -use Doctrine\Common\Persistence\ObjectRepository; use Doctrine\ORM\AbstractQuery; use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityRepository; @@ -227,29 +226,4 @@ public function testDeleteExpired() $this->assertSame($randomResult, $this->instance->deleteExpired()); } - - public function testExceptionWithObjectRepository() - { - $this->repository = $this->getMockBuilder(ObjectRepository::class) - ->disableOriginalConstructor() - ->getMock() - ; - - $this->entityManager = $this->getMockBuilder(EntityManager::class) - ->disableOriginalConstructor() - ->getMock() - ; - - $this->entityManager - ->expects($this->once()) - ->method('getRepository') - ->with($this->className) - ->willReturn($this->repository) - ; - - $this->instance = new TokenManager($this->entityManager, $this->className); - - $this->expectException(\RuntimeException::class); - $this->instance->findTokenBy([]); - } }