diff --git a/expression-src/main/src/interpreter/Environment.cls b/expression-src/main/src/interpreter/Environment.cls index aa6ce8e..f5423ed 100644 --- a/expression-src/main/src/interpreter/Environment.cls +++ b/expression-src/main/src/interpreter/Environment.cls @@ -26,6 +26,16 @@ public with sharing class Environment { */ private static final Map GLOBAL_CONTEXT_VARIABLES = new Map(); + /** + * @description A snapshot of the global context variables (e.g. custom `@` variables) as they + * were when this Environment was constructed. Context variables live in the static + * GLOBAL_CONTEXT_VARIABLES map, which is reset after each evaluation, so they are not part of + * the instance state otherwise. Capturing a copy here makes them part of the Environment's + * identity (see equals/hashCode), so result caches keyed by Environment do not incorrectly + * reuse results across evaluations that differ only by their context variable values. + */ + private final Map contextSnapshot = new Map(GLOBAL_CONTEXT_VARIABLES); + private static Map getAllGlobalVariables() { Map allGlobalVariables = new Map(); allGlobalVariables.putAll(GLOBAL_VARIABLES); @@ -190,11 +200,17 @@ public with sharing class Environment { public Boolean equals(Object obj) { Environment other = (Environment)obj; - return parentEnvironment == other.parentEnvironment && context == other.context && variables.equals(other.variables); + return parentEnvironment == other.parentEnvironment && + context == other.context && + variables.equals(other.variables) && + contextSnapshot.equals(other.contextSnapshot); } public override Integer hashCode() { - return (parentEnvironment != null ? parentEnvironment.hashCode() : 0) ^ (context != null ? context.hashCode() : 0) ^ variables.hashCode(); + return (parentEnvironment != null ? parentEnvironment.hashCode() : 0) ^ + (context != null ? context.hashCode() : 0) ^ + variables.hashCode() ^ + contextSnapshot.hashCode(); } public class ClearContext implements EvaluatorEventListener { diff --git a/expression-src/main/src/interpreter/tests/EvaluatorCustomContextCacheTest.cls b/expression-src/main/src/interpreter/tests/EvaluatorCustomContextCacheTest.cls index c001b03..df1a547 100644 --- a/expression-src/main/src/interpreter/tests/EvaluatorCustomContextCacheTest.cls +++ b/expression-src/main/src/interpreter/tests/EvaluatorCustomContextCacheTest.cls @@ -32,4 +32,72 @@ private class EvaluatorCustomContextCacheTest { ); } + @IsTest + static void standardFunctionCacheEnabled_respectsChangingCustomContextValues() { + Configuration config1 = new Configuration(); + config1.cacheStandardFunctionResults = true; + config1.customContext = new Map{ + 'value' => 'first' + }; + + Configuration config2 = new Configuration(); + config2.cacheStandardFunctionResults = true; + config2.customContext = new Map{ + 'value' => 'second' + }; + + Test.startTest(); + Object firstResult = Evaluator.run('UPPER(@value)', config1); + Object secondResult = Evaluator.run('UPPER(@value)', config2); + Test.stopTest(); + + System.assertEquals('FIRST', firstResult, 'First evaluation must use the first customContext.'); + System.assertEquals( + 'SECOND', + secondResult, + 'Second evaluation must respect the updated context variable even when caching is enabled.' + ); + } + + @IsTest + static void queryCacheRespectsChangingCustomContextValues() { + // Two records that the same query can match depending on the @name context variable. + Account first = new Account(Name = 'CtxCacheFirst'); + Account second = new Account(Name = 'CtxCacheSecond'); + insert new List{ first, second }; + + Configuration config1 = new Configuration(); + config1.cacheStandardFunctionResults = true; + config1.customContext = new Map{ + 'name' => 'CtxCacheFirst' + }; + + Configuration config2 = new Configuration(); + config2.cacheStandardFunctionResults = true; + config2.customContext = new Map{ + 'name' => 'CtxCacheSecond' + }; + + String expr = 'QUERY(Account(where: Name = @name) [Id, Name])'; + + Test.startTest(); + List firstResult = (List) Evaluator.run(expr, config1); + List secondResult = (List) Evaluator.run(expr, config2); + Test.stopTest(); + + System.assertEquals(1, firstResult.size(), 'First query should match exactly one Account.'); + System.assertEquals( + 'CtxCacheFirst', + firstResult[0].Name, + 'First query must match the Account named by the first customContext.' + ); + + System.assertEquals(1, secondResult.size(), 'Second query should match exactly one Account.'); + System.assertEquals( + 'CtxCacheSecond', + secondResult[0].Name, + 'Second query must respect the updated context variable, not return the cached result from the first run.' + ); + } + } \ No newline at end of file diff --git a/expression-src/main/src/resolver/EvaluatorResolver.cls b/expression-src/main/src/resolver/EvaluatorResolver.cls index 9409392..be753fd 100644 --- a/expression-src/main/src/resolver/EvaluatorResolver.cls +++ b/expression-src/main/src/resolver/EvaluatorResolver.cls @@ -206,12 +206,12 @@ public with sharing abstract class EvaluatorResolver { if (records != null && records.size() > 0) { record = records[0]; } - Environment env = new Environment(record); - + // Register the global context variables before constructing the Environment so that + // the Environment's context snapshot (used as part of its cache-key identity) is complete. Environment.addGlobalContextVariable(contextPrefix, 'id', recordId); Environment.addGlobalContextVariable(contextPrefix, 'context', record); - return env; + return new Environment(record); } } @@ -230,15 +230,15 @@ public with sharing abstract class EvaluatorResolver { } ContextResolver ctxInterpreter = new ContextResolver(recordContexts, contextPrefix, customFunctionDeclarations); List records = ctxInterpreter.build(expressions); - // The environment is created without a record, as we are dealing with a list of records, - // so it would not be possible to interpret references to record fields. Instead, the @context - // variable should be used to access the list of records. - Environment env = new Environment(); - + // Register the global context variables before constructing the Environment so that + // the Environment's context snapshot (used as part of its cache-key identity) is complete. Environment.addGlobalContextVariable(contextPrefix, 'ids', recordIds); Environment.addGlobalContextVariable(contextPrefix, 'context', records); - return env; + // The environment is created without a record, as we are dealing with a list of records, + // so it would not be possible to interpret references to record fields. Instead, the @context + // variable should be used to access the list of records. + return new Environment(); } } @@ -251,8 +251,6 @@ public with sharing abstract class EvaluatorResolver { public override Environment prepareEnvironment(List expressions, List customFunctionDeclarations, String contextPrefix) { - Environment env = new Environment(); - Map> contextsBySObjectTypes = new Map>(); for (CustomRecordContext context : this.contexts) { SObjectType sObjectType = context.recordId.getSobjectType(); @@ -288,7 +286,9 @@ public with sharing abstract class EvaluatorResolver { } } - return env; + // Construct the Environment after all global context variables have been registered so + // that its context snapshot (used as part of its cache-key identity) is complete. + return new Environment(); } }