Skip to content

Commit 4cd3fbf

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1669: GraphQl\Config & Search\Request\Config need to be cleaned when attributes change
Magento\Framework\GraphQl\Config and Magento\Framework\Search\Request both use only the default cache tags (MAGE & CONFIG). Because both of those configurations have data that comes from the current attributes, when attributes change, they need to be invalidated/cleaned. Therefore, they need to have the EAV tag added to them.
1 parent c333d32 commit 4cd3fbf

File tree

4 files changed

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

4 files changed

+153
-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: 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+
}

‎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)