Fix bugs in how macro functions reconstruct the current path

HubSpot · Sep 6, 2024 · 74d9a4d · 74d9a4d
1 parent 789cfe2
commit 74d9a4d
Show file tree

Hide file tree

Showing 12 changed files with 85 additions and 10 deletions.
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
@@ -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<String> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,4 @@
+{% macro foo_importer() -%}
+  {%- import "../../../eager/wraps-current-path-around-macro/dir2/imported.jinja" -%}
+  {%- print foo -%}
+{%- endmacro %}
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
@@ -0,0 +1 @@
+{% set foo = deferred %}
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
@@ -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__ %}
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
@@ -0,0 +1,2 @@
+{% from "../../eager/wraps-current-path-around-macro/dir1/macro.jinja" import foo_importer -%}
+{% print foo_importer() %}