Issue #3432599 by amateescu, alecsmrekar, larowlan: The default configuration...

Issue #3432599 by amateescu, alecsmrekar, larowlan: The default configuration of the oEmbed source plugin makes pages uncacheable

core/modules/media/src/Plugin/media/Source/OEmbed.php

@@ -257,7 +257,7 @@ public function getMetadata(MediaInterface $media, $name) {
         return parent::getMetadata($media, 'default_name');
 
       case 'thumbnail_uri':
-        return $this->getLocalThumbnailUri($resource) ?: parent::getMetadata($media, 'thumbnail_uri');
+        return $this->getLocalThumbnailUri($resource, $media) ?: parent::getMetadata($media, 'thumbnail_uri');
 
       case 'type':
         return $resource->getType();
@@ -387,6 +387,8 @@ public function defaultConfiguration() {
    *
    * @param \Drupal\media\OEmbed\Resource $resource
    *   The oEmbed resource.
+   * @param \Drupal\media\MediaInterface|null $media
+   *   The media entity that contains the resource.
    *
    * @return string|null
    *   The local thumbnail URI, or NULL if it could not be downloaded, or if the
@@ -397,7 +399,15 @@ public function defaultConfiguration() {
    * toggle-able. See https://www.drupal.org/project/drupal/issues/2962751 for
    * more information.
    */
-  protected function getLocalThumbnailUri(Resource $resource) {
+  protected function getLocalThumbnailUri(Resource $resource, MediaInterface $media = NULL) {
+    if (is_null($media)) {
+      @trigger_error('Calling ' . __METHOD__ . '() without the $media argument is deprecated in drupal:10.3.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3432920', E_USER_DEPRECATED);
+      $token_data = [];
+    }
+    else {
+      $token_data = ['date' => $media->getCreatedTime()];
+    }
+
     // If there is no remote thumbnail, there's nothing for us to fetch here.
     $remote_thumbnail_url = $resource->getThumbnailUrl();
     if (!$remote_thumbnail_url) {
@@ -409,7 +419,11 @@ protected function getLocalThumbnailUri(Resource $resource) {
     // contain HTML, the tags will be removed and XML entities will be decoded.
     $configuration = $this->getConfiguration();
     $directory = $configuration['thumbnails_directory'];
-    $directory = $this->token->replace($directory);
+    // The thumbnail directory might contain a date token, so we pass in the
+    // creation date of the media entity so that the token won't rely on the
+    // current request time, making the current request have a max-age of 0.
+    // @see system_tokens() for $type == 'date'.
+    $directory = $this->token->replace($directory, $token_data);
     $directory = PlainTextOutput::renderFromHtml($directory);
 
     // The local thumbnail doesn't exist yet, so try to download it. First,

core/modules/media/tests/src/Kernel/OEmbedSourceTest.php

@@ -3,6 +3,8 @@
 namespace Drupal\Tests\media\Kernel;
 
 use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Cache\Cache;
+use Drupal\Core\Render\RenderContext;
 use Drupal\media\Entity\Media;
 use Drupal\media\OEmbed\Resource;
 use Drupal\media\OEmbed\ResourceFetcherInterface;
@@ -170,6 +172,16 @@ public function testThumbnailUri(string $remote_thumbnail_url, array $thumbnail_
     /** @var \Drupal\Core\Image\Image $image */
     $image = $this->container->get('image.factory')->get($expected_uri);
     $this->assertTrue($image->isValid());
+
+    // Check that the current date token as per the default configuration of the
+    // oEmbed source plugin doesn't make a render context uncacheable.
+    $context = new RenderContext();
+    \Drupal::service('renderer')->executeInRenderContext($context, function () use ($source, $media) {
+      return $source->getMetadata($media, 'thumbnail_uri');
+    });
+    /** @var \Drupal\Core\Render\BubbleableMetadata $bubbleable_metadata */
+    $bubbleable_metadata = $context->pop();
+    $this->assertSame(Cache::PERMANENT, $bubbleable_metadata->getCacheMaxAge());
   }
 
 }