Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
EntityReferenceRevisionsFieldItemList::referencedEntities loads the referenced entities by revision id from the database instead of using the existing entities on the reference item. This leads to potential data loss if the referenced entity has a change compared to the database.
Proposed resolution
This was implemented based on EntityReferenceFieldItemList::referencedEntities but there are two fundamental differences a) there's no static cache for revision list loads which means the object on the entity property will differ from what load returns -- for plain entity references the load will return the handle of an object in the memory cache b) there's no point because there's no loadMultiple for revisions. The whole method should be collapsed into
foreach ($this->list as $delta => $item) {
if ($item->entity) {
$target_entities[$delta] = $item->entity;
}
}
return $target_entities;
and be done. This will actually be faster in many cases because the EntityReferenceRevisions / entity_revision_reference data type plugin optimizes getTarget().
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3098924-12.patch | 1.47 KB | travis-bradbury |
| #10 | 3098924-10.patch | 1.44 KB | travis-bradbury |
| #9 | 3098924-9.patch | 641 bytes | spadxiii |
Comments
Comment #2
ghost of drupal pastComment #3
ghost of drupal pastData loss bugs are critical.
Comment #4
ghost of drupal pastComment #5
ghost of drupal pastComment #6
ghost of drupal pastComment #7
berdirThanks for reporting. This will also help with #3095329: Optimise entity reference autocomplete widget loading referencable entities
I have some ideas on how it could be improved further, like doing a loadMultiple() first but yes, if we don't have a isLoaded() or something on ->entity then that's also a bit of overhead.
+1 to just simplifying it to that.
Comment #8
super_romeo commentedYes.
returns correct data,
but
returns wrong data.
Comment #9
spadxiii commentedHere's a small patch with a less-intrusive fix than the proposed solution. I've added the entity-check in the existing code.
I didn't check if the proposed solution works in all cases. My patch seems to fix the problems for me, but I'm unsure if the rest of the code can be removed here.
Comment #10
travis-bradbury commentedThis patch is the change described in the issue summary - replacing the whole function with just
$item->entityfor each item.Comment #11
travis-bradbury commentedComment #12
travis-bradbury commentedOops, #10 has a typo.
Comment #13
dieterholvoet commentedComment #14
vpowner commentedRegarding data loss, I noticed this function does not account for orphaned references. The ER on a node that points to a no longer existing node needs to be reported (or exception thrown) so we can investigate/fix.
Comment #15
mstrelan commentedI came across this working on a ConstraintValidator on an entity that checked the value of a field inside a paragraphs item on the entity. This was previously only able to access the saved revision of the paragraphs item, and not the revision about to be saved. I tested #12 for this use case and it's working correctly.
Comment #16
berdirTests would be nice for the actual regression, but the change makes sense and passes the existing tests.