ACP2E-2982: Order refund results in duplicate credit memo

thiaramus · thiaramus · commit 64eb22da7d5e · 2024-04-12T07:12:19.000-05:00
diff --git a/app/code/Magento/Sales/Model/RefundInvoice.php b/app/code/Magento/Sales/Model/RefundInvoice.php
@@ -5,11 +5,16 @@
  */
 namespace Magento\Sales\Model;
 
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\ResourceConnection;
 use Magento\Sales\Api\CreditmemoRepositoryInterface;
+use Magento\Sales\Api\Data\CreditmemoCommentCreationInterface;
+use Magento\Sales\Api\Data\CreditmemoCreationArgumentsInterface;
 use Magento\Sales\Api\InvoiceRepositoryInterface;
 use Magento\Sales\Api\OrderRepositoryInterface;
 use Magento\Sales\Api\RefundInvoiceInterface;
+use Magento\Sales\Exception\CouldNotRefundException;
+use Magento\Sales\Exception\DocumentValidationException;
 use Magento\Sales\Model\Order\Config as OrderConfig;
 use Magento\Sales\Model\Order\Creditmemo\NotifierInterface;
 use Magento\Sales\Model\Order\CreditmemoDocumentFactory;
@@ -19,7 +24,6 @@
 use Psr\Log\LoggerInterface;
 
 /**
- * Class RefundInvoice
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class RefundInvoice implements RefundInvoiceInterface
@@ -79,6 +83,11 @@ class RefundInvoice implements RefundInvoiceInterface
      */
     private $validator;
 
+    /**
+     * @var OrderMutexInterface
+     */
+    private $orderMutex;
+
     /**
      * RefundInvoice constructor.
      *
@@ -93,6 +102,7 @@ class RefundInvoice implements RefundInvoiceInterface
      * @param NotifierInterface $notifier
      * @param OrderConfig $config
      * @param LoggerInterface $logger
+     * @param OrderMutexInterface|null $orderMutex
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -106,7 +116,8 @@ public function __construct(
         CreditmemoDocumentFactory $creditmemoDocumentFactory,
         NotifierInterface $notifier,
         OrderConfig $config,
-        LoggerInterface $logger
+        LoggerInterface $logger,
+        ?OrderMutexInterface $orderMutex = null
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->orderStateResolver = $orderStateResolver;
@@ -119,6 +130,7 @@ public function __construct(
         $this->notifier = $notifier;
         $this->config = $config;
         $this->logger = $logger;
+        $this->orderMutex = $orderMutex ?: ObjectManager::getInstance()->get(OrderMutexInterface::class);
     }
 
     /**
@@ -133,7 +145,48 @@ public function execute(
         \Magento\Sales\Api\Data\CreditmemoCommentCreationInterface $comment = null,
         \Magento\Sales\Api\Data\CreditmemoCreationArgumentsInterface $arguments = null
     ) {
-        $connection = $this->resourceConnection->getConnection('sales');
+        $invoice = $this->invoiceRepository->get($invoiceId);
+        $order = $this->orderRepository->get($invoice->getOrderId());
+
+        return $this->orderMutex->execute(
+            (int) $order->getEntityId(),
+            \Closure::fromCallable([$this, 'processRefund']),
+            [
+                $invoiceId,
+                $items,
+                $isOnline,
+                $notify,
+                $appendComment,
+                $comment,
+                $arguments
+            ]
+        );
+    }
+
+    /**
+     * Refund process logic
+     *
+     * @param int $invoiceId
+     * @param array $items
+     * @param bool $isOnline
+     * @param bool $notify
+     * @param bool $appendComment
+     * @param CreditmemoCommentCreationInterface|null $comment
+     * @param CreditmemoCreationArgumentsInterface|null $arguments
+     * @return int|null
+     * @throws CouldNotRefundException
+     * @throws DocumentValidationException
+     * @SuppressWarnings(PHPMD.UnusedPrivateMethod) This method is used in closure callback
+     */
+    private function processRefund(
+        $invoiceId,
+        array $items = [],
+        bool $isOnline = false,
+        bool $notify = false,
+        bool $appendComment = false,
+        \Magento\Sales\Api\Data\CreditmemoCommentCreationInterface $comment = null,
+        \Magento\Sales\Api\Data\CreditmemoCreationArgumentsInterface $arguments = null
+    ) {
         $invoice = $this->invoiceRepository->get($invoiceId);
         $order = $this->orderRepository->get($invoice->getOrderId());
         $creditmemo = $this->creditmemoDocumentFactory->createFromInvoice(
@@ -160,7 +213,6 @@ public function execute(
                 __("Creditmemo Document Validation Error(s):\n" . implode("\n", $validationMessages->getMessages()))
             );
         }
-        $connection->beginTransaction();
         try {
             $creditmemo->setState(\Magento\Sales\Model\Order\Creditmemo::STATE_REFUNDED);
             $order->setCustomerNoteNotify($notify);
@@ -178,10 +230,8 @@ public function execute(
             $this->invoiceRepository->save($invoice);
             $order = $this->orderRepository->save($order);
             $creditmemo = $this->creditmemoRepository->save($creditmemo);
-            $connection->commit();
         } catch (\Exception $e) {
             $this->logger->critical($e);
-            $connection->rollBack();
             throw new \Magento\Sales\Exception\CouldNotRefundException(
                 __('Could not save a Creditmemo, see error log for details')
             );
diff --git a/app/code/Magento/Sales/Test/Unit/Model/RefundInvoiceTest.php b/app/code/Magento/Sales/Test/Unit/Model/RefundInvoiceTest.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\CreditmemoRepositoryInterface;
 use Magento\Sales\Api\Data\CreditmemoCommentCreationInterface;
 use Magento\Sales\Api\Data\CreditmemoCreationArgumentsInterface;
@@ -28,6 +29,7 @@
 use Magento\Sales\Model\Order\OrderStateResolverInterface;
 use Magento\Sales\Model\Order\RefundAdapterInterface;
 use Magento\Sales\Model\Order\Validation\RefundInvoiceInterface;
+use Magento\Sales\Model\OrderMutex;
 use Magento\Sales\Model\RefundInvoice;
 use Magento\Sales\Model\ValidatorResultInterface;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -222,7 +224,8 @@ protected function setUp(): void
             $this->creditmemoDocumentFactoryMock,
             $this->notifierMock,
             $this->configMock,
-            $this->loggerMock
+            $this->loggerMock,
+            new OrderMutex($this->resourceConnectionMock)
         );
     }
 
@@ -238,14 +241,17 @@ protected function setUp(): void
      */
     public function testOrderCreditmemo($invoiceId, $isOnline, $items, $notify, $appendComment)
     {
-        $this->resourceConnectionMock->expects($this->once())
-            ->method('getConnection')
-            ->with('sales')
-            ->willReturn($this->adapterInterface);
-        $this->invoiceRepositoryMock->expects($this->once())
+        $this->mockConnection($invoiceId);
+        $this->invoiceMock->expects($this->exactly(2))
+            ->method('getOrderId')
+            ->willReturn($invoiceId);
+        $this->orderMock->expects($this->once())
+            ->method('getEntityId')
+            ->willReturn($invoiceId);
+        $this->invoiceRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->invoiceMock);
-        $this->orderRepositoryMock->expects($this->once())
+        $this->orderRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->orderMock);
         $this->creditmemoDocumentFactoryMock->expects($this->once())
