Fixing sub request buffer and cache metadata collection. (#547)

fubhy · web-flow · commit 58a9b7464bc6 · 2018-03-09T11:24:36.000+01:00
diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/Blocks/BlocksByRegion.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/Blocks/BlocksByRegion.php
@@ -141,6 +141,8 @@ protected function resolveValues($value, array $args, ResolveContext $context, R
       });
 
       return function ($value, array $args, ResolveContext $context, ResolveInfo $info) use ($resolve) {
+        /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */
+        $response = $resolve();
         $metadata = new CacheableMetadata();
         $metadata->addCacheTags(['config:block_list']);
         $blocks = array_map(function (Block $block) {
@@ -151,10 +153,10 @@ protected function resolveValues($value, array $args, ResolveContext $context, R
           else {
             return $block;
           }
-        }, $resolve());
+        }, $response->getValue());
 
         foreach ($blocks as $block) {
-          yield new CacheableValue($block, [$metadata]);
+          yield new CacheableValue($block, [$metadata, $response]);
         }
       };
     }
diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/Breadcrumbs/Breadcrumbs.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/Breadcrumbs/Breadcrumbs.php
@@ -7,6 +7,7 @@
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\Core\Url;
 use Drupal\graphql\GraphQL\Buffers\SubRequestBuffer;
+use Drupal\graphql\GraphQL\Cache\CacheableValue;
 use Drupal\graphql\GraphQL\Execution\ResolveContext;
 use Drupal\graphql\Plugin\GraphQL\Fields\FieldPluginBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -101,9 +102,12 @@ protected function resolveValues($value, array $args, ResolveContext $context, R
       });
 
       return function ($value, array $args, ResolveContext $context, ResolveInfo $info) use ($resolve) {
-        $links = $resolve();
+        /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */
+        $response = $resolve();
+        $links = $response->getValue();
+
         foreach ($links as $link) {
-          yield $link;
+          yield new CacheableValue($link, [$response]);
         }
       };
     }
diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/LanguageSwitch/LanguageSwitchLinks.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/LanguageSwitch/LanguageSwitchLinks.php
@@ -7,6 +7,7 @@
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Url;
 use Drupal\graphql\GraphQL\Buffers\SubRequestBuffer;
+use Drupal\graphql\GraphQL\Cache\CacheableValue;
 use Drupal\graphql\GraphQL\Execution\ResolveContext;
 use Drupal\graphql\Plugin\GraphQL\Fields\FieldPluginBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -82,16 +83,18 @@ protected function resolveValues($value, array $args, ResolveContext $context, R
       });
 
       return function () use ($resolve) {
-        list($current, $links) = $resolve();
+        /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */
+        $response = $resolve();
+        list($current, $links) = $response->getValue();
 
         if (!empty($links->links)) {
           foreach ($links->links as $link) {
             // Yield the link array and the language object of the language
             // context resolved from the sub-request.
-            yield [
+            yield new CacheableValue([
               'link' => $link,
               'context' => $current,
-            ];
+            ], [$response]);
           }
         }
       };
diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/Routing/InternalUrl/InternalRequest.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/Routing/InternalUrl/InternalRequest.php
@@ -5,6 +5,7 @@
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Url;
 use Drupal\graphql\GraphQL\Buffers\SubRequestBuffer;
+use Drupal\graphql\GraphQL\Cache\CacheableValue;
 use Drupal\graphql\GraphQL\Execution\ResolveContext;
 use Drupal\graphql\Plugin\GraphQL\Fields\FieldPluginBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -114,7 +115,9 @@ protected function resolveValues($value, array $args, ResolveContext $context, R
       });
 
       return function ($value, array $args, ResolveContext $context, ResolveInfo $info) use ($resolve) {
-        yield $resolve();
+        /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */
+        $response = $resolve();
+        yield new CacheableValue($response->getValue(), [$response]);
       };
     }
   }
diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/Routing/RouteEntity.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/Routing/RouteEntity.php
@@ -172,8 +172,11 @@ protected function resolveEntityTranslation(EntityInterface $entity, Url $url, a
     });
 
     return function ($value, array $args, ResolveContext $context, ResolveInfo $info) use ($resolve, $entity) {
-      $language = $resolve();
-      $entity = $this->entityRepository->getTranslationFromContext($entity, $language);
+      /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */
+      $response = $resolve();
+      $entity = $this->entityRepository->getTranslationFromContext($entity, $response->getValue());
+      $entity->addCacheableDependency($response);
+
       return $this->resolveEntity($entity, $value, $args, $info);
     };
   }
diff --git a/src/Controller/SubrequestExtractionController.php b/src/Controller/SubrequestExtractionController.php
@@ -4,6 +4,8 @@
 
 use Drupal\Core\Controller\ControllerBase;
 use Drupal\Core\Language\LanguageManagerInterface;
