PIMAG-747 | Bugfix: Prevent race conditions when saving shipment trackings

Observer/LockUnlockOrder.php

@@ -1,4 +1,8 @@
 <?php
+/*
+ * Copyright Magmodules.eu. All rights reserved.
+ * See COPYING.txt for license details.
+ */
 
 namespace Mollie\Payment\Observer;
 
@@ -7,7 +11,6 @@
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Sales\Api\Data\OrderInterface;
 use Magento\Sales\Api\Data\ShipmentInterface;
-use Mollie\Payment\Config;
 use Mollie\Payment\Service\LockService;
 
 class LockUnlockOrder implements ObserverInterface
@@ -18,16 +21,14 @@ class LockUnlockOrder implements ObserverInterface
     private $lockService;
 
     /**
-     * @var Config
+     * @var string
      */
-    private $config;
+    private $reason = '';
 
     public function __construct(
-        Config $config,
         LockService $lockService
     ) {
         $this->lockService = $lockService;
-        $this->config = $config;
     }
 
     public function execute(Observer $observer)
@@ -41,7 +42,7 @@ public function execute(Observer $observer)
                 throw new LocalizedException(__('Unable to get lock for %1', $key));
             }
 
-            $this->lockService->lock($key);
+            $this->lockService->lock($key, -1, $this->reason);
         }
 
         if (strpos($name, 'save_after') !== false) {
@@ -54,6 +55,13 @@ private function getOrder(string $name, Observer $observer): OrderInterface
         /** @var ShipmentInterface $shipment */
         $shipment = $observer->getEvent()->getData('shipment');
 
-        return $shipment->getOrder();
+        if ($shipment) {
+            $this->reason = 'shipment';
+            return $shipment->getOrder();
+        }
+
+        $this->reason = 'shipment tracking';
+        $track = $observer->getEvent()->getData('track');
+        return $track->getShipment()->getOrder();
     }
 }

Service/LockService.php

@@ -61,16 +61,18 @@ public function __construct(
      *
      * @param string $name lock name
      * @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
+     * @param ?string $reason Reason for locking, will be logged only
      * @return bool
      */
-    public function lock(string $name, int $timeout = -1): bool
+    public function lock(string $name, int $timeout = -1, ?string $reason = null): bool
     {
         // Make sure we only lock once per request.
         if ($this->alreadyLocked) {
             return true;
         }
 
-        $this->config->addToLog('info', 'Locking: ' . $name);
+        $message = 'Locking: ' . $name . ($reason ? ' - Reaseon: ' . $reason : '');
+        $this->config->addToLog('info', $message);
         if ($this->isLockManagerAvailable()) {
             return $this->alreadyLocked = $this->lockManager->lock($name, $timeout);
         }

etc/events.xml

@@ -19,8 +19,13 @@
     <event name="sales_order_shipment_save_after">
         <observer name="mollie_lock_unlock_order" instance="Mollie\Payment\Observer\LockUnlockOrder" />
     </event>
+    <event name="sales_order_shipment_track_save_before">
+        <observer name="mollie_aaaa_lock_unlock_order" instance="Mollie\Payment\Observer\LockUnlockOrder" />
+    </event>
     <event name="sales_order_shipment_track_save_after">
         <observer name="mollie_create_shipment" instance="Mollie\Payment\Observer\SalesOrderShipmentTrackSaveAfter"/>
+        <!-- Order of execution is important -->
+        <observer name="mollie_zzzz_lock_unlock_order" instance="Mollie\Payment\Observer\LockUnlockOrder" />
     </event>
     <event name="sales_order_creditmemo_save_after">
         <observer name="mollie_create_online_refund" instance="Mollie\Payment\Observer\SalesOrderCreditmemoSaveAfter"/>