diff --git a/models/BaseEntity.cfc b/models/BaseEntity.cfc index f2fa40e..043ae6b 100644 --- a/models/BaseEntity.cfc +++ b/models/BaseEntity.cfc @@ -2960,11 +2960,13 @@ component accessors="true" { message = 'This instance is missing `accessors="true"` in the component metadata. This is required for Quick to work properly. Please add it to your component metadata and reinit your application.' ); } - meta[ "fullName" ] = meta.originalMetadata.fullname; - param meta.originalMetadata.mapping = listLast( meta.originalMetadata.fullname, "." ); - meta[ "mapping" ] = meta.originalMetadata.mapping; - param meta.originalMetadata.entityName = listLast( meta.originalMetadata.name, "." ); - meta[ "entityName" ] = meta.originalMetadata.entityName; + meta[ "fullName" ] = meta.originalMetadata.fullname; + param meta.originalMetadata.mapping = listLast( meta.originalMetadata.fullname, "." ); + meta[ "mapping" ] = meta.originalMetadata.mapping; + param meta.originalMetadata.entityName = listLast( meta.originalMetadata.name, "." ); + meta[ "entityName" ] = meta.originalMetadata.entityName; + param meta.localMetadata.properties = []; + guardDuplicatePropertyNames( meta.localMetadata, meta.mapping ); param meta.originalMetadata.table = variables._str.plural( variables._str.snake( meta.entityName ) ); meta[ "table" ] = meta.originalMetadata.table; param meta.originalMetadata.readonly = false; @@ -3212,6 +3214,75 @@ component accessors="true" { return variables._meta.nonPersistentProperties.keyExists( arguments.name ); } + private void function guardDuplicatePropertyNames( required struct metadata, required string mapping ) { + var propertyNames = {}; + var entityMapping = arguments.mapping; + arguments.metadata.properties.each( function( prop ) { + if ( propertyNames.keyExists( arguments.prop.name ) ) { + throwDuplicateProperty( entityMapping, arguments.prop.name ); + } + propertyNames[ arguments.prop.name ] = true; + } ); + + // Some engines collapse duplicate declarations in component metadata. In + // that case, inspect the local component source when it is available. + if ( !arguments.metadata.keyExists( "path" ) || !fileExists( arguments.metadata.path ) ) { + return; + } + + propertyNames = {}; + var source = fileRead( arguments.metadata.path ); + source = reReplace( + source, + "(?s)/[*].*?[*]/|", + " ", + "all" + ); + source = reReplace( source, "(?m)//.*$", " ", "all" ); + var propertyToken = chr( 60 ) & "cfproperty"; + var declarations = reMatchNoCase( "(?is)(^|[^a-z0-9_])(property|#propertyToken#)\s[^;>]*", source ); + declarations.each( function( declaration ) { + var nameAssignment = reFindNoCase( + "name\s*=\s*", + arguments.declaration, + 1, + true + ); + if ( nameAssignment.pos[ 1 ] == 0 ) { + return; + } + var valueStart = nameAssignment.pos[ 1 ] + nameAssignment.len[ 1 ]; + var quote = mid( arguments.declaration, valueStart, 1 ); + if ( quote != chr( 34 ) && quote != chr( 39 ) ) { + return; + } + var valueEnd = find( + quote, + arguments.declaration, + valueStart + 1 + ); + if ( valueEnd == 0 ) { + return; + } + var propertyName = mid( + arguments.declaration, + valueStart + 1, + valueEnd - valueStart - 1 + ); + if ( propertyNames.keyExists( propertyName ) ) { + throwDuplicateProperty( entityMapping, propertyName ); + } + propertyNames[ propertyName ] = true; + } ); + } + + private void function throwDuplicateProperty( required string mapping, required string propertyName ) { + throw( + type = "QuickDuplicateProperty", + message = "[#arguments.mapping#] declares more than one property named [#arguments.propertyName#]. Property names must be unique." + ); + } + private struct function generateCastsFromProperties( required array properties ) { return arguments.properties.reduce( function( acc, prop ) { if ( !arguments.prop.keyExists( "casts" ) || arguments.prop.casts == "" ) { diff --git a/tests/resources/app/models/AliasedUsernameUser.cfc b/tests/resources/app/models/AliasedUsernameUser.cfc new file mode 100644 index 0000000..044b6e1 --- /dev/null +++ b/tests/resources/app/models/AliasedUsernameUser.cfc @@ -0,0 +1,6 @@ +component extends="quick.models.BaseEntity" accessors="true" table="users" { + + property name="id"; + property name="username" column="first_name" sqltype="cf_sql_varchar"; + +} diff --git a/tests/resources/app/models/DuplicateUsernamePropertyUser.cfc b/tests/resources/app/models/DuplicateUsernamePropertyUser.cfc new file mode 100644 index 0000000..7bb0745 --- /dev/null +++ b/tests/resources/app/models/DuplicateUsernamePropertyUser.cfc @@ -0,0 +1,14 @@ +component + extends ="quick.models.BaseEntity" + accessors="true" + table ="users" +{ + + property name="id"; + property + name ="username" + column ="first_name" + sqltype="cf_sql_varchar"; + property name="username"; + +} diff --git a/tests/specs/integration/BaseEntity/AttributeSpec.cfc b/tests/specs/integration/BaseEntity/AttributeSpec.cfc index bb18c87..cce831f 100644 --- a/tests/specs/integration/BaseEntity/AttributeSpec.cfc +++ b/tests/specs/integration/BaseEntity/AttributeSpec.cfc @@ -39,6 +39,12 @@ component extends="tests.resources.ModuleIntegrationSpec" { expect( entity.getActivoSN() ).toBeFalse(); } ); + it( "rejects duplicate property names", function() { + expect( function() { + getInstance( "DuplicateUsernamePropertyUser" ); + } ).toThrow(); + } ); + it( "can set a value to null using the `setColumnName` magic methods", function() { var user = getInstance( "User" ).find( 1 ); expect( user.getUsername() ).toBe( "elpete" ); @@ -216,6 +222,14 @@ component extends="tests.resources.ModuleIntegrationSpec" { } ); } ); + it( "uses an explicit column when the property name is also a database column", function() { + var user = getInstance( "AliasedUsernameUser" ).findOrFail( 1 ); + + expect( user.getUsername() ).toBe( "Eric" ); + expect( user.retrieveAttributesData() ).toHaveKey( "first_name" ); + expect( user.retrieveAttributesData() ).notToHaveKey( "username" ); + } ); + // https://github.com/coldbox-modules/quick/issues/127 it( "can clear an attribute", () => { var elpete = getInstance( "User" ).findOrFail( 1 );