@@ -341,11 +347,17 @@ public function testDocumentValidationException()
         $appendComment = true;
         $isOnline = false;
         $errorMessages = ['error1', 'error2'];
-
-        $this->invoiceRepositoryMock->expects($this->once())
+        $this->mockConnection($invoiceId);
+        $this->invoiceMock->expects($this->exactly(2))
+            ->method('getOrderId')
+            ->willReturn($invoiceId);
+        $this->orderMock->expects($this->once())
+            ->method('getEntityId')
+            ->willReturn($invoiceId);
+        $this->invoiceRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->invoiceMock);
-        $this->orderRepositoryMock->expects($this->once())
+        $this->orderRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->orderMock);
 
@@ -401,15 +413,18 @@ public function testCouldNotCreditmemoException()
         $notify = true;
         $appendComment = true;
         $isOnline = false;
-        $this->resourceConnectionMock->expects($this->once())
-            ->method('getConnection')
-            ->with('sales')
-            ->willReturn($this->adapterInterface);
 
-        $this->invoiceRepositoryMock->expects($this->once())
+        $this->mockConnection($invoiceId);
+        $this->invoiceMock->expects($this->exactly(2))
+            ->method('getOrderId')
+            ->willReturn($invoiceId);
+        $this->orderMock->expects($this->once())
+            ->method('getEntityId')
+            ->willReturn($invoiceId);
+        $this->invoiceRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->invoiceMock);
-        $this->orderRepositoryMock->expects($this->once())
+        $this->orderRepositoryMock->expects($this->exactly(2))
             ->method('get')
             ->willReturn($this->orderMock);
 
@@ -479,4 +494,31 @@ public function dataProvider()
             'TestWithNotifyFalse' => [1, true,  [1 => $creditmemoItemCreationMock], false, true],
         ];
     }
+
+    private function mockConnection(int $orderId)
+    {
+        $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);
+    }
 }
