From 699f4f448a574931f2a02db964a3adee5456c99e Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Mon, 20 Nov 2023 20:48:06 +0000 Subject: [PATCH 1/8] [twgit] Init feature 'feature-PRESIDECMS-2745_rules-engine-condition-result-caching'. From 6e0d546e84bafd983eada7efa01428ca2db5acd7 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Mon, 20 Nov 2023 20:48:27 +0000 Subject: [PATCH 2/8] PRESIDECMS-2745 add various request level caching to rules engine. * Calculation of any given context's payload * Evaluation of an individual expression * Evaluation of an entire condition * Various other calculations The idea here is that the result of condition and expression evaluation will not change within the scope of a request and neither will a payload for a given context. In complicated pages with a lot of conditions to run, we can save a lot of resource by caching results. n.b. While here, also reduce use of member functions. --- .../RulesEngineConditionService.cfc | 18 ++++--- .../rulesEngine/RulesEngineContextService.cfc | 44 ++++++++++------- .../RulesEngineExpressionService.cfc | 49 ++++++++++++------- 3 files changed, 67 insertions(+), 44 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineConditionService.cfc b/system/services/rulesEngine/RulesEngineConditionService.cfc index 894a2a2ede..5a1b332c7a 100644 --- a/system/services/rulesEngine/RulesEngineConditionService.cfc +++ b/system/services/rulesEngine/RulesEngineConditionService.cfc @@ -41,11 +41,14 @@ component displayName="RulesEngine Condition Service" { var conditionRecord = $getPresideObject( "rules_engine_condition" ).selectData( id=arguments.conditionId ); if ( conditionRecord.recordCount ) { + if ( IsSimpleValue( conditionRecord.expressions ) ) { + conditionRecord.expressions[ 1 ] = DeSerializeJson( conditionRecord.expressions ); // deliberately against a cached query to avoid doing this multiple times + } return { id = arguments.conditionId , name = conditionRecord.condition_name , context = conditionRecord.context - , expressions = DeSerializeJson( conditionRecord.expressions ) + , expressions = conditionRecord.expressions }; } @@ -97,14 +100,14 @@ component displayName="RulesEngine Condition Service" { , required string context , struct payload = {} ) { - var condition = getCondition( arguments.conditionId ); + var condition = getCondition( arguments.conditionId ); - if ( condition.isEmpty() ) { + if ( StructIsEmpty( condition ) ) { return false; } - var finalPayload = Duplicate( arguments.payload ); - finalPayload.append( _getContextService().getContextPayload( + var finalPayload = StructCopy( arguments.payload ); + StructAppend( finalPayload, _getContextService().getContextPayload( context = arguments.context , args = arguments.payload ), false ); @@ -261,9 +264,10 @@ component displayName="RulesEngine Condition Service" { var currentEvaluation = true; var currentJoin = "and"; var expressionResult = true; + var arryLen = ArrayLen( arguments.expressionArray ) - for( var i=1; i<=arguments.expressionArray.len(); i++ ) { - var item = arguments.expressionArray[i]; + for( var i=1; i<=arryLen; i++ ) { + var item = arguments.expressionArray[ i ]; var isOddRow = ( i mod 2 == 1 ); if ( isOddRow ) { diff --git a/system/services/rulesEngine/RulesEngineContextService.cfc b/system/services/rulesEngine/RulesEngineContextService.cfc index ae5ba14ec6..463d22187d 100644 --- a/system/services/rulesEngine/RulesEngineContextService.cfc +++ b/system/services/rulesEngine/RulesEngineContextService.cfc @@ -130,15 +130,15 @@ component displayName="RulesEngine Context Service" { var objects = []; if ( arguments.includeChildContexts ) { - if ( mainObject.len() ) { - objects.append( mainObject ); + if ( Len( mainObject ) ) { + ArrayAppend( objects, mainObject ); } var subContexts = contexts[ arguments.context ].subcontexts ?: []; for( var subContext in subContexts ) { var subobjects = getContextObject( subContext, true ); - if ( subobjects.len() ) { - objects.append( subobjects, true ); + if ( Len( subobjects ) ) { + ArrayAppend( objects, subobjects, true ); } } @@ -192,24 +192,30 @@ component displayName="RulesEngine Context Service" { * @args.hint Optional set of args to send to the context getPayload() handler */ public struct function getContextPayload( required string context, struct args={} ) { - var coldboxController = $getColdbox(); - var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); - var payload = {}; - - for( var cx in expanded ) { - var handlerAction = "rules.contexts.#cx#.getPayload"; - - if ( coldboxController.handlerExists( handlerAction ) ) { - payload.append( $getColdbox().runEvent( - event = handlerAction - , eventArguments = arguments.args - , private = true - , prePostExempt = true - ) ); + var cb = $getColdbox(); + var cacheKey = arguments.context & Hash( SerializeJson( args ) ); + + if ( !StructKeyExists( request, "_rulesEngineContextPayloadCache" ) || !StructKeyExists( request._rulesEngineContextPayloadCache, cacheKey ) ) { + var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); + var payload = {}; + + for( var cx in expanded ) { + var handlerAction = "rules.contexts.#cx#.getPayload"; + + if ( cb.handlerExists( handlerAction ) ) { + StructAppend( payload, cb.runEvent( + event = handlerAction + , eventArguments = arguments.args + , private = true + , prePostExempt = true + ) ); + } } + + request._rulesEngineContextPayloadCache[ cacheKey ] = payload; } - return payload; + return request._rulesEngineContextPayloadCache[ cacheKey ]; } // GETTERS AND SETTERS diff --git a/system/services/rulesEngine/RulesEngineExpressionService.cfc b/system/services/rulesEngine/RulesEngineExpressionService.cfc index f6a5e75d47..bfaba88f85 100644 --- a/system/services/rulesEngine/RulesEngineExpressionService.cfc +++ b/system/services/rulesEngine/RulesEngineExpressionService.cfc @@ -298,6 +298,7 @@ component displayName="RulesEngine Expression Service" { , required struct configuredFields ) { _lazyLoadDynamicExpressions( context=arguments.context ) + try { var expression = _getRawExpression( expressionId ); } catch( any e ) { @@ -307,30 +308,34 @@ component displayName="RulesEngine Expression Service" { var contexts = expression.contexts ?: []; - if ( !contexts.findNoCase( arguments.context ) && !contexts.findNoCase( "global" ) ) { + if ( !ArrayFindNoCase( contexts, arguments.context ) && !ArrayFindNoCase( contexts, "global" ) ) { throw( type = "preside.rule.expression.invalid.context" , message = "The expression [#arguments.expressionId#] cannot be used in the [#arguments.context#] context." ); } - var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; - var eventArgs = { - context = arguments.context - , payload = arguments.payload - }; + var requestCacheKey = arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ); - eventArgs.append( expression.expressionHandlerArgs ?: {} ); - eventArgs.append( preProcessConfiguredFields( arguments.expressionId, arguments.configuredFields ) ); + if ( !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { + var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; + var eventArgs = { + context = arguments.context + , payload = arguments.payload + }; - var result = $getColdbox().runEvent( - event = handlerAction - , private = true - , prePostExempt = true - , eventArguments = eventArgs - ); + eventArgs.append( expression.expressionHandlerArgs ?: {} ); + eventArgs.append( preProcessConfiguredFields( arguments.expressionId, arguments.configuredFields ) ); - return result; + request._rulesEngineEvaluateExpressionCache[ requestCacheKey ] = $getColdbox().runEvent( + event = handlerAction + , private = true + , prePostExempt = true + , eventArguments = eventArgs + ); + } + + return request._rulesEngineEvaluateExpressionCache[ requestCacheKey ]; } /** @@ -618,18 +623,24 @@ component displayName="RulesEngine Expression Service" { } private void function _lazyLoadDynamicExpressions( string context="", string filterObject="" ) { + var contextCacheKey = "context=#arguments.context#,filterObject=#arguments.filterObject#"; + variables._lazyLoadDone = variables._lazyLoadDone ?: {}; + if ( StructKeyExists( variables._lazyLoadDone, contextCacheKey ) ) { + return; + } + var objects = []; var contextService = _getContextService(); if ( Len( Trim( arguments.filterObject ) ) ) { - objects.append( arguments.filterObject ); + ArrayAppend( objects, arguments.filterObject ); } if ( Len( Trim( arguments.context ) ) ) { var contextObjects = contextService.getContextObject( arguments.context, true ); - if ( contextObjects.len() ) { - objects.append( contextObjects, true ); + if ( ArrayLen( contextObjects ) ) { + ArrayAppend( objects, contextObjects, true ); } } @@ -645,6 +656,8 @@ component displayName="RulesEngine Expression Service" { variables._lazyLoadDone[ objectName ] = true; } } + + variables._lazyLoadDone[ contextCacheKey ] = true; } private array function _getAdminUserRoles( string adminUserId=$getAdminLoginService().getLoggedInUserId() ) { From 80833cad780c871ce532dbeece2bcdd8e0df6420 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Mon, 20 Nov 2023 20:48:27 +0000 Subject: [PATCH 3/8] PRESIDECMS-2745 add various request level caching to rules engine. * Calculation of any given context's payload * Evaluation of an individual expression * Evaluation of an entire condition * Various other calculations The idea here is that the result of condition and expression evaluation will not change within the scope of a request and neither will a payload for a given context. In complicated pages with a lot of conditions to run, we can save a lot of resource by caching results. n.b. While here, also reduce use of member functions. --- .../RulesEngineConditionService.cfc | 18 ++++--- .../rulesEngine/RulesEngineContextService.cfc | 44 ++++++++++------- .../RulesEngineExpressionService.cfc | 49 ++++++++++++------- 3 files changed, 67 insertions(+), 44 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineConditionService.cfc b/system/services/rulesEngine/RulesEngineConditionService.cfc index 894a2a2ede..5a1b332c7a 100644 --- a/system/services/rulesEngine/RulesEngineConditionService.cfc +++ b/system/services/rulesEngine/RulesEngineConditionService.cfc @@ -41,11 +41,14 @@ component displayName="RulesEngine Condition Service" { var conditionRecord = $getPresideObject( "rules_engine_condition" ).selectData( id=arguments.conditionId ); if ( conditionRecord.recordCount ) { + if ( IsSimpleValue( conditionRecord.expressions ) ) { + conditionRecord.expressions[ 1 ] = DeSerializeJson( conditionRecord.expressions ); // deliberately against a cached query to avoid doing this multiple times + } return { id = arguments.conditionId , name = conditionRecord.condition_name , context = conditionRecord.context - , expressions = DeSerializeJson( conditionRecord.expressions ) + , expressions = conditionRecord.expressions }; } @@ -97,14 +100,14 @@ component displayName="RulesEngine Condition Service" { , required string context , struct payload = {} ) { - var condition = getCondition( arguments.conditionId ); + var condition = getCondition( arguments.conditionId ); - if ( condition.isEmpty() ) { + if ( StructIsEmpty( condition ) ) { return false; } - var finalPayload = Duplicate( arguments.payload ); - finalPayload.append( _getContextService().getContextPayload( + var finalPayload = StructCopy( arguments.payload ); + StructAppend( finalPayload, _getContextService().getContextPayload( context = arguments.context , args = arguments.payload ), false ); @@ -261,9 +264,10 @@ component displayName="RulesEngine Condition Service" { var currentEvaluation = true; var currentJoin = "and"; var expressionResult = true; + var arryLen = ArrayLen( arguments.expressionArray ) - for( var i=1; i<=arguments.expressionArray.len(); i++ ) { - var item = arguments.expressionArray[i]; + for( var i=1; i<=arryLen; i++ ) { + var item = arguments.expressionArray[ i ]; var isOddRow = ( i mod 2 == 1 ); if ( isOddRow ) { diff --git a/system/services/rulesEngine/RulesEngineContextService.cfc b/system/services/rulesEngine/RulesEngineContextService.cfc index ae5ba14ec6..463d22187d 100644 --- a/system/services/rulesEngine/RulesEngineContextService.cfc +++ b/system/services/rulesEngine/RulesEngineContextService.cfc @@ -130,15 +130,15 @@ component displayName="RulesEngine Context Service" { var objects = []; if ( arguments.includeChildContexts ) { - if ( mainObject.len() ) { - objects.append( mainObject ); + if ( Len( mainObject ) ) { + ArrayAppend( objects, mainObject ); } var subContexts = contexts[ arguments.context ].subcontexts ?: []; for( var subContext in subContexts ) { var subobjects = getContextObject( subContext, true ); - if ( subobjects.len() ) { - objects.append( subobjects, true ); + if ( Len( subobjects ) ) { + ArrayAppend( objects, subobjects, true ); } } @@ -192,24 +192,30 @@ component displayName="RulesEngine Context Service" { * @args.hint Optional set of args to send to the context getPayload() handler */ public struct function getContextPayload( required string context, struct args={} ) { - var coldboxController = $getColdbox(); - var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); - var payload = {}; - - for( var cx in expanded ) { - var handlerAction = "rules.contexts.#cx#.getPayload"; - - if ( coldboxController.handlerExists( handlerAction ) ) { - payload.append( $getColdbox().runEvent( - event = handlerAction - , eventArguments = arguments.args - , private = true - , prePostExempt = true - ) ); + var cb = $getColdbox(); + var cacheKey = arguments.context & Hash( SerializeJson( args ) ); + + if ( !StructKeyExists( request, "_rulesEngineContextPayloadCache" ) || !StructKeyExists( request._rulesEngineContextPayloadCache, cacheKey ) ) { + var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); + var payload = {}; + + for( var cx in expanded ) { + var handlerAction = "rules.contexts.#cx#.getPayload"; + + if ( cb.handlerExists( handlerAction ) ) { + StructAppend( payload, cb.runEvent( + event = handlerAction + , eventArguments = arguments.args + , private = true + , prePostExempt = true + ) ); + } } + + request._rulesEngineContextPayloadCache[ cacheKey ] = payload; } - return payload; + return request._rulesEngineContextPayloadCache[ cacheKey ]; } // GETTERS AND SETTERS diff --git a/system/services/rulesEngine/RulesEngineExpressionService.cfc b/system/services/rulesEngine/RulesEngineExpressionService.cfc index f6a5e75d47..bfaba88f85 100644 --- a/system/services/rulesEngine/RulesEngineExpressionService.cfc +++ b/system/services/rulesEngine/RulesEngineExpressionService.cfc @@ -298,6 +298,7 @@ component displayName="RulesEngine Expression Service" { , required struct configuredFields ) { _lazyLoadDynamicExpressions( context=arguments.context ) + try { var expression = _getRawExpression( expressionId ); } catch( any e ) { @@ -307,30 +308,34 @@ component displayName="RulesEngine Expression Service" { var contexts = expression.contexts ?: []; - if ( !contexts.findNoCase( arguments.context ) && !contexts.findNoCase( "global" ) ) { + if ( !ArrayFindNoCase( contexts, arguments.context ) && !ArrayFindNoCase( contexts, "global" ) ) { throw( type = "preside.rule.expression.invalid.context" , message = "The expression [#arguments.expressionId#] cannot be used in the [#arguments.context#] context." ); } - var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; - var eventArgs = { - context = arguments.context - , payload = arguments.payload - }; + var requestCacheKey = arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ); - eventArgs.append( expression.expressionHandlerArgs ?: {} ); - eventArgs.append( preProcessConfiguredFields( arguments.expressionId, arguments.configuredFields ) ); + if ( !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { + var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; + var eventArgs = { + context = arguments.context + , payload = arguments.payload + }; - var result = $getColdbox().runEvent( - event = handlerAction - , private = true - , prePostExempt = true - , eventArguments = eventArgs - ); + eventArgs.append( expression.expressionHandlerArgs ?: {} ); + eventArgs.append( preProcessConfiguredFields( arguments.expressionId, arguments.configuredFields ) ); - return result; + request._rulesEngineEvaluateExpressionCache[ requestCacheKey ] = $getColdbox().runEvent( + event = handlerAction + , private = true + , prePostExempt = true + , eventArguments = eventArgs + ); + } + + return request._rulesEngineEvaluateExpressionCache[ requestCacheKey ]; } /** @@ -618,18 +623,24 @@ component displayName="RulesEngine Expression Service" { } private void function _lazyLoadDynamicExpressions( string context="", string filterObject="" ) { + var contextCacheKey = "context=#arguments.context#,filterObject=#arguments.filterObject#"; + variables._lazyLoadDone = variables._lazyLoadDone ?: {}; + if ( StructKeyExists( variables._lazyLoadDone, contextCacheKey ) ) { + return; + } + var objects = []; var contextService = _getContextService(); if ( Len( Trim( arguments.filterObject ) ) ) { - objects.append( arguments.filterObject ); + ArrayAppend( objects, arguments.filterObject ); } if ( Len( Trim( arguments.context ) ) ) { var contextObjects = contextService.getContextObject( arguments.context, true ); - if ( contextObjects.len() ) { - objects.append( contextObjects, true ); + if ( ArrayLen( contextObjects ) ) { + ArrayAppend( objects, contextObjects, true ); } } @@ -645,6 +656,8 @@ component displayName="RulesEngine Expression Service" { variables._lazyLoadDone[ objectName ] = true; } } + + variables._lazyLoadDone[ contextCacheKey ] = true; } private array function _getAdminUserRoles( string adminUserId=$getAdminLoginService().getLoggedInUserId() ) { From 9b4e8f477a24cbb642dceef128011d6ee13253af Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Tue, 21 Nov 2023 12:38:46 +0000 Subject: [PATCH 4/8] PRESIDECMS-2745 fix test to not use the request cache (that interferes with the unnatural nature of the test --- .../api/rulesEngine/RulesEngineContextServiceTest.cfc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc b/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc index 24da9f30d1..650d376f6a 100644 --- a/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc +++ b/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc @@ -218,6 +218,8 @@ component extends="resources.HelperObjects.PresideBddTestCase" { mockColdbox = createEmptyMock( "preside.system.coldboxModifications.Controller" ); service.$( "$getColdbox", mockColdbox ); + request._rulesEngineContextPayloadCache = {}; + return service; } From 3fa397515a7386eb022daec9b8491a3a7e00e2b2 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Wed, 22 Nov 2023 10:34:42 +0000 Subject: [PATCH 5/8] PRESIDECMS-2745 do not use request cache for background threads/ email sends where the context can change in the life of the 'request' --- .../rulesEngine/RulesEngineContextService.cfc | 13 +++++++++---- .../rulesEngine/RulesEngineExpressionService.cfc | 13 ++++++++++--- .../rulesEngine/RulesEngineContextServiceTest.cfc | 5 ++++- .../RulesEngineExpressionServiceTest.cfc | 5 +++++ 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineContextService.cfc b/system/services/rulesEngine/RulesEngineContextService.cfc index 463d22187d..3933ee64cd 100644 --- a/system/services/rulesEngine/RulesEngineContextService.cfc +++ b/system/services/rulesEngine/RulesEngineContextService.cfc @@ -193,11 +193,12 @@ component displayName="RulesEngine Context Service" { */ public struct function getContextPayload( required string context, struct args={} ) { var cb = $getColdbox(); - var cacheKey = arguments.context & Hash( SerializeJson( args ) ); + var noCache = $getRequestContext().isEmailRenderingContext() || $getRequestContext().isBackgroundThread(); + var cacheKey = noCache ? "" : arguments.context & Hash( SerializeJson( args ) ); - if ( !StructKeyExists( request, "_rulesEngineContextPayloadCache" ) || !StructKeyExists( request._rulesEngineContextPayloadCache, cacheKey ) ) { - var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); - var payload = {}; + if ( noCache || ( !StructKeyExists( request, "_rulesEngineContextPayloadCache" ) || !StructKeyExists( request._rulesEngineContextPayloadCache, cacheKey ) ) ) { + var expanded = listValidExpressionContextsForParentContexts( [ arguments.context ] ); + var payload = {}; for( var cx in expanded ) { var handlerAction = "rules.contexts.#cx#.getPayload"; @@ -212,6 +213,10 @@ component displayName="RulesEngine Context Service" { } } + if ( noCache ) { + return payload; + } + request._rulesEngineContextPayloadCache[ cacheKey ] = payload; } diff --git a/system/services/rulesEngine/RulesEngineExpressionService.cfc b/system/services/rulesEngine/RulesEngineExpressionService.cfc index bfaba88f85..3d1fd3b418 100644 --- a/system/services/rulesEngine/RulesEngineExpressionService.cfc +++ b/system/services/rulesEngine/RulesEngineExpressionService.cfc @@ -315,9 +315,10 @@ component displayName="RulesEngine Expression Service" { ); } - var requestCacheKey = arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ); + var noCache = $getRequestContext().isEmailRenderingContext() || $getRequestContext().isBackgroundThread(); + var requestCacheKey = noCache ? "" : ( arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ) ); - if ( !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { + if ( noCache || !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; var eventArgs = { context = arguments.context @@ -327,12 +328,18 @@ component displayName="RulesEngine Expression Service" { eventArgs.append( expression.expressionHandlerArgs ?: {} ); eventArgs.append( preProcessConfiguredFields( arguments.expressionId, arguments.configuredFields ) ); - request._rulesEngineEvaluateExpressionCache[ requestCacheKey ] = $getColdbox().runEvent( + var result = $getColdbox().runEvent( event = handlerAction , private = true , prePostExempt = true , eventArguments = eventArgs ); + + if ( noCache ) { + return result; + } + + request._rulesEngineEvaluateExpressionCache[ requestCacheKey ] = result; } return request._rulesEngineEvaluateExpressionCache[ requestCacheKey ]; diff --git a/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc b/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc index 650d376f6a..d3ce29c9a3 100644 --- a/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc +++ b/tests/integration/api/rulesEngine/RulesEngineContextServiceTest.cfc @@ -216,9 +216,12 @@ component extends="resources.HelperObjects.PresideBddTestCase" { ) ); mockColdbox = createEmptyMock( "preside.system.coldboxModifications.Controller" ); + mockRequestContext = createStub(); service.$( "$getColdbox", mockColdbox ); + service.$( "$getRequestContext", mockRequestContext ); - request._rulesEngineContextPayloadCache = {}; + mockRequestContext.$( "isEmailRenderingContext", false ); + mockRequestContext.$( "isBackgroundThread" , true ); return service; } diff --git a/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc b/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc index e1e13cda7c..b912d9ca4d 100644 --- a/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc +++ b/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc @@ -711,6 +711,7 @@ component extends="resources.HelperObjects.PresideBddTestCase" { variables.mockI18n = createStub(); variables.mockExpressions = arguments.expressions; variables.mockColdboxController = CreateStub(); + variables.mockRequestContext = createStub(); mockReaderService.$( "getExpressionsFromDirectories" ).$args( mockDirectories ).$results( mockExpressions ); mockI18n.$( "getFWLanguageCode" ).$results( "en" ); mockI18n.$( "getFWCountryCode" ).$results( "" ); @@ -729,6 +730,7 @@ component extends="resources.HelperObjects.PresideBddTestCase" { service = createMock( object=service ); service.$( "$getColdbox", mockColdboxController ); + service.$( "$getRequestContext", mockRequestContext ); service.$( "_lazyLoadDynamicExpressions" ); mockContextService.$( "getContextObject" ).$args( "request" ).$results( "request_object" ); mockContextService.$( "getContextObject" ).$args( "" ).$results( "" ); @@ -736,6 +738,9 @@ component extends="resources.HelperObjects.PresideBddTestCase" { service.$( "$getAdminLoginService", mockAdminLoginService ); service.$( "$getAdminPermissionService", mockAdminPermissionService ); + mockRequestContext.$( "isEmailRenderingContext", false ); + mockRequestContext.$( "isBackgroundThread" , true ); + return service; } From a44dcc0b419a33d6a246cff9a83959883b8d3d48 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Wed, 22 Nov 2023 10:41:11 +0000 Subject: [PATCH 6/8] PRESIDECMS-2745 refactor to not interfere with existing fields in a cached query This should prevent any compatibility issues where other logic performs this same query and uses the expressions field --- .../services/rulesEngine/RulesEngineConditionService.cfc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineConditionService.cfc b/system/services/rulesEngine/RulesEngineConditionService.cfc index 5a1b332c7a..d1ca0ff52b 100644 --- a/system/services/rulesEngine/RulesEngineConditionService.cfc +++ b/system/services/rulesEngine/RulesEngineConditionService.cfc @@ -41,14 +41,17 @@ component displayName="RulesEngine Condition Service" { var conditionRecord = $getPresideObject( "rules_engine_condition" ).selectData( id=arguments.conditionId ); if ( conditionRecord.recordCount ) { - if ( IsSimpleValue( conditionRecord.expressions ) ) { - conditionRecord.expressions[ 1 ] = DeSerializeJson( conditionRecord.expressions ); // deliberately against a cached query to avoid doing this multiple times + // deliberately against a cached query to avoid doing this multiple times + // where possible + if ( !QueryColumnExists( conditionRecord, "serializedExpressions" ) ) { + QueryAddColumn( conditionRecord, "serializedExpressions", [ DeSerializeJson( conditionRecord.expressions ) ] ) } + return { id = arguments.conditionId , name = conditionRecord.condition_name , context = conditionRecord.context - , expressions = conditionRecord.expressions + , expressions = conditionRecord.serializedExpressions }; } From 38ed7c137ae6969f8ee3806aaaba894f4aeba225 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Wed, 22 Nov 2023 11:40:56 +0000 Subject: [PATCH 7/8] PRESIDECMS-2745 undo cache skipping in condition evaluation This is not necessary as the cache key contains the context payload + expression config so will be fine when context changing in a bg thread --- .../rulesEngine/RulesEngineExpressionService.cfc | 9 ++------- .../api/rulesEngine/RulesEngineExpressionServiceTest.cfc | 5 ----- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineExpressionService.cfc b/system/services/rulesEngine/RulesEngineExpressionService.cfc index 3d1fd3b418..832755d917 100644 --- a/system/services/rulesEngine/RulesEngineExpressionService.cfc +++ b/system/services/rulesEngine/RulesEngineExpressionService.cfc @@ -315,10 +315,9 @@ component displayName="RulesEngine Expression Service" { ); } - var noCache = $getRequestContext().isEmailRenderingContext() || $getRequestContext().isBackgroundThread(); - var requestCacheKey = noCache ? "" : ( arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ) ); + var requestCacheKey = arguments.expressionId & arguments.context & SerializeJson( arguments.payload ) & SerializeJson( arguments.configuredFields ); - if ( noCache || !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { + if ( !StructKeyExists( request, "_rulesEngineEvaluateExpressionCache" ) || !StructKeyExists( request._rulesEngineEvaluateExpressionCache, requestCacheKey ) ) { var handlerAction = expression.expressionhandler ?: "rules.expressions." & arguments.expressionId & ".evaluateExpression"; var eventArgs = { context = arguments.context @@ -335,10 +334,6 @@ component displayName="RulesEngine Expression Service" { , eventArguments = eventArgs ); - if ( noCache ) { - return result; - } - request._rulesEngineEvaluateExpressionCache[ requestCacheKey ] = result; } diff --git a/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc b/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc index b912d9ca4d..e1e13cda7c 100644 --- a/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc +++ b/tests/integration/api/rulesEngine/RulesEngineExpressionServiceTest.cfc @@ -711,7 +711,6 @@ component extends="resources.HelperObjects.PresideBddTestCase" { variables.mockI18n = createStub(); variables.mockExpressions = arguments.expressions; variables.mockColdboxController = CreateStub(); - variables.mockRequestContext = createStub(); mockReaderService.$( "getExpressionsFromDirectories" ).$args( mockDirectories ).$results( mockExpressions ); mockI18n.$( "getFWLanguageCode" ).$results( "en" ); mockI18n.$( "getFWCountryCode" ).$results( "" ); @@ -730,7 +729,6 @@ component extends="resources.HelperObjects.PresideBddTestCase" { service = createMock( object=service ); service.$( "$getColdbox", mockColdboxController ); - service.$( "$getRequestContext", mockRequestContext ); service.$( "_lazyLoadDynamicExpressions" ); mockContextService.$( "getContextObject" ).$args( "request" ).$results( "request_object" ); mockContextService.$( "getContextObject" ).$args( "" ).$results( "" ); @@ -738,9 +736,6 @@ component extends="resources.HelperObjects.PresideBddTestCase" { service.$( "$getAdminLoginService", mockAdminLoginService ); service.$( "$getAdminPermissionService", mockAdminPermissionService ); - mockRequestContext.$( "isEmailRenderingContext", false ); - mockRequestContext.$( "isBackgroundThread" , true ); - return service; } From 407b402fb02b469bd2a4eacc2b6d719ebf0a7297 Mon Sep 17 00:00:00 2001 From: Dominic Watson Date: Wed, 22 Nov 2023 11:52:58 +0000 Subject: [PATCH 8/8] PRESIDECMS-2745 correct variable name for the situation --- system/services/rulesEngine/RulesEngineConditionService.cfc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system/services/rulesEngine/RulesEngineConditionService.cfc b/system/services/rulesEngine/RulesEngineConditionService.cfc index d1ca0ff52b..d7d5f811f5 100644 --- a/system/services/rulesEngine/RulesEngineConditionService.cfc +++ b/system/services/rulesEngine/RulesEngineConditionService.cfc @@ -43,15 +43,15 @@ component displayName="RulesEngine Condition Service" { if ( conditionRecord.recordCount ) { // deliberately against a cached query to avoid doing this multiple times // where possible - if ( !QueryColumnExists( conditionRecord, "serializedExpressions" ) ) { - QueryAddColumn( conditionRecord, "serializedExpressions", [ DeSerializeJson( conditionRecord.expressions ) ] ) + if ( !QueryColumnExists( conditionRecord, "deserializedExpressions" ) ) { + QueryAddColumn( conditionRecord, "deserializedExpressions", [ DeSerializeJson( conditionRecord.expressions ) ] ) } return { id = arguments.conditionId , name = conditionRecord.condition_name , context = conditionRecord.context - , expressions = conditionRecord.serializedExpressions + , expressions = conditionRecord.deserializedExpressions }; }