Skip to content

Commit a3d15ef

Browse files
rganinrganin
committed
B2B-2931: [Graphql Resolver Cache] improve reliability and developer awareness on cache keyspace
1 parent 665d6e4 commit a3d15ef

File tree

10 files changed

+94
-35
lines changed

10 files changed

+94
-35
lines changed

‎app/code/Magento/CmsGraphQl/etc/graphql/di.xml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,24 @@
3030
</argument>
3131
</arguments>
3232
</type>
33+
<type name="Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator\Provider">
34+
<arguments>
35+
<argument name="customFactorProviders" xsi:type="array">
36+
<item name="Magento\CmsGraphQl\Model\Resolver\Page" xsi:type="array">
37+
<item name="currency" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Currency</item>
38+
<item name="store" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Store</item>
39+
<item name="customergroup" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerGroup</item>
40+
<item name="customertaxrate" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerTaxRate</item>
41+
<item name="isloggedin" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\IsLoggedIn</item>
42+
</item>
43+
<item name="Magento\CmsGraphQl\Model\Resolver\Blocks" xsi:type="array">
44+
<item name="currency" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Currency</item>
45+
<item name="store" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Store</item>
46+
<item name="customergroup" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerGroup</item>
47+
<item name="customertaxrate" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerTaxRate</item>
48+
<item name="isloggedin" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\IsLoggedIn</item>
49+
</item>
50+
</argument>
51+
</arguments>
52+
</type>
3353
</config>

‎app/code/Magento/CustomerGraphQl/etc/graphql/di.xml

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,6 @@
9090
</argument>
9191
</arguments>
9292
</type>
93-
<type name="Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator">
94-
<arguments>
95-
<argument name="factorProviders" xsi:type="array">
96-
<item name="customergroup" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerGroup</item>
97-
<item name="customertaxrate" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\CustomerTaxRate</item>
98-
<item name="isloggedin" xsi:type="string">Magento\CustomerGraphQl\Model\Resolver\CacheKey\FactorProvider\IsLoggedIn</item>
99-
</argument>
100-
</arguments>
101-
</type>
10293
<type name="Magento\Framework\GraphQl\Schema\Type\Enum\DefaultDataMapper">
10394
<arguments>
10495
<argument name="map" xsi:type="array">

‎app/code/Magento/GraphQlResolverCache/Model/Resolver/Result/CacheKey/Calculator/Provider.php

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function __construct(
4444
}
4545

4646
/**
47-
* Initialize custom cache key calculator for the given resolver.
47+
* Initialize cache key calculator for the given resolver.
4848
*
4949
* @param ResolverInterface $resolver
5050
*
@@ -57,8 +57,11 @@ private function initForResolver(ResolverInterface $resolver): void
5757
return;
5858
}
5959
$customKeyFactorProviders = $this->getCustomFactorProvidersForResolver($resolver);
60-
if (empty($customKeyFactorProviders)) {
61-
$this->keyCalculatorInstances[$resolverClass] = $this->objectManager->get(Calculator::class);
60+
if ($customKeyFactorProviders === null) {
61+
throw new \InvalidArgumentException(
62+
"Key factors are not determined for {$resolverClass} or its parents." .
63+
"An empty array of factors is expected for the resolvers with no factors involved."
64+
);
6265
} else {
6366
$runtimePoolKey = $this->generateCustomProvidersKey($customKeyFactorProviders);
6467
if (!isset($this->keyCalculatorInstances[$runtimePoolKey])) {
@@ -79,6 +82,9 @@ private function initForResolver(ResolverInterface $resolver): void
7982
*/
8083
private function generateCustomProvidersKey(array $customProviders): string
8184
{
85+
if (empty($customProviders)) {
86+
return "empty";
87+
}
8288
$keyArray = array_keys($customProviders);
8389
sort($keyArray);
8490
return implode('_', $keyArray);
@@ -115,15 +121,19 @@ private function getResolverClassChain(ResolverInterface $resolver): array
115121
* Get custom cache key factor providers for the given resolver object.
116122
*
117123
* @param ResolverInterface $resolver
118-
* @return array
124+
* @return array|null
119125
*/
120-
private function getCustomFactorProvidersForResolver(ResolverInterface $resolver): array
126+
private function getCustomFactorProvidersForResolver(ResolverInterface $resolver): ?array
121127
{
128+
$resultsToMerge = [];
122129
foreach ($this->getResolverClassChain($resolver) as $resolverClass) {
123-
if (!empty($this->customFactorProviders[$resolverClass])) {
124-
return $this->customFactorProviders[$resolverClass];
130+
if (isset($this->customFactorProviders[$resolverClass])
131+
&& is_array($this->customFactorProviders[$resolverClass])
132+
) {
133+
$resultsToMerge []= $this->customFactorProviders[$resolverClass];
125134
}
126135
}
127-
return [];
136+
// avoid using array_merge in a loop
137+
return !empty($resultsToMerge) ? array_merge(...$resultsToMerge) : null;
128138
}
129139
}

‎app/code/Magento/GraphQlResolverCache/Model/Resolver/Result/CacheKey/Calculator/ProviderInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ interface ProviderInterface
2020
*
2121
* @param ResolverInterface $resolver
2222
* @return Calculator
23+
*
24+
* @throws \InvalidArgumentException
2325
*/
2426
public function getKeyCalculatorForResolver(ResolverInterface $resolver): Calculator;
2527
}

