Merge pull request #822 from HubSpot/revert-821-revert-820-handle-nested-import-file-paths

Revert 821 revert 820 handle nested import file paths

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java

@@ -182,14 +182,20 @@ public boolean equals(Object o) {
     MacroFunction that = (MacroFunction) o;
     return (
       caller == that.caller &&
-      definitionLineNumber == that.definitionLineNumber &&
-      definitionStartPosition == that.definitionStartPosition &&
-      content.equals(that.content)
+      Objects.equals(getName(), that.getName()) &&
+      Objects.equals(
+        localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY),
+        that.localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY)
+      )
     );
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(content, caller, definitionLineNumber, definitionStartPosition);
+    return Objects.hash(
+      getName(),
+      localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY),
+      caller
+    );
   }
 }

src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java

@@ -5,6 +5,7 @@
 import com.hubspot.jinjava.el.ext.AstMacroFunction;
 import com.hubspot.jinjava.interpret.Context;
 import com.hubspot.jinjava.interpret.DeferredMacroValueImpl;
+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;
@@ -106,11 +107,15 @@ public String reconstructImage() {
     String prefix = "";
     String suffix = "";
     Optional<String> importFile = macroFunction.getImportFile(interpreter);
+    Object currentDeferredImportResource = null;
     if (importFile.isPresent()) {
       interpreter.getContext().getCurrentPathStack().pop();
-      String currentDeferredImportResource = (String) interpreter
-        .getContext()
-        .get(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY);
+      currentDeferredImportResource =
+        interpreter.getContext().get(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY);
+      if (currentDeferredImportResource instanceof DeferredValue) {
+        currentDeferredImportResource =
+          ((DeferredValue) currentDeferredImportResource).getOriginalValue();
+      }
       prefix =
         EagerReconstructionUtils.buildSetTag(
           ImmutableMap.of(
@@ -120,6 +125,9 @@ public String reconstructImage() {
           interpreter,
           false
         );
+      interpreter
+        .getContext()
+        .put(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, importFile.get());
       suffix =
         EagerReconstructionUtils.buildSetTag(
           ImmutableMap.of(
@@ -150,6 +158,10 @@ public String reconstructImage() {
             .map(arg -> DeferredMacroValueImpl.instance())
             .toArray()
         );
+
+        interpreter
+          .getContext()
+          .put(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, currentDeferredImportResource);
         if (interpreter.getContext().getEagerTokens().size() > numEagerTokensStart) {
           evaluation =
             (String) evaluate(

src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java

@@ -23,6 +23,7 @@
 import com.hubspot.jinjava.tree.parse.TagToken;
 import com.hubspot.jinjava.util.HelperStringTokenizer;
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -151,19 +152,29 @@ public static void integrateChild(
         parent.getContext().addGlobalMacro(macro);
       }
       childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
-      parent.getContext().putAll(childBindings);
-    } else {
-      for (Map.Entry<String, MacroFunction> macro : child
+      parent
         .getContext()
-        .getGlobalMacros()
-        .entrySet()) {
-        childBindings.put(macro.getKey(), macro.getValue());
-      }
+        .putAll(getChildBindingsWithoutImportResourcePath(childBindings));
+    } else {
+      childBindings.putAll(child.getContext().getGlobalMacros());
       childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
       parent.getContext().put(contextVar, childBindings);
     }
   }
 
+  public static Map<String, Object> getChildBindingsWithoutImportResourcePath(
+    Map<String, Object> childBindings
+  ) {
+    Map<String, Object> filteredMap = new HashMap<>();
+    // Don't remove them from childBindings, because it is needed in a macro function's localContextScope
+    childBindings
+      .entrySet()
+      .stream()
+      .filter(entry -> !entry.getKey().equals(Context.IMPORT_RESOURCE_PATH_KEY))
+      .forEach(entry -> filteredMap.put(entry.getKey(), entry.getValue()));
+    return filteredMap;
+  }
+
   public static void handleDeferredNodesDuringImport(
     Node node,
     String contextVar,

src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java

@@ -338,12 +338,11 @@ public static void integrateChild(
       }
       childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
       childBindings.remove(Context.IMPORT_RESOURCE_ALIAS_KEY);
-      parent.getContext().putAll(childBindings);
+      parent
+        .getContext()
+        .putAll(ImportTag.getChildBindingsWithoutImportResourcePath(childBindings));
     } else {
-      Map<String, MacroFunction> globalMacros = child.getContext().getGlobalMacros();
-      for (Map.Entry<String, MacroFunction> macro : globalMacros.entrySet()) {
-        childBindings.put(macro.getKey(), macro.getValue());
-      }
+      childBindings.putAll(child.getContext().getGlobalMacros());
       Map<String, Object> mapForCurrentContextAlias = getMapForCurrentContextAlias(
         currentImportAlias,
         child

src/test/java/com/hubspot/jinjava/lib/tag/ImportTagTest.java

@@ -11,6 +11,7 @@
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.RenderResult;
 import com.hubspot.jinjava.lib.fn.MacroFunction;
+import com.hubspot.jinjava.lib.tag.eager.EagerImportTagTest.PrintPathFilter;
 import com.hubspot.jinjava.loader.LocationResolver;
 import com.hubspot.jinjava.loader.RelativePathResolver;
 import com.hubspot.jinjava.loader.ResourceLocator;
@@ -39,6 +40,7 @@ public void setup() {
     );
 
     context.put("padding", 42);
+    context.registerFilter(new PrintPathFilter());
   }
 
   @Test
@@ -76,6 +78,21 @@ public void itAvoidsNestedImportCycle() throws IOException {
       .contains("Import cycle detected", "b-imports-a.jinja");
   }
 
+  @Test
+  public void itHandlesNullImportedValues() throws IOException {
+    Jinjava jinjava = new Jinjava();
+    interpreter = new JinjavaInterpreter(jinjava, context, jinjava.getGlobalConfig());
+
+    interpreter.render(
+      Resources.toString(
+        Resources.getResource("tags/importtag/imports-null.jinja"),
+        StandardCharsets.UTF_8
+      )
+    );
+    assertThat(context.get("foo")).isEqualTo("foo");
+    assertThat(context.get("bar")).isEqualTo(null);
+  }
+
   @Test
   public void importedContextExposesVars() {
     assertThat(fixture("import"))
@@ -352,6 +369,17 @@ public void itSetsErrorLineNumbersCorrectlyForImportedMacros() throws IOExceptio
       .isEqualTo("tags/importtag/errors/macro-with-error.jinja");
   }
 
+  @Test
+  public void itCorrectlySetsNestedPaths() {
+    context.put("foo", "foo");
+    assertThat(
+        interpreter.render(
+          "{% import 'double-import-macro.jinja' %}{{ print_path_macro2(foo) }}"
+        )
+      )
+      .isEqualTo("double-import-macro.jinja\n\nimport-macro.jinja\nfoo\n");
+  }
+
   private String fixture(String name) {
     try {
       return interpreter.renderFlat(

src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java

@@ -535,6 +535,27 @@ public void itCorrectlySetsPathForSecondPass() {
       .isEqualTo("import-macro.jinja\nfoo");
   }
 
+  @Test
+  public void itCorrectlySetsNestedPathsForSecondPass() {
+    setupResourceLocator();
+    context.put("foo", DeferredValue.instance());
+    String firstPassResult = interpreter.render(
+      "{% import 'double-import-macro.jinja' %}{{ print_path_macro2(foo) }}"
+    );
+    assertThat(firstPassResult)
+      .isEqualTo(
+        "{% set deferred_import_resource_path = 'double-import-macro.jinja' %}{% macro print_path_macro2(var) %}{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" +
+        "{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro print_path_macro(var) %}\n" +
+        "{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" +
+        "{{ var }}\n" +
+        "{% endmacro %}{% set deferred_import_resource_path = null %}{{ print_path_macro(var) }}{% endmacro %}{% set deferred_import_resource_path = null %}{{ print_path_macro2(foo) }}"
+      );
+    context.put("foo", "foo");
+    context.put(Context.GLOBAL_MACROS_SCOPE_KEY, null);
+    assertThat(interpreter.render(firstPassResult).trim())
+      .isEqualTo("double-import-macro.jinja\n\nimport-macro.jinja\nfoo");
+  }
+
   @Test
   public void itImportsDoublyNamed() {
     setupResourceLocator();

src/test/resources/tags/eager/importtag/double-import-macro.jinja

@@ -0,0 +1,5 @@
+{% import 'import-macro.jinja' -%}
+{% macro print_path_macro2(var) -%}
+{{ var|print_path }}
+{{ print_path_macro(var) }}
+{%- endmacro %}

src/test/resources/tags/importtag/imports-null.jinja

@@ -0,0 +1,4 @@
+{% set bar = 'bar' %}
+{% import 'tags/importtag/null-value.jinja' %}
+{{ foo }}
+{{ bar }}

src/test/resources/tags/importtag/null-value.jinja

@@ -0,0 +1,2 @@
+{% set foo = 'foo' %}
+{% set bar = null %}

src/test/resources/tags/macrotag/double-import-macro.jinja

@@ -0,0 +1,5 @@
+{% import 'import-macro.jinja' -%}
+{% macro print_path_macro2(var) -%}
+{{ var|print_path }}
+{{ print_path_macro(var) }}
+{%- endmacro %}