Fix some side effects in update subscribers

Setono · Jan 9, 2025 · f782ed2 · f782ed2
1 parent f64e6c6
commit f782ed2
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 48 deletions.
diff --git a/src/EventSubscriber/UpdateCatalogPromotionSubscriber.php b/src/EventSubscriber/UpdateCatalogPromotionSubscriber.php
@@ -12,7 +12,6 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\Messenger\MessageBusInterface;
-use Webmozart\Assert\Assert;
 
 /**
  * Notice that we don't need to handle the removal of catalog promotions because although the catalog promotion
@@ -21,9 +20,9 @@
 final class UpdateCatalogPromotionSubscriber implements EventSubscriberInterface
 {
     /**
-     * A list of catalog promotions to update
+     * An array of catalog promotions to update indexed by code
      *
-     * @var list<CatalogPromotionInterface>
+     * @var array<string, CatalogPromotionInterface>
      */
     private array $catalogPromotions = [];
 
@@ -43,30 +42,17 @@ public static function getSubscribedEvents(): array
 
     public function update(ResourceControllerEvent $event): void
     {
-        $obj = $event->getSubject();
-        Assert::isInstanceOf($obj, CatalogPromotionInterface::class);
-
-        $this->catalogPromotions[] = $obj;
+        $this->addCatalogPromotion($event->getSubject());
     }
 
     public function postPersist(LifecycleEventArgs $eventArgs): void
     {
-        $obj = $eventArgs->getObject();
-        if (!$obj instanceof CatalogPromotionInterface) {
-            return;
-        }
-
-        $this->catalogPromotions[] = $obj;
+        $this->addCatalogPromotion($eventArgs->getObject());
     }
 
     public function postUpdate(LifecycleEventArgs $eventArgs): void
     {
-        $obj = $eventArgs->getObject();
-        if (!$obj instanceof CatalogPromotionInterface) {
-            return;
-        }
-
-        $this->catalogPromotions[] = $obj;
+        $this->addCatalogPromotion($eventArgs->getObject());
     }
 
     public function dispatch(): void
@@ -85,4 +71,13 @@ public function dispatch(): void
 
         $this->catalogPromotions = [];
     }
+
+    private function addCatalogPromotion(mixed $catalogPromotion): void
+    {
+        if (!$catalogPromotion instanceof CatalogPromotionInterface) {
+            return;
+        }
+
+        $this->catalogPromotions[(string) $catalogPromotion->getCode()] = $catalogPromotion;
+    }
 }
diff --git a/src/EventSubscriber/UpdateProductSubscriber.php b/src/EventSubscriber/UpdateProductSubscriber.php
@@ -12,14 +12,17 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\Messenger\MessageBusInterface;
-use Webmozart\Assert\Assert;
 
+/**
+ * NOTICE that we don't handle the Doctrine postUpdate event because products are updated when processing a catalog promotion
+ * and hence this would lead to a lot of double work
+ */
 final class UpdateProductSubscriber implements EventSubscriberInterface
 {
     /**
-     * A list of products to update
+     * An array  of products to update index by id
      *
-     * @var list<ProductInterface>
+     * @var array<int, ProductInterface>
      */
     private array $products = [];
 
@@ -39,30 +42,12 @@ public static function getSubscribedEvents(): array
 
     public function updateProduct(ResourceControllerEvent $event): void
     {
-        $product = $event->getSubject();
-        Assert::isInstanceOf($product, ProductInterface::class);
-
-        $this->products[] = $product;
+        $this->addProduct($event->getSubject());
     }
 
     public function postPersist(LifecycleEventArgs $eventArgs): void
     {
-        $obj = $eventArgs->getObject();
-        if (!$obj instanceof ProductInterface) {
-            return;
-        }
-
-        $this->products[] = $obj;
-    }
-
-    public function postUpdate(LifecycleEventArgs $eventArgs): void
-    {
-        $obj = $eventArgs->getObject();
-        if (!$obj instanceof ProductInterface) {
-            return;
-        }
-
-        $this->products[] = $obj;
+        $this->addProduct($eventArgs->getObject());
     }
 
     public function dispatch(): void
@@ -81,4 +66,13 @@ public function dispatch(): void
 
         $this->products = [];
     }
+
+    private function addProduct(mixed $product): void
+    {
+        if (!$product instanceof ProductInterface) {
+            return;
+        }
+
+        $this->products[(int) $product->getId()] = $product;
+    }
 }
diff --git a/src/Message/Command/StartCatalogPromotionUpdate.php b/src/Message/Command/StartCatalogPromotionUpdate.php
@@ -27,15 +27,13 @@ final class StartCatalogPromotionUpdate
     public readonly array $products;
 
     /**
-     * @param list<string|CatalogPromotionInterface> $catalogPromotions
-     * @param list<int|ProductInterface> $products
+     * @param array<array-key, string|CatalogPromotionInterface> $catalogPromotions
+     * @param array<array-key, int|ProductInterface> $products
      */
     public function __construct(
         array $catalogPromotions = [],
         array $products = [],
-        /**
-         * If you want to give information about what started the update, you can provide a string here
-         */
+        /** If you want to give information about what started the update, you can provide a string here */
         public readonly ?string $triggeredBy = null,
     ) {
         $this->catalogPromotions = array_values(array_unique(array_map(

diff --git a/src/Resources/config/services/event_subscriber.xml b/src/Resources/config/services/event_subscriber.xml
@@ -21,7 +21,6 @@
 
             <tag name="kernel.event_subscriber"/>
             <tag name="doctrine.event_listener" event="postPersist"/>
-            <tag name="doctrine.event_listener" event="postUpdate"/>
         </service>
     </services>
 </container>