Issue #3271688 by smustgrave, larowlan, phenaproxima, borisson_, ravi.shankar,...

Issue #3271688 by smustgrave, larowlan, phenaproxima, borisson_, ravi.shankar, dianacastillo, TimoZura: Remove source from media mappings

core/modules/media/media.post_update.php

@@ -8,6 +8,7 @@
 use Drupal\Core\Config\Entity\ConfigEntityUpdater;
 use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
 use Drupal\media\MediaConfigUpdater;
+use Drupal\media\MediaTypeInterface;
 
 /**
  * Implements hook_removed_post_updates().
@@ -45,3 +46,31 @@ function media_post_update_set_blank_iframe_domain_to_null() {
       ->save(TRUE);
   }
 }
+
+/**
+ * Make sure no Media types are using the source field in the meta mappings.
+ */
+function media_post_update_remove_mappings_targeting_source_field(array &$sandbox = NULL): void {
+  \Drupal::classResolver(ConfigEntityUpdater::class)
+    ->update($sandbox, 'media_type', function (MediaTypeInterface $media_type): bool {
+      $source_field = $media_type->getSource()
+        ->getSourceFieldDefinition($media_type);
+
+      if ($source_field) {
+        $source_field_name = $source_field->getName();
+
+        $original_field_map = $media_type->getFieldMap();
+        $field_map = array_diff($original_field_map, [$source_field_name]);
+
+        // Check if old field map matches new field map.
+        if (empty(array_diff($original_field_map, $field_map))) {
+          return FALSE;
+        }
+
+        $media_type->setFieldMap($field_map);
+        return TRUE;
+      }
+
+      return FALSE;
+    });
+}

core/modules/media/src/Entity/MediaType.php

@@ -61,6 +61,7 @@
  *   },
  *   constraints = {
  *     "ImmutableProperties" = {"id", "source"},
+ *     "MediaMappingsConstraint" = { },
  *   }
  * )
  */

core/modules/media/src/MediaTypeForm.php

