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 69c4749da..267446b14 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java @@ -130,19 +130,28 @@ public String getEvaluationResult( ) { interpreter.setLineNumber(definitionLineNumber); interpreter.setPosition(definitionStartPosition); - for (Map.Entry scopeEntry : localContextScope.getScope().entrySet()) { - if (scopeEntry.getValue() instanceof MacroFunction) { - interpreter.getContext().addGlobalMacro((MacroFunction) scopeEntry.getValue()); - } else if (scopeEntry.getKey().equals(Context.GLOBAL_MACROS_SCOPE_KEY)) { - interpreter - .getContext() - .put( - Context.GLOBAL_MACROS_SCOPE_KEY, - new HashMap<>((Map) scopeEntry.getValue()) - ); - } else { - if (!alreadyDeferredInEarlierCall(scopeEntry.getKey(), interpreter)) { - interpreter.getContext().put(scopeEntry.getKey(), scopeEntry.getValue()); + if ( + !Objects.equals( + interpreter.getContext().get(Context.IMPORT_RESOURCE_PATH_KEY), + localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY) + ) + ) { + for (Map.Entry scopeEntry : localContextScope + .getScope() + .entrySet()) { + if (scopeEntry.getValue() instanceof MacroFunction) { + interpreter.getContext().addGlobalMacro((MacroFunction) scopeEntry.getValue()); + } else if (scopeEntry.getKey().equals(Context.GLOBAL_MACROS_SCOPE_KEY)) { + interpreter + .getContext() + .put( + Context.GLOBAL_MACROS_SCOPE_KEY, + new HashMap<>((Map) scopeEntry.getValue()) + ); + } else { + if (!alreadyDeferredInEarlierCall(scopeEntry.getKey(), interpreter)) { + interpreter.getContext().put(scopeEntry.getKey(), scopeEntry.getValue()); + } } } } 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 24b1bdd0c..489a5b09c 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 @@ -198,8 +198,8 @@ private boolean differentMacroWithSameNameExists() { return true; } while ( - !context.getGlobalMacros().containsKey(macroFunction.getName()) || - context.getParent().getParent() == null + !context.getGlobalMacros().containsKey(macroFunction.getName()) && + context.getParent().getParent() != null ) { context = context.getParent(); } diff --git a/src/test/java/com/hubspot/jinjava/FullSnippetsTest.java b/src/test/java/com/hubspot/jinjava/FullSnippetsTest.java new file mode 100644 index 000000000..ad7d0fff1 --- /dev/null +++ b/src/test/java/com/hubspot/jinjava/FullSnippetsTest.java @@ -0,0 +1,104 @@ +package com.hubspot.jinjava; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.io.Resources; +import com.hubspot.jinjava.interpret.Context; +import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.loader.LocationResolver; +import com.hubspot.jinjava.loader.RelativePathResolver; +import com.hubspot.jinjava.loader.ResourceLocator; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.Optional; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class FullSnippetsTest { + private JinjavaInterpreter interpreter; + private Jinjava jinjava; + private ExpectedTemplateInterpreter expectedTemplateInterpreter; + Context globalContext = new Context(); + Context localContext; // ref to context created with global as parent + + @Before + public void setup() { + JinjavaInterpreter.popCurrent(); + jinjava = new Jinjava(); + jinjava.setResourceLocator( + new ResourceLocator() { + private RelativePathResolver relativePathResolver = new RelativePathResolver(); + + @Override + public String getString( + String fullName, + Charset encoding, + JinjavaInterpreter interpreter + ) + throws IOException { + return Resources.toString( + Resources.getResource(String.format("tags/macrotag/%s", fullName)), + StandardCharsets.UTF_8 + ); + } + + @Override + public Optional getLocationResolver() { + return Optional.of(relativePathResolver); + } + } + ); + JinjavaConfig config = JinjavaConfig + .newBuilder() + .withNestedInterpretationEnabled(true) + .withLegacyOverrides( + LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() + ) + .withMaxMacroRecursionDepth(20) + .withEnableRecursiveMacroCalls(true) + .build(); + JinjavaInterpreter parentInterpreter = new JinjavaInterpreter( + jinjava, + globalContext, + config + ); + interpreter = new JinjavaInterpreter(parentInterpreter); + expectedTemplateInterpreter = + new ExpectedTemplateInterpreter(jinjava, interpreter, "snippets"); + localContext = interpreter.getContext(); + + JinjavaInterpreter.pushCurrent(interpreter); + } + + @After + public void teardown() { + try { + assertThat(interpreter.getErrors()).isEmpty(); + } finally { + JinjavaInterpreter.popCurrent(); + } + } + + @Test + public void itDoesNotOverrideCallTagFromHigherScope() { + expectedTemplateInterpreter.assertExpectedOutput( + "does-not-override-call-tag-from-higher-scope" + ); + } + + @Test + public void itDoesNotOverrideMacroFunctionsFromHigherScope() { + expectedTemplateInterpreter.assertExpectedOutput( + "does-not-override-macro-functions-from-higher-scope" + ); + } + + @Test + public void itUsesLowerScopeValueInMacroEvaluation() { + expectedTemplateInterpreter.assertExpectedOutput( + "uses-lower-scope-value-in-macro-evaluation" + ); + } +} diff --git a/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java b/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java index d9f906215..208c5e612 100644 --- a/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.BaseInterpretingTest; import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.LegacyOverrides; +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; @@ -142,12 +143,18 @@ public void itDefersDifferentMacrosWithSameName() { String foo1Code = "{% macro foo(var) %}This is the {{ var }}{% endmacro %}"; String barCode = "{% macro bar(var) %}^{{ foo(var) }}^{% endmacro %}"; String foo2Code = "{% macro foo(var) %}~{{ bar(var) }}~{% endmacro %}"; - MacroFunction foo1Macro = makeMacroFunction("foo", foo1Code); - foo1Macro.setDeferred(true); - MacroFunction barMacro = makeMacroFunction("bar", barCode); - barMacro.setDeferred(true); + MacroFunction foo1Macro; + MacroFunction barMacro; + try (InterpreterScopeClosable c = interpreter.enterScope()) { // Imitate importing + interpreter.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, "some_path"); + foo1Macro = makeMacroFunction("foo", foo1Code); + foo1Macro.setDeferred(true); + barMacro = makeMacroFunction("bar", barCode); + barMacro.setDeferred(true); + } interpreter.getContext().addGlobalMacro(foo1Macro); interpreter.getContext().addGlobalMacro(barMacro); + MacroFunction foo2Macro; String output; try (InterpreterScopeClosable c = interpreter.enterScope()) { diff --git a/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.expected.jinja b/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.expected.jinja new file mode 100644 index 000000000..b0fbdbe5b --- /dev/null +++ b/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.expected.jinja @@ -0,0 +1,5 @@ +1st macro +1st caller + +2nd macro +2nd caller \ No newline at end of file diff --git a/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.jinja b/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.jinja new file mode 100644 index 000000000..ff9001fe6 --- /dev/null +++ b/src/test/resources/snippets/does-not-override-call-tag-from-higher-scope.jinja @@ -0,0 +1,14 @@ +{% macro first() -%} +1st macro +{{ caller() }} +{% endmacro %} +{% call first() -%} +1st caller +{% macro second() -%} +2nd macro +{{ caller() }} +{% endmacro %} +{% call second() -%} +2nd caller +{% endcall %} +{% endcall %} diff --git a/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.expected.jinja b/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.expected.jinja new file mode 100644 index 000000000..0c6bd20f4 --- /dev/null +++ b/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.expected.jinja @@ -0,0 +1,3 @@ +bar + +2nd foo \ No newline at end of file diff --git a/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.jinja b/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.jinja new file mode 100644 index 000000000..8f4d1658d --- /dev/null +++ b/src/test/resources/snippets/does-not-override-macro-functions-from-higher-scope.jinja @@ -0,0 +1,13 @@ +{% macro foo() %} +1st foo +{% endmacro %} +{% macro bar() %} +bar +{{ foo() }} +{% endmacro %} +{% for i in range(1) %} +{% macro foo() %} +2nd foo +{% endmacro %} +{{ bar() }} +{% endfor %} diff --git a/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.expected.jinja b/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.expected.jinja new file mode 100644 index 000000000..e13c5bfaa --- /dev/null +++ b/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.expected.jinja @@ -0,0 +1,3 @@ +1 +2 +1 diff --git a/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.jinja b/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.jinja new file mode 100644 index 000000000..5947a7144 --- /dev/null +++ b/src/test/resources/snippets/uses-lower-scope-value-in-macro-evaluation.jinja @@ -0,0 +1,10 @@ +{% set val = 1 %} +{% macro foo() -%} +{{ val }} +{%- endmacro %} +{% for i in range(1) %} +{{ foo() }} +{%- set val = 2 %} +{{ foo() }} +{%- endfor %} +{{ val }}