From 78587bb66869106e6d2e520b0e290e83643e6fa1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Skowro=C5=84ski?= <j.skowronski39@gmail.com>
Date: Sun, 17 Mar 2024 23:48:30 +0100
Subject: [PATCH] Replace doctrine subscribers with listeners

---
 config/services.yaml                          |   1 -
 .../Doctrine/ModGroupUpdatedListener.php      |  31 +++++
 .../Doctrine/ModGroupUpdatedSubscriber.php    |  42 ------
 .../BlamableEntityCreatedListener.php         |  28 ++++
 .../BlamableEntityUpdatedListener.php         |  28 ++++
 .../Doctrine/EntityBlamableSubscriber.php     |  51 -------
 .../Doctrine/ModGroupUpdatedListenerTest.php  |  67 ++++++++++
 .../BlamableEntityCreatedListenerTest.php     |  46 +++++++
 .../BlamableEntityUpdatedListenerTest.php     |  46 +++++++
 .../Doctrine/EntityBlamableSubscriberTest.php | 126 ------------------
 10 files changed, 246 insertions(+), 220 deletions(-)
 create mode 100644 src/Mods/EventListener/Doctrine/ModGroupUpdatedListener.php
 delete mode 100644 src/Mods/EventSubscriber/Doctrine/ModGroupUpdatedSubscriber.php
 create mode 100644 src/Shared/EventListener/Doctrine/BlamableEntityCreatedListener.php
 create mode 100644 src/Shared/EventListener/Doctrine/BlamableEntityUpdatedListener.php
 delete mode 100644 src/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriber.php
 create mode 100644 tests/unit/Mods/EventListener/Doctrine/ModGroupUpdatedListenerTest.php
 create mode 100644 tests/unit/Shared/EventListener/Doctrine/BlamableEntityCreatedListenerTest.php
 create mode 100644 tests/unit/Shared/EventListener/Doctrine/BlamableEntityUpdatedListenerTest.php
 delete mode 100644 tests/unit/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriberTest.php

diff --git a/config/services.yaml b/config/services.yaml
index 156e5e05..c5f207ab 100644
--- a/config/services.yaml
+++ b/config/services.yaml
@@ -58,7 +58,6 @@ services:
         arguments:
             - '%env(DATABASE_URL)%'
 
-    # Register Doctrine subscribers
     App\Shared\Service\Mission\MissionClient:
         arguments:
             $missionApiUrl: '%env(APP_URL_API_MISSION)%'
