diff --git a/Command/AutoClosingCommand.php b/Command/AutoClosingCommand.php index c162628c..36559253 100644 --- a/Command/AutoClosingCommand.php +++ b/Command/AutoClosingCommand.php @@ -24,7 +24,7 @@ */ class AutoClosingCommand extends ContainerAwareCommand { - use UserManagerTrait; + use UserManagerAwareTrait; protected static $defaultName = 'ticket:autoclosing'; diff --git a/Command/TicketManagerCommand.php b/Command/TicketManagerCommand.php index c2f0bf66..0b3d76dd 100644 --- a/Command/TicketManagerCommand.php +++ b/Command/TicketManagerCommand.php @@ -23,7 +23,7 @@ */ class TicketManagerCommand extends ContainerAwareCommand { - use UserManagerTrait; + use UserManagerAwareTrait; protected static $defaultName = 'ticket:create'; diff --git a/Command/UserManagerTrait.php b/Command/UserManagerAwareTrait.php similarity index 67% rename from Command/UserManagerTrait.php rename to Command/UserManagerAwareTrait.php index a5cf4a06..c604ff55 100644 --- a/Command/UserManagerTrait.php +++ b/Command/UserManagerAwareTrait.php @@ -11,29 +11,28 @@ namespace Hackzilla\Bundle\TicketBundle\Command; -use Doctrine\Persistence\ObjectRepository; +use Hackzilla\Bundle\TicketBundle\Manager\UserManagerInterface; use Hackzilla\Bundle\TicketBundle\Model\UserInterface; /** - * NEXT_MAJOR: Inject the object repository and the user class directly in the command - * classes and remove this trait. + * NEXT_MAJOR: Inject the user manager directly in the command classes and remove this trait. * * @author Javier Spagnoletti */ -trait UserManagerTrait +trait UserManagerAwareTrait { /** - * @var ObjectRepository + * @var UserManagerInterface */ - private $userRepository; + private $userManager; - public function __construct(string $name = null, ?ObjectRepository $userRepository = null) + public function __construct(string $name = null, ?UserManagerInterface $userManager = null) { parent::__construct($name); - $this->userRepository = $userRepository; + $this->userManager = $userManager; - if (null === $userRepository) { + if (null === $userManager) { @trigger_error(sprintf( 'Omitting or passing null as argument 2 for "%s()" is deprecated since hackzilla/ticket-bundle 3.x.', __METHOD__ @@ -43,8 +42,8 @@ public function __construct(string $name = null, ?ObjectRepository $userReposito private function findUser(string $username): ?UserInterface { - if (null !== $this->userRepository) { - return $this->userRepository->findBy(['username' => $username]); + if (null !== $this->userManager) { + return $this->userManager->findUserByUsername($username); } if (!$this->getContainer()->has('fos_user.user_manager')) { diff --git a/EventListener/UserLoad.php b/EventListener/UserLoad.php index 44344b2b..504af679 100644 --- a/EventListener/UserLoad.php +++ b/EventListener/UserLoad.php @@ -12,6 +12,7 @@ namespace Hackzilla\Bundle\TicketBundle\EventListener; use Doctrine\ORM\Event\LifecycleEventArgs; +use Hackzilla\Bundle\TicketBundle\Manager\UserManagerInterface; use Hackzilla\Bundle\TicketBundle\Model\TicketInterface; use Hackzilla\Bundle\TicketBundle\Model\TicketMessageInterface; @@ -20,11 +21,14 @@ */ class UserLoad { - protected $userRepository; + /** + * @var UserManagerInterface + */ + protected $userManager; - public function __construct($userRepository) + public function __construct(UserManagerInterface $userManager) { - $this->userRepository = $userRepository; + $this->userManager = $userManager; } public function getSubscribedEvents() @@ -38,23 +42,21 @@ public function postLoad(LifecycleEventArgs $args) { $entity = $args->getEntity(); - // Ignore any entity lifecycle events not relating to this bundles entities. + // Ignore any entity lifecycle events not related to this bundle's entities. if (!$entity instanceof TicketInterface && !$entity instanceof TicketMessageInterface) { return; } - $userRepository = $args->getEntityManager()->getRepository($this->userRepository); - if ($entity instanceof TicketInterface) { if (null === $entity->getUserCreatedObject()) { - $entity->setUserCreated($userRepository->find($entity->getUserCreated())); + $entity->setUserCreated($this->userManager->getUserById($entity->getUserCreated())); } if (null === $entity->getLastUserObject()) { - $entity->setLastUser($userRepository->find($entity->getLastUser())); + $entity->setLastUser($this->userManager->getUserById($entity->getLastUser())); } } elseif ($entity instanceof TicketMessageInterface) { if (null === $entity->getUserObject()) { - $entity->setUser($userRepository->find($entity->getUser())); + $entity->setUser($this->userManager->getUserById($entity->getUser())); } } } diff --git a/Manager/UserManager.php b/Manager/UserManager.php index b76bf087..a1c20344 100644 --- a/Manager/UserManager.php +++ b/Manager/UserManager.php @@ -45,6 +45,15 @@ public function __construct( AuthorizationCheckerInterface $authorizationChecker ) { $this->tokenStorage = $tokenStorage; + + if (!is_subclass_of($userRepository->getClassName(), UserInterface::class)) { + throw new \InvalidArgumentException(sprintf( + 'Argument 2 passed to "%s()" MUST be an object repository for a class implementing "%s".', + __METHOD__, + UserInterface::class + )); + } + $this->userRepository = $userRepository; $this->authorizationChecker = $authorizationChecker; } @@ -58,6 +67,11 @@ public function getCurrentUser() if ('anon.' === $user) { $user = 0; + } elseif (!$user instanceof UserInterface) { + throw new \LogicException(sprintf( + 'The object representing the authenticated user MUST implement "%s".', + UserInterface::class + )); } return $user; @@ -100,4 +114,9 @@ public function hasPermission($user, TicketInterface $ticket) throw new AccessDeniedHttpException(); } } + + public function findUserByUsername(string $username): ?UserInterface + { + return $this->userRepository->findOneBy(['username' => $username]); + } } diff --git a/Manager/UserManagerInterface.php b/Manager/UserManagerInterface.php index e06f20c0..cf5827cb 100644 --- a/Manager/UserManagerInterface.php +++ b/Manager/UserManagerInterface.php @@ -14,6 +14,9 @@ use Hackzilla\Bundle\TicketBundle\Model\TicketInterface; use Hackzilla\Bundle\TicketBundle\Model\UserInterface; +/** + * @method ?UserInterface findUserByUsername(string $username) + */ interface UserManagerInterface { public function getCurrentUser(); @@ -26,4 +29,7 @@ public function hasRole(UserInterface $user, $role); * @param \Hackzilla\Bundle\TicketBundle\Model\UserInterface|string $user */ public function hasPermission($user, TicketInterface $ticket); + + // NEXT_MAJOR: Uncomment this method. + // public function findUserByUsername(string $username): ?UserInterface; } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 4660f427..a5a01650 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -75,7 +75,7 @@ services: class: Hackzilla\Bundle\TicketBundle\Command\AutoClosingCommand arguments: - null - - '@hackzilla_ticket.user_repository' + - '@hackzilla_ticket.user_manager' tags: - { name: console.command, command: 'ticket:autoclosing' } @@ -83,6 +83,6 @@ services: class: Hackzilla\Bundle\TicketBundle\Command\TicketManagerCommand arguments: - null - - '@hackzilla_ticket.user_repository' + - '@hackzilla_ticket.user_manager' tags: - { name: console.command, command: 'ticket:create' } diff --git a/Tests/EventListener/UserLoadTest.php b/Tests/EventListener/UserLoadTest.php index 1f571651..a76b5a67 100644 --- a/Tests/EventListener/UserLoadTest.php +++ b/Tests/EventListener/UserLoadTest.php @@ -11,33 +11,126 @@ namespace Hackzilla\Bundle\TicketBundle\Tests\EventListener; +use Doctrine\ORM\Event\LifecycleEventArgs; +use Doctrine\Persistence\ObjectManager; +use Hackzilla\Bundle\TicketBundle\Entity\Ticket; +use Hackzilla\Bundle\TicketBundle\Entity\TicketMessage; use Hackzilla\Bundle\TicketBundle\EventListener\UserLoad; use Hackzilla\Bundle\TicketBundle\Manager\UserManager; +use Hackzilla\Bundle\TicketBundle\Model\UserInterface; +use Hackzilla\Bundle\TicketBundle\Tests\Functional\Entity\User; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; final class UserLoadTest extends WebTestCase { - private $object; + private const USER_CREATED_ID = 42; + private const LAST_USER_ID = 43; + private const USER_MESSAGE_ID = 44; + + /** + * @var UserLoad + */ + private $userLoad; + + /** + * @var UserInterface + */ + private $userCreated; + + /** + * @var UserInterface + */ + private $lastUser; + + /** + * @var UserInterface + */ + private $userMessage; protected function setUp(): void { + $this->userCreated = new User(); + $this->lastUser = new User(); + $this->userMessage = new User(); + + (\Closure::bind(function ($id): void { + $this->id = $id; + }, $this->userCreated, User::class))(self::USER_CREATED_ID); + + (\Closure::bind(function ($id): void { + $this->id = $id; + }, $this->lastUser, User::class))(self::LAST_USER_ID); + + (\Closure::bind(function ($id): void { + $this->id = $id; + }, $this->userMessage, User::class))(self::USER_MESSAGE_ID); + $userManager = $this->getUserManagerMock(); - $this->object = new UserLoad($userManager); + $this->userLoad = new UserLoad($userManager); } protected function tearDown(): void { - $this->object = null; + $this->userCreated = null; + $this->lastUser = null; + $this->userMessage = null; + $this->userLoad = null; } - public function getUserManagerMock() + public function testObjectCreated(): void { - return $this->createMock(UserManager::class); + $this->assertInstanceOf(UserLoad::class, $this->userLoad); } - public function testObjectCreated() + public function testPostLoad(): void { - $this->assertInstanceOf(UserLoad::class, $this->object); + $objectManager = $this->createStub(ObjectManager::class); + + $ticket = new Ticket(); + $ticket->setUserCreated(self::USER_CREATED_ID); + $ticket->setLastUser(self::LAST_USER_ID); + + $this->assertNull($ticket->getUserCreatedObject()); + $this->assertNull($ticket->getLastUserObject()); + + $this->userLoad->postLoad(new LifecycleEventArgs($ticket, $objectManager)); + + $this->assertSame($this->userCreated, $ticket->getUserCreatedObject()); + $this->assertSame($this->lastUser, $ticket->getLastUserObject()); + + $message = new TicketMessage(); + $message->setUser(self::USER_MESSAGE_ID); + + $this->assertNull($message->getUserObject()); + + $this->userLoad->postLoad(new LifecycleEventArgs($message, $objectManager)); + + $this->assertSame($this->userMessage, $message->getUserObject()); + } + + private function getUserManagerMock(): MockObject + { + $userManager = $this->createMock(UserManager::class); + $userManager + ->method('getUserById') + ->willReturnCallback(function ($userId): ?UserInterface { + if ($userId === $this->userCreated->getId()) { + return $this->userCreated; + } + + if ($userId === $this->lastUser->getId()) { + return $this->lastUser; + } + + if ($userId === $this->userMessage->getId()) { + return $this->userMessage; + } + + return null; + }); + + return $userManager; } } diff --git a/Tests/Manager/TicketManagerTest.php b/Tests/Manager/TicketManagerTest.php index 7cdfbebf..ceb77abf 100644 --- a/Tests/Manager/TicketManagerTest.php +++ b/Tests/Manager/TicketManagerTest.php @@ -9,7 +9,7 @@ * file that was distributed with this source code. */ -namespace Hackzilla\Bundle\TicketBundle\Tests\User; +namespace Hackzilla\Bundle\TicketBundle\Tests\Manager; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; diff --git a/Tests/Manager/UserManagerTest.php b/Tests/Manager/UserManagerTest.php index ebaa24c5..c00a862e 100644 --- a/Tests/Manager/UserManagerTest.php +++ b/Tests/Manager/UserManagerTest.php @@ -9,10 +9,11 @@ * file that was distributed with this source code. */ -namespace Hackzilla\Bundle\TicketBundle\Tests\User; +namespace Hackzilla\Bundle\TicketBundle\Tests\Manager; use Doctrine\ORM\EntityRepository; use Hackzilla\Bundle\TicketBundle\Manager\UserManager; +use Hackzilla\Bundle\TicketBundle\Tests\Functional\Entity\User; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager; use Symfony\Component\Security\Core\Authentication\Provider\AnonymousAuthenticationProvider; @@ -54,6 +55,11 @@ public function testObjectCreated() private function getMockUserRepository() { - return $this->createMock(EntityRepository::class); + $userRepository = $this->createMock(EntityRepository::class); + $userRepository + ->method('getClassName') + ->willReturn(User::class); + + return $userRepository; } }