Fix apq with page caches

.github/workflows/testing.yml

@@ -91,7 +91,8 @@ jobs:
           phpstan/phpstan-deprecation-rules:^1.0.0 \
           jangregor/phpstan-prophecy:^1.0.0 \
           phpstan/phpstan-phpunit:^1.0.0 \
-          phpstan/extension-installer:^1.0
+          phpstan/extension-installer:^1.0 \
+          guzzlehttp/guzzle:^7.4.5
 
       # We install Coder separately because updating did not work in the local
       # Drupal vendor dir.
@@ -104,10 +105,12 @@ jobs:
 
       - name: Run PHPUnit
         run: |
+          php -S 127.0.0.1:8080 >/dev/null 2>&1 &
           cp modules/graphql/phpunit.xml.dist core/phpunit.xml
           ./vendor/bin/phpunit --configuration core/phpunit.xml modules/graphql
         env:
-          SIMPLETEST_DB: "sqlite://localhost/:memory:"
+          SIMPLETEST_DB: "sqlite://127.0.0.1/sites/default/files/db.sqlite"
+          SIMPLETEST_BASE_URL: http://127.0.0.1:8080
 
       - name: Run PHPStan
         # phpstan-drupal bug, so we remove 1 stub file

phpunit.xml.dist

@@ -11,6 +11,9 @@
     <testsuite name="kernel">
       <file>./tests/TestSuites/KernelTestSuite.php</file>
     </testsuite>
+    <testsuite name="functional">
+      <file>./tests/TestSuites/FunctionalTestSuite.php</file>
+    </testsuite>
   </testsuites>
   <filter>
     <whitelist>

src/EventSubscriber/ApqSubscriber.php

@@ -49,6 +49,10 @@ public function onBeforeOperation(OperationEvent $event): void {
       if ($queryHash !== $computedQueryHash) {
         throw new Error('Provided sha does not match query');
       }
+      // Add cache context for dynamic page cache compatibility.
+      $event->getContext()->addCacheContexts(
+        ['url.query_args:variables', 'url.query_args:extensions']
+      );
       $this->cache->set($queryHash, $query);
     }
   }

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');
   }
 

tests/modules/graphql_testing_schema/graphql/testing.graphqls

@@ -0,0 +1,12 @@
+schema {
+  query: Query
+}
+
+type Query {
+  article(id: Int!): Article
+}
+
+type Article {
+  id: Int!
+  title: String!
+}

tests/modules/graphql_testing_schema/graphql_testing_schema.info.yml

@@ -0,0 +1,8 @@
+type: module
+name: GraphQL Testing Schema
+description: A simple GraphQL schema for tests.
+package: Testing
+hidden: TRUE
+dependencies:
+  - graphql:graphql
+  - drupal:node

tests/modules/graphql_testing_schema/src/Plugin/GraphQL/Schema/TestingSchema.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace Drupal\graphql_testing_schema\Plugin\GraphQL\Schema;
+
+use Drupal\graphql\GraphQL\ResolverBuilder;
+use Drupal\graphql\GraphQL\ResolverRegistry;
+use Drupal\graphql\Plugin\GraphQL\Schema\SdlSchemaPluginBase;
+
+/**
+ * A simple schema for testing purposes.
+ *
+ * @Schema(
+ *   id = "testing",
+ *   name = "Test schema"
+ * )
+ */
+class TestingSchema extends SdlSchemaPluginBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getResolverRegistry() {
+    $builder = new ResolverBuilder();
+    $registry = new ResolverRegistry();
+
+    $registry->addFieldResolver('Query', 'article',
+      $builder->produce('entity_load')
+        ->map('type', $builder->fromValue('node'))
+        ->map('bundles', $builder->fromValue(['article']))
+        ->map('id', $builder->fromArgument('id'))
+    );
+
+    $registry->addFieldResolver('Article', 'id',
+      $builder->produce('entity_id')
+        ->map('entity', $builder->fromParent())
+    );
+
+    $registry->addFieldResolver('Article', 'title',
+      $builder->produce('entity_label')
+        ->map('entity', $builder->fromParent()),
+    );
+
+    return $registry;
+  }
+
+}

tests/queries/article_id.gql

@@ -0,0 +1,5 @@
+query ($id: Int!) {
+  article(id: $id) {
+    id
+  }
+}

tests/queries/article_title.gql

@@ -0,0 +1,5 @@
+query ($id: Int!) {
+  article(id: $id) {
+    title
+  }
+}

tests/src/Functional/Framework/AutomaticPersistedQueriesWithPageCacheTest.php

