Skip to content

Commit 95cdf3c

Browse files
andimovandimov
authored
Merge pull request #573 from magento-performance/ACPT-1679-and-ACPT-1660-and-ACPT-1634
ACPT-1679 and ACPT-1660 and ACPT-1634
2 parents 672e7b2 + ee2f6b4 commit 95cdf3c

File tree

19 files changed

+265
-40
lines changed

19 files changed

+265
-40
lines changed

‎app/code/Magento/ApplicationPerformanceMonitor/Profiler/Profiler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function doProfile(callable $functionBeingProfiled, Application $applicat
7373
*
7474
* @param Metrics $beforeMetrics
7575
* @param Metrics $afterMetrics
76-
* @param Metrics|null $previousAfterMetrics,
76+
* @param Metrics|null $previousAfterMetrics
7777
* @param array $information extra information that we send to output
7878
* @return void
7979
*/

‎app/code/Magento/ApplicationPerformanceMonitorNewRelic/Profiler/Output/NewRelicOutput.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ class NewRelicOutput implements OutputInterface
2121
public const CONFIG_VERBOSE_KEY = 'application/performance_monitor/newrelic_output_verbose';
2222

2323
public function __construct(
24-
private readonly DeploymentConfig $deploymentConfig, private readonly NewRelicWrapper $newRelicWrapper)
25-
{
24+
private readonly DeploymentConfig $deploymentConfig,
25+
private readonly NewRelicWrapper $newRelicWrapper
26+
) {
2627
}
2728

2829
/**
@@ -59,6 +60,11 @@ public function doOutput(array $metrics, array $information) : void
5960
}
6061
}
6162

63+
/**
64+
* Is configured to output verbose
65+
*
66+
* @return bool
67+
*/
6268
private function isVerbose(): bool
6369
{
6470
return match ($this->deploymentConfig->get(static::CONFIG_VERBOSE_KEY)) {

‎app/code/Magento/CatalogGraphQl/etc/di.xml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@
2626
</argument>
2727
</arguments>
2828
</virtualType>
29+
<virtualType name="Magento\Framework\GraphQl\Config\Data" type="Magento\Framework\Config\Data">
30+
<arguments>
31+
<argument name="cacheTags" xsi:type="array">
32+
<!-- Note: Because of DynamicAttributeReaders, this cache needs to be cleaned when attributes change-->
33+
<item name="EAV" xsi:type="string">EAV</item>
34+
</argument>
35+
</arguments>
36+
</virtualType>
2937
<type name="Magento\Framework\GraphQl\Query\FieldTranslator">
3038
<arguments>
3139
<argument name="translationMap" xsi:type="array">
@@ -97,6 +105,14 @@
97105
<type name="Magento\Framework\Search\Request\Config\FilesystemReader">
98106
<plugin name="productAttributesDynamicFields" type="Magento\CatalogGraphQl\Plugin\Search\Request\ConfigReader" />
99107
</type>
108+
<type name="Magento\Framework\Search\Request\Config">
109+
<arguments>
110+
<argument name="cacheTags" xsi:type="array">
111+
<!-- Note: We have to add EAV to the cache tags because productAttributesDynamicFields uses EAV -->
112+
<item name="EAV" xsi:type="string">EAV</item>
113+
</argument>
114+
</arguments>
115+
</type>
100116

101117
<preference type="Magento\CatalogGraphQl\Model\Resolver\Product\Price\Provider" for="Magento\CatalogGraphQl\Model\Resolver\Product\Price\ProviderInterface"/>
102118

‎app/code/Magento/Sales/Model/Order/Config.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,17 @@
66
namespace Magento\Sales\Model\Order;
77

88
use Magento\Framework\App\Area;
9-
use Magento\Framework\Exception\LocalizedException;
109
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\Exception\LocalizedException;
11+
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
1112

1213
/**
1314
* Order configuration model
1415
*
1516
* @api
1617
* @since 100.0.2
1718
*/
18-
class Config
19+
class Config implements ResetAfterRequestInterface
1920
{
2021
/**
2122
* @var \Magento\Sales\Model\ResourceModel\Order\Status\Collection
@@ -84,6 +85,15 @@ public function __construct(
8485
$this->statusLabel = $statusLabel ?: ObjectManager::getInstance()->get(StatusLabel::class);
8586
}
8687

88+
/**
89+
* @inheritDoc
90+
*/
91+
public function _resetState(): void
92+
{
93+
$this->collection = null;
94+
}
95+
96+
8797
/**
8898
* Get collection.
8999
*

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ResolverCache/MediaGalleryTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,8 @@ public function testResolverCacheRecordIsCreatedForEachStoreView()
284284
*/
285285
public function testMediaGalleryForProductVideos(callable $actionMechanismCallable, bool $isInvalidationAction)
286286
{
287+
$this->markTestSkipped('Fixing this test in ACPT-1660');
288+
287289
// Test simple product with media
288290
$simpleProductWithMedia = $this->productRepository->get('simple_product_with_media');
289291

‎dev/tests/api-functional/testsuite/Magento/GraphQl/ConfigurableProduct/AddConfigurableProductToCartTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ private function getAvailableProductCustomOption(string $productSku): array
753753
{
754754
$query = <<<QUERY
755755
{
756-
products(filter: {sku: {eq: "${productSku}"}}) {
756+
products(filter: {sku: {eq: "{$productSku}"}}) {
757757
items {
758758
name
759759
... on CustomizableProductInterface {

‎dev/tests/integration/testsuite/Magento/CatalogGraphQl/Model/Config/ConfigTest.php‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
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\CatalogGraphQl\Model\Config;
9+
10+
use Magento\CatalogGraphQl\Model\Resolver\Products\Attributes\Collection as ProductsAttributesCollection;
11+
use Magento\Framework\GraphQl\Config\Data as GraphQlConfig;
12+
use Magento\Framework\Search\Request\Config as SearchRequestConfig;
13+
use Magento\TestFramework\Helper\Bootstrap;
14+
use Magento\TestFramework\Helper\CacheCleaner;
15+
16+
/**
17+
* @magentoDbIsolation disabled
18+
* @magentoAppArea graphql
19+
*/
20+
class ConfigTest extends \PHPUnit\Framework\TestCase
21+
{
22+
/**
23+
* @var bool|null $isFixtureLoaded
24+
*/
25+
private ?bool $isFixtureLoaded = null;
26+
27+
/**
28+
* @inheritDoc
29+
*/
30+
protected function setUp(): void
31+
{
32+
$this->isFixtureLoaded = false;
33+
CacheCleaner::cleanAll();
34+
}
35+
36+
/**
37+
* @inheritDoc
38+
*/
39+
protected function tearDown(): void
40+
{
41+
if ($this->isFixtureLoaded) {
42+
$rollBackPath = __DIR__
43+
. '/../../../Catalog/_files/products_with_layered_navigation_attribute_store_options_rollback.php';
44+
require $rollBackPath;
45+
}
46+
}
47+
48+
/**
49+
* Test to confirm that we don't load cached configuration before attribute existed
50+
*
51+
* @covers \Magento\Framework\Search\Request\Config
52+
* @return void
53+
* @magentoApiDataFixture Magento/Store/_files/store.php
54+
*/
55+
public function testAttributesChangeCleansSearchRequestConfigCache(): void
56+
{
57+
$objectManager = Bootstrap::getObjectManager();
58+
/** @var SearchRequestConfig $configInstance1 */
59+
$configInstance1 = $objectManager->create(SearchRequestConfig::class);
60+
$aggregations1 = $configInstance1->get('graphql_product_search_with_aggregation/aggregations');
61+
$this->assertArrayNotHasKey('test_configurable_bucket', $aggregations1);
62+
require __DIR__ . '/../../../Catalog/_files/products_with_layered_navigation_attribute_store_options.php';
63+
$this->isFixtureLoaded = true;
64+
/** @var SearchRequestConfig $configInstance2 */
65+
$configInstance2 = $objectManager->create(SearchRequestConfig::class);
66+
$aggregations2 = $configInstance2->get('graphql_product_search_with_aggregation/aggregations');
67+
$this->assertArrayHasKey('test_configurable_bucket', $aggregations2);
68+
}
69+
70+
/**
71+
* Test to confirm that we don't load cached configuration before attribute existed
72+
*
73+
* @return void
74+
* @magentoApiDataFixture Magento/Store/_files/store.php
75+
*/
76+
public function testAttributesChangeCleansGraphQlConfigCache(): void
77+
{
78+
$objectManager = Bootstrap::getObjectManager();
79+
$this->resetStateProductsAttributesCollection($objectManager);
80+
/** @var GraphQlConfig $configInstance1 */
81+
$configInstance1 = $objectManager->create(GraphQlConfig::class);
82+
$aggregations1 = $configInstance1->get('SimpleProduct/fields');
83+
$this->assertArrayNotHasKey('test_configurable', $aggregations1);
84+
require __DIR__ . '/../../../Catalog/_files/products_with_layered_navigation_attribute_store_options.php';
85+
$this->isFixtureLoaded = true;
86+
$this->resetStateProductsAttributesCollection($objectManager);
87+
/** @var GraphQlConfig $configInstance2 */
88+
$configInstance2 = $objectManager->create(GraphQlConfig::class);
89+
$aggregations2 = $configInstance2->get('SimpleProduct/fields');
90+
$this->assertArrayHasKey('test_configurable', $aggregations2);
91+
}
92+
93+
/**
94+
* Test to confirm that we don't load cached configuration before attribute existed
95+
*
96+
* @return void
97+
* @magentoApiDataFixture Magento/Store/_files/store.php
98+
* @magentoAppArea global
99+
*/
100+
public function testGraphQlConfigCacheShouldntBreakWhenLoadedByGlobalArea(): void
101+
{
102+
/*
103+
* When Magento\Framework\GraphQl\Config\Data is loaded from area outside of graphql, and its cache doesn't
104+
* currently exist, then it will load incorrect data and save it in its cache, which will then be used by
105+
* Magento\Framework\GraphQl\Config\Data when it actually is in the graphql area.
106+
* See AC-10465 for more details on this bug.
107+
*/
108+
$this->markTestSkipped('Fix this in AC-10465');
109+
$this->testAttributesChangeCleansGraphQlConfigCache();
110+
}
111+
112+
/**
113+
* Magento\CatalogGraphQl\Model\Config\AttributeReader has mutable state in
114+
* Magento\CatalogGraphQl\Model\Resolver\Products\Attributes\Collection $collection, so we must reset it.
115+
*
116+
* @param $objectManager
117+
* @return void
118+
*/
119+
private function resetStateProductsAttributesCollection($objectManager) : void
120+
{
121+
/** @var ProductsAttributesCollection $productsAttributesCollection */
122+
$productsAttributesCollection = $objectManager->get(ProductsAttributesCollection::class);
123+
$productsAttributesCollection->_resetState();
124+
}
125+
}

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/Collector.php‎

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

1010
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
1111
use Magento\Framework\ObjectManagerInterface;
12+
use Magento\GraphQl\App\State\ObjectManagerInterface as StateObjectManagerInterface;
1213

1314
/**
1415
* Collects shared objects from ObjectManager and copies properties for later comparison
16+
*
17+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1518
*/
1619
class Collector
1720
{
@@ -45,7 +48,7 @@ public function __construct(
4548
*/
4649
private function copyArray(
4750
array $array,
48-
string $compareType,
51+
CompareType $compareType,
4952
int $recursionLevel,
5053
int $arrayRecursionLevel = 100
5154
) : array {
@@ -86,9 +89,9 @@ function ($element) use (
8689
* @param ShouldResetState $shouldResetState
8790
* @return CollectedObject[]
8891
*/
89-
public function getSharedObjects(string $shouldResetState): array
92+
public function getSharedObjects(ShouldResetState $shouldResetState): array
9093
{
91-
if ($this->objectManager instanceof ObjectManager) {
94+
if ($this->objectManager instanceof ObjectManagerInterface) {
9295
$sharedInstances = $this->objectManager->getSharedInstances();
9396
} else {
9497
$obj = new \ReflectionObject($this->objectManager);
@@ -128,7 +131,7 @@ public function getPropertiesConstructedAndCurrent(): array
128131
{
129132
/** @var ObjectManager $objectManager */
130133
$objectManager = $this->objectManager;
131-
if (!($objectManager instanceof ObjectManager)) {
134+
if (!($objectManager instanceof StateObjectManagerInterface)) {
132135
throw new \Exception("Not the correct type of ObjectManager");
133136
}
134137
// Calling _resetState helps us avoid adding skip/filter for these classes.
@@ -156,7 +159,7 @@ public function getPropertiesConstructedAndCurrent(): array
156159
*/
157160
public function getPropertiesFromObject(
158161
object $object,
159-
string $compareType,
162+
CompareType $compareType,
160163
int $recursionLevel = 1,
161164
): CollectedObject {
162165
$className = get_class($object);
@@ -165,7 +168,7 @@ public function getPropertiesFromObject(
165168
if (array_key_exists($className, $skipList)) {
166169
return CollectedObject::getSkippedObject();
167170
}
168-
if ($this->objectManager instanceof ObjectManager) {
171+
if ($this->objectManager instanceof StateObjectManagerInterface) {
169172
$serviceName = array_search($object, $this->objectManager->getSharedInstances(), true);
170173
if ($serviceName && array_key_exists($serviceName, $skipList)) {
171174
return CollectedObject::getSkippedObject();

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/Comparator.php‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,21 @@ private function compare(
179179
}
180180
}
181181
}
182-
if ($objectState) {
183-
return [
184-
'objectClassBefore' => $before->getClassName(),
185-
'objectClassAfter' => $after->getClassName(),
186-
'properties' => $objectState,
187-
'objectIdBefore' => $before->getObjectId(),
188-
'objectIdAfter' => $after->getObjectId(),
189-
];
182+
if (!$objectState) {
183+
return [];
190184
}
191-
return [];
185+
$returnValue = [
186+
'objectClassBefore' => $before->getClassName(),
187+
'properties' => $objectState,
188+
];
189+
if ($returnValue['objectClassBefore'] !== $after->getClassName()) {
190+
$returnValue['objectClassAfter'] = $after->getClassName();
191+
}
192+
if ($before->getObjectId() != $after->getObjectId()) {
193+
$returnValue['objectIdBefore'] = $before->getObjectId();
194+
$returnValue['objectIdAfter'] = $after->getObjectId();
195+
}
196+
return $returnValue;
192197
}
193198

194199
/**

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/CompareType.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@
99

1010
/**
1111
* What type of comparison
12-
*
13-
* TODO: change this back into enum once magento-semvar is fixed
1412
*/
15-
class CompareType
13+
enum CompareType
1614
{
17-
public const CompareBetweenRequests = "CompareBetweenRequests";
18-
public const CompareConstructedAgainstCurrent = "CompareConstructedAgainstCurrent";
15+
case CompareBetweenRequests;
16+
case CompareConstructedAgainstCurrent;
1917
}

0 commit comments

Comments
 (0)