diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java index 97d8173bd..d9b0df0ea 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java @@ -136,7 +136,7 @@ static String reconstructNode( .getELResolver() .getValue(context, null, ExtendedParser.INTERPRETER) ).getContext() - .getMetaContextVariables() + .getComputedMetaContextVariables() .contains(name) ) { return name; diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 22e938199..b619a6238 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -20,6 +20,7 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.SetMultimap; +import com.google.common.collect.Sets; import com.hubspot.jinjava.lib.Importable; import com.hubspot.jinjava.lib.expression.DefaultExpressionStrategy; import com.hubspot.jinjava.lib.expression.ExpressionStrategy; @@ -34,6 +35,7 @@ import com.hubspot.jinjava.lib.tag.Tag; import com.hubspot.jinjava.lib.tag.TagLibrary; import com.hubspot.jinjava.lib.tag.eager.DeferredToken; +import com.hubspot.jinjava.mode.EagerExecutionMode; import com.hubspot.jinjava.tree.Node; import com.hubspot.jinjava.util.DeferredValueUtils; import com.hubspot.jinjava.util.ScopeMap; @@ -117,6 +119,7 @@ public enum Library { private boolean unwrapRawOverride = false; private DynamicVariableResolver dynamicVariableResolver = null; private final Set metaContextVariables; // These variable names aren't tracked in eager execution + private final Set overriddenNonMetaContextVariables; private Node currentNode; public Context() { @@ -209,6 +212,8 @@ public Context( new FunctionLibrary(parent == null, disabled.get(Library.FUNCTION)); this.metaContextVariables = parent == null ? new HashSet<>() : parent.metaContextVariables; + this.overriddenNonMetaContextVariables = + parent == null ? new HashSet<>() : parent.overriddenNonMetaContextVariables; if (parent != null) { this.expressionStrategy = parent.expressionStrategy; this.partialMacroEvaluation = parent.partialMacroEvaluation; @@ -348,10 +353,37 @@ public void addResolvedFunction(String function) { } } + @Deprecated + @Beta public Set getMetaContextVariables() { return metaContextVariables; } + @Beta + public Set getComputedMetaContextVariables() { + return Sets.difference(metaContextVariables, overriddenNonMetaContextVariables); + } + + @Beta + public void addMetaContextVariables(Collection variables) { + metaContextVariables.addAll(variables); + } + + @Beta + public void addNonMetaContextVariables(Collection variables) { + overriddenNonMetaContextVariables.addAll( + variables + .stream() + .filter(var -> !EagerExecutionMode.STATIC_META_CONTEXT_VARIABLES.contains(var)) + .collect(Collectors.toList()) + ); + } + + @Beta + public void removeNonMetaContextVariables(Collection variables) { + overriddenNonMetaContextVariables.removeAll(variables); + } + public void handleDeferredNode(Node node) { if ( JinjavaInterpreter diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index dc7bee8f4..f14ccfa00 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -111,7 +111,6 @@ public JinjavaInterpreter( this.context = context; this.config = renderConfig; this.application = application; - this.config.getExecutionMode().prepareContext(this.context); switch (config.getRandomNumberGeneratorStrategy()) { diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java index 0b227b260..8bc2c888b 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java @@ -37,7 +37,6 @@ import com.hubspot.jinjava.tree.Node; import com.hubspot.jinjava.tree.TagNode; import com.hubspot.jinjava.tree.parse.TagToken; -import com.hubspot.jinjava.util.EagerReconstructionUtils; import com.hubspot.jinjava.util.ForLoop; import com.hubspot.jinjava.util.HelperStringTokenizer; import com.hubspot.jinjava.util.LengthLimitingStringBuilder; @@ -48,7 +47,6 @@ import java.util.List; import java.util.Map.Entry; import java.util.Optional; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.lang3.tuple.Pair; @@ -165,11 +163,7 @@ public String renderForCollection( ) { ForLoop loop = ObjectIterator.getLoop(collection); - Set removedMetaContextVariables = - EagerReconstructionUtils.removeMetaContextVariables( - loopVars.stream(), - interpreter.getContext() - ); + interpreter.getContext().addNonMetaContextVariables(loopVars); try (InterpreterScopeClosable c = interpreter.enterScope()) { if (interpreter.isValidationMode() && !loop.hasNext()) { loop = ObjectIterator.getLoop(new DummyObject()); @@ -298,10 +292,7 @@ public String renderForCollection( } return checkLoopVariable(interpreter, buff); } finally { - interpreter - .getContext() - .getMetaContextVariables() - .addAll(removedMetaContextVariables); + interpreter.getContext().removeNonMetaContextVariables(loopVars); } } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/DeferredToken.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/DeferredToken.java index dc741cb59..306eeaf1f 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/DeferredToken.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/DeferredToken.java @@ -378,7 +378,7 @@ private static Collection markDeferredWordsAndFindSources( } return !(val instanceof DeferredValue); }) - .filter(prop -> !context.getMetaContextVariables().contains(prop)) + .filter(prop -> !context.getComputedMetaContextVariables().contains(prop)) .filter(prop -> { DeferredValue deferredValue = convertToDeferredValue(context, prop); context.put(prop, deferredValue); diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java index a17390207..c16fb1c6b 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java @@ -185,11 +185,7 @@ private EagerExecutionResult runLoopOnce( List loopVars = getTag() .getLoopVarsAndExpression((TagToken) tagNode.getMaster()) .getLeft(); - Set removedMetaContextVariables = - EagerReconstructionUtils.removeMetaContextVariables( - loopVars.stream(), - interpreter.getContext() - ); + interpreter.getContext().addNonMetaContextVariables(loopVars); loopVars.forEach(var -> interpreter.getContext().put(var, DeferredValue.instance()) ); @@ -198,10 +194,7 @@ private EagerExecutionResult runLoopOnce( renderChildren(tagNode, eagerInterpreter) ); } finally { - interpreter - .getContext() - .getMetaContextVariables() - .addAll(removedMetaContextVariables); + interpreter.getContext().removeNonMetaContextVariables(loopVars); if (clearDeferredWords) { interpreter .getContext() diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java index 228468b84..09e45b68b 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java @@ -40,11 +40,11 @@ public String run(TagNode tagNode, JinjavaInterpreter interpreter) { variables = new String[] { var }; expression = tagNode.getHelpers(); } - - EagerReconstructionUtils.removeMetaContextVariables( - Arrays.stream(variables).map(String::trim), - interpreter.getContext() - ); + interpreter + .getContext() + .addNonMetaContextVariables( + Arrays.stream(variables).map(String::trim).collect(Collectors.toList()) + ); EagerExecutionResult eagerExecutionResult = getEagerExecutionResult( tagNode, @@ -167,7 +167,7 @@ public static String getSuffixToPreserveState( if ( maybeTemporaryImportAlias.isPresent() && !AliasedEagerImportingStrategy.isTemporaryImportAlias(variables) && - !interpreter.getContext().getMetaContextVariables().contains(variables) + !interpreter.getContext().getComputedMetaContextVariables().contains(variables) ) { if (!interpreter.getContext().containsKey(maybeTemporaryImportAlias.get())) { if ( diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/FlatEagerImportingStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/FlatEagerImportingStrategy.java index 6a596d3ba..bb274441e 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/FlatEagerImportingStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/FlatEagerImportingStrategy.java @@ -72,7 +72,7 @@ public String getFinalOutput( Set metaContextVariables = importingData .getOriginalInterpreter() .getContext() - .getMetaContextVariables(); + .getComputedMetaContextVariables(); // defer imported variables EagerReconstructionUtils.buildSetTag( child diff --git a/src/main/java/com/hubspot/jinjava/mode/EagerExecutionMode.java b/src/main/java/com/hubspot/jinjava/mode/EagerExecutionMode.java index 00276918a..82a82ad90 100644 --- a/src/main/java/com/hubspot/jinjava/mode/EagerExecutionMode.java +++ b/src/main/java/com/hubspot/jinjava/mode/EagerExecutionMode.java @@ -53,6 +53,6 @@ public void prepareContext(Context context) { .filter(Optional::isPresent) .forEach(maybeEagerTag -> context.registerTag(maybeEagerTag.get())); context.setExpressionStrategy(new EagerExpressionStrategy()); - context.getMetaContextVariables().addAll(STATIC_META_CONTEXT_VARIABLES); + context.addMetaContextVariables(STATIC_META_CONTEXT_VARIABLES); } } diff --git a/src/main/java/com/hubspot/jinjava/util/DeferredValueUtils.java b/src/main/java/com/hubspot/jinjava/util/DeferredValueUtils.java index 487d0921e..ba26fb6d5 100644 --- a/src/main/java/com/hubspot/jinjava/util/DeferredValueUtils.java +++ b/src/main/java/com/hubspot/jinjava/util/DeferredValueUtils.java @@ -119,7 +119,7 @@ private static void markDeferredProperties(Context context, Set props) { props .stream() .filter(prop -> !(context.get(prop) instanceof DeferredValue)) - .filter(prop -> !context.getMetaContextVariables().contains(prop)) + .filter(prop -> !context.getComputedMetaContextVariables().contains(prop)) .forEach(prop -> { Object value = context.get(prop); if (value != null) { diff --git a/src/main/java/com/hubspot/jinjava/util/EagerContextWatcher.java b/src/main/java/com/hubspot/jinjava/util/EagerContextWatcher.java index 0242e2e00..c848fe454 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerContextWatcher.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerContextWatcher.java @@ -50,7 +50,7 @@ public static EagerExecutionResult executeInChildContext( ) { final Set metaContextVariables = interpreter .getContext() - .getMetaContextVariables(); + .getComputedMetaContextVariables(); final EagerExecutionResult initialResult; final Map speculativeBindings; if (eagerChildContextConfig.checkForContextChanges) { diff --git a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java index 1f1ecdf1d..9467c2bb0 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java @@ -2,7 +2,6 @@ import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Sets; import com.hubspot.jinjava.el.ext.AbstractCallableMethod; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.Context.Library; @@ -25,7 +24,6 @@ import com.hubspot.jinjava.lib.tag.eager.importing.AliasedEagerImportingStrategy; import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategyFactory; import com.hubspot.jinjava.loader.RelativePathResolver; -import com.hubspot.jinjava.mode.EagerExecutionMode; import com.hubspot.jinjava.objects.serialization.PyishBlockSetSerializable; import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.objects.serialization.PyishSerializable; @@ -313,7 +311,9 @@ private static PrefixToPreserveState hydrateReconstructionOfVariablesBeforeDefer JinjavaInterpreter interpreter, int depth ) { - Set metaContextVariables = interpreter.getContext().getMetaContextVariables(); + Set metaContextVariables = interpreter + .getContext() + .getComputedMetaContextVariables(); deferredWords .stream() .filter(w -> !metaContextVariables.contains(w)) @@ -753,22 +753,6 @@ public static String wrapInChildScope(String toWrap, JinjavaInterpreter interpre ); } - public static Set removeMetaContextVariables( - Stream varStream, - Context context - ) { - Set metaSetVars = Sets - .intersection( - context.getMetaContextVariables(), - varStream - .filter(var -> !EagerExecutionMode.STATIC_META_CONTEXT_VARIABLES.contains(var)) - .collect(Collectors.toSet()) - ) - .immutableCopy(); - context.getMetaContextVariables().removeAll(metaSetVars); - return metaSetVars; - } - public static Boolean isDeferredExecutionMode() { return JinjavaInterpreter .getCurrentMaybe() diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index fc23ebf4d..e333c827f 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -22,6 +22,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -990,7 +991,7 @@ public void itHandlesImportInDeferredIf() { @Test public void itAllowsMetaContextVarOverriding() { - interpreter.getContext().getMetaContextVariables().add("meta"); + interpreter.getContext().addMetaContextVariables(Collections.singleton("meta")); interpreter.getContext().put("meta", "META"); expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( "allows-meta-context-var-overriding" @@ -1471,7 +1472,7 @@ public void itReconstructsNestedValueInStringRepresentationSecondPass() { @Test public void itDefersLoopSettingMetaContextVar() { - interpreter.getContext().getMetaContextVariables().add("content"); + interpreter.getContext().addMetaContextVariables(Collections.singleton("content")); expectedTemplateInterpreter.assertExpectedOutput( "defers-loop-setting-meta-context-var" ); @@ -1480,7 +1481,7 @@ public void itDefersLoopSettingMetaContextVar() { @Test public void itDefersLoopSettingMetaContextVarSecondPass() { interpreter.getContext().put("deferred", "resolved"); - interpreter.getContext().getMetaContextVariables().add("content"); + interpreter.getContext().addMetaContextVariables(Collections.singleton("content")); expectedTemplateInterpreter.assertExpectedOutput( "defers-loop-setting-meta-context-var.expected" ); @@ -1572,4 +1573,21 @@ public void itReconstructsBlockPathWhenDeferredNestedSecondPass() { "reconstructs-block-path-when-deferred-nested/test.expected" ); } + + @Test + public void itKeepsMetaContextVariablesThroughImport() { + setupWithExecutionMode( + new EagerExecutionMode() { + @Override + public void prepareContext(Context context) { + super.prepareContext(context); + context.addMetaContextVariables(Collections.singleton("meta")); + } + } + ); + interpreter.getContext().put("meta", new ArrayList<>()); + expectedTemplateInterpreter.assertExpectedOutput( + "keeps-meta-context-variables-through-import/test" + ); + } } diff --git a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java index 6946be4f8..7a24f9e5b 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java @@ -16,6 +16,7 @@ import com.hubspot.jinjava.objects.collections.PyMap; import com.hubspot.jinjava.random.RandomNumberGeneratorStrategy; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -225,7 +226,7 @@ public void itPreservesAstTuple() { @Test public void itPreservesUnresolvable() { interpreter.getContext().put("foo_object", new Foo()); - interpreter.getContext().getMetaContextVariables().add("foo_object"); + interpreter.getContext().addMetaContextVariables(Collections.singleton("foo_object")); try { interpreter.resolveELExpression("foo_object.deferred|upper", -1); fail("Should throw DeferredParsingException"); diff --git a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java index a6c40ab2f..fca09dd45 100644 --- a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java +++ b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java @@ -600,7 +600,7 @@ public void itHandlesPyishSerializable() { @Test public void itHandlesPyishSerializableWithProcessingException() { context.put("foo", new SomethingExceptionallyPyish("yes")); - context.getMetaContextVariables().add("foo"); + context.addMetaContextVariables(Collections.singleton("foo")); assertThat(interpreter.render("{{ deferred && (1 == 2 || foo) }}")) .isEqualTo("{{ deferred && (false || foo) }}"); } diff --git a/src/test/java/com/hubspot/jinjava/util/EagerReconstructionUtilsTest.java b/src/test/java/com/hubspot/jinjava/util/EagerReconstructionUtilsTest.java index 04ee6917e..5bc68dad3 100644 --- a/src/test/java/com/hubspot/jinjava/util/EagerReconstructionUtilsTest.java +++ b/src/test/java/com/hubspot/jinjava/util/EagerReconstructionUtilsTest.java @@ -2,8 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -34,7 +33,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Stream; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -350,27 +348,33 @@ public void itIgnoresMetaContextVariables() { } @Test - public void itDoesNotRemoveStaticMetaContextVariables() { + public void itRemovesOtherMetaContextVariables() { String variableName = "foo"; - interpreter.getContext().getMetaContextVariables().add(variableName); - assertThat(interpreter.getContext().getMetaContextVariables()).contains(variableName); - EagerReconstructionUtils.removeMetaContextVariables( - Stream.of(variableName), - interpreter.getContext() - ); - assertThat(interpreter.getContext().getMetaContextVariables()) + interpreter.getContext().addMetaContextVariables(Collections.singleton(variableName)); + assertThat(interpreter.getContext().getComputedMetaContextVariables()) + .contains(variableName); + interpreter + .getContext() + .addNonMetaContextVariables(Collections.singleton(variableName)); + assertThat(interpreter.getContext().getComputedMetaContextVariables()) .doesNotContain(variableName); + interpreter + .getContext() + .removeNonMetaContextVariables(Collections.singleton(variableName)); + assertThat(interpreter.getContext().getComputedMetaContextVariables()) + .contains(variableName); } @Test - public void itRemovesOtherMetaContextVariables() { - assertThat(interpreter.getContext().getMetaContextVariables()) + public void itDoesNotRemoveStaticMetaContextVariables() { + assertThat(interpreter.getContext().getComputedMetaContextVariables()) .contains(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY); - EagerReconstructionUtils.removeMetaContextVariables( - Stream.of(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY), - interpreter.getContext() - ); - assertThat(interpreter.getContext().getMetaContextVariables()) + interpreter + .getContext() + .addNonMetaContextVariables( + Collections.singleton(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY) + ); + assertThat(interpreter.getContext().getComputedMetaContextVariables()) .contains(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY); } diff --git a/src/test/resources/eager/keeps-meta-context-variables-through-import/import-target.jinja b/src/test/resources/eager/keeps-meta-context-variables-through-import/import-target.jinja new file mode 100644 index 000000000..707978124 --- /dev/null +++ b/src/test/resources/eager/keeps-meta-context-variables-through-import/import-target.jinja @@ -0,0 +1 @@ +I am a boring import \ No newline at end of file diff --git a/src/test/resources/eager/keeps-meta-context-variables-through-import/test.expected.jinja b/src/test/resources/eager/keeps-meta-context-variables-through-import/test.expected.jinja new file mode 100644 index 000000000..dbced8528 --- /dev/null +++ b/src/test/resources/eager/keeps-meta-context-variables-through-import/test.expected.jinja @@ -0,0 +1,4 @@ +{% set list = deferred %} + +{% set meta = ['overridden'] %}{% do list.append(meta) %} +{{ list }} \ No newline at end of file diff --git a/src/test/resources/eager/keeps-meta-context-variables-through-import/test.jinja b/src/test/resources/eager/keeps-meta-context-variables-through-import/test.jinja new file mode 100644 index 000000000..5b1adc00c --- /dev/null +++ b/src/test/resources/eager/keeps-meta-context-variables-through-import/test.jinja @@ -0,0 +1,5 @@ +{% set meta = ['overridden'] %} +{% set list = deferred %} +{% import '../../eager/keeps-meta-context-variables-through-import/import-target.jinja' %} +{% do list.append(meta) %} +{{ list }} \ No newline at end of file