@@ -0,0 +1,130 @@
+<?php
+
+namespace Drupal\Tests\graphql\Functional\Framework;
+
+use Drupal\graphql\Entity\Server;
+use Drupal\node\Entity\Node;
+use Drupal\node\Entity\NodeType;
+use Drupal\Tests\graphql\Functional\GraphQLFunctionalTestBase;
+use Drupal\user\Entity\Role;
+
+/**
+ * Tests the automatic persisted query plugin with page cache.
+ *
+ * @group graphql
+ */
+class AutomaticPersistedQueriesWithPageCacheTest extends GraphQLFunctionalTestBase {
+
+  /**
+   * The GraphQL server.
+   *
+   * @var \Drupal\graphql\Entity\Server
+   */
+  protected $server;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'page_cache',
+    'dynamic_page_cache',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    // Node Type used to create test articles.
+    NodeType::create([
+      'type' => 'article',
+    ])->save();
+
+    // Create some test articles.
+    Node::create([
+      'nid' => 1,
+      'type' => 'article',
+      'title' => 'Test Article 1',
+    ])->save();
+
+    Node::create([
+      'nid' => 2,
+      'type' => 'article',
+      'title' => 'Test Article 2',
+    ])->save();
+
+    $config = [
+      'schema' => 'testing',
+      'name' => 'testing',
+      'endpoint' => '/graphql-testing',
+      'persisted_queries_settings' => [
+        'automatic_persisted_query' => [
+          'weight' => 0,
+        ],
+      ],
+    ];
+
+    $this->server = Server::create($config);
+    $this->server->save();
+    \Drupal::service('router.builder')->rebuild();
+
+    $anonymousRole = Role::load(Role::ANONYMOUS_ID);
+    $this->grantPermissions($anonymousRole, [
+      'execute ' . $this->server->id() . ' persisted graphql requests',
+      'execute ' . $this->server->id() . ' arbitrary graphql requests',
+    ]
+    );
+  }
+
+  /**
+   * 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 {
+    $query1 = $this->getQueryFromFile('article_title.gql');
+    $query2 = $this->getQueryFromFile('article_id.gql');
+    $variables1 = '{"id": 1}';
+    $variables2 = '{"id": 2}';
+
+    // Test that requests with different variables but same query hash return
+    // different responses. Requesting in both instances with query first,
+    // to make sure the query is registered.
+    $this->apqRequest($this->server->endpoint, $query1, $variables1, TRUE);
+    $response = $this->apqRequest($this->server->endpoint, $query1, $variables1);
+    $this->assertEquals('Test Article 1', $response['data']['article']['title']);
+
+    $this->apqRequest($this->server->endpoint, $query1, $variables2, TRUE);
+    $response = $this->apqRequest($this->server->endpoint, $query1, $variables2);
+    $this->assertEquals('Test Article 2', $response['data']['article']['title']);
+
+    // Test that requests with same variables but different query hash return
+    // different responses.
+    $this->apqRequest($this->server->endpoint, $query2, $variables2, TRUE);
+    $response = $this->apqRequest($this->server->endpoint, $query2, $variables2);
+    $this->assertEquals(2, $response['data']['article']['id']);
+
+  }
+
+  /**
+   * Test PersistedQueryNotFound error is not cached in page cache.
+   */
+  public function testPersistedQueryNotFoundNotCached(): void {
+    $query = $this->getQueryFromFile('article_title.gql');
+    $variables = '{"id": 1}';
+
+    // The first request should return an PersistedQueryNotFound error.
+    $this->assertPersistedQueryNotFound($query, $variables);
+
+    // Retry with the query included.
+    $response = $this->apqRequest($this->server->endpoint, $query, $variables, TRUE);
+    $this->assertEquals('Test Article 1', $response['data']['article']['title']);
+
+    // Finally, a request without the query should return the correct data.
+    $response = $this->apqRequest($this->server->endpoint, $query, $variables);
+    $this->assertEquals('Test Article 1', $response['data']['article']['title']);
+  }
+
+}

tests/src/Functional/GraphQLFunctionalTestBase.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace Drupal\Tests\graphql\Functional;
+
+use Drupal\Tests\BrowserTestBase;
+use Drupal\Tests\graphql\Traits\GraphQLFunctionalTestsTrait;
+use Drupal\Tests\graphql\Traits\QueryFileTrait;
+
+/**
+ * The base class for all functional GraphQL tests.
+ */
+abstract class GraphQLFunctionalTestBase extends BrowserTestBase {
+
+  use GraphQLFunctionalTestsTrait;
+  use QueryFileTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'graphql',
+    'typed_data',
+    'graphql_testing_schema',
+    'node',
+    'text',
+    'field',
+    'filter',
+  ];
+
+  /**
+   * This can be removed, once the linked issue is resolved.
+   *
+   * @todo See https://github.com/drupal-graphql/graphql/issues/1177.
+   *
+   * {@inheritdoc}
+   */
+  protected static $configSchemaCheckerExclusions = [
+    'graphql.graphql_servers.testing',
+  ];
+
+}