Commit 57818db8 authored by catch's avatar catch

Issue #2990896 by phenaproxima, effulgentsia, seanB, xjm: Move Media::save()...

Issue #2990896 by phenaproxima, effulgentsia, seanB, xjm: Move Media::save() logic into storage handler to prevent data integrity issues when media items with remote content are saved

(cherry picked from commit 1e7b231b)
parent 906bd12f
......@@ -11,3 +11,10 @@
function media_post_update_collection_route() {
// Empty post-update hook.
}
/**
* Clear caches due to the addition of a Media-specific entity storage handler.
*/
function media_post_update_storage_handler() {
// Empty post-update hook.
}
......@@ -29,7 +29,7 @@
* ),
* bundle_label = @Translation("Media type"),
* handlers = {
* "storage" = "Drupal\Core\Entity\Sql\SqlContentEntityStorage",
* "storage" = "Drupal\media\MediaStorage",
* "view_builder" = "Drupal\Core\Entity\EntityViewBuilder",
* "list_builder" = "Drupal\media\MediaListBuilder",
* "access" = "Drupal\media\MediaAccessControlHandler",
......@@ -374,15 +374,22 @@ public function preSaveRevision(EntityStorageInterface $storage, \stdClass $reco
}
/**
* {@inheritdoc}
* Sets the media entity's field values from the source's metadata.
*
* Fetching the metadata could be slow (e.g., if requesting it from a remote
* API), so this is called by \Drupal\media\MediaStorage::save() prior to it
* beginning the database transaction, whereas static::preSave() executes
* after the transaction has already started.
*
* @internal
* Expose this as an API in
* https://www.drupal.org/project/drupal/issues/2992426.
*/
public function save() {
public function prepareSave() {
// @todo If the source plugin talks to a remote API (e.g. oEmbed), this code
// might be performing a fair number of HTTP requests. This is dangerously
// brittle and should probably be handled by a queue, to avoid doing HTTP
// operations during entity save. As it is, doing this before calling
// parent::save() is a quick-fix to avoid doing HTTP requests in the middle
// of a database transaction (which begins once we call parent::save()). See
// operations during entity save. See
// https://www.drupal.org/project/drupal/issues/2976875 for more.
// In order for metadata to be mapped correctly, $this->original must be
......@@ -419,7 +426,6 @@ public function save() {
}
}
}
return parent::save();
}
/**
......
<?php
namespace Drupal\media;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
/**
* Defines the storage handler class for media.
*
* The default storage is overridden to handle metadata fetching outside of the
* database transaction.
*/
class MediaStorage extends SqlContentEntityStorage {
/**
* {@inheritdoc}
*/
public function save(EntityInterface $media) {
// For backwards compatibility, modules that override the Media entity
// class, are not required to implement the prepareSave() method.
// @todo For Drupal 8.7, consider throwing a deprecation notice if the
// method doesn't exist. See
// https://www.drupal.org/project/drupal/issues/2992426 for further
// discussion.
if (method_exists($media, 'prepareSave')) {
$media->prepareSave();
}
return parent::save($media);
}
}
......@@ -15,6 +15,73 @@
*/
class MediaSourceTest extends MediaKernelTestBase {
/**
* Tests that metadata is correctly mapped irrespective of how media is saved.
*/
public function testSave() {
$field_storage = FieldStorageConfig::create([
'entity_type' => 'media',
'field_name' => 'field_to_map_to',
'type' => 'string',
]);
$field_storage->save();
FieldConfig::create([
'field_storage' => $field_storage,
'bundle' => $this->testMediaType->id(),
'label' => 'Field to map to',
])->save();
// Set an arbitrary metadata value to be mapped.
$this->container->get('state')
->set('media_source_test_attributes', [
'attribute_to_map' => [
'title' => 'Attribute to map',
'value' => 'Snowball',
],
'thumbnail_uri' => [
'title' => 'Thumbnail',
'value' => 'public://TheSisko.png',
],
]);
$this->testMediaType->setFieldMap([
'attribute_to_map' => 'field_to_map_to',
])->save();
/** @var \Drupal\Core\Entity\EntityStorageInterface $storage */
$storage = $this->container->get('entity_type.manager')
->getStorage('media');
/** @var \Drupal\media\MediaInterface $a */
$a = $storage->create([
'bundle' => $this->testMediaType->id(),
]);
/** @var \Drupal\media\MediaInterface $b */
$b = $storage->create([
'bundle' => $this->testMediaType->id(),
]);
// Set a random source value on both items.
$a->set($a->getSource()->getSourceFieldDefinition($a->bundle->entity)->getName(), $this->randomString());
$b->set($b->getSource()->getSourceFieldDefinition($b->bundle->entity)->getName(), $this->randomString());
$a->save();
$storage->save($b);
// Assert that the default name was mapped into the name field for both
// media items.
$this->assertFalse($a->get('name')->isEmpty());
$this->assertFalse($b->get('name')->isEmpty());
// Assert that arbitrary metadata was mapped correctly.
$this->assertFalse($a->get('field_to_map_to')->isEmpty());
$this->assertFalse($b->get('field_to_map_to')->isEmpty());
// Assert that the thumbnail was mapped correctly from the source.
$this->assertSame('public://TheSisko.png', $a->thumbnail->entity->getFileUri());
$this->assertSame('public://TheSisko.png', $b->thumbnail->entity->getFileUri());
}
/**
* Tests default media name functionality.
*/
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment