Skip to content

Commit b251932

Browse files
author
Anna Bukatar
committed
ACP2E-1194: Duplicate orders with 2 parallel GraphQL requests
1 parent 65c71f2 commit b251932

File tree

5 files changed

+230
-1
lines changed

5 files changed

+230
-1
lines changed
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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\QuoteGraphQl\Model\Cart;
9+
10+
use Magento\Framework\Exception\LocalizedException;
11+
use Magento\Framework\GraphQl\Exception\GraphQlAlreadyExistsException;
12+
use Magento\Framework\Lock\LockManagerInterface;
13+
14+
/**
15+
* @inheritdoc
16+
*/
17+
class PlaceOrderMutex implements PlaceOrderMutexInterface
18+
{
19+
private const LOCK_PREFIX = 'quote_lock_';
20+
21+
private const LOCK_TIMEOUT = 10;
22+
23+
/**
24+
* @var LockManagerInterface
25+
*/
26+
private $lockManager;
27+
28+
/**
29+
* @var int
30+
*/
31+
private $lockWaitTimeout;
32+
33+
/**
34+
* @param LockManagerInterface $lockManager
35+
* @param int $lockWaitTimeout
36+
*/
37+
public function __construct(
38+
LockManagerInterface $lockManager,
39+
int $lockWaitTimeout = self::LOCK_TIMEOUT
40+
) {
41+
$this->lockManager = $lockManager;
42+
$this->lockWaitTimeout = $lockWaitTimeout;
43+
}
44+
45+
/**
46+
* @inheritDoc
47+
*/
48+
public function execute(string $maskedId, callable $callable, array $args = [])
49+
{
50+
if (empty($maskedId)) {
51+
throw new \InvalidArgumentException('Quote masked id must be provided');
52+
}
53+
54+
if ($this->lockManager->isLocked(self::LOCK_PREFIX . $maskedId)) {
55+
throw new GraphQlAlreadyExistsException(
56+
__('The order has already been placed and is currently processing.')
57+
);
58+
}
59+
60+
if ($this->lockManager->lock(self::LOCK_PREFIX . $maskedId, $this->lockWaitTimeout)) {
61+
try {
62+
return $callable(...$args);
63+
} finally {
64+
$this->lockManager->unlock(self::LOCK_PREFIX . $maskedId);
65+
}
66+
} else {
67+
throw new LocalizedException(
68+
__('Could not acquire lock for the quote id: %1', $maskedId)
69+
);
70+
}
71+
}
72+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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\QuoteGraphQl\Model\Cart;
9+
10+
use Magento\Framework\Exception\LocalizedException;
11+
12+
/**
13+
* Intended to prevent race conditions during order place operation by concurrent requests.
14+
*/
15+
interface PlaceOrderMutexInterface
16+
{
17+
/**
18+
* Acquires a lock for quote, executes callable and releases the lock after.
19+
*
20+
* @param string $maskedId
21+
* @param callable $callable
22+
* @param array $args
23+
* @return mixed
24+
* @throws LocalizedException
25+
*/
26+
public function execute(string $maskedId, callable $callable, array $args = []);
27+
}

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@
77

88
namespace Magento\QuoteGraphQl\Model\Resolver;
99

10+
use Magento\Framework\App\ObjectManager;
1011
use Magento\Framework\Exception\LocalizedException;
1112
use Magento\Framework\GraphQl\Config\Element\Field;
1213
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
1314
use Magento\Framework\GraphQl\Query\ResolverInterface;
1415
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
1516
use Magento\GraphQl\Helper\Error\AggregateExceptionMessageFormatter;
17+
use Magento\GraphQl\Model\Query\ContextInterface;
1618
use Magento\QuoteGraphQl\Model\Cart\GetCartForUser;
1719
use Magento\QuoteGraphQl\Model\Cart\PlaceOrder as PlaceOrderModel;
20+
use Magento\QuoteGraphQl\Model\Cart\PlaceOrderMutexInterface;
1821
use Magento\Sales\Api\OrderRepositoryInterface;
1922

2023
/**
@@ -42,22 +45,30 @@ class PlaceOrder implements ResolverInterface
4245
*/
4346
private $errorMessageFormatter;
4447

48+
/**
49+
* @var PlaceOrderMutexInterface
50+
*/
51+
private $placeOrderMutex;
52+
4553
/**
4654
* @param GetCartForUser $getCartForUser
4755
* @param PlaceOrderModel $placeOrder
4856
* @param OrderRepositoryInterface $orderRepository
4957
* @param AggregateExceptionMessageFormatter $errorMessageFormatter
58+
* @param PlaceOrderMutexInterface|null $placeOrderMutex
5059
*/
5160
public function __construct(
5261
GetCartForUser $getCartForUser,
5362
PlaceOrderModel $placeOrder,
5463
OrderRepositoryInterface $orderRepository,
55-
AggregateExceptionMessageFormatter $errorMessageFormatter
64+
AggregateExceptionMessageFormatter $errorMessageFormatter,
65+
?PlaceOrderMutexInterface $placeOrderMutex = null
5666
) {
5767
$this->getCartForUser = $getCartForUser;
5868
$this->placeOrder = $placeOrder;
5969
$this->orderRepository = $orderRepository;
6070
$this->errorMessageFormatter = $errorMessageFormatter;
71+
$this->placeOrderMutex = $placeOrderMutex ?: ObjectManager::getInstance()->get(PlaceOrderMutexInterface::class);
6172
}
6273

6374
/**
@@ -68,6 +79,26 @@ public function resolve(Field $field, $context, ResolveInfo $info, array $value
6879
if (empty($args['input']['cart_id'])) {
6980
throw new GraphQlInputException(__('Required parameter "cart_id" is missing'));
7081
}
82+
83+
return $this->placeOrderMutex->execute(
84+
$args['input']['cart_id'],
85+
\Closure::fromCallable([$this, 'run']),
86+
[$field, $context, $info, $args]
87+
);
88+
}
89+
90+
/**
91+
* Run the resolver.
92+
*
93+
* @param Field $field
94+
* @param ContextInterface $context
95+
* @param ResolveInfo $info
96+
* @param array|null $args
97+
* @return array[]
98+
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
99+
*/
100+
private function run(Field $field, ContextInterface $context, ResolveInfo $info, ?array $args): array
101+
{
71102
$maskedCartId = $args['input']['cart_id'];
72103
$userId = (int)$context->getUserId();
73104
$storeId = (int)$context->getExtensionAttributes()->getStore()->getId();

app/code/Magento/QuoteGraphQl/etc/graphql/di.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
type="Magento\QuoteGraphQl\Model\Cart\SetShippingMethodsOnCart"/>
1313
<preference for="Magento\QuoteGraphQl\Model\Cart\MergeCarts\CartQuantityValidatorInterface"
1414
type="Magento\QuoteGraphQl\Model\Cart\MergeCarts\CartQuantityValidator"/>
15+
<preference for="Magento\QuoteGraphQl\Model\Cart\PlaceOrderMutexInterface"
16+
type="Magento\QuoteGraphQl\Model\Cart\PlaceOrderMutex" />
1517
<type name="Magento\QuoteGraphQl\Model\Cart\BuyRequest\BuyRequestBuilder">
1618
<arguments>
1719
<argument name="providers" xsi:type="array">
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
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\QuoteGraphQl\Model\Cart;
9+
10+
use Magento\Quote\Api\GuestCartManagementInterface;
11+
use Magento\TestFramework\Helper\Bootstrap as BootstrapHelper;
12+
13+
class PlaceOrderMutexTest extends \PHPUnit\Framework\TestCase
14+
{
15+
/**
16+
* @var GuestCartManagementInterface
17+
*/
18+
private $guestCartManagement;
19+
20+
/**
21+
* @var PlaceOrderMutexInterface
22+
*/
23+
private $placeOrderMutex;
24+
25+
protected function setUp(): void
26+
{
27+
$objectManager = BootstrapHelper::getObjectManager();
28+
$this->placeOrderMutex = $objectManager->create(PlaceOrderMutexInterface::class);
29+
$this->guestCartManagement = $objectManager->create(GuestCartManagementInterface::class);
30+
}
31+
32+
/**
33+
* Tests place order 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->placeOrderMutex->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+
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
79+
*/
80+
private function privateMethod(string $var): string
81+
{
82+
return $var;
83+
}
84+
85+
/**
86+
* Tests exception when empty maskIds array has been provided.
87+
*
88+
* @return void
89+
*/
90+
public function testWithEmptyMaskIdsArgument(): void
91+
{
92+
$this->expectException(\InvalidArgumentException::class);
93+
$callable = function () {
94+
};
95+
$this->placeOrderMutex->execute('', $callable);
96+
}
97+
}

0 commit comments

Comments
 (0)