Issue #3026184 by phenaproxima, nmeegama, kyuubi, vsujeetkumar, carolpettirossi, alexpott, larowlan, seanB, danflanagan8, chr.fritsch: To avoid performance issues, allow tokens in oEmbed thumbnail paths

merge-requests/1009/merge
Alex Pott 2021-10-11 12:20:41 +01:00
parent 9d8e7da4fc
commit adf864ee84
No known key found for this signature in database
GPG Key ID: BDA67E7EE836E5CE
4 changed files with 45 additions and 8 deletions

View File

@ -2,6 +2,7 @@
namespace Drupal\media\Plugin\media\Source;
use Drupal\Component\Render\PlainTextOutput;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\Display\EntityFormDisplayInterface;
@ -14,6 +15,7 @@ use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Url;
use Drupal\Core\Utility\Token;
use Drupal\media\IFrameUrlHelper;
use Drupal\media\OEmbed\Resource;
use Drupal\media\OEmbed\ResourceException;
@ -126,6 +128,13 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
*/
protected $fileSystem;
/**
* The token replacement service.
*
* @var \Drupal\Core\Utility\Token
*/
protected $token;
/**
* Constructs a new OEmbed instance.
*
@ -157,8 +166,10 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
* The iFrame URL helper service.
* @param \Drupal\Core\File\FileSystemInterface $file_system
* The file system.
* @param \Drupal\Core\Utility\Token $token
* The token replacement service.
*/
public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, ConfigFactoryInterface $config_factory, FieldTypePluginManagerInterface $field_type_manager, LoggerInterface $logger, MessengerInterface $messenger, ClientInterface $http_client, ResourceFetcherInterface $resource_fetcher, UrlResolverInterface $url_resolver, IFrameUrlHelper $iframe_url_helper, FileSystemInterface $file_system) {
public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, ConfigFactoryInterface $config_factory, FieldTypePluginManagerInterface $field_type_manager, LoggerInterface $logger, MessengerInterface $messenger, ClientInterface $http_client, ResourceFetcherInterface $resource_fetcher, UrlResolverInterface $url_resolver, IFrameUrlHelper $iframe_url_helper, FileSystemInterface $file_system, Token $token = NULL) {
parent::__construct($configuration, $plugin_id, $plugin_definition, $entity_type_manager, $entity_field_manager, $field_type_manager, $config_factory);
$this->logger = $logger;
$this->messenger = $messenger;
@ -167,6 +178,11 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
$this->urlResolver = $url_resolver;
$this->iFrameUrlHelper = $iframe_url_helper;
$this->fileSystem = $file_system;
if (empty($token)) {
@trigger_error('The token service should be passed to ' . __METHOD__ . '() and is required in drupal:10.0.0. See https://www.drupal.org/node/3240036', E_USER_DEPRECATED);
$token = \Drupal::token();
}
$this->token = $token;
}
/**
@ -187,7 +203,8 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
$container->get('media.oembed.resource_fetcher'),
$container->get('media.oembed.url_resolver'),
$container->get('media.oembed.iframe_url_helper'),
$container->get('file_system')
$container->get('file_system'),
$container->get('token')
);
}
@ -362,7 +379,7 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
*/
public function defaultConfiguration() {
return [
'thumbnails_directory' => 'public://oembed_thumbnails',
'thumbnails_directory' => 'public://oembed_thumbnails/[date:custom:Y-m]',
'providers' => [],
] + parent::defaultConfiguration();
}
@ -392,10 +409,13 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface {
return NULL;
}
// Ensure that we can write to the local directory where thumbnails are
// stored.
// Use the configured directory to store thumbnails. The directory can
// contain basic (i.e., global) tokens. If any of the replaced tokens
// 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);
$directory = PlainTextOutput::renderFromHtml($directory);
// The local thumbnail doesn't exist yet, so try to download it. First,
// ensure that the destination directory is writable, and if it's not,

View File

@ -135,19 +135,36 @@ class OEmbedSourceTest extends MediaKernelTestBase {
$media_type = $this->createMediaType('oembed:video');
$source = $media_type->getSource();
// Add some HTML to the global site slogan, and use the site:slogan token in
// the thumbnail path, in order to prove that the final thumbnail path is
// stripped of HTML tags, and XML entities are decoded.
$this->config('system.site')
->set('slogan', '<h1>this&amp;that</h1>')
->save();
$configuration = $source->getConfiguration();
$configuration['thumbnails_directory'] .= '/[site:slogan]';
$source->setConfiguration($configuration);
$media_type->save();
$media = Media::create([
'bundle' => $media_type->id(),
$source->getSourceFieldDefinition($media_type)->getName() => $this->randomString(),
]);
$media->save();
// The thumbnail directory should include the current date, as per the
// default configuration of the oEmbed source plugin.
$date = date('Y-m', $this->container->get('datetime.time')->getRequestTime());
// The thumbnail should have a file extension, even if it wasn't in the URL.
$expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64($thumbnail_url) . ".$expected_extension";
$expected_uri = "public://oembed_thumbnails/$date/this&that/" . Crypt::hashBase64($thumbnail_url) . ".$expected_extension";
$this->assertSame($expected_uri, $source->getMetadata($media, 'thumbnail_uri'));
// Even if we get the thumbnail_uri more than once, it should only be
// downloaded once. The HTTP client will throw an exception if we try to
// do another request without setting up another response.
$source->getMetadata($media, 'thumbnail_uri');
// The downloaded thumbnail should be usable by the image toolkit.
$this->assertFileExists($expected_uri);
/** @var \Drupal\Core\Image\Image $image */

View File

@ -8,7 +8,7 @@ source: 'oembed:video'
queue_thumbnail_downloads: false
new_revision: true
source_configuration:
thumbnails_directory: 'public://oembed_thumbnails'
thumbnails_directory: 'public://oembed_thumbnails/[date:custom:Y-m]'
providers:
- YouTube
- Vimeo

View File

@ -8,7 +8,7 @@ source: 'oembed:video'
queue_thumbnail_downloads: false
new_revision: true
source_configuration:
thumbnails_directory: 'public://oembed_thumbnails'
thumbnails_directory: 'public://oembed_thumbnails/[date:custom:Y-m]'
providers:
- YouTube
- Vimeo