Skip to content

Commit c604b15

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1666: Fix race conditions in _resetState methods
* Fixing static test failures * Removed FIXME lines from state-skip-list.php
1 parent 3a9161c commit c604b15

File tree

22 files changed

+98
-214
lines changed

22 files changed

+98
-214
lines changed

‎app/code/Magento/Quote/Model/Quote/Address/Total/AbstractTotal.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ protected function _setAddress(\Magento\Quote\Model\Quote\Address $address)
151151
/**
152152
* Get quote address object
153153
*
154-
* @return \Magento\Quote\Model\Quote\Address
155-
* @throws \Magento\Framework\Exception\LocalizedException if address not declared
154+
* @return \Magento\Quote\Model\Quote\Address
155+
* @throws \Magento\Framework\Exception\LocalizedException if address not declared
156156
*/
157157
protected function _getAddress()
158158
{
@@ -163,6 +163,8 @@ protected function _getAddress()
163163
}
164164

165165
/**
166+
* Sets the total
167+
*
166168
* @param \Magento\Quote\Model\Quote\Address\Total $total
167169
* @return $this
168170
*/
@@ -173,6 +175,8 @@ public function _setTotal(\Magento\Quote\Model\Quote\Address\Total $total)
173175
}
174176

175177
/**
178+
* Gets the total
179+
*
176180
* @return \Magento\Quote\Model\Quote\Address\Total
177181
*/
178182
protected function _getTotal()
@@ -183,7 +187,7 @@ protected function _getTotal()
183187
/**
184188
* Set total model amount value to address
185189
*
186-
* @param float $amount
190+
* @param float $amount
187191
* @return $this
188192
*/
189193
protected function _setAmount($amount)
@@ -293,6 +297,7 @@ public function getIsItemRowTotalCompoundable(\Magento\Quote\Model\Quote\Item\Ab
293297

294298
/**
295299
* Process model configuration array.
300+
*
296301
* This method can be used for changing models apply sort order
297302
*
298303
* @param array $config

‎app/code/Magento/Tax/Model/Sales/Total/Quote/CommonTaxCollector.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,15 @@ public function __construct(
193193
ObjectManager::getInstance()->get(CustomerAccountManagement::class);
194194
}
195195

196+
/**
197+
* @inheritDoc
198+
*/
199+
public function _resetState(): void
200+
{
201+
parent::_resetState();
202+
$this->counter = 0;
203+
}
204+
196205
/**
197206
* Map quote address to customer address
198207
*

‎app/code/Magento/Tax/Model/Sales/Total/Quote/Tax.php

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,7 @@
2020
*/
2121
class Tax extends CommonTaxCollector
2222
{
23-
/**
24-
* Counter
25-
*
26-
* @var int
27-
*/
23+
/** @var int */
2824
protected $counter = 0;
2925

3026
/**
@@ -320,13 +316,12 @@ protected function processExtraTaxables(Address\Total $total, array $itemsByType
320316
*
321317
* @param \Magento\Quote\Model\Quote $quote
322318
* @param Address\Total $total
323-
* @return array|null
319+
* @return array
324320
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
325321
* @SuppressWarnings(PHPMD.NPathComplexity)
326322
*/
327323
public function fetch(\Magento\Quote\Model\Quote $quote, \Magento\Quote\Model\Quote\Address\Total $total)
328324
{
329-
$totals = [];
330325
$store = $quote->getStore();
331326
$applied = $total->getAppliedTaxes();
332327
if (is_string($applied)) {
@@ -338,20 +333,17 @@ public function fetch(\Magento\Quote\Model\Quote $quote, \Magento\Quote\Model\Qu
338333
$amount = $total->getTaxAmount();
339334
}
340335
$taxAmount = $amount + $total->getTotalAmount('discount_tax_compensation');
341-
342336
$area = null;
343337
if ($this->_config->displayCartTaxWithGrandTotal($store) && $total->getGrandTotal()) {
344338
$area = 'taxes';
345339
}
346-
347-
$totals[] = [
340+
$totals = [[
348341
'code' => $this->getCode(),
349342
'title' => __('Tax'),
350343
'full_info' => $applied ? $applied : [],
351344
'value' => $amount,
352345
'area' => $area,
353-
];
354-
346+
]];
355347
/**
356348
* Modify subtotal
357349
*/
@@ -361,7 +353,6 @@ public function fetch(\Magento\Quote\Model\Quote $quote, \Magento\Quote\Model\Qu
361353
} else {
362354
$subtotalInclTax = $total->getSubtotal() + $taxAmount - $total->getShippingTaxAmount();
363355
}
364-
365356
$totals[] = [
366357
'code' => 'subtotal',
367358
'title' => __('Subtotal'),
@@ -370,10 +361,6 @@ public function fetch(\Magento\Quote\Model\Quote $quote, \Magento\Quote\Model\Qu
370361
'value_excl_tax' => $total->getSubtotal(),
371362
];
372363
}
373-
374-
if (empty($totals)) {
375-
return null;
376-
}
377364
return $totals;
378365
}
379366

@@ -382,7 +369,7 @@ public function fetch(\Magento\Quote\Model\Quote $quote, \Magento\Quote\Model\Qu
382369
*
383370
* @param \Magento\Quote\Model\Quote $quote
384371
* @param Address\Total $total
385-
* @return null
372+
* @return void
386373
*/
387374
protected function enhanceTotalData(
388375
\Magento\Quote\Model\Quote $quote,
@@ -391,13 +378,11 @@ protected function enhanceTotalData(
391378
$taxAmount = 0;
392379
$shippingTaxAmount = 0;
393380
$discountTaxCompensation = 0;
394-
395381
$subtotalInclTax = $total->getSubtotalInclTax();
396382
$computeSubtotalInclTax = true;
397383
if ($total->getSubtotalInclTax() > 0) {
398384
$computeSubtotalInclTax = false;
399385
}
400-
401386
/** @var \Magento\Quote\Model\Quote\Address $address */
402387
foreach ($quote->getAllAddresses() as $address) {
403388
$taxAmount += $address->getTaxAmount();
@@ -407,7 +392,6 @@ protected function enhanceTotalData(
407392
$subtotalInclTax += $address->getSubtotalInclTax();
408393
}
409394
}
410-
411395
$total->setTaxAmount($taxAmount);
412396
$total->setShippingTaxAmount($shippingTaxAmount);
413397
$total->setDiscountTaxCompensationAmount($discountTaxCompensation); // accessed via 'discount_tax_compensation'

‎app/code/Magento/Theme/Model/Theme/ThemeProvider.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ private function getThemeList()
186186
return $this->themeList;
187187
}
188188

189+
/**
190+
* @inheritDoc
191+
*/
189192
public function _resetState(): void
190193
{
191194
$this->themeList = null;

‎app/code/Magento/Ui/Config/Converter.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,37 @@ class Converter implements ConfigConverterInterface, ResetAfterRequestInterface
1919
/**
2020
* The key attributes of a node
2121
*/
22-
const DATA_ATTRIBUTES_KEY = 'attributes';
22+
public const DATA_ATTRIBUTES_KEY = 'attributes';
2323

2424
/**
2525
* The key for the data arguments
2626
*/
27-
const DATA_ARGUMENTS_KEY = 'arguments';
27+
public const DATA_ARGUMENTS_KEY = 'arguments';
2828

2929
/**
3030
* The key of sub components
3131
*/
32-
const DATA_COMPONENTS_KEY = 'children';
32+
public const DATA_COMPONENTS_KEY = 'children';
3333

3434
/**
3535
* The key of the arguments node
3636
*/
37-
const ARGUMENT_KEY = 'argument';
37+
public const ARGUMENT_KEY = 'argument';
3838

3939
/**
4040
* The key of the settings component
4141
*/
42-
const SETTINGS_KEY = 'settings';
42+
public const SETTINGS_KEY = 'settings';
4343

4444
/**
4545
* Key name attribute value
4646
*/
47-
const NAME_ATTRIBUTE_KEY = 'name';
47+
public const NAME_ATTRIBUTE_KEY = 'name';
4848

4949
/**
5050
* Key class attribute value
5151
*/
52-
const CLASS_ATTRIBUTE_KEY = 'class';
52+
public const CLASS_ATTRIBUTE_KEY = 'class';
5353

5454
/**
5555
* @var Parser
@@ -225,6 +225,8 @@ private function processAttributes(array $attributes)
225225
}
226226

227227
/**
228+
* Process child result
229+
*
228230
* @param \DOMNode $node
229231
* @param array $childResult
230232
* @return array

‎app/code/Magento/Webapi/Model/Authorization/OauthUserContext.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function __construct(
6464
}
6565

6666
/**
67-
* {@inheritdoc}
67+
* @inheritDoc
6868
*/
6969
public function getUserId()
7070
{
@@ -86,13 +86,16 @@ public function getUserId()
8686
}
8787

8888
/**
89-
* {@inheritdoc}
89+
* @inheritDoc
9090
*/
9191
public function getUserType()
9292
{
9393
return UserContextInterface::USER_TYPE_INTEGRATION;
9494
}
9595

96+
/**
97+
* @inheritDoc
98+
*/
9699
public function _resetState(): void
97100
{
98101
$this->integrationId = null;

‎app/code/Magento/Weee/Model/Total/Quote/Weee.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,15 @@ public function __construct(
8888
$this->weeeCodeToItemMap = [];
8989
}
9090

91+
/**
92+
* @inheritDoc
93+
*/
94+
public function _resetState(): void
95+
{
96+
parent::_resetState();
97+
$this->setCode('weee');
98+
}
99+
91100
/**
92101
* Collect Weee amounts for the quote / order
93102
*

‎dev/tests/integration/framework/Magento/TestFramework/ApplicationStateComparator/_files/state-skip-list.php

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
'*' => [
1111
// phpcs:disable Generic.Files.LineLength.TooLong
1212
// list of the latest failures started
13-
// Magento\Store\Model\ResourceModel\Group\Collection\FetchStrategy::class => null,
1413
Magento\Framework\ObjectManager\Resetter\WeakMapSorter::class => null,
1514
Magento\Sales\Api\Data\ShippingAssignmentInterfaceFactory::class => null,
1615
Magento\Sales\Model\Order\ShippingBuilderFactory::class => null,
@@ -366,8 +365,7 @@
366365
Magento\Framework\App\Area::class => null,
367366
Magento\Store\Model\Store\Interceptor::class => null,
368367
Magento\Framework\TestFramework\ApplicationStateComparator\Comparator::class => null, // Yes, our test uses mutable state itself :-)
369-
Magento\Framework\GraphQl\Query\QueryParser::class =>
370-
null, // TODO: Do we need to add a reset for when config changes? Adding it now. Need to add to di.xml
368+
Magento\Framework\GraphQl\Query\QueryParser::class => null, // reloads as a ReloadProcessor
371369
Magento\Framework\App\Http\Context\Interceptor::class => null,
372370
Magento\Framework\HTTP\LaminasClient::class => null,
373371
Magento\TestFramework\App\State\Interceptor::class => null,
@@ -378,9 +376,7 @@
378376
Magento\Customer\Model\Group\Interceptor::class => null,
379377
Magento\Store\Model\Group\Interceptor::class => null,
380378
Magento\Directory\Model\Currency\Interceptor::class => null,
381-
// Magento\Theme\Model\Theme\ThemeProvider::class => null, // Needs _resetState for themes
382379
Magento\Catalog\Model\Category\AttributeRepository::class => null, // Note: has reloadState
383-
// Magento\Framework\Search\Request\Cleaner::class => null, // FIXME: Needs resetState
384380
Magento\Catalog\Model\ResourceModel\Category\Interceptor::class => null,
385381
Magento\Catalog\Model\Attribute\Backend\DefaultBackend\Interceptor::class => null,
386382
Magento\GraphQlCache\Model\Resolver\IdentityPool::class => null,
@@ -430,41 +426,21 @@
430426
null, // Note: We may need to check to see if this needs to be reset when config changes
431427
Magento\ConfigurableProduct\Model\Product\Type\Configurable\Interceptor::class => null,
432428
Magento\Catalog\Model\Product\Type\Simple\Interceptor::class => null,
433-
// Magento\Customer\Model\Session\Storage::class =>
434-
// null, // FIXME: race condition with Magento\Customer\Model\Session::_resetState()
435-
// Magento\Eav\Api\Data\AttributeExtension::class
436-
// => null, // FIXME: This needs to be fixed. is_pagebuilder_enabled 0 => null
437429
Magento\TestFramework\Event\Magento::class => null,
438430
Magento\Store\Model\Website\Interceptor::class => null, // reset by poison pill
439431
Magento\Eav\Model\Entity\Type::class => null, // attribute types should be destroyed by poison pill
440432
Magento\Eav\Model\Entity\Attribute\Backend\DefaultBackend\Interceptor::class =>
441433
null, // attribute types should be destroyed by poison pill
442434
Magento\TestFramework\Mail\Template\TransportBuilderMock\Interceptor::class => null, // only for testing
443-
Magento\Customer\Model\Data\Customer::class =>
444-
null, // FIXME: looks like a bug. Why is this not destroyed?
445-
Magento\Customer\Model\Customer\Interceptor::class =>
446-
null, // FIXME: looks like a bug. Why is this not destroyed?
447435
Magento\Framework\ForeignKey\ObjectRelationProcessor\EnvironmentConfig::class =>
448436
null, // OK; shouldn't change outside of deployment
449-
Magento\Indexer\Model\Indexer\Interceptor::class =>
450-
null, // FIXME: looks like this needs to be reset ?
451-
Magento\Indexer\Model\Indexer\State::class =>
452-
null, // FIXME: looks like this needs to be reset ?
453437
Magento\Customer\Model\ResourceModel\Attribute\Collection\Interceptor::class =>
454438
null, // Note: We don't _resetState these attributes on purpose. Gets reset by Magento\ApplicationServer\Eav\Model\Config\ClearWithoutCleaningCache
455439
Magento\Customer\Model\ResourceModel\Address\Attribute\Collection\Interceptor::class =>
456440
null, // Note: We don't _resetState these attributes on purpose. Gets reset by Magento\ApplicationServer\Eav\Model\Config\ClearWithoutCleaningCache
457441
Magento\Config\Model\Config\Structure\Data::class => null, // should be cleaned after poison pill
458442
Magento\Customer\Model\ResourceModel\Address\Interceptor::class =>
459443
null, // customer_address_entity table info
460-
Magento\Quote\Model\Quote\Address\Total\Subtotal::class => null, // FIXME: these should not be reused.
461-
Magento\Quote\Model\Quote\Address\Total\Grand::class =>
462-
null, // FIXME: these should not be reused.
463-
Magento\SalesRule\Model\Quote\Address\Total\ShippingDiscount::class =>
464-
null, // FIXME: these should not be reused.
465-
Magento\Weee\Model\Total\Quote\WeeeTax::class => null, // FIXME: these should not be reused.
466-
Magento\Tax\Model\Sales\Total\Quote\Shipping\Interceptor::class => null, // FIXME: these should not be reused.
467-
Magento\Tax\Model\Sales\Total\Quote\Subtotal\Interceptor::class => null, // FIXME: these should not be reused.
468444
Magento\SalesRule\Model\ResourceModel\Rule::class => null,
469445
Magento\SalesRule\Model\Plugin\QuoteConfigProductAttributes::class => null,
470446
//Create Empty Cart

‎lib/internal/Magento/Framework/Css/PreProcessor/Instruction/MagentoImport.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class MagentoImport implements PreProcessorInterface, ResetAfterRequestInterface
2424
/**
2525
* PCRE pattern that matches @magento_import instruction
2626
*/
27-
const REPLACE_PATTERN =
27+
public const REPLACE_PATTERN =
2828
'#//@magento_import(?P<reference>\s+\(reference\))?\s+[\'\"](?P<path>(?![/\\\]|\w:[/\\\])[^\"\']+)[\'\"]\s*?;#';
2929

3030
/**
@@ -50,6 +50,7 @@ class MagentoImport implements PreProcessorInterface, ResetAfterRequestInterface
5050
/**
5151
* @var \Magento\Framework\View\Design\Theme\ListInterface
5252
* @deprecated 100.0.2
53+
* @see not used
5354
*/
5455
protected $themeList;
5556

@@ -80,7 +81,7 @@ public function __construct(
8081
}
8182

8283
/**
83-
* {@inheritdoc}
84+
* @inheritDoc
8485
*/
8586
public function process(\Magento\Framework\View\Asset\PreProcessor\Chain $chain)
8687
{
@@ -138,8 +139,9 @@ protected function getTheme(LocalInterface $asset)
138139
}
139140

140141
/**
142+
* Gets themeProvider, lazy loading it when needed
143+
*
141144
* @return ThemeProviderInterface
142-
* @deprecated 100.1.1
143145
*/
144146
private function getThemeProvider()
145147
{

0 commit comments

Comments
 (0)