From 913a9ebdc65e1ff6f704633b09978ec2dbc49367 Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Wed, 28 Jul 2021 17:29:05 +1000 Subject: [PATCH] Issue #3222632 by phenaproxima, imalabya, longwave, catch, larowlan: oEmbed services' cache backends should not be optional --- .../media/src/OEmbed/ProviderRepository.php | 20 +++++++++---- .../media/src/OEmbed/ResourceFetcher.php | 21 ++++++++----- core/modules/media/src/OEmbed/UrlResolver.php | 21 ++++++++----- .../src/ProviderRepository.php | 6 ++-- .../src/Kernel/ProviderRepositoryTest.php | 29 ++++++++++++++++++ .../tests/src/Kernel/ResourceFetcherTest.php | 28 +++++++++++++++++ .../tests/src/Kernel/UrlResolverTest.php | 30 +++++++++++++++++++ .../tests/src/Unit/ResourceFetcherTest.php | 3 +- 8 files changed, 135 insertions(+), 23 deletions(-) create mode 100644 core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php create mode 100644 core/modules/media/tests/src/Kernel/ResourceFetcherTest.php create mode 100644 core/modules/media/tests/src/Kernel/UrlResolverTest.php diff --git a/core/modules/media/src/OEmbed/ProviderRepository.php b/core/modules/media/src/OEmbed/ProviderRepository.php index 8b0a66f87b45..5ee7055ad472 100644 --- a/core/modules/media/src/OEmbed/ProviderRepository.php +++ b/core/modules/media/src/OEmbed/ProviderRepository.php @@ -5,7 +5,6 @@ use Drupal\Component\Datetime\TimeInterface; use Drupal\Component\Serialization\Json; use Drupal\Core\Cache\CacheBackendInterface; -use Drupal\Core\Cache\UseCacheBackendTrait; use Drupal\Core\Config\ConfigFactoryInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\TransferException; @@ -15,8 +14,6 @@ */ class ProviderRepository implements ProviderRepositoryInterface { - use UseCacheBackendTrait; - /** * How long the provider data should be cached, in seconds. * @@ -45,6 +42,13 @@ class ProviderRepository implements ProviderRepositoryInterface { */ protected $time; + /** + * The cache backend. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $cacheBackend; + /** * Constructs a ProviderRepository instance. * @@ -55,7 +59,7 @@ class ProviderRepository implements ProviderRepositoryInterface { * @param \Drupal\Component\Datetime\TimeInterface $time * The time service. * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend - * (optional) The cache backend. + * The cache backend. * @param int $max_age * (optional) How long the cache data should be kept. Defaults to a week. */ @@ -63,6 +67,10 @@ public function __construct(ClientInterface $http_client, ConfigFactoryInterface $this->httpClient = $http_client; $this->providersUrl = $config_factory->get('media.settings')->get('oembed_providers_url'); $this->time = $time; + if (empty($cache_backend)) { + $cache_backend = \Drupal::cache(); + @trigger_error('Passing NULL as the $cache_backend parameter to ' . __METHOD__ . '() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594', E_USER_DEPRECATED); + } $this->cacheBackend = $cache_backend; $this->maxAge = (int) $max_age; } @@ -73,7 +81,7 @@ public function __construct(ClientInterface $http_client, ConfigFactoryInterface public function getAll() { $cache_id = 'media:oembed_providers'; - $cached = $this->cacheGet($cache_id); + $cached = $this->cacheBackend->get($cache_id); if ($cached) { return $cached->data; } @@ -103,7 +111,7 @@ public function getAll() { } } - $this->cacheSet($cache_id, $keyed_providers, $this->time->getCurrentTime() + $this->maxAge); + $this->cacheBackend->set($cache_id, $keyed_providers, $this->time->getCurrentTime() + $this->maxAge); return $keyed_providers; } diff --git a/core/modules/media/src/OEmbed/ResourceFetcher.php b/core/modules/media/src/OEmbed/ResourceFetcher.php index 534102cb5921..921e75a60620 100644 --- a/core/modules/media/src/OEmbed/ResourceFetcher.php +++ b/core/modules/media/src/OEmbed/ResourceFetcher.php @@ -4,7 +4,6 @@ use Drupal\Component\Serialization\Json; use Drupal\Core\Cache\CacheBackendInterface; -use Drupal\Core\Cache\UseCacheBackendTrait; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\TransferException; @@ -13,8 +12,6 @@ */ class ResourceFetcher implements ResourceFetcherInterface { - use UseCacheBackendTrait; - /** * The HTTP client. * @@ -29,6 +26,13 @@ class ResourceFetcher implements ResourceFetcherInterface { */ protected $providers; + /** + * The cache backend. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $cacheBackend; + /** * Constructs a ResourceFetcher object. * @@ -37,13 +41,16 @@ class ResourceFetcher implements ResourceFetcherInterface { * @param \Drupal\media\OEmbed\ProviderRepositoryInterface $providers * The oEmbed provider repository service. * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend - * (optional) The cache backend. + * The cache backend. */ public function __construct(ClientInterface $http_client, ProviderRepositoryInterface $providers, CacheBackendInterface $cache_backend = NULL) { $this->httpClient = $http_client; $this->providers = $providers; + if (empty($cache_backend)) { + $cache_backend = \Drupal::cache(); + @trigger_error('Passing NULL as the $cache_backend parameter to ' . __METHOD__ . '() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594', E_USER_DEPRECATED); + } $this->cacheBackend = $cache_backend; - $this->useCaches = isset($cache_backend); } /** @@ -52,7 +59,7 @@ public function __construct(ClientInterface $http_client, ProviderRepositoryInte public function fetchResource($url) { $cache_id = "media:oembed_resource:$url"; - $cached = $this->cacheGet($cache_id); + $cached = $this->cacheBackend->get($cache_id); if ($cached) { return $this->createResource($cached->data, $url); } @@ -82,7 +89,7 @@ public function fetchResource($url) { throw new ResourceException('The oEmbed resource could not be decoded.', $url); } - $this->cacheSet($cache_id, $data); + $this->cacheBackend->set($cache_id, $data); return $this->createResource($data, $url); } diff --git a/core/modules/media/src/OEmbed/UrlResolver.php b/core/modules/media/src/OEmbed/UrlResolver.php index c8c2d4ee48cd..eb2e6cffbb3b 100644 --- a/core/modules/media/src/OEmbed/UrlResolver.php +++ b/core/modules/media/src/OEmbed/UrlResolver.php @@ -5,7 +5,6 @@ use Drupal\Component\Utility\Html; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Cache\CacheBackendInterface; -use Drupal\Core\Cache\UseCacheBackendTrait; use Drupal\Core\Extension\ModuleHandlerInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\TransferException; @@ -15,8 +14,6 @@ */ class UrlResolver implements UrlResolverInterface { - use UseCacheBackendTrait; - /** * The HTTP client. * @@ -55,6 +52,13 @@ class UrlResolver implements UrlResolverInterface { */ protected $urlCache = []; + /** + * The cache backend. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $cacheBackend; + /** * Constructs a UrlResolver object. * @@ -67,15 +71,18 @@ class UrlResolver implements UrlResolverInterface { * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler service. * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend - * (optional) The cache backend. + * The cache backend. */ public function __construct(ProviderRepositoryInterface $providers, ResourceFetcherInterface $resource_fetcher, ClientInterface $http_client, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache_backend = NULL) { $this->providers = $providers; $this->resourceFetcher = $resource_fetcher; $this->httpClient = $http_client; $this->moduleHandler = $module_handler; + if (empty($cache_backend)) { + $cache_backend = \Drupal::cache(); + @trigger_error('Passing NULL as the $cache_backend parameter to ' . __METHOD__ . '() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594', E_USER_DEPRECATED); + } $this->cacheBackend = $cache_backend; - $this->useCaches = isset($cache_backend); } /** @@ -151,7 +158,7 @@ public function getResourceUrl($url, $max_width = NULL, $max_height = NULL) { // Try to get the resource URL from the persistent cache. $cache_id = "media:oembed_resource_url:$url:$max_width:$max_height"; - $cached = $this->cacheGet($cache_id); + $cached = $this->cacheBackend->get($cache_id); if ($cached) { $this->urlCache[$url] = $cached->data; return $this->urlCache[$url]; @@ -174,7 +181,7 @@ public function getResourceUrl($url, $max_width = NULL, $max_height = NULL) { $resource_url = $parsed_url['path'] . '?' . rawurldecode(UrlHelper::buildQuery($parsed_url['query'])); $this->urlCache[$url] = $resource_url; - $this->cacheSet($cache_id, $resource_url); + $this->cacheBackend->set($cache_id, $resource_url); return $resource_url; } diff --git a/core/modules/media/tests/modules/media_test_oembed/src/ProviderRepository.php b/core/modules/media/tests/modules/media_test_oembed/src/ProviderRepository.php index dc4cb8cbf6fb..aaec13a33fec 100644 --- a/core/modules/media/tests/modules/media_test_oembed/src/ProviderRepository.php +++ b/core/modules/media/tests/modules/media_test_oembed/src/ProviderRepository.php @@ -2,6 +2,7 @@ namespace Drupal\media_test_oembed; +use Drupal\Core\Cache\NullBackend; use Drupal\media\OEmbed\Provider; use Drupal\media\OEmbed\ProviderRepository as BaseProviderRepository; @@ -16,8 +17,9 @@ class ProviderRepository extends BaseProviderRepository { /** * {@inheritdoc} */ - protected function cacheGet($cid) { - return FALSE; + public function __construct(...$arguments) { + parent::__construct(...$arguments); + $this->cacheBackend = new NullBackend('default'); } /** diff --git a/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php b/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php new file mode 100644 index 000000000000..efa0bacc0dd6 --- /dev/null +++ b/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php @@ -0,0 +1,29 @@ +<?php + +namespace Drupal\Tests\media\Kernel; + +use Drupal\KernelTests\KernelTestBase; +use Drupal\media\OEmbed\ProviderRepository; + +/** + * @coversDefaultClass \Drupal\media\OEmbed\ProviderRepository + * + * @group media + */ +class ProviderRepositoryTest extends KernelTestBase { + + /** + * @covers ::__construct + * + * @group legacy + */ + public function testDeprecations(): void { + $this->expectDeprecation('Passing NULL as the $cache_backend parameter to Drupal\media\OEmbed\ProviderRepository::__construct() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594'); + new ProviderRepository( + $this->container->get('http_client'), + $this->container->get('config.factory'), + $this->container->get('datetime.time') + ); + } + +} diff --git a/core/modules/media/tests/src/Kernel/ResourceFetcherTest.php b/core/modules/media/tests/src/Kernel/ResourceFetcherTest.php new file mode 100644 index 000000000000..e68810f348a0 --- /dev/null +++ b/core/modules/media/tests/src/Kernel/ResourceFetcherTest.php @@ -0,0 +1,28 @@ +<?php + +namespace Drupal\Tests\media\Kernel; + +use Drupal\KernelTests\KernelTestBase; +use Drupal\media\OEmbed\ResourceFetcher; + +/** + * @coversDefaultClass \Drupal\media\OEmbed\ResourceFetcher + * + * @group media + */ +class ResourceFetcherTest extends KernelTestBase { + + /** + * @covers ::__construct + * + * @group legacy + */ + public function testDeprecations(): void { + $this->expectDeprecation('Passing NULL as the $cache_backend parameter to Drupal\media\OEmbed\ResourceFetcher::__construct() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594'); + new ResourceFetcher( + $this->container->get('http_client'), + $this->createMock('\Drupal\media\OEmbed\ProviderRepositoryInterface') + ); + } + +} diff --git a/core/modules/media/tests/src/Kernel/UrlResolverTest.php b/core/modules/media/tests/src/Kernel/UrlResolverTest.php new file mode 100644 index 000000000000..7f2b0130bfcd --- /dev/null +++ b/core/modules/media/tests/src/Kernel/UrlResolverTest.php @@ -0,0 +1,30 @@ +<?php + +namespace Drupal\Tests\media\Kernel; + +use Drupal\KernelTests\KernelTestBase; +use Drupal\media\OEmbed\UrlResolver; + +/** + * @coversDefaultClass \Drupal\media\OEmbed\UrlResolver + * + * @group media + */ +class UrlResolverTest extends KernelTestBase { + + /** + * @covers ::__construct + * + * @group legacy + */ + public function testDeprecations(): void { + $this->expectDeprecation('Passing NULL as the $cache_backend parameter to Drupal\media\OEmbed\UrlResolver::__construct() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3223594'); + new UrlResolver( + $this->createMock('\Drupal\media\OEmbed\ProviderRepositoryInterface'), + $this->createMock('\Drupal\media\OEmbed\ResourceFetcherInterface'), + $this->container->get('http_client'), + $this->container->get('module_handler') + ); + } + +} diff --git a/core/modules/media/tests/src/Unit/ResourceFetcherTest.php b/core/modules/media/tests/src/Unit/ResourceFetcherTest.php index 0aef16ed5c06..3d28ee75e763 100644 --- a/core/modules/media/tests/src/Unit/ResourceFetcherTest.php +++ b/core/modules/media/tests/src/Unit/ResourceFetcherTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\media\Unit; use Drupal\Component\Serialization\Json; +use Drupal\Core\Cache\NullBackend; use Drupal\media\OEmbed\ResourceException; use Drupal\media\OEmbed\ResourceFetcher; use Drupal\Tests\UnitTestCase; @@ -50,7 +51,7 @@ public function testUnknownContentTypeHeader(): void { ]); $providers = $this->createMock('\Drupal\media\OEmbed\ProviderRepositoryInterface'); - $fetcher = new ResourceFetcher($client, $providers); + $fetcher = new ResourceFetcher($client, $providers, new NullBackend('default')); /** @var \Drupal\media\OEmbed\Resource $resource */ $resource = $fetcher->fetchResource('valid'); // The resource should have been successfully decoded as JSON. -- GitLab