Skip to content

Commit 2807445

Browse files
andimovandimov
authored
Merge pull request #8620 from magento-performance/ACPT-1669
ACPT-1669: GraphQl\Config & Search\Request\Config need to be cleaned when attributes change
2 parents efe5560 + 5384483 commit 2807445

File tree

4 files changed

+150
-4
lines changed
  • app/code/Magento/CatalogGraphQl/etc
  • dev/tests/integration/testsuite/Magento/CatalogGraphQl/Model/Config
  • lib/internal/Magento/Framework

4 files changed

+150
-4
lines changed

‎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

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

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

‎lib/internal/Magento/Framework/Config/Data.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,22 @@ class Data implements \Magento\Framework\Config\DataInterface
7777
* @param CacheInterface $cache
7878
* @param string $cacheId
7979
* @param SerializerInterface|null $serializer
80+
* @param array|null $cacheTags
8081
*/
8182
public function __construct(
8283
ReaderInterface $reader,
8384
CacheInterface $cache,
8485
$cacheId,
85-
SerializerInterface $serializer = null
86+
SerializerInterface $serializer = null,
87+
?array $cacheTags = null,
8688
) {
8789
$this->reader = $reader;
8890
$this->cache = $cache;
8991
$this->cacheId = $cacheId;
9092
$this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class);
93+
if ($cacheTags) {
94+
$this->cacheTags = $cacheTags;
95+
}
9196
$this->initData();
9297
}
9398

‎lib/internal/Magento/Framework/Search/Request/Config.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class Config extends \Magento\Framework\Config\Data
1515
/**
1616
* Cache identifier
1717
*/
18-
const CACHE_ID = 'request_declaration';
18+
public const CACHE_ID = 'request_declaration';
1919

2020
/**
2121
* Constructor
@@ -24,13 +24,16 @@ class Config extends \Magento\Framework\Config\Data
2424
* @param \Magento\Framework\Config\CacheInterface $cache
2525
* @param string|null $cacheId
2626
* @param SerializerInterface|null $serializer
27+
* @param array|null $cacheTags
28+
* phpcs:disable Generic.CodeAnalysis.UselessOverridingMethod
2729
*/
2830
public function __construct(
2931
\Magento\Framework\Search\Request\Config\FilesystemReader $reader,
3032
\Magento\Framework\Config\CacheInterface $cache,
3133
$cacheId = self::CACHE_ID,
32-
SerializerInterface $serializer = null
34+
SerializerInterface $serializer = null,
35+
?array $cacheTags = null,
3336
) {
34-
parent::__construct($reader, $cache, $cacheId, $serializer);
37+
parent::__construct($reader, $cache, $cacheId, $serializer, $cacheTags);
3538
}
3639
}

0 commit comments

Comments
 (0)