Problem/Motivation
Content entities in Drupal allowed changing their UUID on an existing entity without throwing any error or warning. Config entities already had this protection via ConfigEntityBase::preSave(), but content entities (in ContentEntityStorageBase) had no equivalent check. This could lead to data corruption, especially for field data where UUIDs are used as identifiers.
This was a follow-up from #1969698 (ConfigEntity::save() disallowing UUID/ID conflicts) and #1969800 (Add UUIDs to default configuration). The original question asked whether all entities should enforce that the UUID cannot change for an existing entity.
Steps to reproduce
Create any content entity (e.g., an entity_test entity) and save it.
Load the saved entity.
Change its UUID via $entity->set('uuid', '').
Call $entity->save().
Expected: An exception should be thrown because the UUID of an existing entity should not be changeable.
Actual: No exception is thrown; the entity saves with the new UUID, which can cause data integrity issues.
Proposed resolution
Add a UUID change validation check in ContentEntityStorageBase::doPreSave(), placed alongside the existing ID change and revision ID change checks. The check:
Only applies to existing entities (not new ones).
Only fires when the entity type has a UUID key defined.
Uses $entity->getOriginal() (already loaded by the parent doPreSave()) to compare the old UUID against the new UUID.
Throws an EntityStorageException with a descriptive message if the UUID was modified.
The fix follows the exact same pattern already used in ConfigEntityBase::preSave() for config entities.
Additionally, the file upload handler (FileUploadHandler::handleFileUpload()) needs to preserve the existing UUID when replacing a file via FileExists::Replace, following the same pattern used in FileRepository::moveFile().
Issue fork drupal-1989674
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- main
changes, plain diff MR !16080
- 1989674-throw-an-exception
compare
Comments
Comment #1
xjmComment #2
xjmSee also: #1609418: Configuration objects are not unique.
Comment #3
swentel commentedAlso related #1740378: Implement renames in the import cycle (but more or less duplicate) - and a big problem for fields at the moment (unless we start dropping features), see this comment http://drupal.org/node/1740378#comment-7130190
Comment #4
tstoecklerThis makes total sense. I see no reason why this should be specific to ConfigEntities. Adjusting title for that.
Comment #5
xjmSee also: #2041887: check that id does not exist before saving a new ConfigEntity
Comment #6
sunLooks like the deps are resolved.
Comment #7
tim.plunkettConfigEntityBase::preSave() has partial handling for this.
Comment #10
tim.plunkettNot actively part of the Blocks-Layouts work.
Comment #22
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #24
smustgrave commentedSeems like a still relevant task.
Tagging for a summary update and test coverage
Comment #28
phthlaap commentedComment #29
smustgrave commentedTag for IS summary is still relevant.
@phthlaap it's super appreciative knocking tests out but please read the tickets too for open questions and tags.
Comment #30
phthlaap commentedComment #31
phthlaap commented