From 4fb69762a0575b6d9c669c4ecabb393d764f3059 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 22 Aug 2026 07:01:43 -0600 Subject: [PATCH 1/2] feat: guard loaded entity keys from mutation Closes #43 --- models/BaseEntity.cfc | 27 +++++++++++++++++++ models/Relationships/HasOneOrMany.cfc | 2 +- .../specs/integration/BaseEntity/SaveSpec.cfc | 24 +++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/models/BaseEntity.cfc b/models/BaseEntity.cfc index c900d867..d89771c2 100644 --- a/models/BaseEntity.cfc +++ b/models/BaseEntity.cfc @@ -1179,6 +1179,11 @@ component accessors="true" { arguments.value = castValueForSetter( arguments.name, arguments.value.keyValues()[ 1 ] ); } + guardAgainstLoadedKeyMutation( + arguments.name, + isNull( arguments.value ) ? javacast( "null", "" ) : arguments.value + ); + variables._data[ retrieveColumnForAlias( arguments.name ) ] = arguments.cast ? castValueForSetter( arguments.name, isNull( arguments.value ) ? javacast( "null", "" ) : arguments.value @@ -1191,6 +1196,28 @@ component accessors="true" { return this; } + private void function guardAgainstLoadedKeyMutation( required string name, any value ) { + if ( !isLoaded() || !arrayContainsNoCase( keyNames(), retrieveAliasForColumn( arguments.name ) ) ) { + return; + } + + var keyColumn = retrieveColumnForAlias( arguments.name ); + var originalIsNull = !variables._originalAttributes.keyExists( keyColumn ) || isNull( + variables._originalAttributes[ keyColumn ] + ); + var replacementIsNull = isNull( arguments.value ); + if ( + originalIsNull != replacementIsNull || + ( !originalIsNull && variables._originalAttributes[ keyColumn ] != arguments.value ) + ) { + throw( + type = "QuickPrimaryKeyMutationException", + message = "A loaded [#entityName()#] entity cannot change its primary key [#retrieveAliasForColumn( arguments.name )#].", + detail = "Create a new entity when a different primary key is required." + ); + } + } + /** * Retrieve an array of qualified column names. * diff --git a/models/Relationships/HasOneOrMany.cfc b/models/Relationships/HasOneOrMany.cfc index 4f8eff66..59da9857 100644 --- a/models/Relationships/HasOneOrMany.cfc +++ b/models/Relationships/HasOneOrMany.cfc @@ -341,10 +341,10 @@ component var keyValues = arguments.entity; arguments.entity = variables.related.newEntity(); var relatedKeyNames = variables.related.keyNames(); - arguments.entity.set_loaded( true ); for ( var i = 1; i <= relatedKeyNames.len(); i++ ) { arguments.entity.forceAssignAttribute( relatedKeyNames[ i ], keyValues[ i ] ); } + arguments.entity.assignOriginalAttributes( arguments.entity.retrieveAttributesData() ).set_loaded( true ); } setForeignAttributesForCreate( arguments.entity ); return arguments.entity.save(); diff --git a/tests/specs/integration/BaseEntity/SaveSpec.cfc b/tests/specs/integration/BaseEntity/SaveSpec.cfc index a3b99ef1..c87282db 100644 --- a/tests/specs/integration/BaseEntity/SaveSpec.cfc +++ b/tests/specs/integration/BaseEntity/SaveSpec.cfc @@ -118,6 +118,30 @@ component extends="tests.resources.ModuleIntegrationSpec" { expect( dateCompare( freshUser.getModifiedDate(), originalModified ) ).toBe( 0 ); } ); + it( "throws a helpful error when changing the key of a loaded entity", function() { + var existingUser = getInstance( "User" ).findOrFail( 1 ); + + expect( function() { + existingUser.setId( 2 ).save(); + } ).toThrow( type = "QuickPrimaryKeyMutationException", regex = "cannot change its primary key" ); + } ); + + it( "allows assigning the existing key value to a loaded entity", function() { + var existingUser = getInstance( "User" ).findOrFail( 1 ); + + expect( function() { + existingUser.setId( 1 ).save(); + } ).notToThrow(); + } ); + + it( "guards every part of a loaded composite key", function() { + var composite = getInstance( "Composite" ).findOrFail( [ 1, 2 ] ); + + expect( function() { + composite.setB( 1 ).save(); + } ).toThrow( type = "QuickPrimaryKeyMutationException", regex = "primary key \[b\]" ); + } ); + it( "does not allow updating of column where update=false in property", function() { var existingUser = getInstance( "User" ).find( 1 ); existingUser.setEmail( "test2@test.com" ); From c62e9cefd853b1fbbc80a57d2465dcaf90ca82b4 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 25 Aug 2026 13:55:32 -0600 Subject: [PATCH 2/2] test: align touch coverage with key mutation guard --- tests/specs/integration/BaseEntity/SaveSpec.cfc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/specs/integration/BaseEntity/SaveSpec.cfc b/tests/specs/integration/BaseEntity/SaveSpec.cfc index c87282db..476651cf 100644 --- a/tests/specs/integration/BaseEntity/SaveSpec.cfc +++ b/tests/specs/integration/BaseEntity/SaveSpec.cfc @@ -83,19 +83,15 @@ component extends="tests.resources.ModuleIntegrationSpec" { var originalFirstName = user.getFirstName(); var originalId = user.getId(); var dirtyFirstName = "This must not be persisted"; - var dirtyId = 9999; user.setFirstName( dirtyFirstName ); - user.setId( dirtyId ); user.touch(); expect( dateCompare( user.getCreatedDate(), originalCreated ) ).toBe( 0 ); expect( dateCompare( user.getModifiedDate(), originalModified ) ).toBe( 0 ); expect( user.getFirstName() ).toBe( dirtyFirstName ); - expect( user.getId() ).toBe( dirtyId ); expect( user.isDirty( "firstName" ) ).toBeTrue(); - expect( user.isDirty( "id" ) ).toBeTrue(); expect( user.isDirty( "createdDate" ) ).toBeFalse(); expect( user.isDirty( "modifiedDate" ) ).toBeFalse(); var freshUser = getInstance( "User" ).findOrFail( originalId );