diff --git a/core/modules/media/media.services.yml b/core/modules/media/media.services.yml
index 1b49c6ed4b9eb66da0c521faa03e91354110d8bb..db7e79a2bc9c734a8836e088630f3a23d4af57b3 100644
--- a/core/modules/media/media.services.yml
+++ b/core/modules/media/media.services.yml
@@ -12,7 +12,7 @@ services:
arguments: ['@media.oembed.provider_repository', '@media.oembed.resource_fetcher', '@http_client', '@module_handler', '@cache.default']
media.oembed.provider_repository:
class: Drupal\media\OEmbed\ProviderRepository
- arguments: ['@http_client', '@config.factory', '@datetime.time', '@cache.default']
+ arguments: ['@http_client', '@config.factory', '@datetime.time', '@keyvalue', '@logger.factory']
media.oembed.resource_fetcher:
class: Drupal\media\OEmbed\ResourceFetcher
arguments: ['@http_client', '@media.oembed.provider_repository', '@cache.default']
diff --git a/core/modules/media/src/OEmbed/ProviderRepository.php b/core/modules/media/src/OEmbed/ProviderRepository.php
index 5ee7055ad4721ecdc048d842acd2c75ec314a0f4..159bafda55755ffb60bce8ac7c1eaefa93211723 100644
--- a/core/modules/media/src/OEmbed/ProviderRepository.php
+++ b/core/modules/media/src/OEmbed/ProviderRepository.php
@@ -4,8 +4,10 @@
use Drupal\Component\Datetime\TimeInterface;
use Drupal\Component\Serialization\Json;
-use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\DependencyInjection\DeprecatedServicePropertyTrait;
+use Drupal\Core\KeyValueStore\KeyValueFactoryInterface;
+use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use GuzzleHttp\ClientInterface;
use GuzzleHttp\Exception\TransferException;
@@ -14,6 +16,17 @@
*/
class ProviderRepository implements ProviderRepositoryInterface {
+ use DeprecatedServicePropertyTrait;
+
+ /**
+ * The service properties that should raise a deprecation error.
+ *
+ * @var string[]
+ */
+ private $deprecatedProperties = [
+ 'cacheBackend' => 'cache.default',
+ ];
+
/**
* How long the provider data should be cached, in seconds.
*
@@ -43,11 +56,18 @@ class ProviderRepository implements ProviderRepositoryInterface {
protected $time;
/**
- * The cache backend.
+ * The key-value store.
+ *
+ * @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface
+ */
+ protected $keyValue;
+
+ /**
+ * The logger channel.
*
- * @var \Drupal\Core\Cache\CacheBackendInterface
+ * @var \Drupal\Core\Logger\LoggerChannelInterface
*/
- protected $cacheBackend;
+ protected $logger;
/**
* Constructs a ProviderRepository instance.
@@ -58,44 +78,76 @@ class ProviderRepository implements ProviderRepositoryInterface {
* The config factory service.
* @param \Drupal\Component\Datetime\TimeInterface $time
* The time service.
- * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend
- * The cache backend.
+ * @param \Drupal\Core\KeyValueStore\KeyValueFactoryInterface $key_value_factory
+ * The key-value store factory.
+ * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory
+ * The logger channel factory.
* @param int $max_age
* (optional) How long the cache data should be kept. Defaults to a week.
*/
- public function __construct(ClientInterface $http_client, ConfigFactoryInterface $config_factory, TimeInterface $time, CacheBackendInterface $cache_backend = NULL, $max_age = 604800) {
+ public function __construct(ClientInterface $http_client, ConfigFactoryInterface $config_factory, TimeInterface $time, $key_value_factory = NULL, $logger_factory = NULL, int $max_age = 604800) {
$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);
+ if (!($key_value_factory instanceof KeyValueFactoryInterface)) {
+ @trigger_error('The keyvalue service should be passed to ' . __METHOD__ . '() since drupal:9.3.0 and is required in drupal:10.0.0. See https://www.drupal.org/node/3186186', E_USER_DEPRECATED);
+ $key_value_factory = \Drupal::service('keyvalue');
+ }
+ if (!($logger_factory instanceof LoggerChannelFactoryInterface)) {
+ // If $max_age was passed in $logger_factory's position, ensure that we
+ // use the correct value.
+ if (is_numeric($logger_factory)) {
+ $max_age = $logger_factory;
+ }
+ @trigger_error('The logger.factory service should be passed to ' . __METHOD__ . '() since drupal:9.3.0 and is required in drupal:10.0.0. See https://www.drupal.org/node/3186186', E_USER_DEPRECATED);
+ $logger_factory = \Drupal::service('logger.factory');
}
- $this->cacheBackend = $cache_backend;
- $this->maxAge = (int) $max_age;
+ $this->maxAge = $max_age;
+ $this->keyValue = $key_value_factory->get('media');
+ $this->logger = $logger_factory->get('media');
}
/**
* {@inheritdoc}
*/
public function getAll() {
- $cache_id = 'media:oembed_providers';
-
- $cached = $this->cacheBackend->get($cache_id);
- if ($cached) {
- return $cached->data;
+ $current_time = $this->time->getCurrentTime();
+ $stored = $this->keyValue->get('oembed_providers');
+ // If we have stored data that hasn't yet expired, return that. We need to
+ // store the data in a key-value store because, if the remote provider
+ // database is unavailable, we'd rather return stale data than throw an
+ // exception. This means we cannot use a normal cache backend or expirable
+ // key-value store, since those could delete the stale data at any time.
+ if ($stored && $stored['expires'] > $current_time) {
+ return $stored['data'];
}
try {
$response = $this->httpClient->request('GET', $this->providersUrl);
}
catch (TransferException $e) {
+ if (isset($stored['data'])) {
+ // Use the stale data to fall back gracefully, but warn site
+ // administrators that we used stale data.
+ $this->logger->warning('Remote oEmbed providers could not be retrieved due to error: @error. Using previously stored data. This may contain out of date information.', [
+ '@error' => $e->getMessage(),
+ ]);
+ return $stored['data'];
+ }
+ // We have no previous data and the request failed.
throw new ProviderException("Could not retrieve the oEmbed provider database from $this->providersUrl", NULL, $e);
}
$providers = Json::decode((string) $response->getBody());
if (!is_array($providers) || empty($providers)) {
+ if (isset($stored['data'])) {
+ // Use the stale data to fall back gracefully, but as above, warn site
+ // administrators that we used stale data.
+ $this->logger->warning('Remote oEmbed providers database returned invalid or empty list. Using previously stored data. This may contain out of date information.');
+ return $stored['data'];
+ }
+ // We have no previous data and the current data is corrupt.
throw new ProviderException('Remote oEmbed providers database returned invalid or empty list.');
}
@@ -111,7 +163,10 @@ public function getAll() {
}
}
- $this->cacheBackend->set($cache_id, $keyed_providers, $this->time->getCurrentTime() + $this->maxAge);
+ $this->keyValue->set('oembed_providers', [
+ 'data' => $keyed_providers,
+ 'expires' => $current_time + $this->maxAge,
+ ]);
return $keyed_providers;
}
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 aaec13a33fecf9404adb3dd53df55d2b97238774..a80ba32fe483d7e661a31186f169c74e3447a381 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,7 +2,6 @@
namespace Drupal\media_test_oembed;
-use Drupal\Core\Cache\NullBackend;
use Drupal\media\OEmbed\Provider;
use Drupal\media\OEmbed\ProviderRepository as BaseProviderRepository;
@@ -14,14 +13,6 @@
*/
class ProviderRepository extends BaseProviderRepository {
- /**
- * {@inheritdoc}
- */
- public function __construct(...$arguments) {
- parent::__construct(...$arguments);
- $this->cacheBackend = new NullBackend('default');
- }
-
/**
* {@inheritdoc}
*/
diff --git a/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php b/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php
index efa0bacc0dd6edf193329fdd64194e3679cfa7db..66dbff2f36aa3b9a7dde0c9b84560b1d53f26017 100644
--- a/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php
+++ b/core/modules/media/tests/src/Kernel/ProviderRepositoryTest.php
@@ -2,6 +2,8 @@
namespace Drupal\Tests\media\Kernel;
+use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Cache\NullBackend;
use Drupal\KernelTests\KernelTestBase;
use Drupal\media\OEmbed\ProviderRepository;
@@ -18,12 +20,26 @@ class ProviderRepositoryTest extends KernelTestBase {
* @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(
+ // Passing a cache backend in the key-value store's place, and the max age
+ // in the logger factory's place, should raise deprecation notices.
+ $this->expectDeprecation('The keyvalue service should be passed to Drupal\media\OEmbed\ProviderRepository::__construct() since drupal:9.3.0 and is required in drupal:10.0.0. See https://www.drupal.org/node/3186186');
+ $this->expectDeprecation('The logger.factory service should be passed to Drupal\media\OEmbed\ProviderRepository::__construct() since drupal:9.3.0 and is required in drupal:10.0.0. See https://www.drupal.org/node/3186186');
+ $providers = new ProviderRepository(
$this->container->get('http_client'),
$this->container->get('config.factory'),
- $this->container->get('datetime.time')
+ $this->container->get('datetime.time'),
+ new NullBackend('test'),
+ 86400
);
+ $this->expectDeprecation('The property cacheBackend (cache.default service) is deprecated in Drupal\media\OEmbed\ProviderRepository and will be removed before Drupal 10.0.0.');
+ $this->assertInstanceOf(CacheBackendInterface::class, $providers->cacheBackend);
+
+ // Ensure that the $max_age was properly set, even though it was passed in
+ // the logger factory's position.
+ $reflector = new \ReflectionClass($providers);
+ $property = $reflector->getProperty('maxAge');
+ $property->setAccessible(TRUE);
+ $this->assertSame(86400, $property->getValue($providers));
}
}
diff --git a/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php b/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..359c3cc87d60f04084e8e1b9a53fb0cd8a441b8b
--- /dev/null
+++ b/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php
@@ -0,0 +1,234 @@
+<?php
+
+namespace Drupal\Tests\media\Unit;
+
+use Drupal\Core\KeyValueStore\KeyValueMemoryFactory;
+use Drupal\Core\Logger\LoggerChannelFactory;
+use Drupal\media\OEmbed\ProviderException;
+use Drupal\media\OEmbed\ProviderRepository;
+use Drupal\Tests\UnitTestCase;
+use GuzzleHttp\Client;
+use GuzzleHttp\Handler\MockHandler;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Psr7\Response;
+
+/**
+ * @coversDefaultClass \Drupal\media\OEmbed\ProviderRepository
+ *
+ * @group media
+ */
+class ProviderRepositoryTest extends UnitTestCase {
+
+ /**
+ * The provider repository under test.
+ *
+ * @var \Drupal\media\OEmbed\ProviderRepository
+ */
+ private $repository;
+
+ /**
+ * The HTTP client handler which will serve responses.
+ *
+ * @var \GuzzleHttp\Handler\MockHandler
+ */
+ private $responses;
+
+ /**
+ * The key-value store.
+ *
+ * @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface
+ */
+ private $keyValue;
+
+ /**
+ * The time that the current test began.
+ *
+ * @var int
+ */
+ private $currentTime;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ $config_factory = $this->getConfigFactoryStub([
+ 'media.settings' => [
+ 'oembed_providers_url' => 'https://oembed.com/providers.json',
+ ],
+ ]);
+
+ $key_value_factory = new KeyValueMemoryFactory();
+ $this->keyValue = $key_value_factory->get('media');
+
+ $this->currentTime = time();
+ $time = $this->prophesize('\Drupal\Component\Datetime\TimeInterface');
+ $time->getCurrentTime()->willReturn($this->currentTime);
+
+ $this->responses = new MockHandler();
+ $client = new Client([
+ 'handler' => HandlerStack::create($this->responses),
+ ]);
+ $this->repository = new ProviderRepository(
+ $client,
+ $config_factory,
+ $time->reveal(),
+ $key_value_factory,
+ new LoggerChannelFactory()
+ );
+ }
+
+ /**
+ * Tests that a successful fetch stores the provider database in key-value.
+ */
+ public function testSuccessfulFetch(): void {
+ $body = <<<END
+[
+ {
+ "provider_name": "YouTube",
+ "provider_url": "https:\/\/www.youtube.com\/",
+ "endpoints": [
+ {
+ "schemes": [
+ "https:\/\/*.youtube.com\/watch*",
+ "https:\/\/*.youtube.com\/v\/*"
+ ],
+ "url": "https:\/\/www.youtube.com\/oembed",
+ "discovery": true
+ }
+ ]
+ }
+]
+END;
+ $response = new Response(200, [], $body);
+ $this->responses->append($response);
+
+ $provider = $this->repository->get('YouTube');
+ $stored_data = [
+ 'data' => [
+ 'YouTube' => $provider,
+ ],
+ 'expires' => $this->currentTime + 604800,
+ ];
+ $this->assertSame($stored_data, $this->keyValue->get('oembed_providers'));
+ }
+
+ /**
+ * Tests handling of invalid JSON when fetching the provider database.
+ *
+ * @param int $expiration_offset
+ * An offset to add to the current time to determine when the primed data,
+ * if any, expires.
+ *
+ * @dataProvider providerInvalidResponse
+ */
+ public function testInvalidResponse(int $expiration_offset): void {
+ $provider = $this->prophesize('\Drupal\media\OEmbed\Provider')
+ ->reveal();
+
+ // This stored data should be returned, irrespective of whether it's fresh.
+ $this->keyValue->set('oembed_providers', [
+ 'data' => [
+ 'YouTube' => $provider,
+ ],
+ 'expires' => $this->currentTime + $expiration_offset,
+ ]);
+
+ $response = new Response(200, [], "This certainly isn't valid JSON.");
+ $this->responses->append($response, $response);
+ $this->assertSame($provider, $this->repository->get('YouTube'));
+
+ // When there is no stored data, we should get an exception.
+ $this->keyValue->delete('oembed_providers');
+ $this->expectException(ProviderException::class);
+ $this->repository->get('YouTube');
+ }
+
+ /**
+ * Data provider for ::testInvalidResponse().
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerInvalidResponse(): array {
+ return [
+ 'expired' => [
+ -86400,
+ ],
+ 'fresh' => [
+ 86400,
+ ],
+ ];
+ }
+
+ /**
+ * Tests handling of exceptions when fetching the provider database.
+ */
+ public function testRequestException(): void {
+ $provider = $this->prophesize('\Drupal\media\OEmbed\Provider')
+ ->reveal();
+
+ // This data is expired (stale), but it should be returned anyway.
+ $this->keyValue->set('oembed_providers', [
+ 'data' => [
+ 'YouTube' => $provider,
+ ],
+ 'expires' => $this->currentTime - 86400,
+ ]);
+
+ $response = new Response(503);
+ $this->responses->append($response, $response);
+ $this->assertSame($provider, $this->repository->get('YouTube'));
+
+ // When there is no stored data, we should get an exception.
+ $this->keyValue->delete('oembed_providers');
+ $this->expectException(ProviderException::class);
+ $this->repository->get('YouTube');
+ }
+
+ /**
+ * Tests a successful fetch but with a single corrupt item.
+ */
+ public function testCorruptProviderIgnored(): void {
+ $body = <<<END
+[
+ {
+ "provider_name": "YouTube",
+ "provider_url": "https:\/\/www.youtube.com\/",
+ "endpoints": [
+ {
+ "schemes": [
+ "https:\/\/*.youtube.com\/watch*",
+ "https:\/\/*.youtube.com\/v\/*"
+ ],
+ "url": "https:\/\/www.youtube.com\/oembed",
+ "discovery": true
+ }
+ ]
+ },
+ {
+ "provider_name": "Uncle Rico's football videos",
+ "provider_url": "not a real url",
+ "endpoints": []
+ }
+]
+END;
+ $response = new Response(200, [], $body);
+ $this->responses->append($response);
+
+ $youtube = $this->repository->get('YouTube');
+ // The corrupt provider should not be stored.
+ $stored_data = [
+ 'data' => [
+ 'YouTube' => $youtube,
+ ],
+ 'expires' => $this->currentTime + 604800,
+ ];
+ $this->assertSame($stored_data, $this->keyValue->get('oembed_providers'));
+
+ $this->expectException('InvalidArgumentException');
+ $this->repository->get("Uncle Rico's football videos");
+ }
+
+}