Skip to content

Commit d65d605

Browse files
committed
ACP2E-75: Duplicate SKU on same order
1 parent a993ccb commit d65d605

File tree

6 files changed

+225
-30
lines changed

6 files changed

+225
-30
lines changed
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Quote\Model;
8+
9+
use Magento\Framework\App\ResourceConnection;
10+
11+
/**
12+
* @inheritDoc
13+
*/
14+
class QuoteMutex implements QuoteMutexInterface
15+
{
16+
/**
17+
* @var ResourceConnection
18+
*/
19+
private $resourceConnection;
20+
21+
/**
22+
* @param ResourceConnection $resourceConnection
23+
*/
24+
public function __construct(
25+
ResourceConnection $resourceConnection
26+
) {
27+
$this->resourceConnection = $resourceConnection;
28+
}
29+
30+
/**
31+
* @inheritDoc
32+
*/
33+
public function execute(array $maskedIds, callable $callable, array $args = [])
34+
{
35+
if (empty($maskedIds)) {
36+
throw new \InvalidArgumentException('Quote masked ids must be provided');
37+
}
38+
39+
$connection = $this->resourceConnection->getConnection();
40+
$connection->beginTransaction();
41+
$query = $connection->select()
42+
->from($this->resourceConnection->getTableName('quote_id_mask'), 'entity_id')
43+
->where('masked_id IN (?)', $maskedIds)
44+
->forUpdate(true);
45+
$connection->query($query);
46+
47+
try {
48+
$result = $callable(...$args);
49+
$this->resourceConnection->getConnection()->commit();
50+
return $result;
51+
} catch (\Throwable $e) {
52+
$this->resourceConnection->getConnection()->rollBack();
53+
throw $e;
54+
}
55+
}
56+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Quote\Model;
8+
9+
/**
10+
* Intended to prevent race conditions during quote update by concurrent requests.
11+
*/
12+
interface QuoteMutexInterface
13+
{
14+
/**
15+
* Acquires a lock for quote, executes callable and releases the lock after.
16+
*
17+
* @param string[] $maskedIds
18+
* @param callable $callable
19+
* @param array $args
20+
* @return mixed
21+
*/
22+
public function execute(array $maskedIds, callable $callable, array $args = []);
23+
}

app/code/Magento/Quote/etc/di.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
<preference for="Magento\Quote\Api\Data\EstimateAddressInterface" type="Magento\Quote\Model\EstimateAddress" />
4545
<preference for="Magento\Quote\Api\Data\ProductOptionInterface" type="Magento\Quote\Model\Quote\ProductOption" />
4646
<preference for="Magento\Quote\Model\ValidationRules\QuoteValidationRuleInterface" type="Magento\Quote\Model\ValidationRules\QuoteValidationComposite\Proxy"/>
47+
<preference for="Magento\Quote\Model\QuoteMutexInterface" type="Magento\Quote\Model\QuoteMutex"/>
4748
<preference for="Magento\Quote\Model\Quote\Item\Option\ComparatorInterface" type="Magento\Quote\Model\Quote\Item\Option\Comparator"/>
4849
<type name="Magento\Webapi\Controller\Rest\ParamsOverrider">
4950
<arguments>

app/code/Magento/QuoteGraphQl/Model/Resolver/AddProductsToCart.php

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
1212
use Magento\Framework\GraphQl\Query\ResolverInterface;
1313
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
14-
use Magento\Framework\Lock\LockManagerInterface;
1514
use Magento\GraphQl\Model\Query\ContextInterface;
1615
use Magento\Quote\Model\Cart\AddProductsToCart as AddProductsToCartService;
1716
use Magento\Quote\Model\Cart\Data\AddProductsToCartOutput;
1817
use Magento\Quote\Model\Cart\Data\CartItemFactory;
18+
use Magento\Quote\Model\QuoteMutexInterface;
1919
use Magento\QuoteGraphQl\Model\Cart\GetCartForUser;
2020
use Magento\Quote\Model\Cart\Data\Error;
2121
use Magento\QuoteGraphQl\Model\CartItem\DataProvider\Processor\ItemDataProcessorInterface;
@@ -43,26 +43,26 @@ class AddProductsToCart implements ResolverInterface
4343
private $itemDataProcessor;
4444

4545
/**
46-
* @var LockManagerInterface
46+
* @var QuoteMutexInterface
4747
*/
48-
private $lockManager;
48+
private $quoteMutex;
4949

5050
/**
5151
* @param GetCartForUser $getCartForUser
5252
* @param AddProductsToCartService $addProductsToCart
5353
* @param ItemDataProcessorInterface $itemDataProcessor
54-
* @param LockManagerInterface $lockManager
54+
* @param QuoteMutexInterface $quoteMutex
5555
*/
5656
public function __construct(
5757
GetCartForUser $getCartForUser,
5858
AddProductsToCartService $addProductsToCart,
5959
ItemDataProcessorInterface $itemDataProcessor,
60-
LockManagerInterface $lockManager
60+
QuoteMutexInterface $quoteMutex
6161
) {
6262
$this->getCartForUser = $getCartForUser;
6363
$this->addProductsToCartService = $addProductsToCart;
6464
$this->itemDataProcessor = $itemDataProcessor;
65-
$this->lockManager = $lockManager;
65+
$this->quoteMutex = $quoteMutex;
6666
}
6767

6868
/**
@@ -78,19 +78,29 @@ public function resolve(Field $field, $context, ResolveInfo $info, array $value
7878
throw new GraphQlInputException(__('Required parameter "cartItems" is missing'));
7979
}
8080

81+
return $this->quoteMutex->execute(
82+
[$args['cartId']],
83+
\Closure::fromCallable([$this, 'run']),
84+
[$context, $args]
85+
);
86+
}
87+
88+
/**
89+
* Run the resolver.
90+
*
91+
* @param ContextInterface $context
92+
* @param array|null $args
93+
* @return array
94+
* @throws GraphQlInputException
95+
*/
96+
private function run($context, ?array $args): array
97+
{
8198
$maskedCartId = $args['cartId'];
8299
$cartItemsData = $args['cartItems'];
83100
$storeId = (int)$context->getExtensionAttributes()->getStore()->getId();
84-
$lockName = 'cart_processing_lock_' . $maskedCartId;
85-
while ($this->lockManager->isLocked($lockName)) {
86-
// wait till other process working with the same cart complete
87-
usleep(rand(100, 600));
88-
}
89-
$this->lockManager->lock($lockName, 1);
90101

91102
// Shopping Cart validation
92103
$this->getCartForUser->execute($maskedCartId, $context->getUserId(), $storeId);
93-
94104
$cartItems = [];
95105
foreach ($cartItemsData as $cartItemData) {
96106
if (!$this->itemIsAllowedToCart($cartItemData, $context)) {
@@ -101,7 +111,6 @@ public function resolve(Field $field, $context, ResolveInfo $info, array $value
101111

102112
/** @var AddProductsToCartOutput $addProductsToCartOutput */
103113
$addProductsToCartOutput = $this->addProductsToCartService->execute($maskedCartId, $cartItems);
104-
$this->lockManager->unlock($lockName);
105114

106115
return [
107116
'cart' => [

app/code/Magento/QuoteGraphQl/Model/Resolver/AddSimpleProductsToCart.php

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@
99

1010
use Magento\Framework\GraphQl\Config\Element\Field;
1111
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
12+
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
1213
use Magento\Framework\GraphQl\Query\ResolverInterface;
1314
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
15+
use Magento\Quote\Model\QuoteMutexInterface;
1416
use Magento\QuoteGraphQl\Model\Cart\AddProductsToCart;
1517
use Magento\QuoteGraphQl\Model\Cart\GetCartForUser;
16-
use Magento\Framework\Lock\LockManagerInterface;
1718

1819
/**
1920
* Add simple products to cart GraphQl resolver
@@ -32,23 +33,23 @@ class AddSimpleProductsToCart implements ResolverInterface
3233
private $addProductsToCart;
3334

3435
/**
35-
* @var LockManagerInterface
36+
* @var QuoteMutexInterface
3637
*/
37-
private $lockManager;
38+
private $quoteMutex;
3839

3940
/**
4041
* @param GetCartForUser $getCartForUser
4142
* @param AddProductsToCart $addProductsToCart
42-
* @param LockManagerInterface $lockManager
43+
* @param QuoteMutexInterface $quoteMutex
4344
*/
4445
public function __construct(
4546
GetCartForUser $getCartForUser,
4647
AddProductsToCart $addProductsToCart,
47-
LockManagerInterface $lockManager
48+
QuoteMutexInterface $quoteMutex
4849
) {
4950
$this->getCartForUser = $getCartForUser;
5051
$this->addProductsToCart = $addProductsToCart;
51-
$this->lockManager = $lockManager;
52+
$this->quoteMutex = $quoteMutex;
5253
}
5354

5455
/**
@@ -59,28 +60,37 @@ public function resolve(Field $field, $context, ResolveInfo $info, array $value
5960
if (empty($args['input']['cart_id'])) {
6061
throw new GraphQlInputException(__('Required parameter "cart_id" is missing'));
6162
}
62-
$maskedCartId = $args['input']['cart_id'];
6363

6464
if (empty($args['input']['cart_items'])
6565
|| !is_array($args['input']['cart_items'])
6666
) {
6767
throw new GraphQlInputException(__('Required parameter "cart_items" is missing'));
6868
}
69-
$cartItems = $args['input']['cart_items'];
70-
$storeId = (int)$context->getExtensionAttributes()->getStore()->getId();
7169

72-
$lockName = 'cart_processing_lock_' . $maskedCartId;
73-
while ($this->lockManager->isLocked($lockName)) {
74-
// wait till other process working with the same cart complete
75-
usleep(rand(100, 600));
76-
}
77-
$this->lockManager->lock($lockName, 1);
70+
return $this->quoteMutex->execute(
71+
[$args['input']['cart_id']],
72+
\Closure::fromCallable([$this, 'run']),
73+
[$context, $args]
74+
);
75+
}
7876

77+
/**
78+
* Run the resolver.
79+
*
80+
* @param ContextInterface $context
81+
* @param array|null $args
82+
* @return array[]
83+
* @throws GraphQlInputException
84+
*/
85+
private function run($context, ?array $args): array
86+
{
87+
$maskedCartId = $args['input']['cart_id'];
88+
$cartItems = $args['input']['cart_items'];
89+
$storeId = (int)$context->getExtensionAttributes()->getStore()->getId();
7990
$cart = $this->getCartForUser->execute($maskedCartId, $context->getUserId(), $storeId);
8091
$this->addProductsToCart->execute($cart, $cartItems);
8192
$cart = $this->getCartForUser->execute($maskedCartId, $context->getUserId(), $storeId);
8293

83-
$this->lockManager->unlock($lockName);
8494
return [
8595
'cart' => [
8696
'model' => $cart,
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Quote\Model;
9+
10+
use Magento\Quote\Api\GuestCartManagementInterface;
11+
use Magento\TestFramework\Helper\Bootstrap as BootstrapHelper;
12+
13+
class QuoteMutexTest extends \PHPUnit\Framework\TestCase
14+
{
15+
/**
16+
* @var GuestCartManagementInterface
17+
*/
18+
private $guestCartManagement;
19+
20+
/**
21+
* @var QuoteMutexInterface
22+
*/
23+
private $quoteMutex;
24+
25+
protected function setUp(): void
26+
{
27+
$objectManager = BootstrapHelper::getObjectManager();
28+
$this->quoteMutex = $objectManager->create(QuoteMutexInterface::class);
29+
$this->guestCartManagement = $objectManager->create(GuestCartManagementInterface::class);
30+
}
31+
32+
/**
33+
* Tests quote mutex execution with different callables.
34+
*
35+
* @param callable $callable
36+
* @param array $args
37+
* @param mixed $expectedResult
38+
* @return void
39+
* @dataProvider callableDataProvider
40+
*/
41+
public function testSuccessfulExecution(callable $callable, array $args, $expectedResult): void
42+
{
43+
$maskedQuoteId = $this->guestCartManagement->createEmptyCart();
44+
$result = $this->quoteMutex->execute([$maskedQuoteId], $callable, $args);
45+
46+
$this->assertEquals($expectedResult, $result);
47+
}
48+
49+
/**
50+
* @return array[]
51+
*/
52+
public function callableDataProvider(): array
53+
{
54+
$functionWithArgs = function (int $a, int $b) {
55+
return $a + $b;
56+
};
57+
58+
$functionWithoutArgs = function () {
59+
return 'Function without args';
60+
};
61+
62+
return [
63+
['callable' => $functionWithoutArgs, 'args' => [], 'expectedResult' => 'Function without args'],
64+
['callable' => $functionWithArgs, 'args' => [1,2], 'expectedResult' => 3],
65+
[
66+
'callable' => \Closure::fromCallable([$this, 'privateMethod']),
67+
'args' => ['test'],
68+
'expectedResult' => 'test'
69+
],
70+
];
71+
}
72+
73+
/**
74+
* Private method for data provider.
75+
*
76+
* @param string $var
77+
* @return string
78+
*/
79+
private function privateMethod(string $var)
80+
{
81+
return $var;
82+
}
83+
84+
/**
85+
* Tests exception when empty maskIds array has been provided.
86+
*
87+
* @return void
88+
*/
89+
public function testWithEmptyMaskIdsArgument(): void
90+
{
91+
$this->expectException(\InvalidArgumentException::class);
92+
$callable = function () {
93+
};
94+
$this->quoteMutex->execute([], $callable);
95+
}
96+
}

0 commit comments

Comments
 (0)