Skip to content

Commit 8271fc0

Browse files
nishant04412nishant04412
committed
AC-6057: Duplicate orders with same QuoteId issue fixes
1 parent 5dc13c3 commit 8271fc0

File tree

2 files changed

+153
-17
lines changed

2 files changed

+153
-17
lines changed

‎app/code/Magento/Quote/Model/QuoteManagement.php‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Magento\Framework\Exception\NoSuchEntityException;
2525
use Magento\Framework\Exception\StateException;
2626
use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
27+
use Magento\Framework\Lock\LockManagerInterface;
2728
use Magento\Framework\Model\AbstractExtensibleModel;
2829
use Magento\Framework\Validator\Exception as ValidatorException;
2930
use Magento\Payment\Model\Method\AbstractMethod;
@@ -32,8 +33,8 @@
3233
use Magento\Quote\Api\Data\PaymentInterface;
3334
use Magento\Quote\Model\Quote\Address\ToOrder as ToOrderConverter;
3435
use Magento\Quote\Model\Quote\Address\ToOrderAddress as ToOrderAddressConverter;
35-
use Magento\Quote\Model\Quote as QuoteEntity;
3636
use Magento\Quote\Model\Quote\AddressFactory;
37+
use Magento\Quote\Model\Quote as QuoteEntity;
3738
use Magento\Quote\Model\Quote\Item\ToOrderItem as ToOrderItemConverter;
3839
use Magento\Quote\Model\Quote\Payment\ToOrderPayment as ToOrderPaymentConverter;
3940
use Magento\Quote\Model\ResourceModel\Quote\Item;
@@ -51,6 +52,10 @@
5152
*/
5253
class QuoteManagement implements CartManagementInterface
5354
{
55+
private const LOCK_PREFIX = 'PLACE_ORDER_';
56+
57+
private const LOCK_TIMEOUT = 10;
58+
5459
/**
5560
* @var EventManager
5661
*/
@@ -151,6 +156,11 @@ class QuoteManagement implements CartManagementInterface
151156
*/
152157
protected $quoteFactory;
153158

159+
/**
160+
* @var LockManagerInterface
161+
*/
162+
private $lockManager;
163+
154164
/**
155165
* @var QuoteIdMaskFactory
156166
*/
@@ -201,6 +211,7 @@ class QuoteManagement implements CartManagementInterface
201211
* @param AddressRepositoryInterface|null $addressRepository
202212
* @param RequestInterface|null $request
203213
* @param RemoteAddress $remoteAddress
214+
* @param LockManagerInterface $lockManager
204215
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
205216
*/
206217
public function __construct(
@@ -227,7 +238,8 @@ public function __construct(
227238
QuoteIdMaskFactory $quoteIdMaskFactory = null,
228239
AddressRepositoryInterface $addressRepository = null,
229240
RequestInterface $request = null,
230-
RemoteAddress $remoteAddress = null
241+
RemoteAddress $remoteAddress = null,
242+
LockManagerInterface $lockManager = null
231243
) {
232244
$this->eventManager = $eventManager;
233245
$this->submitQuoteValidator = $submitQuoteValidator;
@@ -257,6 +269,8 @@ public function __construct(
257269
->get(RequestInterface::class);
258270
$this->remoteAddress = $remoteAddress ?: ObjectManager::getInstance()
259271
->get(RemoteAddress::class);
272+
$this->lockManager = $lockManager ?: ObjectManager::getInstance()
273+
->get(LockManagerInterface::class);
260274
}
261275

262276
/**
@@ -324,7 +338,7 @@ public function assignCustomer($cartId, $customerId, $storeId)
324338
$customerActiveQuote->setIsActive(0);
325339
$this->quoteRepository->save($customerActiveQuote);
326340

327-
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
341+
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
328342
} catch (NoSuchEntityException $e) {
329343
}
330344

@@ -584,25 +598,29 @@ protected function submitQuote(QuoteEntity $quote, $orderData = [])
584598
$order->setCustomerFirstname($quote->getCustomerFirstname());
585599
$order->setCustomerMiddlename($quote->getCustomerMiddlename());
586600
$order->setCustomerLastname($quote->getCustomerLastname());
587-
588601
if ($quote->getOrigOrderId()) {
589602
$order->setEntityId($quote->getOrigOrderId());
590603
}
591-
592604
if ($quote->getReservedOrderId()) {
593605
$order->setIncrementId($quote->getReservedOrderId());
594606
}
595-
596607
$this->submitQuoteValidator->validateOrder($order);
597-
598608
$this->eventManager->dispatch(
599609
'sales_model_service_quote_submit_before',
600610
[
601611
'order' => $order,
602612
'quote' => $quote
603613
]
604614
);
615+
616+
$lockedName = self::LOCK_PREFIX . $quote->getId();
617+
if ($this->lockManager->isLocked($lockedName)) {
618+
throw new LocalizedException(__(
619+
'A server error stopped your order from being placed. Please try to place your order again.'
620+
));
621+
}
605622
try {
623+
$this->lockManager->lock($lockedName, self::LOCK_TIMEOUT);
606624
$order = $this->orderManagement->place($order);
607625
$quote->setIsActive(false);
608626
$this->eventManager->dispatch(
@@ -613,6 +631,7 @@ protected function submitQuote(QuoteEntity $quote, $orderData = [])
613631
]
614632
);
615633
$this->quoteRepository->save($quote);
634+
$this->lockManager->unlock($lockedName);
616635
} catch (\Exception $e) {
617636
$this->rollbackAddresses($quote, $order, $e);
618637
throw $e;
@@ -648,7 +667,7 @@ protected function _prepareCustomerQuote($quote)
648667
if ($defaultShipping) {
649668
try {
650669
$shippingAddress = $this->addressRepository->getById($defaultShipping);
651-
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
670+
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
652671
} catch (LocalizedException $e) {
653672
// no address
654673
}
@@ -682,7 +701,7 @@ protected function _prepareCustomerQuote($quote)
682701
if ($defaultBilling) {
683702
try {
684703
$billingAddress = $this->addressRepository->getById($defaultBilling);
685-
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
704+
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
686705
} catch (LocalizedException $e) {
687706
// no address
688707
}

‎app/code/Magento/Quote/Test/Unit/Model/QuoteManagementTest.php‎

Lines changed: 125 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Magento\Framework\Exception\NoSuchEntityException;
2626
use Magento\Framework\Exception\StateException;
2727
use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
28+
use Magento\Framework\Lock\LockManagerInterface;
2829
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
2930
use Magento\Quote\Api\CartRepositoryInterface;
3031
use Magento\Quote\Model\CustomerManagement;
@@ -292,6 +293,9 @@ protected function setUp(): void
292293
$this->addressRepositoryMock = $this->getMockBuilder(AddressRepositoryInterface::class)
293294
->getMockForAbstractClass();
294295

296+
$this->lockManagerMock = $this->getMockBuilder(LockManagerInterface::class)
297+
->getMockForAbstractClass();
298+
295299
$this->model = $objectManager->getObject(
296300
QuoteManagement::class,
297301
[
@@ -315,7 +319,8 @@ protected function setUp(): void
315319
'customerSession' => $this->customerSessionMock,
316320
'accountManagement' => $this->accountManagementMock,
317321
'quoteFactory' => $this->quoteFactoryMock,
318-
'addressRepository' => $this->addressRepositoryMock
322+
'addressRepository' => $this->addressRepositoryMock,
323+
'lockManager' => $this->lockManagerMock
319324
]
320325
);
321326

@@ -761,7 +766,8 @@ public function testSubmit(): void
761766
$customerId,
762767
$quoteId,
763768
$quoteItems,
764-
$shippingAddress
769+
$shippingAddress,
770+
false
765771
);
766772

767773
$this->submitQuoteValidator->expects($this->once())
@@ -826,6 +832,7 @@ public function testSubmit(): void
826832
['order' => $order, 'quote' => $quote]
827833
]
828834
);
835+
$this->lockManagerMock->method('isLocked')->willReturn(false);
829836
$this->quoteRepositoryMock->expects($this->once())->method('save')->with($quote);
830837
$this->assertEquals($order, $this->model->submit($quote, $orderData));
831838
}
@@ -1063,7 +1070,8 @@ protected function getQuote(
10631070
int $customerId,
10641071
int $id,
10651072
array $quoteItems,
1066-
Address $shippingAddress = null
1073+
Address $shippingAddress = null,
1074+
bool $setIsActive
10671075
): MockObject {
10681076
$quote = $this->getMockBuilder(Quote::class)
10691077
->addMethods(['getCustomerEmail', 'getCustomerId'])
@@ -1085,9 +1093,11 @@ protected function getQuote(
10851093
)
10861094
->disableOriginalConstructor()
10871095
->getMock();
1088-
$quote->expects($this->once())
1089-
->method('setIsActive')
1090-
->with(false);
1096+
if ($setIsActive) {
1097+
$quote->expects($this->once())
1098+
->method('setIsActive')
1099+
->with(false);
1100+
}
10911101
$quote->expects($this->any())
10921102
->method('getAllVisibleItems')
10931103
->willReturn($quoteItems);
@@ -1129,7 +1139,7 @@ protected function getQuote(
11291139
$quote->expects($this->any())
11301140
->method('getCustomer')
11311141
->willReturn($customer);
1132-
$quote->expects($this->once())
1142+
$quote->expects($this->exactly(2))
11331143
->method('getId')
11341144
->willReturn($id);
11351145
$this->customerRepositoryMock->expects($this->any())->method('getById')->willReturn($customer);
@@ -1292,7 +1302,8 @@ public function testSubmitForCustomer(): void
12921302
$customerId,
12931303
$quoteId,
12941304
$quoteItems,
1295-
$shippingAddress
1305+
$shippingAddress,
1306+
false
12961307
);
12971308

12981309
$this->submitQuoteValidator->method('validateQuote')
@@ -1386,4 +1397,110 @@ private function createPartialMockForAbstractClass(string $className, array $met
13861397
$methods
13871398
);
13881399
}
1400+
1401+
/**
1402+
* @return void
1403+
*
1404+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
1405+
*/
1406+
public function testSubmitWithLockException(): void
1407+
{
1408+
$orderData = [];
1409+
$isGuest = true;
1410+
$isVirtual = false;
1411+
$customerId = 1;
1412+
$quoteId = 1;
1413+
$quoteItem = $this->createMock(Item::class);
1414+
$billingAddress = $this->createMock(Address::class);
1415+
$shippingAddress = $this->getMockBuilder(Address::class)
1416+
->addMethods(['getQuoteId'])
1417+
->onlyMethods(['getShippingMethod', 'getId'])
1418+
->disableOriginalConstructor()
1419+
->getMock();
1420+
$payment = $this->createMock(Payment::class);
1421+
$baseOrder = $this->getMockForAbstractClass(OrderInterface::class);
1422+
$convertedBilling = $this->createPartialMockForAbstractClass(OrderAddressInterface::class, ['setData']);
1423+
$convertedShipping = $this->createPartialMockForAbstractClass(OrderAddressInterface::class, ['setData']);
1424+
$convertedPayment = $this->getMockForAbstractClass(OrderPaymentInterface::class);
1425+
$convertedQuoteItem = $this->getMockForAbstractClass(OrderItemInterface::class);
1426+
$addresses = [$convertedShipping, $convertedBilling];
1427+
$quoteItems = [$quoteItem];
1428+
$convertedItems = [$convertedQuoteItem];
1429+
$quote = $this->getQuote(
1430+
$isGuest,
1431+
$isVirtual,
1432+
$billingAddress,
1433+
$payment,
1434+
$customerId,
1435+
$quoteId,
1436+
$quoteItems,
1437+
$shippingAddress,
1438+
false
1439+
);
1440+
1441+
$this->submitQuoteValidator->expects($this->once())
1442+
->method('validateQuote')
1443+
->with($quote);
1444+
$this->quoteAddressToOrder->expects($this->once())
1445+
->method('convert')
1446+
->with($shippingAddress, $orderData)
1447+
->willReturn($baseOrder);
1448+
$this->quoteAddressToOrderAddress
1449+
->method('convert')
1450+
->withConsecutive(
1451+
[
1452+
$shippingAddress,
1453+
[
1454+
'address_type' => 'shipping',
1455+
'email' => 'customer@example.com'
1456+
]
1457+
],
1458+
[
1459+
$billingAddress,
1460+
[
1461+
'address_type' => 'billing',
1462+
'email' => 'customer@example.com'
1463+
]
1464+
]
1465+
)->willReturnOnConsecutiveCalls($convertedShipping, $convertedBilling);
1466+
$billingAddress->expects($this->once())->method('getId')->willReturn(4);
1467+
$convertedBilling->expects($this->once())->method('setData')->with('quote_address_id', 4);
1468+
1469+
$this->quoteItemToOrderItem->expects($this->once())->method('convert')
1470+
->with($quoteItem, ['parent_item' => null])
1471+
->willReturn($convertedQuoteItem);
1472+
$this->quotePaymentToOrderPayment->expects($this->once())->method('convert')->with($payment)
1473+
->willReturn($convertedPayment);
1474+
$shippingAddress->expects($this->once())->method('getShippingMethod')->willReturn('free');
1475+
$shippingAddress->expects($this->once())->method('getId')->willReturn(5);
1476+
$convertedShipping->expects($this->once())->method('setData')->with('quote_address_id', 5);
1477+
$order = $this->prepareOrderFactory(
1478+
$baseOrder,
1479+
$convertedBilling,
1480+
$addresses,
1481+
$convertedPayment,
1482+
$convertedItems,
1483+
$quoteId,
1484+
$convertedShipping
1485+
);
1486+
1487+
$this->eventManager
1488+
->method('dispatch')
1489+
->withConsecutive(
1490+
[
1491+
'sales_model_service_quote_submit_before',
1492+
['order' => $order, 'quote' => $quote]
1493+
],
1494+
[
1495+
'sales_model_service_quote_submit_success',
1496+
['order' => $order, 'quote' => $quote]
1497+
]
1498+
);
1499+
$this->lockManagerMock->method('isLocked')->willReturn(true);
1500+
1501+
$this->expectExceptionMessage(
1502+
'A server error stopped your order from being placed. Please try to place your order again.'
1503+
);
1504+
$this->assertEquals($order, $this->model->submit($quote, $orderData));
1505+
}
13891506
}

0 commit comments

Comments
 (0)