‎app/code/Magento/GraphQlResolverCache/Model/Resolver/Result/HydratorDehydratorProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,6 @@ private function getResolverClassChain(ResolverInterface $resolver): array
210210
foreach (class_parents($resolver) as $classParent) {
211211
$resolverClasses[] = trim($classParent, '\\');
212212
}
213-
return $resolverClasses;
213+
return array_reverse($resolverClasses);
214214
}
215215
}

‎app/code/Magento/StoreGraphQl/etc/graphql/di.xml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,4 @@
4141
</argument>
4242
</arguments>
4343
</type>
44-
<type name="Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator">
45-
<arguments>
46-
<argument name="factorProviders" xsi:type="array">
47-
<item name="currency" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Currency</item>
48-
<item name="store" xsi:type="string">Magento\StoreGraphQl\Model\Resolver\CacheKey\FactorProvider\Store</item>
49-
</argument>
50-
</arguments>
51-
</type>
5244
</config>

‎dev/tests/integration/testsuite/Magento/GraphQlResolverCache/Model/Plugin/Resolver/CacheTest.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ public function testCachingSkippedOnKeyCalculationFailure()
104104
$this->keyFactorMock->expects($this->any())
105105
->method('getFactorValue')
106106
->willThrowException(new \Exception("Test key factor exception"));
107-
$this->loggerMock->expects($this->once())->method('warning');
108107
$this->graphqlResolverCacheMock->expects($this->never())
109108
->method('load');
110109
$this->graphqlResolverCacheMock->expects($this->never())
@@ -167,6 +166,20 @@ private function preconfigureMocks()
167166
]
168167
);
169168

169+
$this->objectManager->configure(
170+
[
171+
\Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator\Provider::class => [
172+
'arguments' => [
173+
'customFactorProviders' => [
174+
\Magento\StoreGraphQl\Model\Resolver\StoreConfigResolver::class => [
175+
'test_failing' => 'TestFailingKeyFactor'
176+
]
177+
]
178+
]
179+
]
180+
]
181+
);
182+
170183
$identityProviderMock = $this->getMockBuilder(IdentityInterface::class)
171184
->disableOriginalConstructor()
172185
->onlyMethods(['getIdentities'])

‎dev/tests/integration/testsuite/Magento/GraphQlResolverCache/Model/Resolver/Result/Cache/KeyCalculator/ProviderTest.php

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use Magento\CustomerGraphQl\Model\Resolver\Customer;
1111
use Magento\CustomerGraphQl\Model\Resolver\CustomerAddresses;
1212
use Magento\Framework\GraphQl\Query\ResolverInterface;
13-
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator;
1413
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator\Provider;
1514
use Magento\StoreGraphQl\CacheIdFactorProviders\CurrencyProvider;
1615
use Magento\StoreGraphQl\CacheIdFactorProviders\StoreProvider;
@@ -19,6 +18,7 @@
1918

2019
/**
2120
* Test for Graphql Resolver-level cache key provider.
21+
* @magentoAppArea graphql
2222
*/
2323
class ProviderTest extends \PHPUnit\Framework\TestCase
2424
{
@@ -42,21 +42,49 @@ public function setUp(): void
4242
}
4343