+use Drupal\Core\Render\RenderContext;
+use Drupal\Core\Render\RendererInterface;
 use Drupal\graphql\GraphQL\Buffers\SubRequestResponse;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -20,22 +22,38 @@ class SubrequestExtractionController extends ControllerBase {
    */
   protected $requestStack;
 
+  /**
+   * The renderer service.
+   *
+   * @var \Drupal\Core\Render\RendererInterface
+   */
+  protected $renderer;
+
   /**
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('request_stack'),
-      $container->get('language_manager')
+      $container->get('language_manager'),
+      $container->get('renderer')
     );
   }
 
   /**
-   * {@inheritdoc}
+   * SubrequestExtractionController constructor.
+   *
+   * @param \Symfony\Component\HttpFoundation\RequestStack $requestStack
+   *   The request stack.
+   * @param \Drupal\Core\Language\LanguageManagerInterface $languageManager
+   *   The language manager service.
+   * @param \Drupal\Core\Render\RendererInterface $renderer
+   *   The rewnderer service.
    */
-  public function __construct(RequestStack $requestStack, LanguageManagerInterface $languageManager) {
+  public function __construct(RequestStack $requestStack, LanguageManagerInterface $languageManager, RendererInterface $renderer) {
     $this->requestStack = $requestStack;
     $this->languageManager = $languageManager;
+    $this->renderer = $renderer;
   }
 
   /**
@@ -51,7 +69,18 @@ public function extract() {
     // TODO: Remove this once https://www.drupal.org/project/drupal/issues/2940036#comment-12479912 is resolved.
     $this->languageManager->reset();
 
-    return new SubRequestResponse($callback());
+    // Collect any potentially leaked cache metadata released by the callback.
+    $context = new RenderContext();
+    $result = $this->renderer->executeInRenderContext($context, function () use ($callback) {
+      return $callback();
+    });
+
+    $response = new SubRequestResponse($result);
+    if (!$context->isEmpty()) {
+      $response->addCacheableDependency($context->pop());
+    }
+
+    return $response;
   }
 
 }
diff --git a/src/GraphQL/Buffers/SubRequestBuffer.php b/src/GraphQL/Buffers/SubRequestBuffer.php
@@ -2,7 +2,9 @@
 
 namespace Drupal\graphql\GraphQL\Buffers;
 
+use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Url;
+use Drupal\graphql\GraphQL\Cache\CacheableValue;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
@@ -60,20 +62,27 @@ public function add(Url $url, callable $extract) {
    * {@inheritdoc}
    */
   protected function getBufferId($item) {
-    /** @var \Drupal\Core\Url $url */
-    $url = $item['url'];
-    return $url->toString();
+    /** @var \Drupal\Core\GeneratedUrl $url */
+    $url = $item['url']->toString(TRUE);
+
+    return hash('sha256', json_encode([
+      'url' => $url->getGeneratedUrl(),
+      'tags' => $url->getCacheTags(),
+      'contexts' => $url->getCacheContexts(),
+      'age' => $url->getCacheMaxAge(),
+    ]));
   }
 
   /**
    * {@inheritdoc}
    */
   public function resolveBufferArray(array $buffer) {
-    /** @var \Drupal\Core\Url $url */
-    $url = reset($buffer)['url'];
+    /** @var \Drupal\Core\GeneratedUrl $url */
+    $url = reset($buffer)['url']->toString(TRUE);
+
     $currentRequest = $this->requestStack->getCurrentRequest();
     $request = Request::create(
-      $url->getOption('routed_path') ?: $url->toString(),
+      $url->getGeneratedUrl(),
       'GET',
       $currentRequest->query->all(),
       $currentRequest->cookies->all(),
@@ -93,6 +102,9 @@ public function resolveBufferArray(array $buffer) {
 
     /** @var \Drupal\graphql\GraphQL\Buffers\SubRequestResponse $response */
     $response = $this->httpKernel->handle($request, HttpKernelInterface::SUB_REQUEST);
+    if ($url instanceof CacheableDependencyInterface) {
+      $response->addCacheableDependency($url);
+    }
 
     // TODO:
     // Remove the request stack manipulation once the core issue described at
@@ -101,7 +113,9 @@ public function resolveBufferArray(array $buffer) {
       $this->requestStack->pop();
     }
 
-    return $response->getResult();
+    return array_map(function ($value) use ($response) {
+      return new CacheableValue($value, [$response]);
+    }, $response->getResult());
   }
 
 }
diff --git a/src/GraphQL/Buffers/SubRequestResponse.php b/src/GraphQL/Buffers/SubRequestResponse.php
@@ -2,9 +2,12 @@
 
 namespace Drupal\graphql\GraphQL\Buffers;
 
+use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
+use Drupal\Core\Cache\RefinableCacheableDependencyTrait;
 use Symfony\Component\HttpFoundation\Response;
 
-class SubRequestResponse extends Response {
+class SubRequestResponse extends Response implements RefinableCacheableDependencyInterface {
+  use RefinableCacheableDependencyTrait;
 
   /**
    * The request result.

Original file line number	Diff line number	Diff line change
`@@ -141,6 +141,8 @@ protected function resolveValues($value, array $args, ResolveContext $context, R`
`141`	`141`	`});`
`142`	`142`
`143`	`143`	`return function ($value, array $args, ResolveContext $context, ResolveInfo $info) use ($resolve) {`
	`144`	`+ /** @var \Drupal\graphql\GraphQL\Cache\CacheableValue $response */`
	`145`	`+ $response = $resolve();`
`144`	`146`	`$metadata = new CacheableMetadata();`
`145`	`147`	`$metadata->addCacheTags(['config:block_list']);`
`146`	`148`	`$blocks = array_map(function (Block $block) {`
`@@ -151,10 +153,10 @@ protected function resolveValues($value, array $args, ResolveContext $context, R`
`151`	`153`	`else {`
`152`	`154`	`return $block;`
`153`	`155`	`}`
`154`		`- }, $resolve());`
	`156`	`+ }, $response->getValue());`
`155`	`157`
`156`	`158`	`foreach ($blocks as $block) {`
`157`		`- yield new CacheableValue($block, [$metadata]);`
	`159`	`+ yield new CacheableValue($block, [$metadata, $response]);`
`158`	`160`	`}`
`159`	`161`	`};`
`160`	`162`	`}`