Skip to content

Commit 53eb21e

Browse files
dbosendbosen
authored
fix(APQ): Fix APQ with page caches (#1317)
1 parent 3711762 commit 53eb21e

File tree

4 files changed

+195
-7
lines changed

4 files changed

+195
-7
lines changed

‎src/EventSubscriber/ApqSubscriber.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,21 @@ public function onBeforeOperation(OperationEvent $event): void {
4444
$query = $event->getContext()->getOperation()->query;
4545
$queryHash = $event->getContext()->getOperation()->extensions['persistedQuery']['sha256Hash'] ?? '';
4646

47-
if (is_string($query) && is_string($queryHash) && $queryHash !== '') {
48-
$computedQueryHash = hash('sha256', $query);
49-
if ($queryHash !== $computedQueryHash) {
50-
throw new Error('Provided sha does not match query');
47+
if (is_string($queryHash) && $queryHash !== '') {
48+
// Add cache context for dynamic page cache compatibility on all
49+
// operations that have the hash set.
50+
$event->getContext()->addCacheContexts(
51+
['url.query_args:variables', 'url.query_args:extensions']
52+
);
53+
54+
// If we have a query and the hash matches then can cache it.
55+
if (is_string($query)) {
56+
$computedQueryHash = hash('sha256', $query);
57+
if ($queryHash !== $computedQueryHash) {
58+
throw new Error('Provided sha does not match query');
59+
}
60+
$this->cache->set($queryHash, $query);
5161
}
52-
$this->cache->set($queryHash, $query);
5362
}
5463
}
5564

‎src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Drupal\graphql\Plugin\GraphQL\PersistedQuery;
44

55
use Drupal\Core\Cache\CacheBackendInterface;
6+
use Drupal\Core\PageCache\ResponsePolicy\KillSwitch;
67
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
78
use Drupal\graphql\PersistedQuery\PersistedQueryPluginBase;
89
use GraphQL\Server\OperationParams;
@@ -27,20 +28,34 @@ class AutomaticPersistedQuery extends PersistedQueryPluginBase implements Contai
2728
*/
2829
protected $cache;
2930

31+
/**
32+
* Page cache kill switch.
33+
*
34+
* @var \Drupal\Core\PageCache\ResponsePolicy\KillSwitch
35+
*/
36+
protected $pageCacheKillSwitch;
37+
3038
/**
3139
* {@inheritdoc}
3240
*/
33-
public function __construct(array $configuration, $plugin_id, $plugin_definition, CacheBackendInterface $cache) {
41+
public function __construct(array $configuration, $plugin_id, $plugin_definition, CacheBackendInterface $cache, KillSwitch $pageCacheKillSwitch) {
3442
parent::__construct($configuration, $plugin_id, $plugin_definition);
3543

3644
$this->cache = $cache;
45+
$this->pageCacheKillSwitch = $pageCacheKillSwitch;
3746
}
3847

3948
/**
4049
* {@inheritdoc}
4150
*/
4251
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
43-
return new static($configuration, $plugin_id, $plugin_definition, $container->get('cache.graphql.apq'));
52+
return new static(
53+
$configuration,
54+
$plugin_id,
55+
$plugin_definition,
56+
$container->get('cache.graphql.apq'),
57+
$container->get('page_cache_kill_switch')
58+
);
4459
}
4560

4661
/**
@@ -50,6 +65,11 @@ public function getQuery($id, OperationParams $operation) {
5065
if ($query = $this->cache->get($id)) {
5166
return $query->data;
5267
}
68+
// Preventing page cache for this request. Otherwise, we would need to add
69+
// a cache tag to the response and flush it when we add the persisted
70+
// query. This is not necessary, because the PersistedQueryNotFound
71+
// response is very short-lived.
72+
$this->pageCacheKillSwitch->trigger();
5373
throw new RequestError('PersistedQueryNotFound');
5474
}
5575

‎tests/src/Kernel/Framework/AutomaticPersistedQueriesDynamicPageCacheTest.php

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
<?php
2+
3+
namespace Drupal\Tests\graphql\Kernel\Framework;
4+
5+
use Drupal\node\Entity\Node;
6+
use Drupal\node\Entity\NodeType;
7+
use Drupal\Tests\graphql\Kernel\GraphQLTestBase;
8+
use Symfony\Component\HttpFoundation\Request;
9+
10+
/**
11+
* Tests the automatic persisted query plugin.
12+
*
13+
* @group graphql
14+
*/
15+
class AutomaticPersistedQueriesDynamicPageCacheTest extends GraphQLTestBase {
16+
17+
/**
18+
* {@inheritdoc}
19+
*/
20+
protected static $modules = [
21+
'dynamic_page_cache',
22+
];
23+
24+
/**
25+
* Test plugin.
26+
*
27+
* @var \Drupal\graphql\Plugin\PersistedQueryPluginInterface
28+
*/
29+
protected $pluginApq;
30+
31+
/**
32+
* {@inheritdoc}
33+
*/
34+
protected function setUp(): void {
35+
parent::setUp();
36+
$this->configureCachePolicy();
37+
38+
$schema = <<<GQL
39+
schema {
40+
query: Query
41+
}
42+
type Query {
43+
node(id: String): Node
44+
}
45+
46+
type Node {
47+
title: String!
48+
id: Int!
49+
}
50+
GQL;
51+
$this->setUpSchema($schema);
52+
$this->mockResolver('Query', 'node',
53+
$this->builder->produce('entity_load')
54+
->map('type', $this->builder->fromValue('node'))
55+
->map('id', $this->builder->fromArgument('id'))
56+
);
57+
58+
$this->mockResolver('Node', 'title',
59+
$this->builder->produce('entity_label')
60+
->map('entity', $this->builder->fromParent())
61+
);
62+
63+
$this->mockResolver('Node', 'id',
64+
$this->builder->produce('entity_id')
65+
->map('entity', $this->builder->fromParent())
66+
);
67+
68+
/** @var \Drupal\graphql\Plugin\DataProducerPluginManager $manager */
69+
$manager = $this->container->get('plugin.manager.graphql.persisted_query');
70+
71+
$this->pluginApq = $manager->createInstance('automatic_persisted_query');
72+
}
73+
74+
/**
75+
* Test APQ with dynamic page cache.
76+
*
77+
* Tests that cache context for different variables parameter is correctly
78+
* added to the dynamic page cache entries.
79+
*/
80+
public function testPageCacheWithDifferentVariables(): void {
81+
// Before adding the persisted query plugins to the server, we want to make
82+
// sure that there are no existing plugins already there.
83+
$this->server->removeAllPersistedQueryInstances();
84+
$this->server->addPersistedQueryInstance($this->pluginApq);
85+
$this->server->save();
86+
$endpoint = $this->server->get('endpoint');
87+
88+
NodeType::create([
89+
'type' => 'test',
90+
'name' => 'Test',
91+
])->save();
92+
93+
$node = Node::create([
94+
'nid' => 1,
95+
'title' => 'Node 1',
96+
'type' => 'test',
97+
]);
98+
$node->save();
99+
100+
$node = Node::create([
101+
'nid' => 2,
102+
'title' => 'Node 2',
103+
'type' => 'test',
104+
]);
105+
$node->save();
106+
107+
$titleQuery = 'query($id: String!) { node(id: $id) { title } }';
108+
$idQuery = 'query($id: String!) { node(id: $id) { id } }';
109+
110+
// Post query to endpoint to register the query hashes.
111+
$parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $titleQuery);
112+
$parameters['variables'] = '{"id": "2"}';
113+
$content = json_encode(['query' => $titleQuery] + $parameters);
114+
$request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content);
115+
$result = $this->container->get('http_kernel')->handle($request);
116+
$this->assertSame(200, $result->getStatusCode());
117+
$this->assertSame(['data' => ['node' => ['title' => 'Node 2']]], json_decode($result->getContent(), TRUE));
118+
119+
$parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $idQuery);
120+
$parameters['variables'] = '{"id": "2"}';
121+
$content = json_encode(['query' => $idQuery] + $parameters);
122+
$request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content);
123+
$result = $this->container->get('http_kernel')->handle($request);
124+
$this->assertSame(200, $result->getStatusCode());
125+
$this->assertSame(['data' => ['node' => ['id' => 2]]], json_decode($result->getContent(), TRUE));
126+
127+
// Execute apq call.
128+
$parameters['variables'] = '{"id": "1"}';
129+
$request = Request::create($endpoint, 'GET', $parameters);
130+
$result = $this->container->get('http_kernel')->handle($request);
131+
$this->assertSame(200, $result->getStatusCode());
132+
$this->assertSame(['data' => ['node' => ['id' => 1]]], json_decode($result->getContent(), TRUE));
133+
134+
// Execute apq call with different variables.
135+
$parameters['variables'] = '{"id": "2"}';
136+
$request = Request::create($endpoint, 'GET', $parameters);
137+
$result = $this->container->get('http_kernel')->handle($request);
138+
$this->assertSame(200, $result->getStatusCode());
139+
$this->assertSame(['data' => ['node' => ['id' => 2]]], json_decode($result->getContent(), TRUE));
140+
141+
// Execute apq call with same parameters, but different query.
142+
$parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $titleQuery);
143+
$parameters['variables'] = '{"id": "2"}';
144+
$request = Request::create($endpoint, 'GET', $parameters);
145+
$result = $this->container->get('http_kernel')->handle($request);
146+
$this->assertSame(200, $result->getStatusCode());
147+
$this->assertSame(['data' => ['node' => ['title' => 'Node 2']]], json_decode($result->getContent(), TRUE));
148+
149+
}
150+
151+
}

‎tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
*/
1313
class AutomaticPersistedQueriesTest extends GraphQLTestBase {
1414

15+
/**
16+
* {@inheritdoc}
17+
*/
18+
protected static $modules = [
19+
'page_cache',
20+
];
21+
1522
/**
1623
* Test plugin.
1724
*
@@ -24,6 +31,7 @@ class AutomaticPersistedQueriesTest extends GraphQLTestBase {
2431
*/
2532
protected function setUp(): void {
2633
parent::setUp();
34+
$this->configureCachePolicy();
2735
$schema = <<<GQL
2836
schema {
2937
query: Query

0 commit comments

Comments
 (0)