From 74d9a4d700354617d87832abcce1ee2efa1f3e4e Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Fri, 6 Sep 2024 15:47:07 -0400 Subject: [PATCH] Fix bugs in how macro functions reconstruct the current path --- .../lib/fn/eager/EagerMacroFunction.java | 46 ++++++++++++++++++- .../com/hubspot/jinjava/lib/tag/FromTag.java | 2 +- .../hubspot/jinjava/lib/tag/ImportTag.java | 2 +- .../jinjava/lib/tag/eager/EagerFromTag.java | 2 +- .../jinjava/lib/tag/eager/EagerImportTag.java | 2 +- .../EagerImportingStrategyFactory.java | 6 +++ .../java/com/hubspot/jinjava/EagerTest.java | 7 +++ .../jinjava/ExpectedTemplateInterpreter.java | 10 ++-- .../dir1/macro.jinja | 4 ++ .../dir2/imported.jinja | 1 + .../test.expected.jinja | 11 +++++ .../test.jinja | 2 + 12 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 src/test/resources/eager/wraps-current-path-around-macro/dir1/macro.jinja create mode 100644 src/test/resources/eager/wraps-current-path-around-macro/dir2/imported.jinja create mode 100644 src/test/resources/eager/wraps-current-path-around-macro/test.expected.jinja create mode 100644 src/test/resources/eager/wraps-current-path-around-macro/test.jinja diff --git a/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java b/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java index 9e805daa3..4dbc553cc 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java @@ -1,5 +1,7 @@ package com.hubspot.jinjava.lib.fn.eager; +import static com.hubspot.jinjava.loader.RelativePathResolver.CURRENT_PATH_CONTEXT_KEY; + import com.google.common.annotations.Beta; import com.hubspot.jinjava.el.ext.AstMacroFunction; import com.hubspot.jinjava.el.ext.DeferredInvocationResolutionException; @@ -13,6 +15,7 @@ import com.hubspot.jinjava.lib.fn.MacroFunction; import com.hubspot.jinjava.lib.tag.MacroTag; import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult; +import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategyFactory; import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.tree.Node; import com.hubspot.jinjava.util.EagerContextWatcher; @@ -80,6 +83,11 @@ public Object doEvaluate( () -> getEvaluationResultDirectly(argMap, kwargMap, varArgs, interpreter), interpreter ); + return wrapCurrentPathSetting( + interpreter, + importFile, + result.asTemplateString() + ); } return result.asTemplateString(); } finally { @@ -121,12 +129,45 @@ public Object doEvaluate( .put( tempVarName, new MacroFunctionTempVariable( - prefixToPreserveState + eagerExecutionResult.asTemplateString() + wrapCurrentPathSetting( + interpreter, + Optional + .ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY)) + .map(Object::toString), + prefixToPreserveState + eagerExecutionResult.asTemplateString() + ) ) ); throw new DeferredInvocationResolutionException(tempVarName); } - return eagerExecutionResult.getResult().toString(true); + return wrapCurrentPathSetting( + interpreter, + Optional + .ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY)) + .map(Object::toString), + eagerExecutionResult.getResult().toString(true) + ); + } + + private static String wrapCurrentPathSetting( + JinjavaInterpreter interpreter, + Optional importFile, + String resultToWrap + ) { + if (!importFile.isPresent()) { + return resultToWrap; + } + final String initialPathSetter = + EagerImportingStrategyFactory.getSetTagForCurrentPath(interpreter); + final String newPathSetter = EagerReconstructionUtils.buildBlockOrInlineSetTag( + CURRENT_PATH_CONTEXT_KEY, + importFile.get(), + interpreter + ); + if (initialPathSetter.equals(newPathSetter)) { + return resultToWrap; + } + return newPathSetter + resultToWrap + initialPathSetter; } private String getEvaluationResultDirectly( @@ -312,6 +353,7 @@ private boolean differentMacroWithSameNameExists(JinjavaInterpreter interpreter) return false; } MacroFunction mostRecent = context.getGlobalMacro(getName()); + //noinspection ErrorProne if (this != mostRecent) { return true; } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java index ce00eceef..0a3068e9e 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java @@ -95,7 +95,7 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) { child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); JinjavaInterpreter.pushCurrent(child); try { - child.render(node); + child.render(node, false); } finally { JinjavaInterpreter.popCurrent(); } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java index aa9ffbd30..5767a54fa 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java @@ -102,7 +102,7 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) { JinjavaInterpreter.pushCurrent(child); try { - child.render(node); + child.render(node, false); } finally { JinjavaInterpreter.popCurrent(); } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java index 937a4ca17..72000d265 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java @@ -79,7 +79,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter JinjavaInterpreter.pushCurrent(child); String output; try { - output = child.render(node); + output = child.render(node, false); } finally { JinjavaInterpreter.popCurrent(); } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java index ea46cae3b..d6828a667 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java @@ -64,7 +64,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter String output; try { eagerImportingStrategy.setup(child); - output = child.render(node); + output = child.render(node, false); } finally { JinjavaInterpreter.popCurrent(); } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/EagerImportingStrategyFactory.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/EagerImportingStrategyFactory.java index c8833852b..3cd547f3d 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/EagerImportingStrategyFactory.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/EagerImportingStrategyFactory.java @@ -34,6 +34,12 @@ public static String getSetTagForCurrentPath(JinjavaInterpreter interpreter) { .getContext() .getCurrentPathStack() .peek() + .map(c -> { + interpreter + .getContext() + .replace(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY, c); + return c; + }) .orElseGet(() -> (String) interpreter .getContext() diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 76d5f170f..142938e3e 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1597,4 +1597,11 @@ public void prepareContext(Context context) { "keeps-meta-context-variables-through-import/test" ); } + + @Test + public void itWrapsCurrentPathAroundMacro() { + expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( + "wraps-current-path-around-macro/test" + ); + } } diff --git a/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java b/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java index 0694a47ac..eba39bb0e 100644 --- a/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java +++ b/src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java @@ -127,10 +127,12 @@ public String getFixtureTemplate(String name) { private String expected(String name) { try { - return Resources.toString( - Resources.getResource(String.format("%s/%s.expected.jinja", path, name)), - StandardCharsets.UTF_8 - ); + return Resources + .toString( + Resources.getResource(String.format("%s/%s.expected.jinja", path, name)), + StandardCharsets.UTF_8 + ) + .replaceAll("\\\\\n\\s*", ""); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/src/test/resources/eager/wraps-current-path-around-macro/dir1/macro.jinja b/src/test/resources/eager/wraps-current-path-around-macro/dir1/macro.jinja new file mode 100644 index 000000000..099408892 --- /dev/null +++ b/src/test/resources/eager/wraps-current-path-around-macro/dir1/macro.jinja @@ -0,0 +1,4 @@ +{% macro foo_importer() -%} + {%- import "../../../eager/wraps-current-path-around-macro/dir2/imported.jinja" -%} + {%- print foo -%} +{%- endmacro %} \ No newline at end of file diff --git a/src/test/resources/eager/wraps-current-path-around-macro/dir2/imported.jinja b/src/test/resources/eager/wraps-current-path-around-macro/dir2/imported.jinja new file mode 100644 index 000000000..27547dad8 --- /dev/null +++ b/src/test/resources/eager/wraps-current-path-around-macro/dir2/imported.jinja @@ -0,0 +1 @@ +{% set foo = deferred %} \ No newline at end of file diff --git a/src/test/resources/eager/wraps-current-path-around-macro/test.expected.jinja b/src/test/resources/eager/wraps-current-path-around-macro/test.expected.jinja new file mode 100644 index 000000000..1bc819fcf --- /dev/null +++ b/src/test/resources/eager/wraps-current-path-around-macro/test.expected.jinja @@ -0,0 +1,11 @@ +{% set __macro_foo_importer_1564912668_temp_variable_0__ %}\ + {% set current_path = '../../eager/wraps-current-path-around-macro/dir1/macro.jinja' %}\ + {% do %}\ + {% set current_path = '../../eager/wraps-current-path-around-macro/dir2/imported.jinja' %}\ + {% set foo = deferred %}\ + {% set current_path = '../../eager/wraps-current-path-around-macro/dir1/macro.jinja' %}\ + {% enddo %}\ + {% print foo %}\ + {% set current_path = '' %}\ +{% endset %}\ +{% print __macro_foo_importer_1564912668_temp_variable_0__ %} \ No newline at end of file diff --git a/src/test/resources/eager/wraps-current-path-around-macro/test.jinja b/src/test/resources/eager/wraps-current-path-around-macro/test.jinja new file mode 100644 index 000000000..d05b5b2ef --- /dev/null +++ b/src/test/resources/eager/wraps-current-path-around-macro/test.jinja @@ -0,0 +1,2 @@ +{% from "../../eager/wraps-current-path-around-macro/dir1/macro.jinja" import foo_importer -%} +{% print foo_importer() %}