4444
/**
45-
* Test that generic key provided for non-customized resolver is a generic key provider with default config.
45+
* Test that missing config triggers an exception.
4646
*
4747
* @magentoAppArea graphql
4848
*
4949
* @return void
5050
*/
51-
public function testProviderForGenericKey()
51+
public function testProviderNotConfigured()
5252
{
5353
$this->provider = $this->objectManager->create(Provider::class);
5454
$resolver = $this->getMockBuilder(ResolverInterface::class)
5555
->disableOriginalConstructor()
5656
->getMock();
57-
$genericCalculator = $this->objectManager->get(Calculator::class);
57+
$this->expectException(\InvalidArgumentException::class);
58+
$resolverClass = get_class($resolver);
59+
$this->expectExceptionMessage(
60+
"Key factors are not determined for {$resolverClass} or its parents."
61+
. "An empty array of factors is expected for the resolvers with no factors involved."
62+
);
63+
$this->provider->getKeyCalculatorForResolver($resolver);
64+
}
65+
66+
/**
67+
* Test that empty provided config is handled properly.
68+
*
69+
* @magentoAppArea graphql
70+
*
71+
* @return void
72+
*/
73+
public function testProviderEmptyConfig()
74+
{
75+
$this->provider = $this->objectManager->create(
76+
Provider::class,
77+
[
78+
'customFactorProviders' => [
79+
'Magento\StoreGraphQl\Model\Resolver\StoreConfigResolver' => [],
80+
]
81+
]
82+
);
83+
$resolver = $this->getMockBuilder(\Magento\StoreGraphQl\Model\Resolver\StoreConfigResolver::class)
84+
->disableOriginalConstructor()
85+
->getMock();
5886
$calc = $this->provider->getKeyCalculatorForResolver($resolver);
59-
$this->assertSame($genericCalculator, $calc);
87+
$this->assertNull($calc->calculateCacheKey());
6088
}
6189

6290
/**
@@ -66,7 +94,7 @@ public function testProviderForGenericKey()
6694
*
6795
* @return void
6896
*/
69-
public function testProviderNonGenericKey()
97+
public function testProviderKeyFactorsConfigured()
7098
{
7199
$this->provider = $this->objectManager->create(Provider::class, [
72100
'customFactorProviders' => [

‎dev/tests/integration/testsuite/Magento/GraphQlResolverCache/Model/Resolver/Result/Cache/KeyCalculatorTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\Calculator;
1313
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\ParentValueFactorProviderInterface;
1414
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\GenericFactorProviderInterface;
15-
use Magento\GraphQlResolverCache\Model\Resolver\Result\CacheKey\ParentValue\ProcessedValueFactorInterface;
1615
use Magento\GraphQlResolverCache\Model\Resolver\Result\ValueProcessorInterface;
1716
use Magento\TestFramework\Helper\Bootstrap;
1817
use Psr\Log\LoggerInterface;

‎dev/tests/integration/testsuite/Magento/GraphQlResolverCache/Model/Resolver/Result/HydratorDehydratorProviderTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
use Magento\Framework\GraphQl\Query\ResolverInterface;
1212
use Magento\StoreGraphQl\Model\Resolver\StoreConfigResolver;
1313
use Magento\TestFramework\Helper\Bootstrap;
14+
use PHPUnit\Framework\TestCase;
1415

15-
class HydratorDehydratorProviderTest extends \PHPUnit\Framework\TestCase
16+
class HydratorDehydratorProviderTest extends TestCase
1617
{
1718
/**
1819
* @var \Magento\TestFramework\ObjectManager
@@ -49,6 +50,8 @@ private function getTestProviderConfig()
4950
'sortOrder' => 15,
5051
'class' => 'TestResolverNestedItemsHydrator'
5152
],
53+
],
54+
'StoreConfigResolverDerivedMock' => [
5255
'model_hydrator' => [
5356
'sortOrder' => 10,
5457
'class' => 'TestResolverModelHydrator'
@@ -75,6 +78,7 @@ public function testHydratorChainProvider()
7578
{
7679
$resolver = $this->getMockBuilder(StoreConfigResolver::class)
7780
->disableOriginalConstructor()
81+
->setMockClassName('StoreConfigResolverDerivedMock')
7882
->getMockForAbstractClass();
7983

8084
$testResolverData = [

0 commit comments

Comments
 (0)