From 1b747d5bdad54dc9ef12b4c6d5163c374d9780cf Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Fri, 22 Mar 2024 13:56:01 +0000 Subject: [PATCH] Issue #3432599 by amateescu, alecsmrekar, larowlan: The default configuration of the oEmbed source plugin makes pages uncacheable --- .../media/src/Plugin/media/Source/OEmbed.php | 20 ++++++++++++++++--- .../tests/src/Kernel/OEmbedSourceTest.php | 12 +++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/core/modules/media/src/Plugin/media/Source/OEmbed.php b/core/modules/media/src/Plugin/media/Source/OEmbed.php index 23c08b9fdd0f..34b8581a1df2 100644 --- a/core/modules/media/src/Plugin/media/Source/OEmbed.php +++ b/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, diff --git a/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php b/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php index 67b465c450f8..c1b16bbbdf3e 100644 --- a/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php +++ b/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()); } } -- GitLab