Problem/Motivation
#3026184: To avoid performance issues, allow tokens in oEmbed thumbnail paths added a date token ([date:custom:Y-m]
) to the thumbnails_directory
default configuration of the oEmbed source. However, whenever a date token is used without specifying a default replacement value for it, system_tokens()
makes the page uncacheable:
if (empty($data['date'])) {
$date = \Drupal::time()->getRequestTime();
// We depend on the current request time, so the tokens are not cacheable
// at all.
$bubbleable_metadata->setCacheMaxAge(0);
}
Steps to reproduce
If you enable cache header debugging and execute this code in a render context (e.g. any Drupal page that displays the thumbnail of an oembed:video media), you will get the x-drupal-dynamic-cache: UNCACHEABLE
header.
$media_type = $this->createMediaType('oembed:video');
$source = $media_type->getSource();
$source->getMetadata($media, 'thumbnail_uri')
Proposed resolution
Pass the media entity creation date as additional data for token replacement in \Drupal\media\Plugin\media\Source\OEmbed::getLocalThumbnailUri()
.
Remaining tasks
Review.
User interface changes
Nope.
API changes
Nope.
Data model changes
Nope.
Release notes snippet
Nope.
Issue fork drupal-3432599
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
amateescu CreditAttribution: amateescu for Tag1 Consulting commentedOpened a MR with the fix and test coverage.
Comment #5
smustgrave CreditAttribution: smustgrave at Mobomo commentedSeems test-only feature has been ran, results can be seen here https://git.drupalcode.org/issue/drupal-3432599/-/jobs/1119277
Passing in a new parameter to a protected function in a plugin don't believe is an issue that needs any BC check.
So making that assumption change looks good.
Comment #6
larowlanI think we should try to add a BC layer here, this feels like something people might have overridden in custom code
Comment #7
amateescu CreditAttribution: amateescu for Tag1 Consulting commentedAdded BC for the new argument and a CR: https://www.drupal.org/node/3432920
Comment #9
catchGood spot. Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!