diff --git a/src/Mods/EventListener/Doctrine/ModGroupUpdatedListener.php b/src/Mods/EventListener/Doctrine/ModGroupUpdatedListener.php
new file mode 100644
index 00000000..9a356881
--- /dev/null
+++ b/src/Mods/EventListener/Doctrine/ModGroupUpdatedListener.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Mods\EventListener\Doctrine;
+
+use App\Mods\Entity\ModGroup\ModGroup;
+use App\Mods\Service\ModListUpdateService\ModListUpdateServiceInterface;
+use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Events;
+
+#[AsEntityListener(event: Events::preUpdate, method: 'preUpdate', entity: ModGroup::class)]
+class ModGroupUpdatedListener
+{
+    public function __construct(
+        private EntityManagerInterface $entityManager,
+        private ModListUpdateServiceInterface $modListUpdateService,
+    ) {
+    }
+
+    public function preUpdate(ModGroup $modGroup): void
+    {
+        // Do nothing if updated entity is not a Mod Group or no changes were made to the entity
+        if (!$this->entityManager->getUnitOfWork()->getEntityChangeSet($modGroup)) {
+            return;
+        }
+
+        $this->modListUpdateService->updateModListsAssociatedWithModGroup($modGroup);
+    }
+}
diff --git a/src/Mods/EventSubscriber/Doctrine/ModGroupUpdatedSubscriber.php b/src/Mods/EventSubscriber/Doctrine/ModGroupUpdatedSubscriber.php
deleted file mode 100644
index 72f6138d..00000000
--- a/src/Mods/EventSubscriber/Doctrine/ModGroupUpdatedSubscriber.php
+++ /dev/null
@@ -1,42 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Mods\EventSubscriber\Doctrine;
-
-use App\Mods\Entity\ModGroup\ModGroup;
-use App\Mods\Service\ModListUpdateService\ModListUpdateServiceInterface;
-use Doctrine\Common\EventSubscriber;
-use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\Event\PreUpdateEventArgs;
-use Doctrine\ORM\Events;
-use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;
-
-#[AutoconfigureTag('doctrine.event_subscriber')]
-class ModGroupUpdatedSubscriber implements EventSubscriber
-{
-    public function __construct(
-        private EntityManagerInterface $entityManager,
-        private ModListUpdateServiceInterface $modListUpdateService,
-    ) {
-    }
-
-    public function getSubscribedEvents(): array
-    {
-        return [
-            Events::preUpdate,
-        ];
-    }
-
-    public function preUpdate(PreUpdateEventArgs $args): void
-    {
-        $modGroup = $args->getObject();
-
-        // Do nothing if updated entity is not a Mod Group or no changes were made to the entity
-        if (!$modGroup instanceof ModGroup || !$this->entityManager->getUnitOfWork()->getEntityChangeSet($modGroup)) {
-            return;
-        }
-
-        $this->modListUpdateService->updateModListsAssociatedWithModGroup($modGroup);
-    }
-}
diff --git a/src/Shared/EventListener/Doctrine/BlamableEntityCreatedListener.php b/src/Shared/EventListener/Doctrine/BlamableEntityCreatedListener.php
new file mode 100644
index 00000000..91801e3b
--- /dev/null
+++ b/src/Shared/EventListener/Doctrine/BlamableEntityCreatedListener.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Shared\EventListener\Doctrine;
+
+use App\Shared\Entity\Common\AbstractBlamableEntity;
+use App\Users\Entity\User\User;
+use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
+use Doctrine\ORM\Events;
+use Symfony\Bundle\SecurityBundle\Security;
+
+#[AsEntityListener(event: Events::prePersist, method: 'prePersist', entity: AbstractBlamableEntity::class)]
+class BlamableEntityCreatedListener
+{
+    public function __construct(
+        private Security $security
+    ) {
+    }
+
+    public function prePersist(AbstractBlamableEntity $entity): void
+    {
+        $currentUser = $this->security->getUser();
+        if ($currentUser instanceof User) {
+            $entity->created($currentUser);
+        }
+    }
+}
diff --git a/src/Shared/EventListener/Doctrine/BlamableEntityUpdatedListener.php b/src/Shared/EventListener/Doctrine/BlamableEntityUpdatedListener.php
new file mode 100644
index 00000000..18dafab7
--- /dev/null
+++ b/src/Shared/EventListener/Doctrine/BlamableEntityUpdatedListener.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Shared\EventListener\Doctrine;
+
+use App\Shared\Entity\Common\AbstractBlamableEntity;
+use App\Users\Entity\User\User;
+use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
+use Doctrine\ORM\Events;
+use Symfony\Bundle\SecurityBundle\Security;
+
+#[AsEntityListener(event: Events::preUpdate, method: 'preUpdate', entity: AbstractBlamableEntity::class)]
+class BlamableEntityUpdatedListener
+{
+    public function __construct(
+        private Security $security
+    ) {
+    }
+
+    public function preUpdate(AbstractBlamableEntity $entity): void
+    {
+        $currentUser = $this->security->getUser();
+        if ($currentUser instanceof User) {
+            $entity->updated($currentUser);
+        }
+    }
+}
diff --git a/src/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriber.php b/src/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriber.php
deleted file mode 100644
index 0c1ba8cd..00000000
--- a/src/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriber.php
+++ /dev/null
@@ -1,51 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Shared\EventSubscriber\Doctrine;
-
-use App\Shared\Entity\Common\AbstractBlamableEntity;
-use App\Users\Entity\User\User;
-use Doctrine\Common\EventSubscriber;
-use Doctrine\ORM\Event\PrePersistEventArgs;
-use Doctrine\ORM\Event\PreUpdateEventArgs;
-use Doctrine\ORM\Events;
-use Symfony\Bundle\SecurityBundle\Security;
-use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;
-
-#[AutoconfigureTag('doctrine.event_subscriber')]
-class EntityBlamableSubscriber implements EventSubscriber
-{
-    public function __construct(
-        private Security $security
-    ) {
-    }
-
-    public function getSubscribedEvents(): array
-    {
-        return [
-            Events::prePersist,
-            Events::preUpdate,
-        ];
-    }
-
-    public function prePersist(PrePersistEventArgs $args): void
-    {
-        $currentUser = $this->security->getUser();
-        $entity = $args->getObject();
-
-        if ($currentUser instanceof User && $entity instanceof AbstractBlamableEntity) {
-            $entity->created($currentUser);
-        }
-    }
-
-    public function preUpdate(PreUpdateEventArgs $args): void
-    {
-        $currentUser = $this->security->getUser();
-        $entity = $args->getObject();
-
-        if ($currentUser instanceof User && $entity instanceof AbstractBlamableEntity) {
-            $entity->updated($currentUser);
-        }
-    }
-}
diff --git a/tests/unit/Mods/EventListener/Doctrine/ModGroupUpdatedListenerTest.php b/tests/unit/Mods/EventListener/Doctrine/ModGroupUpdatedListenerTest.php
new file mode 100644
index 00000000..085b18fd
--- /dev/null
+++ b/tests/unit/Mods/EventListener/Doctrine/ModGroupUpdatedListenerTest.php
@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Mods\EventListener\Doctrine;
+
+use App\Mods\Entity\ModGroup\ModGroup;
+use App\Mods\EventListener\Doctrine\ModGroupUpdatedListener;
+use App\Mods\Service\ModListUpdateService\ModListUpdateServiceInterface;
+use Codeception\Test\Unit;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\UnitOfWork;
+
+class ModGroupUpdatedListenerTest extends Unit
+{
+    public function testMarkAssociatedModListsAsUpdated(): void
+    {
+        $changeSet = [
+            'name' => [
+                'old name',
+                'new name',
+            ],
+        ];
+
+        $modGroup = $this->createMock(ModGroup::class);
+        $entityManager = $this->createEntityManagerMock($changeSet);
+
+        $modListUpdateService = $this->createMock(ModListUpdateServiceInterface::class);
+        $modListUpdateService
+            ->expects(self::once())
+            ->method('updateModListsAssociatedWithModGroup')
+            ->with($modGroup)
+        ;
+
+        $modGroupUpdatedListener = new ModGroupUpdatedListener($entityManager, $modListUpdateService);
+        $modGroupUpdatedListener->preUpdate($modGroup);
+    }
+
+    public function testDoNotMarkAssociatedModListsAsUpdatedIfThereAreNoChangesToEntity(): void
+    {
+        $changeSet = [];
+
+        $modGroup = $this->createMock(ModGroup::class);
+        $entityManager = $this->createEntityManagerMock($changeSet);
+
+        $modListUpdateService = $this->createMock(ModListUpdateServiceInterface::class);
+        $modListUpdateService
+            ->expects(self::never())
+            ->method('updateModListsAssociatedWithModGroup')
+            ->with($modGroup)
+        ;
+
+        $modGroupUpdatedListener = new ModGroupUpdatedListener($entityManager, $modListUpdateService);
+        $modGroupUpdatedListener->preUpdate($modGroup);
+    }
+
+    private function createEntityManagerMock(array $changeSet): EntityManagerInterface
+    {
+        $unitOfWork = $this->createMock(UnitOfWork::class);
+        $unitOfWork->method('getEntityChangeSet')->willReturn($changeSet);
+
+        $entityManager = $this->createMock(EntityManagerInterface::class);
+        $entityManager->method('getUnitOfWork')->willReturn($unitOfWork);
+
+        return $entityManager;
+    }
+}
diff --git a/tests/unit/Shared/EventListener/Doctrine/BlamableEntityCreatedListenerTest.php b/tests/unit/Shared/EventListener/Doctrine/BlamableEntityCreatedListenerTest.php
new file mode 100644
index 00000000..0f4a1083
--- /dev/null
+++ b/tests/unit/Shared/EventListener/Doctrine/BlamableEntityCreatedListenerTest.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Shared\EventListener\Doctrine;
+
+use App\Shared\Entity\Common\AbstractBlamableEntity;
+use App\Shared\EventListener\Doctrine\BlamableEntityCreatedListener;
+use App\Users\Entity\User\User;
+use Codeception\Test\Unit;
+use Symfony\Bundle\SecurityBundle\Security;
+
+class BlamableEntityCreatedListenerTest extends Unit
+{
+    public function testMarkEntityAsCreated(): void
+    {
+        $user = $this->createMock(User::class);
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn($user);
+
+        $entity = $this->createMock(AbstractBlamableEntity::class);
+        $entity
+            ->expects(self::once())
+            ->method('created')
+            ->with(self::isInstanceOf(User::class))
+        ;
+
+        $blamableEntityCreatedListener = new BlamableEntityCreatedListener($security);
+        $blamableEntityCreatedListener->prePersist($entity);
+    }
+
+    public function testDoNotMarkEntityAsCreated(): void
+    {
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn(null);
+
+        $entity = $this->createMock(AbstractBlamableEntity::class);
+        $entity
+            ->expects(self::never())
+            ->method('created')
+        ;
+
+        $blamableEntityCreatedListener = new BlamableEntityCreatedListener($security);
+        $blamableEntityCreatedListener->prePersist($entity);
+    }
+}
diff --git a/tests/unit/Shared/EventListener/Doctrine/BlamableEntityUpdatedListenerTest.php b/tests/unit/Shared/EventListener/Doctrine/BlamableEntityUpdatedListenerTest.php
new file mode 100644
index 00000000..52a1ef0e
--- /dev/null
+++ b/tests/unit/Shared/EventListener/Doctrine/BlamableEntityUpdatedListenerTest.php
@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Shared\EventListener\Doctrine;
+
+use App\Shared\Entity\Common\AbstractBlamableEntity;
+use App\Shared\EventListener\Doctrine\BlamableEntityUpdatedListener;
+use App\Users\Entity\User\User;
+use Codeception\Test\Unit;
+use Symfony\Bundle\SecurityBundle\Security;
+
+class BlamableEntityUpdatedListenerTest extends Unit
+{
+    public function testMarkEntityAsUpdated(): void
+    {
+        $user = $this->createMock(User::class);
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn($user);
+
+        $entity = $this->createMock(AbstractBlamableEntity::class);
+        $entity
+            ->expects(self::once())
+            ->method('updated')
+            ->with(self::isInstanceOf(User::class))
+        ;
+
+        $blamableEntityUpdatedListener = new BlamableEntityUpdatedListener($security);
+        $blamableEntityUpdatedListener->preUpdate($entity);
+    }
+
+    public function testDoNotMarkEntityAsUpdated(): void
+    {
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn(null);
+
+        $entity = $this->createMock(AbstractBlamableEntity::class);
+        $entity
+            ->expects(self::never())
+            ->method('updated')
+        ;
+
+        $blamableEntityUpdatedListener = new BlamableEntityUpdatedListener($security);
+        $blamableEntityUpdatedListener->preUpdate($entity);
+    }
+}
diff --git a/tests/unit/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriberTest.php b/tests/unit/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriberTest.php
deleted file mode 100644
index 770192c1..00000000
--- a/tests/unit/Shared/EventSubscriber/Doctrine/EntityBlamableSubscriberTest.php
+++ /dev/null
@@ -1,126 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Tests\Unit\Shared\EventSubscriber\Doctrine;
-
-use App\Shared\Entity\Common\AbstractBlamableEntity;
-use App\Shared\EventSubscriber\Doctrine\EntityBlamableSubscriber;
-use App\Users\Entity\User\User;
-use Codeception\Attribute\DataProvider;
-use Codeception\Test\Unit;
-use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\Event\PrePersistEventArgs;
-use Doctrine\ORM\Event\PreUpdateEventArgs;
-use Doctrine\ORM\Events;
-use Symfony\Bundle\SecurityBundle\Security;
-
-final class EntityBlamableSubscriberTest extends Unit
-{
-    public function testGetSubscribedEvents(): void
-    {
-        $security = $this->createMock(Security::class);
-        $entityBlamableSubscriberTest = new EntityBlamableSubscriber($security);
-
-        $expectedEvents = [
-            Events::prePersist,
-            Events::preUpdate,
-        ];
-
-        $subscribedEvents = $entityBlamableSubscriberTest->getSubscribedEvents();
-        self::assertSame($expectedEvents, $subscribedEvents);
-    }
-
-    public function testMarkEntityAsCreated(): void
-    {
-        $user = $this->createMock(User::class);
-        $security = $this->createMock(Security::class);
-        $security->method('getUser')->willReturn($user);
-
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-        $entity = $this->createMock(AbstractBlamableEntity::class);
-        $entity
-            ->expects(self::once())
-            ->method('created')
-            ->with(self::isInstanceOf(User::class))
-        ;
-
-        $lifecycleEventArgs = new PrePersistEventArgs($entity, $entityManager);
-
-        $entityBlamableSubscriberTest = new EntityBlamableSubscriber($security);
-        $entityBlamableSubscriberTest->prePersist($lifecycleEventArgs);
-    }
-
-    #[DataProvider('invalidEventArgs')]
-    public function testDoNotMarkEntityAsCreated(mixed $user, mixed $entity): void
-    {
-        $security = $this->createMock(Security::class);
-        $security->method('getUser')->willReturn($user);
-
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-        $lifecycleEventArgs = new PrePersistEventArgs($entity, $entityManager);
-
-        $entityBlamableSubscriberTest = new EntityBlamableSubscriber($security);
-        $entityBlamableSubscriberTest->prePersist($lifecycleEventArgs);
-    }
-
-    public function testMarkEntityAsUpdated(): void
-    {
-        $user = $this->createMock(User::class);
-        $security = $this->createMock(Security::class);
-        $security->method('getUser')->willReturn($user);
-
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-        $entity = $this->createMock(AbstractBlamableEntity::class);
-        $entity
-            ->expects(self::once())
-            ->method('updated')
-            ->with(self::isInstanceOf(User::class))
-        ;
-
-        $changeSet = [];
-        $lifecycleEventArgs = new PreUpdateEventArgs($entity, $entityManager, $changeSet);
-
-        $entityBlamableSubscriberTest = new EntityBlamableSubscriber($security);
-        $entityBlamableSubscriberTest->preUpdate($lifecycleEventArgs);
-    }
-
-    #[DataProvider('invalidEventArgs')]
-    public function testDoNotMarkEntityAsUpdated(mixed $user, mixed $entity): void
-    {
-        $security = $this->createMock(Security::class);
-        $security->method('getUser')->willReturn($user);
-
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-
-        $changeSet = [];
-        $lifecycleEventArgs = new PreUpdateEventArgs($entity, $entityManager, $changeSet);
-
-        $entityBlamableSubscriberTest = new EntityBlamableSubscriber($security);
-        $entityBlamableSubscriberTest->preUpdate($lifecycleEventArgs);
-    }
-
-    protected function invalidEventArgs(): iterable
-    {
-        $validUser = $this->createMock(User::class);
-        $invalidUser = null;
-
-        $validEntity = $this->createMock(AbstractBlamableEntity::class);
-        $validEntity->expects(self::never())->method('created');
-        $validEntity->expects(self::never())->method('updated');
-
-        $invalidEntity = $this->getMockBuilder(\stdClass::class)->getMock();
-        $invalidEntity->expects(self::never())->method(self::anything());
-
-        return [
-            'valid user, invalid entity' => [
-                $validUser,
-                $invalidEntity,
-            ],
-            'invalid user, valid entity' => [
-                $invalidUser,
-                $validEntity,
-            ],
-        ];
-    }
-}