From 10b75fc5d1827fa92bd1d3eb25a06f9a94ba1732 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Fri, 29 Nov 2019 14:53:27 +0100 Subject: [PATCH 01/14] Add `ON DELETE CASCADE` to `AccessToken`'s Client identifier constraint --- Resources/config/doctrine/model/AccessToken.orm.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index 247c68fe..0243c7b0 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -13,7 +13,7 @@ - + From 8d667bdadf281c0a6f3b8d2933d8ba834cd8000b Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 11:59:38 +0100 Subject: [PATCH 02/14] Add test to cover delete integrity violation --- Tests/Acceptance/DeleteClientCommandTest.php | 61 +++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index de132f19..3c38e00a 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -4,11 +4,17 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; +use DateTime; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; +use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; +/** + * @covers \Trikoder\Bundle\OAuth2Bundle\Command\DeleteClientCommand + */ final class DeleteClientCommandTest extends AbstractAcceptanceTest { public function testDeleteClient(): void @@ -29,6 +35,31 @@ public function testDeleteClient(): void $this->assertNull($client); } + public function testDeleteClientWithAccessTokens(): void + { + $client = $this->fakeAClient('foobar'); + $this->getClientManager()->save($client); + + $accessToken = $this->fakeAnAccessToken( + 'bazqux', + 'xyzzy', + $client + ); + $this->getAccessTokenManager()->save($accessToken); + + $command = $this->command(); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + 'identifier' => $client->getIdentifier(), + ]); + $output = $commandTester->getDisplay(); + $this->assertStringContainsString('Given oAuth2 client deleted successfully', $output); + + $client = $this->findClient($client->getIdentifier()); + $this->assertNull($client); + } + public function testDeleteNonExistentClient(): void { $identifierName = 'invalid identifier'; @@ -42,7 +73,7 @@ public function testDeleteNonExistentClient(): void $this->assertStringContainsString(sprintf('oAuth2 client identified as "%s" does not exist', $identifierName), $output); } - private function findClient($identifier): ?Client + private function findClient(string $identifier): ?Client { return $this @@ -53,11 +84,27 @@ private function findClient($identifier): ?Client ; } - private function fakeAClient($identifier): Client + private function fakeAClient(string $identifier): Client { return new Client($identifier, 'quzbaz'); } + private function fakeAnAccessToken( + string $identifier, + string $userIdentifier, + Client $client, + array $scopes = [], + string $timeModifier = '+1 day' + ): AccessToken { + return new AccessToken( + $identifier, + (new DateTime('now'))->modify($timeModifier), + $client, + $userIdentifier, + $scopes + ); + } + private function getClientManager(): ClientManagerInterface { return $this->client @@ -66,6 +113,16 @@ private function getClientManager(): ClientManagerInterface ; } + private function getAccessTokenManager(): AccessTokenManagerInterface + { + return + $this + ->client + ->getContainer() + ->get(AccessTokenManagerInterface::class) + ; + } + private function command(): Command { return $this->application->find('trikoder:oauth2:delete-client'); From 350e944b142014696b15e16677fc7f823e2bd56a Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 12:53:49 +0100 Subject: [PATCH 03/14] Remove extra space --- Resources/config/doctrine/model/AccessToken.orm.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index 0243c7b0..14f8eb58 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -13,7 +13,7 @@ - + From 06bb0d9409d0d5d17b73b1d731e78b0404565645 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 12:55:01 +0100 Subject: [PATCH 04/14] Add access token deleted check in `DeleteClientCommandTest#testDeleteClientWithAccessTokens` --- Tests/Acceptance/DeleteClientCommandTest.php | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index 3c38e00a..ec8ed2c1 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -53,11 +53,17 @@ public function testDeleteClientWithAccessTokens(): void 'command' => $command->getName(), 'identifier' => $client->getIdentifier(), ]); + + $this->clearEM(); + $output = $commandTester->getDisplay(); $this->assertStringContainsString('Given oAuth2 client deleted successfully', $output); $client = $this->findClient($client->getIdentifier()); $this->assertNull($client); + + $accessToken = $this->findAccessToken($accessToken->getIdentifier()); + $this->assertNull($accessToken); } public function testDeleteNonExistentClient(): void @@ -84,6 +90,17 @@ private function findClient(string $identifier): ?Client ; } + private function findAccessToken(string $identifier): ?AccessToken + { + return + $this + ->client + ->getContainer() + ->get(AccessTokenManagerInterface::class) + ->find($identifier) + ; + } + private function fakeAClient(string $identifier): Client { return new Client($identifier, 'quzbaz'); @@ -127,4 +144,15 @@ private function command(): Command { return $this->application->find('trikoder:oauth2:delete-client'); } + + private function clearEM() + { + $this + ->client + ->getContainer() + ->get('doctrine') + ->getManager() + ->clear() + ; + } } From d58a7256e6e94ab424267e2fe070bf56a2d893bd Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 13:07:23 +0100 Subject: [PATCH 05/14] Add refresh token interaction check --- Tests/Acceptance/DeleteClientCommandTest.php | 70 ++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index ec8ed2c1..e5c7f678 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -9,8 +9,10 @@ use Symfony\Component\Console\Tester\CommandTester; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; +use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; /** * @covers \Trikoder\Bundle\OAuth2Bundle\Command\DeleteClientCommand @@ -66,6 +68,44 @@ public function testDeleteClientWithAccessTokens(): void $this->assertNull($accessToken); } + public function testDeleteClientWithAccessAndRefreshTokens(): void + { + $client = $this->fakeAClient('foobar'); + $this->getClientManager()->save($client); + + $accessToken = $this->fakeAnAccessToken( + 'bazqux', + 'xyzzy', + $client + ); + $this->getAccessTokenManager()->save($accessToken); + + $refreshToken = $this->fakeARefreshToken('killroy', $accessToken); + $this->getRefreshTokenManager()->save($refreshToken); + + $command = $this->command(); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + 'identifier' => $client->getIdentifier(), + ]); + + $this->clearEM(); + + $output = $commandTester->getDisplay(); + $this->assertStringContainsString('Given oAuth2 client deleted successfully', $output); + + $client = $this->findClient($client->getIdentifier()); + $this->assertNull($client); + + $accessToken = $this->findAccessToken($accessToken->getIdentifier()); + $this->assertNull($accessToken); + + $refreshToken = $this->findRefreshToken($refreshToken->getIdentifier()); + $this->assertNotNull($refreshToken); + $this->assertNull($refreshToken->getAccessToken()); + } + public function testDeleteNonExistentClient(): void { $identifierName = 'invalid identifier'; @@ -101,6 +141,17 @@ private function findAccessToken(string $identifier): ?AccessToken ; } + private function findRefreshToken(string $identifier): ?RefreshToken + { + return + $this + ->client + ->getContainer() + ->get(RefreshTokenManagerInterface::class) + ->find($identifier) + ; + } + private function fakeAClient(string $identifier): Client { return new Client($identifier, 'quzbaz'); @@ -122,6 +173,15 @@ private function fakeAnAccessToken( ); } + private function fakeARefreshToken(string $identifier, AccessToken $accessToken, string $timeModifier = '+1 day') + { + return new RefreshToken( + $identifier, + (new DateTime('now'))->modify($timeModifier), + $accessToken + ); + } + private function getClientManager(): ClientManagerInterface { return $this->client @@ -140,6 +200,16 @@ private function getAccessTokenManager(): AccessTokenManagerInterface ; } + private function getRefreshTokenManager(): RefreshTokenManagerInterface + { + return + $this + ->client + ->getContainer() + ->get(RefreshTokenManagerInterface::class) + ; + } + private function command(): Command { return $this->application->find('trikoder:oauth2:delete-client'); From 9d1eaf06ac4adfabf4e80ba98d474d8b675164f0 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 13:08:40 +0100 Subject: [PATCH 06/14] Use `get*Manager` methods in `find*` methods --- Tests/Acceptance/DeleteClientCommandTest.php | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index e5c7f678..964ef72c 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -123,9 +123,7 @@ private function findClient(string $identifier): ?Client { return $this - ->client - ->getContainer() - ->get(ClientManagerInterface::class) + ->getClientManager() ->find($identifier) ; } @@ -134,9 +132,7 @@ private function findAccessToken(string $identifier): ?AccessToken { return $this - ->client - ->getContainer() - ->get(AccessTokenManagerInterface::class) + ->getAccessTokenManager() ->find($identifier) ; } @@ -145,9 +141,7 @@ private function findRefreshToken(string $identifier): ?RefreshToken { return $this - ->client - ->getContainer() - ->get(RefreshTokenManagerInterface::class) + ->getRefreshTokenManager() ->find($identifier) ; } @@ -184,9 +178,11 @@ private function fakeARefreshToken(string $identifier, AccessToken $accessToken, private function getClientManager(): ClientManagerInterface { - return $this->client - ->getContainer() - ->get(ClientManagerInterface::class) + return + $this + ->client + ->getContainer() + ->get(ClientManagerInterface::class) ; } From ec1cbbda0ade7b655fcd456f92331cde78c3c157 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 13:10:08 +0100 Subject: [PATCH 07/14] Move `command` method to bottom of class --- Tests/Acceptance/DeleteClientCommandTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index 964ef72c..5aa91566 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -206,11 +206,6 @@ private function getRefreshTokenManager(): RefreshTokenManagerInterface ; } - private function command(): Command - { - return $this->application->find('trikoder:oauth2:delete-client'); - } - private function clearEM() { $this @@ -221,4 +216,9 @@ private function clearEM() ->clear() ; } + + private function command(): Command + { + return $this->application->find('trikoder:oauth2:delete-client'); + } } From ce2946003b13133c811d71e1e17d0f1859aa0746 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 15:22:03 +0100 Subject: [PATCH 08/14] Move ClientManager related tests from DeleteClientCommandTest to DoctrineClientManagerTest --- Tests/Acceptance/DeleteClientCommandTest.php | 148 ------------------ .../Acceptance/DoctrineClientManagerTest.php | 142 +++++++++++++++++ 2 files changed, 142 insertions(+), 148 deletions(-) create mode 100644 Tests/Acceptance/DoctrineClientManagerTest.php diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index 5aa91566..eda37db7 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -4,15 +4,10 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; -use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; -use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; -use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; -use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; /** * @covers \Trikoder\Bundle\OAuth2Bundle\Command\DeleteClientCommand @@ -37,75 +32,6 @@ public function testDeleteClient(): void $this->assertNull($client); } - public function testDeleteClientWithAccessTokens(): void - { - $client = $this->fakeAClient('foobar'); - $this->getClientManager()->save($client); - - $accessToken = $this->fakeAnAccessToken( - 'bazqux', - 'xyzzy', - $client - ); - $this->getAccessTokenManager()->save($accessToken); - - $command = $this->command(); - $commandTester = new CommandTester($command); - $commandTester->execute([ - 'command' => $command->getName(), - 'identifier' => $client->getIdentifier(), - ]); - - $this->clearEM(); - - $output = $commandTester->getDisplay(); - $this->assertStringContainsString('Given oAuth2 client deleted successfully', $output); - - $client = $this->findClient($client->getIdentifier()); - $this->assertNull($client); - - $accessToken = $this->findAccessToken($accessToken->getIdentifier()); - $this->assertNull($accessToken); - } - - public function testDeleteClientWithAccessAndRefreshTokens(): void - { - $client = $this->fakeAClient('foobar'); - $this->getClientManager()->save($client); - - $accessToken = $this->fakeAnAccessToken( - 'bazqux', - 'xyzzy', - $client - ); - $this->getAccessTokenManager()->save($accessToken); - - $refreshToken = $this->fakeARefreshToken('killroy', $accessToken); - $this->getRefreshTokenManager()->save($refreshToken); - - $command = $this->command(); - $commandTester = new CommandTester($command); - $commandTester->execute([ - 'command' => $command->getName(), - 'identifier' => $client->getIdentifier(), - ]); - - $this->clearEM(); - - $output = $commandTester->getDisplay(); - $this->assertStringContainsString('Given oAuth2 client deleted successfully', $output); - - $client = $this->findClient($client->getIdentifier()); - $this->assertNull($client); - - $accessToken = $this->findAccessToken($accessToken->getIdentifier()); - $this->assertNull($accessToken); - - $refreshToken = $this->findRefreshToken($refreshToken->getIdentifier()); - $this->assertNotNull($refreshToken); - $this->assertNull($refreshToken->getAccessToken()); - } - public function testDeleteNonExistentClient(): void { $identifierName = 'invalid identifier'; @@ -128,54 +54,11 @@ private function findClient(string $identifier): ?Client ; } - private function findAccessToken(string $identifier): ?AccessToken - { - return - $this - ->getAccessTokenManager() - ->find($identifier) - ; - } - - private function findRefreshToken(string $identifier): ?RefreshToken - { - return - $this - ->getRefreshTokenManager() - ->find($identifier) - ; - } - private function fakeAClient(string $identifier): Client { return new Client($identifier, 'quzbaz'); } - private function fakeAnAccessToken( - string $identifier, - string $userIdentifier, - Client $client, - array $scopes = [], - string $timeModifier = '+1 day' - ): AccessToken { - return new AccessToken( - $identifier, - (new DateTime('now'))->modify($timeModifier), - $client, - $userIdentifier, - $scopes - ); - } - - private function fakeARefreshToken(string $identifier, AccessToken $accessToken, string $timeModifier = '+1 day') - { - return new RefreshToken( - $identifier, - (new DateTime('now'))->modify($timeModifier), - $accessToken - ); - } - private function getClientManager(): ClientManagerInterface { return @@ -186,37 +69,6 @@ private function getClientManager(): ClientManagerInterface ; } - private function getAccessTokenManager(): AccessTokenManagerInterface - { - return - $this - ->client - ->getContainer() - ->get(AccessTokenManagerInterface::class) - ; - } - - private function getRefreshTokenManager(): RefreshTokenManagerInterface - { - return - $this - ->client - ->getContainer() - ->get(RefreshTokenManagerInterface::class) - ; - } - - private function clearEM() - { - $this - ->client - ->getContainer() - ->get('doctrine') - ->getManager() - ->clear() - ; - } - private function command(): Command { return $this->application->find('trikoder:oauth2:delete-client'); diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php new file mode 100644 index 00000000..5c23f4f3 --- /dev/null +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -0,0 +1,142 @@ +client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineAccessTokenManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + $doctrineAccessTokenManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->findOneBy( + [ + 'identifier' => $client->getIdentifier(), + ] + ) + ); + } + + public function testClientDeleteCascadesToAccessTokens(): void + { + /** @var $em EntityManagerInterface */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineAccessTokenManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + timecop_freeze(new DateTime()); + + try { + $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $doctrineAccessTokenManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->findOneBy( + [ + 'identifier' => $client->getIdentifier(), + ] + ) + ); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->findOneBy( + [ + 'identifier' => $accessToken->getIdentifier(), + ] + ) + ); + } finally { + timecop_return(); + } + } + + public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void + { + /** @var $em EntityManagerInterface */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineAccessTokenManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + timecop_freeze(new DateTime()); + + try { + $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $refreshToken = new RefreshToken('refresh token', (new DateTime())->modify('+1 day'), $accessToken); + $em->persist($refreshToken); + $em->flush(); + + $doctrineAccessTokenManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->findOneBy( + [ + 'identifier' => $client->getIdentifier(), + ] + ) + ); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->findOneBy( + [ + 'identifier' => $accessToken->getIdentifier(), + ] + ) + ); + + $em->clear(); + + /** @var $refreshToken RefreshToken */ + $refreshToken = $em + ->getRepository(RefreshToken::class) + ->findOneBy( + [ + 'identifier' => $refreshToken->getIdentifier(), + ] + ) + ; + $this->assertNotNull($refreshToken); + $this->assertNull($refreshToken->getAccessToken()); + } finally { + timecop_return(); + } + } +} From bfedb1540cfb8f14fbe41fc1d7c795b1fc2fe32d Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 14:56:53 +0100 Subject: [PATCH 09/14] =?UTF-8?q?Fix=20test=20namespaces=20=F0=9F=8C=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Tests/Acceptance/DoctrineAccessTokenManagerTest.php | 2 +- Tests/Acceptance/DoctrineRefreshTokenManagerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index b0194a93..08a3ad62 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; +namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; use DateTime; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager as DoctrineAccessTokenManager; diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index 7213678e..685ba944 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; +namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; use DateTime; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\RefreshTokenManager as DoctrineRefreshTokenManager; From 4d06370b0105e9f88817b7b865a47ffff6458040 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 15:29:57 +0100 Subject: [PATCH 10/14] Lint fixes --- Tests/Acceptance/DoctrineAccessTokenManagerTest.php | 1 - Tests/Acceptance/DoctrineRefreshTokenManagerTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index 08a3ad62..5b5f97d3 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -9,7 +9,6 @@ use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -use Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance\AbstractAcceptanceTest; /** * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index 685ba944..3d5f734d 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -9,7 +9,6 @@ use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -use Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance\AbstractAcceptanceTest; /** * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. From 08058f54d04e0f145dc2afd28b42bc310c0e53cd Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 15:36:02 +0100 Subject: [PATCH 11/14] Fix whoopsies --- .../Acceptance/DoctrineClientManagerTest.php | 148 ++++++++---------- 1 file changed, 68 insertions(+), 80 deletions(-) diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php index 5c23f4f3..c6d181fe 100644 --- a/Tests/Acceptance/DoctrineClientManagerTest.php +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -17,13 +17,13 @@ public function testSimpleDelete(): void { /** @var $em EntityManagerInterface */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $doctrineAccessTokenManager = new DoctrineClientManager($em); + $doctrineClientManager = new DoctrineClientManager($em); $client = new Client('client', 'secret'); $em->persist($client); $em->flush(); - $doctrineAccessTokenManager->remove($client); + $doctrineClientManager->remove($client); $this->assertNull( $em @@ -40,103 +40,91 @@ public function testClientDeleteCascadesToAccessTokens(): void { /** @var $em EntityManagerInterface */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $doctrineAccessTokenManager = new DoctrineClientManager($em); + $doctrineClientManager = new DoctrineClientManager($em); $client = new Client('client', 'secret'); $em->persist($client); $em->flush(); - timecop_freeze(new DateTime()); - - try { - $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); - $em->persist($accessToken); - $em->flush(); - - $doctrineAccessTokenManager->remove($client); - - $this->assertNull( - $em - ->getRepository(Client::class) - ->findOneBy( - [ - 'identifier' => $client->getIdentifier(), - ] - ) - ); - - $this->assertNull( - $em - ->getRepository(AccessToken::class) - ->findOneBy( - [ - 'identifier' => $accessToken->getIdentifier(), - ] - ) - ); - } finally { - timecop_return(); - } + $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $doctrineClientManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->findOneBy( + [ + 'identifier' => $client->getIdentifier(), + ] + ) + ); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->findOneBy( + [ + 'identifier' => $accessToken->getIdentifier(), + ] + ) + ); } public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void { /** @var $em EntityManagerInterface */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $doctrineAccessTokenManager = new DoctrineClientManager($em); + $doctrineClientManager = new DoctrineClientManager($em); $client = new Client('client', 'secret'); $em->persist($client); $em->flush(); - timecop_freeze(new DateTime()); - - try { - $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); - $em->persist($accessToken); - $em->flush(); - - $refreshToken = new RefreshToken('refresh token', (new DateTime())->modify('+1 day'), $accessToken); - $em->persist($refreshToken); - $em->flush(); - - $doctrineAccessTokenManager->remove($client); - - $this->assertNull( - $em - ->getRepository(Client::class) - ->findOneBy( - [ - 'identifier' => $client->getIdentifier(), - ] - ) - ); - - $this->assertNull( - $em - ->getRepository(AccessToken::class) - ->findOneBy( - [ - 'identifier' => $accessToken->getIdentifier(), - ] - ) - ); - - $em->clear(); - - /** @var $refreshToken RefreshToken */ - $refreshToken = $em - ->getRepository(RefreshToken::class) + $accessToken = new AccessToken('access token', (new DateTime())->modify('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $refreshToken = new RefreshToken('refresh token', (new DateTime())->modify('+1 day'), $accessToken); + $em->persist($refreshToken); + $em->flush(); + + $doctrineClientManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) ->findOneBy( [ - 'identifier' => $refreshToken->getIdentifier(), + 'identifier' => $client->getIdentifier(), ] ) - ; - $this->assertNotNull($refreshToken); - $this->assertNull($refreshToken->getAccessToken()); - } finally { - timecop_return(); - } + ); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->findOneBy( + [ + 'identifier' => $accessToken->getIdentifier(), + ] + ) + ); + + $em->clear(); + + /** @var $refreshToken RefreshToken */ + $refreshToken = $em + ->getRepository(RefreshToken::class) + ->findOneBy( + [ + 'identifier' => $refreshToken->getIdentifier(), + ] + ) + ; + $this->assertNotNull($refreshToken); + $this->assertNull($refreshToken->getAccessToken()); } } From 3865e43f25b65327deb710f48a8d8198c9328585 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Tue, 3 Dec 2019 15:37:00 +0100 Subject: [PATCH 12/14] Add comment for clearing entity manger --- Tests/Acceptance/DoctrineClientManagerTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php index c6d181fe..0d3866c7 100644 --- a/Tests/Acceptance/DoctrineClientManagerTest.php +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -113,6 +113,8 @@ public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void ) ); + // The entity manager has to be cleared manually + // because it doesn't process deep integrity constraints $em->clear(); /** @var $refreshToken RefreshToken */ From 249795dcaeab8152b5c389622b7c6a20dde5c1c0 Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Wed, 4 Dec 2019 01:04:34 +0100 Subject: [PATCH 13/14] Implement simple QC fixes --- .../Acceptance/DoctrineClientManagerTest.php | 53 +++++++------------ 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php index 0d3866c7..d8d471a2 100644 --- a/Tests/Acceptance/DoctrineClientManagerTest.php +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -11,7 +11,10 @@ use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -class DoctrineClientManagerTest extends AbstractAcceptanceTest +/** + * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\ClientManager + */ +final class DoctrineClientManagerTest extends AbstractAcceptanceTest { public function testSimpleDelete(): void { @@ -28,11 +31,7 @@ public function testSimpleDelete(): void $this->assertNull( $em ->getRepository(Client::class) - ->findOneBy( - [ - 'identifier' => $client->getIdentifier(), - ] - ) + ->find($client->getIdentifier()) ); } @@ -55,21 +54,17 @@ public function testClientDeleteCascadesToAccessTokens(): void $this->assertNull( $em ->getRepository(Client::class) - ->findOneBy( - [ - 'identifier' => $client->getIdentifier(), - ] - ) + ->find($client->getIdentifier()) ); + // The entity manager has to be cleared manually + // because it doesn't process deep integrity constraints + $em->clear(); + $this->assertNull( $em ->getRepository(AccessToken::class) - ->findOneBy( - [ - 'identifier' => $accessToken->getIdentifier(), - ] - ) + ->find($accessToken->getIdentifier()) ); } @@ -96,35 +91,23 @@ public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void $this->assertNull( $em ->getRepository(Client::class) - ->findOneBy( - [ - 'identifier' => $client->getIdentifier(), - ] - ) + ->find($client->getIdentifier()) ); + // The entity manager has to be cleared manually + // because it doesn't process deep integrity constraints + $em->clear(); + $this->assertNull( $em ->getRepository(AccessToken::class) - ->findOneBy( - [ - 'identifier' => $accessToken->getIdentifier(), - ] - ) + ->find($accessToken->getIdentifier()) ); - // The entity manager has to be cleared manually - // because it doesn't process deep integrity constraints - $em->clear(); - /** @var $refreshToken RefreshToken */ $refreshToken = $em ->getRepository(RefreshToken::class) - ->findOneBy( - [ - 'identifier' => $refreshToken->getIdentifier(), - ] - ) + ->find($refreshToken->getIdentifier()) ; $this->assertNotNull($refreshToken); $this->assertNull($refreshToken->getAccessToken()); From 2b71617863737dc9bc036b1bbd27de356bfc3dfd Mon Sep 17 00:00:00 2001 From: Josip Igrec Date: Thu, 5 Dec 2019 14:11:59 +0100 Subject: [PATCH 14/14] Add missing @TODO and @covers tags --- Tests/Acceptance/DoctrineAccessTokenManagerTest.php | 3 ++- Tests/Acceptance/DoctrineClientManagerTest.php | 1 + Tests/Acceptance/DoctrineRefreshTokenManagerTest.php | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index 5b5f97d3..1661fb0c 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -11,7 +11,8 @@ use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; /** - * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager */ final class DoctrineAccessTokenManagerTest extends AbstractAcceptanceTest { diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php index d8d471a2..f0b17fe2 100644 --- a/Tests/Acceptance/DoctrineClientManagerTest.php +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -12,6 +12,7 @@ use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; /** + * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\ClientManager */ final class DoctrineClientManagerTest extends AbstractAcceptanceTest diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index 3d5f734d..54014090 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -11,7 +11,8 @@ use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; /** - * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\RefreshTokenManager */ final class DoctrineRefreshTokenManagerTest extends AbstractAcceptanceTest {