From 7b0ea5fbd18ba1634233d596e56dc30458269040 Mon Sep 17 00:00:00 2001 From: Mingo Hagen Date: Wed, 19 May 2021 20:46:58 +0200 Subject: [PATCH 1/4] fix broken test and add test for 'null' function --- tests/Application.cfc | 42 +++++++++----------- tests/orm/logging/logentry.cfc | 4 +- tests/specs/default.cfc | 72 ++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 61 deletions(-) diff --git a/tests/Application.cfc b/tests/Application.cfc index 988361e..307c511 100644 --- a/tests/Application.cfc +++ b/tests/Application.cfc @@ -1,5 +1,5 @@ component { - this.name = 'basecfc_test_1001'; + this.name = 'basecfc_tests'; this.root = getDirectoryFromPath( getCurrentTemplatePath() ).replace( '\', '/', 'all' ); this.basecfcRoot = this.root.listDeleteAt( this.root.listLen( '/' ), '/' ); @@ -16,25 +16,17 @@ component { this.ormEnabled = true; - this.datasource = 'basecfc'; + this.datasource = 'basecfc'; // need global ds, not just in orm this.ormSettings.dbCreate = 'dropcreate'; this.ormSettings.cfcLocation = this.mappings[ '/root' ] & 'orm'; this.ormSettings.sqlScript = 'nuke.sql'; - // this.ormSettings.secondaryCacheEnabled = false; - // this.ormSettings.useDBForMapping = false; - // this.ormSettings.autoManageSession = false; - // this.ormSettings.flushAtRequestEnd = false; - // this.ormSettings.cacheConfig = 'ehcache-config_ORM__basecfc.xml'; - function onRequest() { - request.appName = 'basecfc'; + request.appName = this.name; request.context.config.root = 'basecfc.tests'; - ormReload(); - - request.allOrmEntities = listAllOrmEntities( this.ormSettings.cfcLocation ); + setupORM(); param url.reporter = "simple"; param url.directory = "root.specs"; @@ -43,19 +35,21 @@ component { include '/testbox/system/runners/HTMLRunner.cfm'; } - private struct function listAllOrmEntities( cfcLocation ) { - var cacheKey = 'orm-entities'; + private void function setupORM() { + ormReload(); - var allOrmEntities = {}; - var storedEntityNames = createObject( 'java', 'java.util.Arrays' ).asList( ormGetSessionFactory().getStatistics().getEntityNames() ); + request.allOrmEntities = {}; - storedEntityNames.each((entityName)=>{ - var entity = getMetadata( entityNew( entityName ) ); - allOrmEntities[ entityName ] = { 'name' = entityName, 'table' = isNull( entity.table ) ? entityName : entity.table }; - }); + var cacheKey = 'orm-entities'; - return allOrmEntities; + createObject( 'java', 'java.util.Arrays' ).asList( ormGetSessionFactory().getStatistics().getEntityNames() ).each( ( entityName )=>{ + try { + var entity = getMetadata( entityNew( entityName ) ); + request.allOrmEntities[ entityName ] = { 'name' = entityName, 'table' = isNull( entity.table ) ? entityName : entity.table }; + } catch ( basecfc.init.invalidPropertiesError e ) { + // allow this error on entity with name "invalid", because that's used for testing + if ( entityName != 'invalid' ) rethrow; + } + } ); } - - -} +} \ No newline at end of file diff --git a/tests/orm/logging/logentry.cfc b/tests/orm/logging/logentry.cfc index 3f21750..bc697c6 100644 --- a/tests/orm/logging/logentry.cfc +++ b/tests/orm/logging/logentry.cfc @@ -58,9 +58,7 @@ component extends=basecfc.base persistent=true { formdata[ 'savedstate' ] = serializeJSON( newstate ); - transaction { - var result = save( formdata ); - } + var result = save( formdata ); return result; } diff --git a/tests/specs/default.cfc b/tests/specs/default.cfc index 4990487..7b546ee 100644 --- a/tests/specs/default.cfc +++ b/tests/specs/default.cfc @@ -42,8 +42,10 @@ component extends="testbox.system.basespec" { describe( title = 'test helper methods.', body = function() { beforeeach( function( currentspec ) { - variables.obj = entityNew( 'test' ); - variables.obj.save( { name = 'helpermethods' } ); + transaction { + variables.obj = entityNew( 'test' ); + variables.obj.save( { name = 'helpermethods' } ); + } } ); aftereach( function( currentspec ) { @@ -57,7 +59,8 @@ component extends="testbox.system.basespec" { } ); it( 'expects tojson( ) to contain all properties of the entity.', function() { - expect( variables.obj.tojson() ).toinclude( '"sortorder"' ) + expect( variables.obj.tojson() ) + .toinclude( '"sortorder"' ) .toinclude( '"id"' ) .toinclude( '"deleted"' ) .toinclude( '"name"' ); @@ -231,20 +234,22 @@ component extends="testbox.system.basespec" { } ); it( 'expects save( {add_data=[data]}) to be able to add multiple one-to-many objects', function() { - var first = entityNew( 'other' ).save( { name = 'first' } ); - var second = entityNew( 'other' ).save( { name = 'second' } ); - - var saved = variables.obj.save( { - add_entityinsubfolder = [ - { id = first.getid() }, - second.getid() - ] - } ); + transaction { + var first = entityNew( 'other' ).save( { name = 'first' } ); + var second = entityNew( 'other' ).save( { name = 'second' } ); + var saved = variables.obj.save( { + add_entityinsubfolder = [ + { id = first.getid() }, + second.getid() + ] + } ); + } var savedentitiesinsubfolder = saved.getentitiesinsubfolder(); expect( savedentitiesinsubfolder ).tobearray().tohavelength( 2 ); + // order is not guaranteed: // expect( savedentitiesinsubfolder[ 1 ].getid() ).tobe( first.getid() ); // expect( savedentitiesinsubfolder[ 2 ].getid() ).tobe( second.getid() ); } ); @@ -302,13 +307,13 @@ component extends="testbox.system.basespec" { it( 'expects update multiple items to not remove old items', function() { transaction { var multiple_1 = entityNew( 'multiple' ).save(); - var multiple_2 = entityNew( 'multiple' ).save(); variables.obj.save( { 'name' = 'tomanyupdatetest', 'multiples' = [ multiple_1 ] } ); } expect( variables.obj.getmultiples() ).tohavelength( 1 ); transaction { + var multiple_2 = entityNew( 'multiple' ).save(); variables.obj.save( { 'name' = 'tomanyupdatetest', 'multiples' = [ multiple_1, multiple_2 ] @@ -318,23 +323,6 @@ component extends="testbox.system.basespec" { expect( variables.obj.getmultiples() ).tohavelength( 2 ); } ); - xit( 'expects update multiple items with nested items pointing back to first item to not remove old items', function() { - transaction { - var multiple_1 = entityNew( 'multiple' ); - // multiple_1.enableDebug(); - multiple_1.save( { test = variables.obj } ); - - var multiple_2 = entityNew( 'multiple' ); - // multiple_2.enableDebug(); - multiple_2.save( { test = variables.obj } ); - } - - // variables.obj.enableDebug(); - variables.obj.save( { 'name' = 'tomanyupdatetest', 'multiples' = [ multiple_2, multiple_1 ] } ); - - expect( variables.obj.getmultiples() ).tohavelength( 1 ); - } ); - it( 'expects set_ to overwrite add_ in save( )', function() { var testobjects_a = entityNew( 'multiple' ).save( { name = 'a' } ); var testobjects_b = entityNew( 'multiple' ).save( { name = 'b' } ); @@ -432,6 +420,20 @@ component extends="testbox.system.basespec" { expect( deeperlinkback ).tobearray().tohavelength( 1 ); expect( deeperlinkback[ 1 ].getid() ).tobe( more.getid() ); } ); + + it( 'expects save( {more=''null''} ) to delete a nested many-to-one object', function() { + transaction { + var saved = variables.obj.save( { more = { name = 'newmore' } } ); + } + + expect( saved.getMore() ).notToBeNull(); + + transaction { + saved.save( { more = 'null' } ); + } + + expect( saved.getMore() ).toBeNull(); + } ); } ); @@ -537,7 +539,7 @@ component extends="testbox.system.basespec" { ormCloseAllSessions(); } ); - it( 'expects objects not to be persisted with transactionrollback', function() { + it( 'expects objects not to be persisted with transactionRollback', function() { expect( entityLoad( 'test' ) ).tohavelength( 0 ); expect( entityLoad( 'more' ) ).tohavelength( 0 ); @@ -555,7 +557,7 @@ component extends="testbox.system.basespec" { expect( allmores ).tohavelength( 0 ); } ); - it( 'expects objects to be persisted without transactionrollback', function() { + it( 'expects objects to be persisted without transactionRollback', function() { expect( entityLoad( 'test' ) ).tohavelength( 0 ); expect( entityLoad( 'more' ) ).tohavelength( 0 ); @@ -672,8 +674,10 @@ component extends="testbox.system.basespec" { var logable = entityNew( 'logable' ); - logable.save( { 'afieldtotest' = 'firstvalue', 'thiswontchange' = 'staticvalue' } ); - logable.save( { 'afieldtotest' = 'secondvalue' } ); + transaction { + logable.save( { 'afieldtotest' = 'firstvalue', 'thiswontchange' = 'staticvalue' } ); + logable.save( { 'afieldtotest' = 'secondvalue' } ); + } var log = entityLoad( 'logentry' ); From 629dd137fb59928ad3855a6a34364336f97d78fd Mon Sep 17 00:00:00 2001 From: Mingo Hagen Date: Wed, 19 May 2021 20:52:04 +0200 Subject: [PATCH 2/4] couple things, see description - for deORM move complexity to functions - remove `variables.instance` from argument defaults, move to params - call `logChanges()` without `savedState` (part of saved state as obj update) - add `isDeletedEntity()` to allow removal of to-one obects using `value="null"` syntax --- base.cfc | 128 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 78 insertions(+), 50 deletions(-) diff --git a/base.cfc b/base.cfc index 9cfc919..87cfaf4 100644 --- a/base.cfc +++ b/base.cfc @@ -27,7 +27,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" hide=true { property name="id" type="string" fieldType="id" generator="uuid"; - this.version = "4.3.0"; + this.version = "4.4.0"; this.sanitizeDataTypes = listToArray( "date,datetime,double,float,int,integer,numeric,percentage,timestamp" ); this.logLevels = listToArray( "debug,information,warning,error,fatal" ); this.logFields = listToArray( "createcontact,createdate,createip,updatecontact,updatedate,updateip" ); @@ -62,8 +62,6 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" throw( logMessage, 'basecfc.global' ); } - var savedState = { }; - for ( var logField in this.logFields ) { formData.delete( logField ); } @@ -168,6 +166,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" var skipMatrix = getSkipMatrix( property, formData, depth ); if ( skipProperty( skipMatrix ) ) { + if ( request.context.debug ) writeOutput( '
skipping #key# (#serializeJson( skipMatrix )#)' ); continue; } @@ -194,9 +193,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" } // log changes if value is not empty and if field is not one of the standard log fields - if ( !isNull( valueToLog ) && !this.logFields.findNoCase( property.name ) ) { - savedState[ property.name ] = valueToLog; - } else if ( request.context.debug ) { + if ( request.context.debug ) { writeOutput( '
not logging anything for "#property.name#" (#isNull(valueToLog)?'null':'not null'#/#this.logFields.findNoCase( property.name )#)' ); } } @@ -223,8 +220,8 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" // Process queued instructions if ( depth == 0 ) { - processQueue( ); - logChanges( savedState ); + processQueue(); + logChanges(); } return this; @@ -286,7 +283,9 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" /** * the entity name (as per CFML ORM standard) */ - public string function getEntityName( string className = variables.instance.className ) { + public string function getEntityName( string className ) { + param className = variables.instance.className; + var basicEntityName = className.listLast( '.' ); if ( request.allOrmEntities.keyExists( basicEntityName ) ) { return request.allOrmEntities[ basicEntityName ].name; @@ -297,7 +296,9 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" /** * the database table name (as per CFML ORM standard) */ - public string function getTableName( string className = variables.instance.className ) { + public string function getTableName( string className ) { + param className = variables.instance.className; + var basicEntityName = className.listLast( '.' ); if ( request.allOrmEntities.keyExists( basicEntityName ) ) { return request.allOrmEntities[ basicEntityName ].table; @@ -531,40 +532,52 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" * @data One or more entities to be converted to a less complex representation */ public any function deORM( any data = this ) { - var deWormed = { }; - if ( isSimpleValue( data ) ) { - deWormed = data; + return data; + } else if ( isObject( data ) ) { - var properties = data.getInheritedProperties( ); - for ( var key in properties ) { - var prop = properties[ key ]; - if ( !structKeyExists( data, 'get' & prop.name ) || ( structKeyExists( prop, 'fieldtype' ) && findNoCase( - "-to-", - prop.fieldtype - ) ) ) { - continue; - } - deWormed[ prop.name ] = invoke( data, 'get#prop.name#' ); - if ( prop.name contains '`' ) { - writeDump(deWormed[ prop.name ]);abort; - } - } + return deWormObject( data ); + } else if ( isStruct( data ) ) { - for ( var key in data ) { - if ( structKeyExists( data, key ) ) { - deWormed[ key ] = deORM( data[ key ] ); - } - } + return deWormStruct( data ); + } else if ( isArray( data ) ) { - var deWormed = [ ]; + return deWormArray( data ); - for ( var el in data ) { - arrayAppend( deWormed, deORM( el ) ); - } } + } + + private struct function deWormObject( data ) { + return data.getInheritedProperties() + .filter( function( key, prop ) { return !isNull( prop ); } ) + .filter( function( key, prop ) { + var propertyHasGetter = structkeyExists( this, 'get#prop.name#' ); + var isSimpleProperty = !( prop.keyExists( 'fieldtype' ) && prop.fieldtype.findNoCase( '-to-' ) ); + return ( propertyHasGetter && isSimpleProperty ); + } ) + .map( function( key, prop ) { + return invoke( this, 'get#prop.name#' ); + } ); + } + + private struct function deWormStruct( data ) { + if ( isNull( data ) ) return {}; + + return data + .filter(function(key, value){ return !isNull( value ) }) + .map( function(key, value) { + return deORM( value ); + } ); + } + + private array function deWormArray( data ) { + var result = []; - return deWormed; + data.each(function(key, value){ + result.append( deORM( value ) ); + }); + + return result; } /** @@ -746,12 +759,14 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" var dataType = getDatatype( property ); // check inside json obj to see if an ID was passed in - try { - var testForJSON = deserializeJSON( nestedData ); - if ( isStruct( testForJSON ) && testForJSON.keyExists( "id" ) ) { - nestedData = testForJSON.id; + if ( property.name != 'savedstate' ) { + try { + var testForJSON = deserializeJSON( nestedData ); + if ( isStruct( testForJSON ) && testForJSON.keyExists( "id" ) ) { + nestedData = testForJSON.id; + } + } catch ( any e ) { } - } catch ( any e ) { } if ( request.basecfc.keyExists( "sanitationService" ) && this.sanitizeDataTypes.findNoCase( dataType ) ) { @@ -809,7 +824,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" if ( !skipToNextPropery ) { // remove data if nestedData is empty - if ( isNull( nestedData ) ) { + if ( isNull( nestedData ) || isDeletedEntity( nestedData ) ) { queueInstruction( this, fn, "null" ); if ( request.context.debug ) { @@ -1022,6 +1037,10 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" } structDelete( formData, "set_#property.name#" ); + + if ( workData.isEmpty() ) { + formData[ 'set_#property.name#' ] = javaCast( 'null', 0 ); + } } if ( request.context.debug ) writeOutput( '
toMany_convertSetToAdd() #getTickCount()-t#ms.' ); @@ -1358,9 +1377,12 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" * component (or passes along the given component) */ private any function toComponent( required any variable, required string entityName, required string cfc ) { + var t = getTickCount(); var parsedVar = variable; + if ( isNull( parsedVar ) || isDeletedEntity( parsedVar ) ) return; + try { if ( isObject( parsedVar ) && isInstanceOf( parsedVar, cfc ) ) { if ( request.context.debug ) writeOutput( '
toComponent() #getTickCount()-t#ms. (early exit)' ); @@ -1572,7 +1594,6 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" , notInFormdata( property, formData ) ? 1 : 0 // , (depth>2&&property.keyExists( 'inverse' )) ? 1 : 0 ]; - return skipMatrix; } @@ -1611,7 +1632,11 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" entitySave( logEntry ); - var logResult = logEntry.enterIntoLog( logAction, savedState, this ); + if ( isNull( arguments.savedState ) || arguments.savedState.isEmpty() ) { + arguments.savedState = deOrm(); + } + + var logResult = logEntry.enterIntoLog( logAction, arguments.savedState, this ); basecfcLog( 'Added log entry for #getName()# (#logResult.getId()#).' ); request.context.log = logResult; // <- that's ugly, but I need the log entry in some controllers. @@ -1772,7 +1797,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" * - sortorder basecfc-entities always have a sortkey, if you don't use it, set it to 0. */ private void function validateBaseProperties() { - if ( variables.instance.className == 'basecfc.base' || variables.instance.className == '' ) { + if ( !variables.keyExists( 'instance' ) || variables.instance.className == 'basecfc.base' || variables.instance.className == '' ) { return; } @@ -1844,10 +1869,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" } private void function setup() { - if ( !request.keyExists( 'allOrmEntities' ) ) { - return; - } - + if ( !request.keyExists( 'allOrmEntities' ) ) return; // Application not set up for ORM if ( variables.keyExists( 'instance' ) ) return; // entity already set up param variables.name=""; @@ -1919,4 +1941,10 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" throw( logMessage, 'basecfc.global' ); } } + + private boolean function isDeletedEntity( value ) { + if ( isSimpleValue( value ) && value == 'null' ) return true; + if ( isSimpleValue( value ) && value == '' ) return true; + return false; + } } \ No newline at end of file From b246b954a5cb0b468bd89c1cce024d0da5b911db Mon Sep 17 00:00:00 2001 From: Mingo Hagen Date: Wed, 19 May 2021 22:15:11 +0200 Subject: [PATCH 3/4] rolleback savedstate changes --- base.cfc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/base.cfc b/base.cfc index 87cfaf4..c5ea6fd 100644 --- a/base.cfc +++ b/base.cfc @@ -27,7 +27,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" hide=true { property name="id" type="string" fieldType="id" generator="uuid"; - this.version = "4.4.0"; + this.version = "4.4.1"; this.sanitizeDataTypes = listToArray( "date,datetime,double,float,int,integer,numeric,percentage,timestamp" ); this.logLevels = listToArray( "debug,information,warning,error,fatal" ); this.logFields = listToArray( "createcontact,createdate,createip,updatecontact,updatedate,updateip" ); @@ -62,6 +62,8 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" throw( logMessage, 'basecfc.global' ); } + var savedState = { }; + for ( var logField in this.logFields ) { formData.delete( logField ); } @@ -193,7 +195,9 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" } // log changes if value is not empty and if field is not one of the standard log fields - if ( request.context.debug ) { + if ( !isNull( valueToLog ) && !this.logFields.findNoCase( property.name ) ) { + savedState[ property.name ] = valueToLog; + } else if ( request.context.debug ) { writeOutput( '
not logging anything for "#property.name#" (#isNull(valueToLog)?'null':'not null'#/#this.logFields.findNoCase( property.name )#)' ); } } @@ -221,7 +225,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" // Process queued instructions if ( depth == 0 ) { processQueue(); - logChanges(); + logChanges( savedState ); } return this; @@ -1377,7 +1381,6 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" * component (or passes along the given component) */ private any function toComponent( required any variable, required string entityName, required string cfc ) { - var t = getTickCount(); var parsedVar = variable; From 0fcf3cc44d85cc84d9f761e1b659189c2ceca48e Mon Sep 17 00:00:00 2001 From: Mingo Hagen Date: Wed, 19 May 2021 22:18:18 +0200 Subject: [PATCH 4/4] rolled back more savedstate changes --- base.cfc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/base.cfc b/base.cfc index c5ea6fd..b4ab011 100644 --- a/base.cfc +++ b/base.cfc @@ -1635,11 +1635,7 @@ component mappedSuperClass=true cacheuse="transactional" defaultSort="sortorder" entitySave( logEntry ); - if ( isNull( arguments.savedState ) || arguments.savedState.isEmpty() ) { - arguments.savedState = deOrm(); - } - - var logResult = logEntry.enterIntoLog( logAction, arguments.savedState, this ); + var logResult = logEntry.enterIntoLog( logAction, savedState, this ); basecfcLog( 'Added log entry for #getName()# (#logResult.getId()#).' ); request.context.log = logResult; // <- that's ugly, but I need the log entry in some controllers.