Merge branch 'ACP2E-153' of https://github.com/magento-l3/magento2ce into PR-2021-20-08

chittima · chittima · commit 7c98c9951cc5 · 2021-09-20T12:41:41.000-05:00
diff --git a/app/code/Magento/Sales/Model/InvoiceOrder.php b/app/code/Magento/Sales/Model/InvoiceOrder.php
@@ -6,6 +6,7 @@
 
 namespace Magento\Sales\Model;
 
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\ResourceConnection;
 use Magento\Sales\Api\Data\InvoiceCommentCreationInterface;
 use Magento\Sales\Api\Data\InvoiceCreationArgumentsInterface;
@@ -21,7 +22,8 @@
 use Psr\Log\LoggerInterface;
 
 /**
- * Class InvoiceOrder
+ * Creates invoice for an order
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class InvoiceOrder implements InvoiceOrderInterface
@@ -76,6 +78,11 @@ class InvoiceOrder implements InvoiceOrderInterface
      */
     private $logger;
 
+    /**
+     * @var OrderMutexInterface
+     */
+    private $orderMutex;
+
     /**
      * InvoiceOrder constructor.
      * @param ResourceConnection $resourceConnection
@@ -88,6 +95,7 @@ class InvoiceOrder implements InvoiceOrderInterface
      * @param InvoiceOrderValidator $invoiceOrderValidator
      * @param NotifierInterface $notifierInterface
      * @param LoggerInterface $logger
+     * @param OrderMutex|null $orderMutex
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -100,7 +108,8 @@ public function __construct(
         InvoiceRepository $invoiceRepository,
         InvoiceOrderValidator $invoiceOrderValidator,
         NotifierInterface $notifierInterface,
-        LoggerInterface $logger
+        LoggerInterface $logger,
+        ?OrderMutexInterface $orderMutex = null
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->orderRepository = $orderRepository;
@@ -112,9 +121,12 @@ public function __construct(
         $this->invoiceOrderValidator = $invoiceOrderValidator;
         $this->notifierInterface = $notifierInterface;
         $this->logger = $logger;
+        $this->orderMutex = $orderMutex ?: ObjectManager::getInstance()->get(OrderMutexInterface::class);
     }
 
     /**
+     * Creates invoice for provided order ID
+     *
      * @param int $orderId
      * @param bool $capture
      * @param array $items
@@ -138,7 +150,48 @@ public function execute(
         InvoiceCommentCreationInterface $comment = null,
         InvoiceCreationArgumentsInterface $arguments = null
     ) {
-        $connection = $this->resourceConnection->getConnection('sales');
+        return $this->orderMutex->execute(
+            (int) $orderId,
+            \Closure::fromCallable([$this, 'createInvoice']),
+            [
+                $orderId,
+                $capture,
+                $items,
+                $notify,
+                $appendComment,
+                $comment,
+                $arguments
+            ]
+        );
+    }
+
+    /**
+     * Creates invoice for provided order ID
+     *
+     * @param int $orderId
+     * @param bool $capture
+     * @param array $items
+     * @param bool $notify
+     * @param bool $appendComment
+     * @param \Magento\Sales\Api\Data\InvoiceCommentCreationInterface|null $comment
+     * @param \Magento\Sales\Api\Data\InvoiceCreationArgumentsInterface|null $arguments
+     * @return int
+     * @throws \Magento\Sales\Api\Exception\DocumentValidationExceptionInterface
+     * @throws \Magento\Sales\Api\Exception\CouldNotInvoiceExceptionInterface
+     * @throws \Magento\Framework\Exception\InputException
+     * @throws \Magento\Framework\Exception\NoSuchEntityException
+     * @throws \DomainException
+     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
+     */
+    private function createInvoice(
+        $orderId,
+        $capture = false,
+        array $items = [],
+        $notify = false,
+        $appendComment = false,
+        InvoiceCommentCreationInterface $comment = null,
+        InvoiceCreationArgumentsInterface $arguments = null
+    ) {
         $order = $this->orderRepository->get($orderId);
         $invoice = $this->invoiceDocumentFactory->create(
             $order,
@@ -162,7 +215,6 @@ public function execute(
                 __("Invoice Document Validation Error(s):\n" . implode("\n", $errorMessages->getMessages()))
             );
         }
-        $connection->beginTransaction();
         try {
             $order = $this->paymentAdapter->pay($order, $invoice, $capture);
             $order->setState(
@@ -172,10 +224,8 @@ public function execute(
             $invoice->setState(\Magento\Sales\Model\Order\Invoice::STATE_PAID);
             $this->invoiceRepository->save($invoice);
             $this->orderRepository->save($order);
-            $connection->commit();
         } catch (\Exception $e) {
             $this->logger->critical($e);
-            $connection->rollBack();
             throw new \Magento\Sales\Exception\CouldNotInvoiceException(
                 __('Could not save an invoice, see error log for details')
             );
diff --git a/app/code/Magento/Sales/Model/OrderMutex.php b/app/code/Magento/Sales/Model/OrderMutex.php
@@ -0,0 +1,53 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Sales\Model;
+
+use Magento\Framework\App\ResourceConnection;
+
+/**
+ * Intended to prevent race conditions during order update by concurrent requests.
+ */
+class OrderMutex implements OrderMutexInterface
+{
+    /**
+     * @var ResourceConnection
+     */
+    private $resourceConnection;
+
+    /**
+     * @param ResourceConnection $resourceConnection
+     */
+    public function __construct(
+        ResourceConnection $resourceConnection
+    ) {
+        $this->resourceConnection = $resourceConnection;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function execute(int $orderId, callable $callable, array $args = [])
+    {
+        $connection = $this->resourceConnection->getConnection('sales');
+        $connection->beginTransaction();
+        $query = $connection->select()
+            ->from($this->resourceConnection->getTableName('sales_order'), 'entity_id')
+            ->where('entity_id = ?', $orderId)
+            ->forUpdate(true);
+        $connection->query($query);
+
+        try {
+            $result = $callable(...$args);
+            $connection->commit();
+            return $result;
+        } catch (\Throwable $e) {
+            $connection->rollBack();
+            throw $e;
+        }
+    }
+}
diff --git a/app/code/Magento/Sales/Model/OrderMutexInterface.php b/app/code/Magento/Sales/Model/OrderMutexInterface.php
@@ -0,0 +1,24 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Sales\Model;
+
+/**
+ * Intended to prevent race conditions during order update by concurrent requests.
+ */
+interface OrderMutexInterface
+{
+    /**
+     * Acquires a lock for order, executes callable and releases the lock after.
+     *
+     * @param int $orderId
+     * @param callable $callable
+     * @param array $args
+     * @return mixed
+     */
+    public function execute(int $orderId, callable $callable, array $args = []);
+}
diff --git a/app/code/Magento/Sales/Test/Unit/Model/InvoiceOrderTest.php b/app/code/Magento/Sales/Test/Unit/Model/InvoiceOrderTest.php
@@ -9,6 +9,7 @@
 
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DB\Adapter\AdapterInterface;
+use Magento\Framework\DB\Select;
 use Magento\Sales\Api\Data\InvoiceCommentCreationInterface;
 use Magento\Sales\Api\Data\InvoiceCreationArgumentsInterface;
 use Magento\Sales\Api\Data\InvoiceInterface;
@@ -26,6 +27,7 @@
 use Magento\Sales\Model\Order\OrderStateResolverInterface;
 use Magento\Sales\Model\Order\PaymentAdapterInterface;
 use Magento\Sales\Model\Order\Validation\InvoiceOrderInterface;
+use Magento\Sales\Model\OrderMutex;
 use Magento\Sales\Model\ValidatorResultInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -200,7 +202,8 @@ protected function setUp(): void
             $this->invoiceRepositoryMock,
             $this->invoiceOrderValidatorMock,
             $this->notifierInterfaceMock,
-            $this->loggerMock
+            $this->loggerMock,
+            new OrderMutex($this->resourceConnectionMock)
         );
     }
 
@@ -216,10 +219,7 @@ protected function setUp(): void
      */
     public function testOrderInvoice($orderId, $capture, $items, $notify, $appendComment)
     {
-        $this->resourceConnectionMock->expects($this->once())
-            ->method('getConnection')
-            ->with('sales')
-            ->willReturn($this->adapterInterface);
+        $this->mockConnection($orderId);
         $this->orderRepositoryMock->expects($this->once())
             ->method('get')
             ->willReturn($this->orderMock);
@@ -315,7 +315,7 @@ public function testDocumentValidationException()
         $notify = true;
         $appendComment = true;
         $errorMessages = ['error1', 'error2'];
-
+        $this->mockConnection($orderId);
         $this->orderRepositoryMock->expects($this->once())
             ->method('get')
             ->willReturn($this->orderMock);
@@ -369,10 +369,7 @@ public function testCouldNotInvoiceException()
         $capture = true;
         $notify = true;
         $appendComment = true;
-        $this->resourceConnectionMock->expects($this->once())
-            ->method('getConnection')
-            ->with('sales')
-            ->willReturn($this->adapterInterface);
+        $this->mockConnection($orderId);
 
         $this->orderRepositoryMock->expects($this->once())
             ->method('get')
@@ -440,4 +437,34 @@ public function dataProvider()
             'TestWithNotifyFalse' => [1, true, [1 => 2], false, true],
         ];
     }
+
+    /**
+     * @param int $orderId
+     */
+    private function mockConnection(int $orderId): void
+    {
+        $select = $this->createMock(Select::class);
+        $select->expects($this->once())
+            ->method('from')
+            ->with('sales_order', 'entity_id')
+            ->willReturnSelf();
+        $select->expects($this->once())
+            ->method('where')
+            ->with('entity_id = ?', $orderId)
+            ->willReturnSelf();
+        $select->expects($this->once())
+            ->method('forUpdate')
+            ->with(true)
+            ->willReturnSelf();
+        $this->adapterInterface->expects($this->once())
+            ->method('select')
+            ->willReturn($select);
+        $this->resourceConnectionMock->expects($this->once())
+            ->method('getConnection')
+            ->with('sales')
+            ->willReturn($this->adapterInterface);
+        $this->resourceConnectionMock->expects($this->once())
+            ->method('getTableName')
+            ->willReturnArgument(0);
+    }
 }
diff --git a/app/code/Magento/Sales/etc/di.xml b/app/code/Magento/Sales/etc/di.xml
@@ -118,6 +118,7 @@
     <preference for="Magento\Sales\Model\ResourceModel\Provider\NotSyncedDataProviderInterface" type="Magento\Sales\Model\ResourceModel\Provider\NotSyncedDataProvider" />
     <preference for="Magento\Sales\Model\ConfigInterface" type="Magento\Sales\Model\Config" />
     <preference for="Magento\Sales\Model\Order\Shipment\ShipmentItemsValidatorInterface" type="Magento\Sales\Model\Order\Shipment\ShipmentItemsValidator" />
+    <preference for="Magento\Sales\Model\OrderMutexInterface" type="Magento\Sales\Model\OrderMutex"/>
     <type name="Magento\Sales\Model\ResourceModel\Provider\NotSyncedDataProvider">
         <arguments>
             <argument name="providers" xsi:type="array">
