Entities: Prevent changing Key property on existing entities

[AndyButland]

Description

We have #21131 raised noting that it was no longer possible to change a Key on a content entity programatically, due to the key being used a a foreign key constraint. This triggered some (internal) discussion about whether this should be supported, and the view was taken that we hadn't expected and shouldn't support the changing of keys. With the management API and backoffice working exclusively with GUID keys rather than integer IDs, pickers storing GUIDs, newer services using keys as database constraints and the backoffice using these as a keys for caching; it's not something we should expect to work without issue.

As such this PR is intended to be explicit about this. It's not enough to just say we don't want packages or projects changing keys when our services and APIs allow it. So this PR, targeted for the next major, locks down at the model level by throwing an InvalidOperationException if a key is changed.

Change Summary

• Add validation in EntityBase.Key setter to throw InvalidOperationException when attempting to change the Key of an existing (persisted) entity
• The Key (GUID) should be immutable once an entity is persisted to the database
• Update DeepCloneWithResetIdentities implementations to call ResetIdentity() before other operations

Change Details

• EntityBase.cs: Added _keyIsAssigned tracking field and validation logic in Key setter. Added [OnDeserialized] callback to ensure correct state after deserialization.
  • Removed [DataMember] from HasIdentity (it's derived from Id). This was only necessary to support testing, but it's a correct change nonetheless (it's read-only, and derived from Id which is serialized)
• Content.cs: Reordered DeepCloneWithResetIdentities to call ResetIdentity() first
• ContentTypeBase.cs: Reordered DeepCloneWithResetIdentities to call ResetIdentity() first
• IDataType.cs: Reordered DeepCloneWithResetIdentities to call ResetIdentity() first

Breaking Change

Attempting to change the Key property on an existing entity will now throw InvalidOperationException. This is intentional - Keys should be immutable once persisted.

Testing and Review

Verifying that newly added and existing unit and integration tests pass should suffice here.

Check over discussion on the linked issue and consider implications of the change.

[Copilot]

Pull request overview

This pull request implements Key immutability for persisted entities in Umbraco CMS. The Key (GUID) property can no longer be changed once an entity has been persisted to the database, which prevents data integrity issues.

Changes:

• Added validation logic in EntityBase to track whether a Key has been explicitly assigned and prevent modifications to Keys of persisted entities
• Reordered DeepCloneWithResetIdentities implementations to call ResetIdentity() first, eliminating redundant Key assignments
• Added comprehensive test coverage for Key immutability scenarios including serialization, factory loading patterns, and edge cases

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Umbraco.Core/Models/Entities/EntityBase.cs	Added _keyIsAssigned tracking field, validation logic in Key setter to prevent changes on persisted entities, OnDeserialized callback for serialization support, and removed [DataMember] from HasIdentity
src/Umbraco.Core/Models/Content.cs	Reordered DeepCloneWithResetIdentities to call ResetIdentity() first, removing redundant Key = Guid.Empty assignment
src/Umbraco.Core/Models/ContentTypeBase.cs	Reordered DeepCloneWithResetIdentities to call ResetIdentity() first, removing redundant Key = Guid.Empty assignment
src/Umbraco.Core/Models/IDataType.cs	Removed redundant Key = Guid.Empty from DeepCloneWithResetIdentities as ResetIdentity() already handles this
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/EntityTests.cs	Added comprehensive unit tests covering all Key immutability scenarios including factory patterns, serialization, cloning, and edge cases
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs	Added integration test verifying Key immutability for persisted content entities

[kjac]

💪

[ronaldbarendse]

Enforcing this at the entity level is probably the easiest to implement, but wouldn't the proper way be to do this in the service/repository layer and return a failed result with a new operation result?

Besides this and the in-line comments, this will require additional testing in Deploy once a v18 preview version has been released 😄

[AndyButland]

I've applied your naming suggestions @ronaldbarendse in 8ac46e9 (as they came in after this PR was merged).

In terms of where this validation takes place, I did consider it at the service level but it felt like that was likely going to lead to more repetition (and potential omission) across the various entities, so would be better to enforce it in the domain model itself. @kjac - thoughts on this and whether it should change as per Ronald's comment?