Revert "Wrap the current path around reconstructed macro functions"

HubSpot · Sep 10, 2024 · 0744ad1 · 0744ad1
1 parent 12bc66c
commit 0744ad1
Show file tree

Hide file tree

Showing 15 changed files with 12 additions and 140 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/Jinjava.java b/src/main/java/com/hubspot/jinjava/Jinjava.java
@@ -35,7 +35,6 @@
 import com.hubspot.jinjava.lib.fn.ELFunctionDefinition;
 import com.hubspot.jinjava.lib.tag.Tag;
 import com.hubspot.jinjava.loader.ClasspathResourceLocator;
-import com.hubspot.jinjava.loader.RelativePathResolver;
 import com.hubspot.jinjava.loader.ResourceLocator;
 import de.odysseus.el.ExpressionFactoryImpl;
 import de.odysseus.el.misc.TypeConverter;
@@ -245,10 +244,6 @@ public RenderResult renderForResult(
     } else {
       context = new Context(copyGlobalContext(), bindings, renderConfig.getDisabled());
     }
-    Object currentPath = context.get(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY);
-    if (currentPath != null) {
-      context.getCurrentPathStack().pushWithoutCycleCheck(currentPath.toString(), 0, 0);
-    }
 
     JinjavaInterpreter interpreter = globalConfig
       .getInterpreterFactory()
@@ -295,9 +290,6 @@ public RenderResult renderForResult(
       );
     } finally {
       globalContext.reset();
-      if (currentPath != null) {
-        context.getCurrentPathStack().pop();
-      }
       JinjavaInterpreter.popCurrent();
     }
   }

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,7 +1,5 @@
 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;
@@ -15,7 +13,6 @@
 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;
@@ -83,11 +80,6 @@ public Object doEvaluate(
               () -> getEvaluationResultDirectly(argMap, kwargMap, varArgs, interpreter),
               interpreter
             );
-          return wrapCurrentPathSetting(
-            interpreter,
-            importFile,
-            result.asTemplateString()
-          );
         }
         return result.asTemplateString();
       } finally {
@@ -129,45 +121,12 @@ public Object doEvaluate(
         .put(
           tempVarName,
           new MacroFunctionTempVariable(
-            wrapCurrentPathSetting(
-              interpreter,
-              Optional
-                .ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY))
-                .map(Object::toString),
-              prefixToPreserveState + eagerExecutionResult.asTemplateString()
-            )
+            prefixToPreserveState + eagerExecutionResult.asTemplateString()
           )
         );
       throw new DeferredInvocationResolutionException(tempVarName);
     }
-    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;
+    return eagerExecutionResult.getResult().toString(true);
   }
 
   private String getEvaluationResultDirectly(
@@ -353,7 +312,6 @@ private boolean differentMacroWithSameNameExists(JinjavaInterpreter interpreter)
       return false;
     }
     MacroFunction mostRecent = context.getGlobalMacro(getName());
-    //noinspection ErrorProne
     if (this != mostRecent) {
       return true;
     }

diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java
@@ -625,7 +625,7 @@ public void itDefersMacroInIf() {
 
   @Test
   public void itPutsDeferredImportedMacroInOutput() {
-    expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
+    expectedTemplateInterpreter.assertExpectedOutput(
       "puts-deferred-imported-macro-in-output"
     );
   }
@@ -643,7 +643,7 @@ public void itPutsDeferredImportedMacroInOutputSecondPass() {
 
   @Test
   public void itPutsDeferredFromedMacroInOutput() {
-    expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
+    expectedTemplateInterpreter.assertExpectedOutput(
       "puts-deferred-fromed-macro-in-output"
     );
   }
@@ -1597,11 +1597,4 @@ 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
@@ -15,7 +15,6 @@ public class ExpectedTemplateInterpreter {
   private Jinjava jinjava;
   private JinjavaInterpreter interpreter;
   private String path;
-  private boolean sensibleCurrentPath = false;
 
   public ExpectedTemplateInterpreter(
     Jinjava jinjava,
@@ -27,26 +26,6 @@ public ExpectedTemplateInterpreter(
     this.path = path;
   }
 
-  public static ExpectedTemplateInterpreter withSensibleCurrentPath(
-    Jinjava jinjava,
-    JinjavaInterpreter interpreter,
-    String path
-  ) {
-    return new ExpectedTemplateInterpreter(jinjava, interpreter, path, true);
-  }
-
-  private ExpectedTemplateInterpreter(
-    Jinjava jinjava,
-    JinjavaInterpreter interpreter,
-    String path,
-    boolean sensibleCurrentPath
-  ) {
-    this.jinjava = jinjava;
-    this.interpreter = interpreter;
-    this.path = path;
-    this.sensibleCurrentPath = sensibleCurrentPath;
-  }
-
   public String assertExpectedOutput(String name) {
     String template = getFixtureTemplate(name);
     String output = JinjavaInterpreter.getCurrent().render(template);
@@ -137,13 +116,6 @@ public String assertExpectedNonEagerOutput(String name) {
 
   public String getFixtureTemplate(String name) {
     try {
-      if (sensibleCurrentPath) {
-        JinjavaInterpreter
-          .getCurrent()
-          .getContext()
-          .getCurrentPathStack()
-          .push(String.format("%s/%s.jinja", path, name), 0, 0);
-      }
       return Resources.toString(
         Resources.getResource(String.format("%s/%s.jinja", path, name)),
         StandardCharsets.UTF_8
@@ -155,12 +127,10 @@ 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
-        )
-        .replaceAll("\\\\\n\\s*", "");
+      return Resources.toString(
+        Resources.getResource(String.format("%s/%s.expected.jinja", path, name)),
+        StandardCharsets.UTF_8
+      );
     } catch (IOException e) {
       throw new RuntimeException(e);
     }

diff --git a/src/test/java/com/hubspot/jinjava/FullSnippetsTest.java b/src/test/java/com/hubspot/jinjava/FullSnippetsTest.java
@@ -39,7 +39,7 @@ public String getString(
           JinjavaInterpreter interpreter
         ) throws IOException {
           return Resources.toString(
-            Resources.getResource(relativePathResolver.resolve(fullName, interpreter)),
+            Resources.getResource(String.format("tags/macrotag/%s", fullName)),
             StandardCharsets.UTF_8
           );
         }
@@ -66,11 +66,7 @@ public Optional<LocationResolver> getLocationResolver() {
     );
     interpreter = new JinjavaInterpreter(parentInterpreter);
     expectedTemplateInterpreter =
-      ExpectedTemplateInterpreter.withSensibleCurrentPath(
-        jinjava,
-        interpreter,
-        "snippets"
-      );
+      new ExpectedTemplateInterpreter(jinjava, interpreter, "snippets");
     localContext = interpreter.getContext();
 
     JinjavaInterpreter.pushCurrent(interpreter);
@@ -105,11 +101,4 @@ public void itUsesLowerScopeValueInMacroEvaluation() {
       "uses-lower-scope-value-in-macro-evaluation"
     );
   }
-
-  @Test
-  public void itFromTagDoesntStealExtends() {
-    expectedTemplateInterpreter.assertExpectedOutput(
-      "from-tag-doesnt-steal-extends/test"
-    );
-  }
 }
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
@@ -1 +1 @@
-{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}{% set current_path = 'simple-with-call.jinja' %}Hello {{ myname }}{% set current_path = '' %}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}
+{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}
diff --git a/src/test/resources/eager/puts-deferred-imported-macro-in-output.expected.jinja b/src/test/resources/eager/puts-deferred-imported-macro-in-output.expected.jinja
@@ -1 +1 @@
-{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}{% set current_path = 'simple-with-call.jinja' %}Hello {{ myname }}{% set current_path = '' %}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}
+{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}
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
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
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
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
diff --git a/src/test/resources/snippets/from-tag-doesnt-steal-extends/base.jinja b/src/test/resources/snippets/from-tag-doesnt-steal-extends/base.jinja
diff --git a/src/test/resources/snippets/from-tag-doesnt-steal-extends/something-to-import.jinja b/src/test/resources/snippets/from-tag-doesnt-steal-extends/something-to-import.jinja
diff --git a/src/test/resources/snippets/from-tag-doesnt-steal-extends/test.expected.jinja b/src/test/resources/snippets/from-tag-doesnt-steal-extends/test.expected.jinja
diff --git a/src/test/resources/snippets/from-tag-doesnt-steal-extends/test.jinja b/src/test/resources/snippets/from-tag-doesnt-steal-extends/test.jinja
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}{% set current_path = 'simple-with-call.jinja' %}Hello {{ myname }}{% set current_path = '' %}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}
		{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}