@@ -176,7 +176,13 @@ public function form(array $form, FormStateInterface $form_state) {
     }
     else {
       $options = [MediaSourceInterface::METADATA_FIELD_EMPTY => $this->t('- Skip field -')];
+      $source_field_name = $source->getSourceFieldDefinition($this->entity)?->getName();
       foreach ($this->entityFieldManager->getFieldDefinitions('media', $this->entity->id()) as $field_name => $field) {
+        // The source field cannot be the target of a field mapping, because
+        // this would cause it to be overwritten, probably with invalid data.
+        if ($field_name === $source_field_name) {
+          continue;
+        }
         if (!($field instanceof BaseFieldDefinition) || $field_name === 'name') {
           $options[$field_name] = $field->getLabel();
         }

core/modules/media/src/Plugin/Validation/Constraint/MediaMappingsConstraint.php

+<?php
+
+namespace Drupal\media\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * Validates media mappings.
+ *
+ * @internal
+ *
+ * @Constraint(
+ *   id = "MediaMappingsConstraint",
+ *   label = @Translation("Media Mapping Constraint", context = "Validation"),
+ *   type = {"string"}
+ * )
+ */
+class MediaMappingsConstraint extends Constraint {
+
+  /**
+   * The error message if source is used in media mapping.
+   *
+   * @var string
+   */
+  public string $invalidMappingMessage = 'It is not possible to map the source field @source_field_name of a media type.';
+
+}

core/modules/media/src/Plugin/Validation/Constraint/MediaMappingsConstraintValidator.php

+<?php
+
+namespace Drupal\media\Plugin\Validation\Constraint;
+
+use Drupal\Component\Plugin\Exception\PluginException;
+use Drupal\media\MediaTypeInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+
+/**
+ * Validates media mappings.
+ */
+class MediaMappingsConstraintValidator extends ConstraintValidator {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validate($value, Constraint $constraint) {
+    if (!$constraint instanceof MediaMappingsConstraint) {
+      throw new UnexpectedTypeException($constraint, __NAMESPACE__ . '\MediaMappingsConstraint');
+    }
+    if (!$value instanceof MediaTypeInterface) {
+      throw new UnexpectedTypeException($value, MediaTypeInterface::class);
+    }
+    // The source field cannot be the target of a field mapping because that
+    // would cause it to be overwritten, possibly with invalid data. This is
+    // also enforced in the UI.
+    if (is_array($value->getFieldMap())) {
+      try {
+        $source_field_name = $value->getSource()
+          ->getSourceFieldDefinition($value)
+          ?->getName();
+
+        if (in_array($source_field_name, $value->getFieldMap(), TRUE)) {
+          $this->context->addViolation($constraint->invalidMappingMessage, [
+            '@source_field_name' => $source_field_name,
+          ]);
+        }
+      }
+      catch (PluginException) {
+        // The source references an invalid plugin.
+      }
+    }
+  }
+
+}

core/modules/media/tests/src/Functional/Update/MediaMappingUpdateTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\media\Functional\Update;
+
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+use Drupal\media\Entity\MediaType;
+use Drupal\media\Plugin\media\Source\File;
+use Drupal\Tests\media\Traits\MediaTypeCreationTrait;
+
+/**
+ * Tests update functions for the Media module.
+ *
+ * @group media
+ */
+class MediaMappingUpdateTest extends UpdatePathTestBase {
+
+  use MediaTypeCreationTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles(): void {
+    $this->databaseDumpFiles = [
+      $this->getDrupalRoot() . '/core/modules/system/tests/fixtures/update/drupal-9.4.0.bare.standard.php.gz',
+      __DIR__ . '/../../../fixtures/update/media.php',
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+    /** @var \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager */
+    $entity_type_manager = $this->container->get('entity_type.manager');
+    /** @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface $update_manager */
+    $update_manager = $this->container->get('entity.definition_update_manager');
+
+    $entity_type = $entity_type_manager->getDefinition('media_type');
+    $update_manager->installEntityType($entity_type);
+    $entity_type = $entity_type_manager->getDefinition('media');
+    $update_manager->installEntityType($entity_type);
+  }
+
+  /**
+   * Tests updating Media types using source field in meta mappings.
+   *
+   * @see media_post_update_remove_mappings_targeting_source_field()
+   */
+  public function testMediaMappingUpdate(): void {
+    $media_type = $this->createMediaType('image', ['id' => 'invalid_mapping']);
+
+    $source_field_name = $media_type->getSource()
+      ->getSourceFieldDefinition($media_type)
+      ->getName();
+    $field_map = $media_type->getFieldMap();
+    $field_map[File::METADATA_ATTRIBUTE_MIME] = $source_field_name;
+    $this->config($media_type->getConfigDependencyName())
+      ->set('field_map', $field_map)
+      ->save();
+
+    $this->runUpdates();
+
+    $this->container->get('entity_type.manager')
+      ->getStorage('media_type')
+      ->resetCache(['invalid_mapping']);
+    $field_map = MediaType::load('invalid_mapping')?->getFieldMap();
+    $this->assertIsArray($field_map);
+    $this->assertNotContains($source_field_name, $field_map);
+  }
+
+}

core/modules/media/tests/src/FunctionalJavascript/MediaTypeCreationTest.php

@@ -107,11 +107,13 @@ public function testMediaTypeCreationFormWithDefaultField() {
     $assert_session->pageTextContains('The media source cannot be changed after the media type is created.');
 
     // Check that the field map options are sorted alphabetically.
+    // Source field should not be included.
     $options = $this->xpath('//select[@name="field_map[attribute_1]"]/option');
-    $this->assertGreaterThanOrEqual(3, count($options));
+    $this->assertGreaterThanOrEqual(2, count($options));
     $this->assertSame('- Skip field -', $options[0]->getText());
     $this->assertSame('Name', $options[1]->getText());
-    $this->assertSame('Test source', $options[2]->getText());
+    // It should not be possible to map the source field.
+    $assert_session->optionNotExists('field_map[attribute_1]', 'Test source');
 
     // Open up the media add form and verify that the source field is right
     // after the name, and before the vertical tabs.

core/modules/media/tests/src/Kernel/MediaMappingsConstraintValidatorTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\media\Kernel;
+
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\media\Plugin\media\Source\File;
+use Drupal\Tests\media\Traits\MediaTypeCreationTrait;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
+
+/**
+ * @coversDefaultClass \Drupal\media\Plugin\Validation\Constraint\MediaMappingsConstraintValidator
+ *
+ * @group media
+ */
+class MediaMappingsConstraintValidatorTest extends KernelTestBase {
+
+  use MediaTypeCreationTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['field', 'file', 'image', 'media', 'user'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+    $this->installEntitySchema('file');
+    $this->installEntitySchema('user');
+  }
+
+  /**
+   * @covers ::validate
+   */
+  public function testMediaMappingSource() {
+    $media_type = $this->createMediaType('image', [
+      'id' => 'test',
+    ]);
+
+    $source_field_name = $media_type->getSource()
+      ->getSourceFieldDefinition($media_type)
+      ->getName();
+    $field_map = $media_type->getFieldMap();
+    $field_map[File::METADATA_ATTRIBUTE_MIME] = $source_field_name;
+    $media_type->setFieldMap($field_map);
+    $media_type->save();
+    $typed_data = $this->container->get('typed_data_manager');
+    $definition = $typed_data->createDataDefinition('entity:media_type');
+    $violations = $typed_data->create($definition, $media_type)->validate();
+    assert($violations instanceof ConstraintViolationListInterface);
+    $this->assertCount(1, $violations);
+    $this->assertEquals('It is not possible to map the source field ' . $source_field_name . ' of a media type.', $violations[0]->getMessage());
+  }
+
+}