diff --git a/src/EventSubscriber/ApqSubscriber.php b/src/EventSubscriber/ApqSubscriber.php index 3594fd49b..ada2b53dd 100644 --- a/src/EventSubscriber/ApqSubscriber.php +++ b/src/EventSubscriber/ApqSubscriber.php @@ -44,12 +44,21 @@ public function onBeforeOperation(OperationEvent $event): void { $query = $event->getContext()->getOperation()->query; $queryHash = $event->getContext()->getOperation()->extensions['persistedQuery']['sha256Hash'] ?? ''; - if (is_string($query) && is_string($queryHash) && $queryHash !== '') { - $computedQueryHash = hash('sha256', $query); - if ($queryHash !== $computedQueryHash) { - throw new Error('Provided sha does not match query'); + if (is_string($queryHash) && $queryHash !== '') { + // Add cache context for dynamic page cache compatibility on all + // operations that have the hash set. + $event->getContext()->addCacheContexts( + ['url.query_args:variables', 'url.query_args:extensions'] + ); + + // If we have a query and the hash matches then can cache it. + if (is_string($query)) { + $computedQueryHash = hash('sha256', $query); + if ($queryHash !== $computedQueryHash) { + throw new Error('Provided sha does not match query'); + } + $this->cache->set($queryHash, $query); } - $this->cache->set($queryHash, $query); } } diff --git a/src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php b/src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php index db5d369da..a599bc048 100644 --- a/src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php +++ b/src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php @@ -3,6 +3,7 @@ namespace Drupal\graphql\Plugin\GraphQL\PersistedQuery; use Drupal\Core\Cache\CacheBackendInterface; +use Drupal\Core\PageCache\ResponsePolicy\KillSwitch; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\graphql\PersistedQuery\PersistedQueryPluginBase; use GraphQL\Server\OperationParams; @@ -27,20 +28,34 @@ class AutomaticPersistedQuery extends PersistedQueryPluginBase implements Contai */ protected $cache; + /** + * Page cache kill switch. + * + * @var \Drupal\Core\PageCache\ResponsePolicy\KillSwitch + */ + protected $pageCacheKillSwitch; + /** * {@inheritdoc} */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, CacheBackendInterface $cache) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, CacheBackendInterface $cache, KillSwitch $pageCacheKillSwitch) { parent::__construct($configuration, $plugin_id, $plugin_definition); $this->cache = $cache; + $this->pageCacheKillSwitch = $pageCacheKillSwitch; } /** * {@inheritdoc} */ public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { - return new static($configuration, $plugin_id, $plugin_definition, $container->get('cache.graphql.apq')); + return new static( + $configuration, + $plugin_id, + $plugin_definition, + $container->get('cache.graphql.apq'), + $container->get('page_cache_kill_switch') + ); } /** @@ -50,6 +65,11 @@ public function getQuery($id, OperationParams $operation) { if ($query = $this->cache->get($id)) { return $query->data; } + // Preventing page cache for this request. Otherwise, we would need to add + // a cache tag to the response and flush it when we add the persisted + // query. This is not necessary, because the PersistedQueryNotFound + // response is very short-lived. + $this->pageCacheKillSwitch->trigger(); throw new RequestError('PersistedQueryNotFound'); } diff --git a/tests/src/Kernel/Framework/AutomaticPersistedQueriesDynamicPageCacheTest.php b/tests/src/Kernel/Framework/AutomaticPersistedQueriesDynamicPageCacheTest.php new file mode 100644 index 000000000..16ee28767 --- /dev/null +++ b/tests/src/Kernel/Framework/AutomaticPersistedQueriesDynamicPageCacheTest.php @@ -0,0 +1,151 @@ +configureCachePolicy(); + + $schema = <<setUpSchema($schema); + $this->mockResolver('Query', 'node', + $this->builder->produce('entity_load') + ->map('type', $this->builder->fromValue('node')) + ->map('id', $this->builder->fromArgument('id')) + ); + + $this->mockResolver('Node', 'title', + $this->builder->produce('entity_label') + ->map('entity', $this->builder->fromParent()) + ); + + $this->mockResolver('Node', 'id', + $this->builder->produce('entity_id') + ->map('entity', $this->builder->fromParent()) + ); + + /** @var \Drupal\graphql\Plugin\DataProducerPluginManager $manager */ + $manager = $this->container->get('plugin.manager.graphql.persisted_query'); + + $this->pluginApq = $manager->createInstance('automatic_persisted_query'); + } + + /** + * Test APQ with dynamic page cache. + * + * Tests that cache context for different variables parameter is correctly + * added to the dynamic page cache entries. + */ + public function testPageCacheWithDifferentVariables(): void { + // Before adding the persisted query plugins to the server, we want to make + // sure that there are no existing plugins already there. + $this->server->removeAllPersistedQueryInstances(); + $this->server->addPersistedQueryInstance($this->pluginApq); + $this->server->save(); + $endpoint = $this->server->get('endpoint'); + + NodeType::create([ + 'type' => 'test', + 'name' => 'Test', + ])->save(); + + $node = Node::create([ + 'nid' => 1, + 'title' => 'Node 1', + 'type' => 'test', + ]); + $node->save(); + + $node = Node::create([ + 'nid' => 2, + 'title' => 'Node 2', + 'type' => 'test', + ]); + $node->save(); + + $titleQuery = 'query($id: String!) { node(id: $id) { title } }'; + $idQuery = 'query($id: String!) { node(id: $id) { id } }'; + + // Post query to endpoint to register the query hashes. + $parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $titleQuery); + $parameters['variables'] = '{"id": "2"}'; + $content = json_encode(['query' => $titleQuery] + $parameters); + $request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content); + $result = $this->container->get('http_kernel')->handle($request); + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame(['data' => ['node' => ['title' => 'Node 2']]], json_decode($result->getContent(), TRUE)); + + $parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $idQuery); + $parameters['variables'] = '{"id": "2"}'; + $content = json_encode(['query' => $idQuery] + $parameters); + $request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content); + $result = $this->container->get('http_kernel')->handle($request); + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame(['data' => ['node' => ['id' => 2]]], json_decode($result->getContent(), TRUE)); + + // Execute apq call. + $parameters['variables'] = '{"id": "1"}'; + $request = Request::create($endpoint, 'GET', $parameters); + $result = $this->container->get('http_kernel')->handle($request); + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame(['data' => ['node' => ['id' => 1]]], json_decode($result->getContent(), TRUE)); + + // Execute apq call with different variables. + $parameters['variables'] = '{"id": "2"}'; + $request = Request::create($endpoint, 'GET', $parameters); + $result = $this->container->get('http_kernel')->handle($request); + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame(['data' => ['node' => ['id' => 2]]], json_decode($result->getContent(), TRUE)); + + // Execute apq call with same parameters, but different query. + $parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $titleQuery); + $parameters['variables'] = '{"id": "2"}'; + $request = Request::create($endpoint, 'GET', $parameters); + $result = $this->container->get('http_kernel')->handle($request); + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame(['data' => ['node' => ['title' => 'Node 2']]], json_decode($result->getContent(), TRUE)); + + } + +} diff --git a/tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php b/tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php index 873eb777d..fd06fc74c 100644 --- a/tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php +++ b/tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php @@ -12,6 +12,13 @@ */ class AutomaticPersistedQueriesTest extends GraphQLTestBase { + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'page_cache', + ]; + /** * Test plugin. * @@ -24,6 +31,7 @@ class AutomaticPersistedQueriesTest extends GraphQLTestBase { */ protected function setUp(): void { parent::setUp(); + $this->configureCachePolicy(); $schema = <<