From 95c4138f7bbeed5841dcf1fa8b7d5388444e9747 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Wed, 5 Oct 2022 10:27:36 -0400 Subject: [PATCH 1/3] Store macro function evaluation in temp variable when result has deferred tokens and partial macro evaluation is off --- .../el/ext/eager/EagerAstMacroFunction.java | 7 ++ .../ext/eager/MacroFunctionTempVariable.java | 39 ++++++++++ .../hubspot/jinjava/lib/fn/MacroFunction.java | 24 +++--- .../PyishBlockSetSerializable.java | 5 ++ .../util/EagerReconstructionUtils.java | 78 ++++++++++++++++++- 5 files changed, 140 insertions(+), 13 deletions(-) create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/eager/MacroFunctionTempVariable.java create mode 100644 src/main/java/com/hubspot/jinjava/objects/serialization/PyishBlockSetSerializable.java diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java index 5dd0cca4d..8b5fc23dd 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.lib.fn.MacroFunction; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstParameters; import java.lang.reflect.Array; @@ -147,6 +148,12 @@ public String getPartiallyResolved( DeferredParsingException deferredParsingException, boolean preserveIdentifier ) { + if ( + deferredParsingException != null && + deferredParsingException.getSourceNode() instanceof MacroFunction + ) { + return deferredParsingException.getDeferredEvalResult(); + } StringBuilder sb = new StringBuilder(); sb.append(getName()); try { diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/MacroFunctionTempVariable.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/MacroFunctionTempVariable.java new file mode 100644 index 000000000..d65a9eae2 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/MacroFunctionTempVariable.java @@ -0,0 +1,39 @@ +package com.hubspot.jinjava.el.ext.eager; + +import com.hubspot.jinjava.objects.serialization.PyishBlockSetSerializable; +import java.util.Objects; + +public class MacroFunctionTempVariable implements PyishBlockSetSerializable { + private static final String CONTEXT_KEY_PREFIX = "__macro_%s_temp_variable_%d__"; + private final String deferredResult; + + public MacroFunctionTempVariable(String deferredResult) { + this.deferredResult = deferredResult; + } + + public static String getVarName(String macroFunctionName, int callCount) { + return String.format(CONTEXT_KEY_PREFIX, macroFunctionName, callCount); + } + + @Override + public String getBlockSetBody() { + return deferredResult; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + MacroFunctionTempVariable that = (MacroFunctionTempVariable) o; + return deferredResult.equals(that.deferredResult); + } + + @Override + public int hashCode() { + return Objects.hash(deferredResult); + } +} diff --git a/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java b/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java index 1e291f983..cb07e89c0 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java @@ -1,11 +1,11 @@ package com.hubspot.jinjava.lib.fn; -import com.google.common.collect.ImmutableList; import com.hubspot.jinjava.el.ext.AbstractCallableMethod; +import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.eager.MacroFunctionTempVariable; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValue; -import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable; import com.hubspot.jinjava.tree.Node; @@ -16,6 +16,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; /** * Function definition parsed from a jinjava template, stored in global macros registry in interpreter context. @@ -36,6 +37,8 @@ public class MacroFunction extends AbstractCallableMethod { private boolean deferred; + private AtomicInteger callCount = new AtomicInteger(); + public MacroFunction( List content, String name, @@ -70,6 +73,7 @@ public Object doEvaluate( Map kwargMap, List varArgs ) { + int currentCallCount = callCount.getAndIncrement(); JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); Optional importFile = getImportFile(interpreter); try (InterpreterScopeClosable c = interpreter.enterScope()) { @@ -83,17 +87,15 @@ public Object doEvaluate( !interpreter.getContext().getDeferredTokens().isEmpty() ) ) { - interpreter - .getContext() - .removeDeferredTokens( - ImmutableList.copyOf(interpreter.getContext().getDeferredTokens()) - ); - // If the macro function could not be fully evaluated, throw a DeferredValueException. - throw new DeferredValueException( + String tempVarName = MacroFunctionTempVariable.getVarName( getName(), - interpreter.getLineNumber(), - interpreter.getPosition() + currentCallCount ); + interpreter + .getContext() + .getParent() + .put(tempVarName, new MacroFunctionTempVariable(result)); + throw new DeferredParsingException(this, tempVarName); } return result; diff --git a/src/main/java/com/hubspot/jinjava/objects/serialization/PyishBlockSetSerializable.java b/src/main/java/com/hubspot/jinjava/objects/serialization/PyishBlockSetSerializable.java new file mode 100644 index 000000000..7cc319a12 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/objects/serialization/PyishBlockSetSerializable.java @@ -0,0 +1,5 @@ +package com.hubspot.jinjava.objects.serialization; + +public interface PyishBlockSetSerializable { + String getBlockSetBody(); +} diff --git a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java index 86df973ce..c1d4a30b7 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java @@ -22,6 +22,7 @@ import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult; import com.hubspot.jinjava.objects.collections.PyList; import com.hubspot.jinjava.objects.collections.PyMap; +import com.hubspot.jinjava.objects.serialization.PyishBlockSetSerializable; import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.tree.TagNode; import com.hubspot.jinjava.tree.parse.TagToken; @@ -296,7 +297,8 @@ public static String reconstructFromContextBeforeDeferring( ) { return ( reconstructMacroFunctionsBeforeDeferring(deferredWords, interpreter) + - reconstructVariablesBeforeDeferring(deferredWords, interpreter) + reconstructBlockSetVariablesBeforeDeferring(deferredWords, interpreter) + + reconstructInlineSetVariablesBeforeDeferring(deferredWords, interpreter) ); } @@ -360,7 +362,79 @@ private static String reconstructMacroFunctionsBeforeDeferring( return result; } - private static String reconstructVariablesBeforeDeferring( + private static String reconstructBlockSetVariablesBeforeDeferring( + Set deferredWords, + JinjavaInterpreter interpreter + ) { + if (interpreter.getContext().isDeferredExecutionMode()) { + Context parent = interpreter.getContext().getParent(); + while (parent.isDeferredExecutionMode()) { + parent = parent.getParent(); + } + final Context finalParent = parent; + deferredWords = + deferredWords + .stream() + .filter(word -> interpreter.getContext().get(word) != finalParent.get(word)) + .collect(Collectors.toSet()); + } + if (deferredWords.isEmpty()) { + return ""; + } + Set metaContextVariables = interpreter.getContext().getMetaContextVariables(); + Map blockSetMap = new HashMap<>(); + + deferredWords + .stream() + .map(w -> w.split("\\.", 2)[0]) // get base prop + .filter(w -> !metaContextVariables.contains(w)) + .filter(w -> interpreter.getContext().get(w) instanceof PyishBlockSetSerializable) + .forEach( + w -> + blockSetMap.put(w, (PyishBlockSetSerializable) interpreter.getContext().get(w)) + ); + deferredWords + .stream() + .map(w -> w.split("\\.", 2)[0]) // get base prop + .filter( + w -> { + Object value = interpreter.getContext().get(w); + return ( + value instanceof DeferredLazyReference && + ( + (DeferredLazyReference) value + ).getOriginalValue() instanceof PyishBlockSetSerializable + ); + } + ) + .forEach( + w -> { + blockSetMap.put( + w, + (PyishBlockSetSerializable) ( + (DeferredLazyReference) interpreter.getContext().get(w) + ).getOriginalValue() + ); + } + ); + String blockSetTags = blockSetMap + .entrySet() + .stream() + .map( + entry -> + buildBlockSetTag( + entry.getKey(), + entry.getValue().getBlockSetBody(), + interpreter, + false + ) + ) + .collect(Collectors.joining()); + deferredWords.removeAll(blockSetMap.keySet()); + return blockSetTags; + } + + private static String reconstructInlineSetVariablesBeforeDeferring( Set deferredWords, JinjavaInterpreter interpreter ) { From f838bade3eb30021c61e26756078982f6e09484e Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Wed, 5 Oct 2022 10:29:17 -0400 Subject: [PATCH 2/3] Fix expected results to expect the macro function temporary variable --- .../eager/eagerly-defers-macro.expected.jinja | 8 ++++---- .../fully-defers-filtered-macro.expected.jinja | 2 +- ...ferred-for-loop-var-from-macro.expected.jinja | 16 ++++++++-------- ...eferred-fromed-macro-in-output.expected.jinja | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/resources/eager/eagerly-defers-macro.expected.jinja b/src/test/resources/eager/eagerly-defers-macro.expected.jinja index cd45e9ec6..bbf836e2d 100644 --- a/src/test/resources/eager/eagerly-defers-macro.expected.jinja +++ b/src/test/resources/eager/eagerly-defers-macro.expected.jinja @@ -1,6 +1,6 @@ -{% macro big_guy() %} +{% set __macro_big_guy_temp_variable_0__ %} {% if deferred %}I am foo{% else %}I am bar{% endif %} -{% endmacro %}{% print big_guy() %} -{% macro big_guy() %} +{% endset %}{% print __macro_big_guy_temp_variable_0__ %} +{% set __macro_big_guy_temp_variable_1__ %} {% if deferred %}No more foo{% else %}I am bar{% endif %} -{% endmacro %}{% print big_guy() %} \ No newline at end of file +{% endset %}{% print __macro_big_guy_temp_variable_1__ %} diff --git a/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja b/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja index 492a80e38..352034b90 100644 --- a/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja +++ b/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja @@ -2,4 +2,4 @@ A flashy {{ deferred }}.{% endmacro %}{{ flashy(flashy('bar')) }} --- -{% macro silly() %}A silly {{ deferred }}.{% endmacro %}{{ filter:upper.filter(silly(), ____int3rpr3t3r____) }} +{% set __macro_silly_temp_variable_0__ %}A silly {{ deferred }}.{% endset %}{{ filter:upper.filter(__macro_silly_temp_variable_0__, ____int3rpr3t3r____) }} diff --git a/src/test/resources/eager/handles-deferred-for-loop-var-from-macro.expected.jinja b/src/test/resources/eager/handles-deferred-for-loop-var-from-macro.expected.jinja index 22ad3b882..c650a5d45 100644 --- a/src/test/resources/eager/handles-deferred-for-loop-var-from-macro.expected.jinja +++ b/src/test/resources/eager/handles-deferred-for-loop-var-from-macro.expected.jinja @@ -1,13 +1,13 @@ -{% macro getData() %} +{% set __macro_getData_temp_variable_0__ %} {% for __ignored__ in [0] %} -{% macro doIt(val) %} -{{ deferred ~ filter:tojson.filter(val, ____int3rpr3t3r____) }} -{% endmacro %}{% set val = {'a': 'a'} %}{{ filter:upper.filter(doIt(val), ____int3rpr3t3r____) }} +{% set __macro_doIt_temp_variable_0__ %} +{{ deferred ~ '{\"a\":\"a\"}' }} +{% endset %}{{ filter:upper.filter(__macro_doIt_temp_variable_0__, ____int3rpr3t3r____) }} -{% macro doIt(val) %} -{{ deferred ~ filter:tojson.filter(val, ____int3rpr3t3r____) }} -{% endmacro %}{% set val = {'b': 'b'} %}{{ filter:upper.filter(doIt(val), ____int3rpr3t3r____) }} +{% set __macro_doIt_temp_variable_1__ %} +{{ deferred ~ '{\"b\":\"b\"}' }} +{% endset %}{{ filter:upper.filter(__macro_doIt_temp_variable_1__, ____int3rpr3t3r____) }} {% endfor %} -{% endmacro %}{{ filter:upper.filter(getData(), ____int3rpr3t3r____) }} +{% endset %}{{ filter:upper.filter(__macro_getData_temp_variable_0__, ____int3rpr3t3r____) }} diff --git a/src/test/resources/eager/puts-deferred-fromed-macro-in-output.expected.jinja b/src/test/resources/eager/puts-deferred-fromed-macro-in-output.expected.jinja index 720db4c8c..9241dd338 100644 --- a/src/test/resources/eager/puts-deferred-fromed-macro-in-output.expected.jinja +++ b/src/test/resources/eager/puts-deferred-fromed-macro-in-output.expected.jinja @@ -1 +1 @@ -{% set myname = deferred + 3 %}{% set deferred_import_resource_path = 'simple-with-call.jinja' %}{% macro getPath() %}Hello {{ myname }}{% endmacro %}{% set deferred_import_resource_path = null %}{% print getPath() %} +{% set myname = deferred + 3 %}{% set __macro_getPath_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_temp_variable_1__ %} From 8d0335bec393e1fc2b6b340b8bdc7ec0d5e13934 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Wed, 5 Oct 2022 10:54:55 -0400 Subject: [PATCH 3/3] Fix variable referencing so that keys are removed from the original set --- .../hubspot/jinjava/util/EagerReconstructionUtils.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java index c1d4a30b7..4f544662c 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java @@ -366,25 +366,26 @@ private static String reconstructBlockSetVariablesBeforeDeferring( Set deferredWords, JinjavaInterpreter interpreter ) { + Set filteredDeferredWords = deferredWords; if (interpreter.getContext().isDeferredExecutionMode()) { Context parent = interpreter.getContext().getParent(); while (parent.isDeferredExecutionMode()) { parent = parent.getParent(); } final Context finalParent = parent; - deferredWords = + filteredDeferredWords = deferredWords .stream() .filter(word -> interpreter.getContext().get(word) != finalParent.get(word)) .collect(Collectors.toSet()); } - if (deferredWords.isEmpty()) { + if (filteredDeferredWords.isEmpty()) { return ""; } Set metaContextVariables = interpreter.getContext().getMetaContextVariables(); Map blockSetMap = new HashMap<>(); - deferredWords + filteredDeferredWords .stream() .map(w -> w.split("\\.", 2)[0]) // get base prop .filter(w -> !metaContextVariables.contains(w)) @@ -393,7 +394,7 @@ private static String reconstructBlockSetVariablesBeforeDeferring( w -> blockSetMap.put(w, (PyishBlockSetSerializable) interpreter.getContext().get(w)) ); - deferredWords + filteredDeferredWords .stream() .map(w -> w.split("\\.", 2)[0]) // get base prop .filter(