Issue #3442022 by phenaproxima, alexpott, larowlan: Trigger entity validation...

Issue #3442022 by phenaproxima, alexpott, larowlan: Trigger entity validation in \Drupal\Core\DefaultContent\Importer::importContent()

(cherry picked from commit 1736da1fcc50cc12fdd4ce2592cc0772cc30c9e4)

core/lib/Drupal/Core/DefaultContent/Finder.php

@@ -43,8 +43,8 @@ public function __construct(string $path) {
     foreach ($finder as $file) {
       /** @var array{_meta: array{uuid: string|null, depends: array<string, string>|null}} $decoded */
       $decoded = Yaml::decode($file->getContents());
-      $uuid = $decoded['_meta']['uuid'] ?? throw new ImportException($file->getPathname() . ' does not have a UUID.');
-      $decoded['_meta']['path'] = $file->getPath();
+      $decoded['_meta']['path'] = $file->getPathname();
+      $uuid = $decoded['_meta']['uuid'] ?? throw new ImportException($decoded['_meta']['path'] . ' does not have a UUID.');
       $files[$uuid] = $decoded;
 
       // For the graph to work correctly, every entity must be mentioned in it.

core/lib/Drupal/Core/DefaultContent/Importer.php

@@ -32,10 +32,10 @@ final class Importer implements LoggerAwareInterface {
   /**
    * The dependencies of the currently importing entity, if any.
    *
-   * The keys are the UUIDs of the dependencies, and the values are their entity
-   * type.
+   * The keys are the UUIDs of the dependencies, and the values are arrays with
+   * two members: the entity type ID of the dependency, and the UUID to load.
    *
-   * @var string[]|null
+   * @var array<string, string[]>|null
    */
   private ?array $dependencies = NULL;
 
@@ -106,7 +106,11 @@ public function importContent(Finder $content, Existing $existing = Existing::Er
         // If a file exists in the same folder, copy it to the designated
         // target URI.
         if ($entity instanceof FileInterface) {
-          $this->copyFileAssociatedWithEntity($path, $entity);
+          $this->copyFileAssociatedWithEntity(dirname($path), $entity);
+        }
+        $violations = $entity->validate();
+        if (count($violations) > 0) {
+          throw new InvalidEntityException($violations, $path);
         }
         $entity->save();
       }
@@ -116,7 +120,7 @@ public function importContent(Finder $content, Existing $existing = Existing::Er
     }
   }
 
-  private function copyFileAssociatedWithEntity(string $path, FileInterface $entity): void {
+  private function copyFileAssociatedWithEntity(string $path, FileInterface &$entity): void {
     $destination = $entity->getFileUri();
     assert(is_string($destination));
 
@@ -130,22 +134,27 @@ private function copyFileAssociatedWithEntity(string $path, FileInterface $entit
       return;
     }
 
+    $copy_file = TRUE;
     if (file_exists($destination)) {
       $source_hash = hash_file('sha256', $source);
       assert(is_string($source_hash));
       $destination_hash = hash_file('sha256', $destination);
       assert(is_string($destination_hash));
 
-      // If we already have the file, we don't need to do anything else.
-      if (hash_equals($source_hash, $destination_hash)) {
-        return;
+      if (hash_equals($source_hash, $destination_hash) && $this->entityTypeManager->getStorage('file')->loadByProperties(['uri' => $destination]) === []) {
+        // If the file hashes match and the file is not already a managed file
+        // then do not copy a new version to the file system. This prevents
+        // re-installs during development from creating unnecessary duplicates.
+        $copy_file = FALSE;
       }
     }
 
     $target_directory = dirname($destination);
     $this->fileSystem->prepareDirectory($target_directory, FileSystemInterface::CREATE_DIRECTORY);
-    $uri = $this->fileSystem->copy($source, $destination);
-    $entity->setFileUri($uri);
+    if ($copy_file) {
+      $uri = $this->fileSystem->copy($source, $destination);
+      $entity->setFileUri($uri);
+    }
   }
 
   /**
@@ -168,16 +177,19 @@ private function toEntity(array $data): ContentEntityInterface {
       throw new ImportException('The uuid metadata must be specified.');
     }
 
-    ['entity_type' => $entity_type] = $data['_meta'];
-    assert(is_string($entity_type));
-
     $is_root = FALSE;
     // @see ::loadEntityDependency()
     if ($this->dependencies === NULL && !empty($data['_meta']['depends'])) {
       $is_root = TRUE;
-      $this->dependencies = $data['_meta']['depends'];
+      foreach ($data['_meta']['depends'] as $uuid => $entity_type) {
+        assert(is_string($uuid));
+        assert(is_string($entity_type));
+        $this->dependencies[$uuid] = [$entity_type, $uuid];
+      }
     }
 
+    ['entity_type' => $entity_type] = $data['_meta'];
+    assert(is_string($entity_type));
     /** @var \Drupal\Core\Entity\EntityTypeInterface $entity_type */
     $entity_type = $this->entityTypeManager->getDefinition($entity_type);
 
@@ -307,7 +319,7 @@ private function getCustomSerializedPropertyNames(FieldItemInterface $field_item
    */
   private function loadEntityDependency(string $target_uuid): ?ContentEntityInterface {
     if ($this->dependencies && array_key_exists($target_uuid, $this->dependencies)) {
-      $entity = $this->entityRepository->loadEntityByUuid($this->dependencies[$target_uuid], $target_uuid);
+      $entity = $this->entityRepository->loadEntityByUuid(...$this->dependencies[$target_uuid]);
       assert($entity instanceof ContentEntityInterface || $entity === NULL);
       return $entity;
     }

core/lib/Drupal/Core/DefaultContent/InvalidEntityException.php

+<?php
+
+namespace Drupal\Core\DefaultContent;
+
+use Drupal\Core\Entity\EntityConstraintViolationListInterface;
+use Symfony\Component\Validator\ConstraintViolationInterface;
+
+/**
+ * Thrown if an entity being imported has validation errors.
+ *
+ * @internal
+ *   This API is experimental.
+ */
+final class InvalidEntityException extends \RuntimeException {
+
+  public function __construct(public readonly EntityConstraintViolationListInterface $violations, public readonly string $filePath) {
+    $messages = [];
+
+    foreach ($violations as $violation) {
+      assert($violation instanceof ConstraintViolationInterface);
+      $messages[] = $violation->getPropertyPath() . '=' . $violation->getMessage();
+    }
+    // Example: "/path/to/file.yml: field_a=Violation 1., field_b=Violation 2.".
+    parent::__construct("$filePath: " . implode('||', $messages));
+  }
+
+}

core/tests/Drupal/FunctionalTests/DefaultContent/ContentImportTest.php

@@ -7,11 +7,14 @@
 use ColinODell\PsrTestLogger\TestLogger;
 use Drupal\block_content\BlockContentInterface;
 use Drupal\block_content\Entity\BlockContentType;
+use Drupal\Component\Serialization\Yaml;
 use Drupal\Core\DefaultContent\Existing;
 use Drupal\Core\DefaultContent\Finder;
 use Drupal\Core\DefaultContent\Importer;
 use Drupal\Core\DefaultContent\ImportException;
+use Drupal\Core\DefaultContent\InvalidEntityException;
 use Drupal\Core\Entity\EntityRepositoryInterface;
+use Drupal\Core\File\FileExists;
 use Drupal\Core\Url;
 use Drupal\field\Entity\FieldConfig;
 use Drupal\field\Entity\FieldStorageConfig;
@@ -103,6 +106,7 @@ protected function setUp(): void {
       ->save();
 
     $this->contentDir = $this->getDrupalRoot() . '/core/tests/fixtures/default_content';
+    \Drupal::service('file_system')->copy($this->contentDir . '/file/druplicon_copy.png', $this->publicFilesDirectory . '/druplicon_copy.png', FileExists::Error);
   }
 
   /**
@@ -154,6 +158,25 @@ public function testDirectContentImport(): void {
     $this->assertTrue($logger->hasRecordThatPasses($predicate, LogLevel::WARNING));
   }
 
+  /**
+   * Tests that the importer validates entities before saving them.
+   */
+  public function testEntityValidationIsTriggered(): void {
+    $dir = uniqid('public://');
+    mkdir($dir);
+
+    /** @var string $data */
+    $data = file_get_contents($this->contentDir . '/node/2d3581c3-92c7-4600-8991-a0d4b3741198.yml');
+    $data = Yaml::decode($data);
+    /** @var array{default: array{sticky: array<int, array{value: mixed}>}} $data */
+    $data['default']['sticky'][0]['value'] = 'not a boolean!';
+    file_put_contents($dir . '/invalid.yml', Yaml::encode($data));
+
+    $this->expectException(InvalidEntityException::class);
+    $this->expectExceptionMessage("$dir/invalid.yml: sticky.0.value=This value should be of the correct primitive type.");
+    $this->container->get(Importer::class)->importContent(new Finder($dir));
+  }
+
   /**
    * Asserts that the default content was imported as expected.
    */
@@ -188,12 +211,12 @@ private function assertContentWasImported(): void {
     $this->assertSame('druplicon.png', $file->getFilename());
     $this->assertSame('d8404562-efcc-40e3-869e-40132d53fe0b', $file->uuid());
 
-    // Another file entity referencing an existing file should have the same
-    // file URI -- in other words, it should have reused the existing file.
-    $duplicate_file = $entity_repository->loadEntityByUuid('file', '23a7f61f-1db3-407d-a6dd-eb4731995c9f');
-    $this->assertInstanceOf(FileInterface::class, $duplicate_file);
-    $this->assertSame('druplicon-duplicate.png', $duplicate_file->getFilename());
-    $this->assertSame($file->getFileUri(), $duplicate_file->getFileUri());
+    // Another file entity referencing an existing file but already in use by
+    // another entity, should be imported.
+    $same_file_different_entity = $entity_repository->loadEntityByUuid('file', '23a7f61f-1db3-407d-a6dd-eb4731995c9f');
+    $this->assertInstanceOf(FileInterface::class, $same_file_different_entity);
+    $this->assertSame('druplicon-duplicate.png', $same_file_different_entity->getFilename());
+    $this->assertStringEndsWith('/druplicon_0.png', (string) $same_file_different_entity->getFileUri());
 
     // Another file entity that references a file with the same name as, but
     // different contents than, an existing file, should be imported and the
@@ -201,7 +224,14 @@ private function assertContentWasImported(): void {
     $different_file = $entity_repository->loadEntityByUuid('file', 'a6b79928-838f-44bd-a8f0-44c2fff9e4cc');
     $this->assertInstanceOf(FileInterface::class, $different_file);
     $this->assertSame('druplicon-different.png', $different_file->getFilename());
-    $this->assertStringEndsWith('/druplicon_0.png', (string) $different_file->getFileUri());
+    $this->assertStringEndsWith('/druplicon_1.png', (string) $different_file->getFileUri());
+
+    // Another file entity referencing an existing file but one that is not in
+    // use by another entity, should be imported but use the existing file.
+    $different_file = $entity_repository->loadEntityByUuid('file', '7fb09f9f-ba5f-4db4-82ed-aa5ccf7d425d');
+    $this->assertInstanceOf(FileInterface::class, $different_file);
+    $this->assertSame('druplicon_copy.png', $different_file->getFilename());
+    $this->assertStringEndsWith('/druplicon_copy.png', (string) $different_file->getFileUri());
 
     // Our node should have a menu link, and it should use the path alias we
     // included with the recipe.
@@ -214,7 +244,7 @@ private function assertContentWasImported(): void {
     $this->assertInstanceOf(BlockContentInterface::class, $block_content);
     $this->assertSame('basic', $block_content->bundle());
     $this->assertSame('Useful Info', $block_content->label());
-    $this->assertSame("<p>I'd love to put some useful info here.</p>", $block_content->body->value);
+    $this->assertSame("I'd love to put some useful info here.", $block_content->body->value);
 
     // A node with a non-existent owner should be reassigned to the current
     // user.
@@ -227,7 +257,7 @@ private function assertContentWasImported(): void {
     $this->assertInstanceOf(NodeInterface::class, $node);
     $translation = $node->getTranslation('fr');
     $this->assertSame('Perdu en traduction', $translation->label());
-    $this->assertSame("<p>Içi c'est la version français.</p>", $translation->body->value);
+    $this->assertSame("Içi c'est la version français.", $translation->body->value);
   }
 
 }

core/tests/fixtures/default_content/block_content/d9b72b2f-a5ea-4a3f-b10c-28deb7b3b7bf.yml

@@ -19,6 +19,6 @@ default:
       value: true
   body:
     -
-      value: "<p>I'd love to put some useful info here.</p>"
-      format: basic_html
+      value: "I'd love to put some useful info here."
+      format: plain_text
       summary: ''

core/tests/fixtures/default_content/file/7fb09f9f-ba5f-4db4-82ed-aa5ccf7d425d.yml

+_meta:
+  version: '1.0'
+  entity_type: file
+  uuid: 7fb09f9f-ba5f-4db4-82ed-aa5ccf7d425d
+  default_langcode: en
+default:
+  uid:
+    -
+      target_id: 1
+  filename:
+    -
+      value: druplicon_copy.png
+  uri:
+    -
+      value: 'public://druplicon_copy.png'
+  filemime:
+    -
+      value: image/png
+  filesize:
+    -
+      value: 3905
+  status:
+    -
+      value: true
+  created:
+    -
+      value: 1711121742

core/tests/fixtures/default_content/node/2d3581c3-92c7-4600-8991-a0d4b3741198.yml

@@ -40,8 +40,8 @@ default:
       value: false
   body:
     -
-      value: "<p>Here's the English version.</p>"
-      format: basic_html
+      value: "Here's the English version."
+      format: plain_text
       summary: ''
 translations:
   fr:
@@ -78,6 +78,6 @@ translations:
         value: false
     body:
       -
-        value: "<p>Içi c'est la version français.</p>"
-        format: basic_html
+        value: "Içi c'est la version français."
+        format: plain_text
         summary: ''

core/tests/fixtures/default_content/node/7f1dd75a-0be2-4d3b-be5d-9d1a868b9267.yml

@@ -39,6 +39,6 @@ default:
       langcode: en
   body:
     -
-      value: '<p>This page was authored by a non-existent user.</p>'
-      format: basic_html
+      value: 'This page was authored by a non-existent user.'
+      format: plain_text
       summary: ''

core/tests/fixtures/default_content/node/e1714f23-70c0-4493-8e92-af1901771921.yml

@@ -39,7 +39,7 @@ default:
   body:
     -
       value: 'Crikey it works!'
-      format: basic_html
+      format: plain_text
       summary: ''
   field_tags:
     -

core/tests/fixtures/default_content/path_alias/9952d42c-4eac-4424-ad25-e18d9612f395.yml

-_meta:
-  version: '1.0'
-  entity_type: path_alias
-  uuid: 9952d42c-4eac-4424-ad25-e18d9612f395
-  default_langcode: en
-default:
-  path:
-    -
-      value: /node/3
-  alias:
-    -
-      value: /test-article
-  status:
-    -
-      value: true

core/tests/fixtures/default_content/user/94503467-be7f-406c-9795-fc25baa22203.yml

@@ -41,6 +41,3 @@ default:
   init:
     -
       value: author@example.com
-  roles:
-    -
-      target_id: content_editor