Issue #3192591 by prudloff, smustgrave: ValidReferenceConstraintValidator...

Issue #3192591 by prudloff, smustgrave: ValidReferenceConstraintValidator reloads the parent entity even it does not need to

core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php

@@ -121,17 +121,17 @@ public function validate($value, Constraint $constraint): void {
 
     // Add violations on deltas with a target_id that is not valid.
     if ($target_ids) {
-      // Get a list of pre-existing references.
-      $previously_referenced_ids = [];
-      if ($entity && !$entity->isNew()) {
-        $existing_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id());
-        foreach ($existing_entity->{$value->getFieldDefinition()->getName()}->getValue() as $item) {
-          $previously_referenced_ids[$item['target_id']] = $item['target_id'];
-        }
-      }
-
       $valid_target_ids = $handler->validateReferenceableEntities($target_ids);
       if ($invalid_target_ids = array_diff($target_ids, $valid_target_ids)) {
+        // Get a list of pre-existing references.
+        $previously_referenced_ids = [];
+        if ($entity && !$entity->isNew()) {
+          $existing_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id());
+          foreach ($existing_entity->{$value->getFieldDefinition()->getName()}->getValue() as $item) {
+            $previously_referenced_ids[$item['target_id']] = $item['target_id'];
+          }
+        }
+
         // For accuracy of the error message, differentiate non-referenceable
         // and non-existent entities.
         $existing_entities = $this->entityTypeManager->getStorage($target_type_id)->loadMultiple($invalid_target_ids);

core/tests/Drupal/KernelTests/Core/Entity/ValidReferenceConstraintValidatorTest.php

@@ -175,9 +175,16 @@ public function testPreExistingItemsValidation(): void {
 
     $this->container->get('account_switcher')->switchTo($user_with_access);
 
+    // We check if the referencing entity was loaded by checking
+    // if it was added to the memory cache.
+    // This requires an empty memory cache for the test to be reliable.
+    $this->container->get('entity.memory_cache')->reset();
     $violations = $referencing_entity->field_test->validate();
     $this->assertCount(0, $violations);
 
+    // No invalid target was found, so the referencing entity was not reloaded.
+    $this->assertFalse($this->container->get('entity.memory_cache')->get('values:' . $referencing_entity->getEntityTypeId() . ':' . $referencing_entity->id()));
+
     // Check that users without access are able pass the validation for fields
     // with pre-existing content.
     $this->container->get('account_switcher')->switchTo($user_without_access);
@@ -204,10 +211,14 @@ public function testPreExistingItemsValidation(): void {
     $field->save();
     $referencing_entity = $this->reloadEntity($referencing_entity);
 
+    $this->container->get('entity.memory_cache')->reset();
     $violations = $referencing_entity->field_test->validate();
     $this->assertCount(1, $violations);
     $this->assertEquals(sprintf('This entity (node: %s) cannot be referenced.', $different_bundle_node->id()), $violations[0]->getMessage());
 
+    // An invalid target was found, so the referencing entity had to be reloaded.
+    $this->assertNotFalse($this->container->get('entity.memory_cache')->get('values:' . $referencing_entity->getEntityTypeId() . ':' . $referencing_entity->id()));
+
     // Delete the last node and check that the pre-existing reference is not
     // valid anymore.
     $deleted_node->delete();