Skip to content

Commit 74d9a4d

Browse files
jasmith-hsjasmith-hs
committed
Fix bugs in how macro functions reconstruct the current path
1 parent 789cfe2 commit 74d9a4d

12 files changed

Lines changed: 85 additions & 10 deletions

File tree

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

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.hubspot.jinjava.lib.fn.eager;
22

3+
import static com.hubspot.jinjava.loader.RelativePathResolver.CURRENT_PATH_CONTEXT_KEY;
4+
35
import com.google.common.annotations.Beta;
46
import com.hubspot.jinjava.el.ext.AstMacroFunction;
57
import com.hubspot.jinjava.el.ext.DeferredInvocationResolutionException;
@@ -13,6 +15,7 @@
1315
import com.hubspot.jinjava.lib.fn.MacroFunction;
1416
import com.hubspot.jinjava.lib.tag.MacroTag;
1517
import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult;
18+
import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategyFactory;
1619
import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
1720
import com.hubspot.jinjava.tree.Node;
1821
import com.hubspot.jinjava.util.EagerContextWatcher;
@@ -80,6 +83,11 @@ public Object doEvaluate(
8083
() -> getEvaluationResultDirectly(argMap, kwargMap, varArgs, interpreter),
8184
interpreter
8285
);
86+
return wrapCurrentPathSetting(
87+
interpreter,
88+
importFile,
89+
result.asTemplateString()
90+
);
8391
}
8492
return result.asTemplateString();
8593
} finally {
@@ -121,12 +129,45 @@ public Object doEvaluate(
121129
.put(
122130
tempVarName,
123131
new MacroFunctionTempVariable(
124-
prefixToPreserveState + eagerExecutionResult.asTemplateString()
132+
wrapCurrentPathSetting(
133+
interpreter,
134+
Optional
135+
.ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY))
136+
.map(Object::toString),
137+
prefixToPreserveState + eagerExecutionResult.asTemplateString()
138+
)
125139
)
126140
);
127141
throw new DeferredInvocationResolutionException(tempVarName);
128142
}
129-
return eagerExecutionResult.getResult().toString(true);
143+
return wrapCurrentPathSetting(
144+
interpreter,
145+
Optional
146+
.ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY))
147+
.map(Object::toString),
148+
eagerExecutionResult.getResult().toString(true)
149+
);
150+
}
151+
152+
private static String wrapCurrentPathSetting(
153+
JinjavaInterpreter interpreter,
154+
Optional<String> importFile,
155+
String resultToWrap
156+
) {
157+
if (!importFile.isPresent()) {
158+
return resultToWrap;
159+
}
160+
final String initialPathSetter =
161+
EagerImportingStrategyFactory.getSetTagForCurrentPath(interpreter);
162+
final String newPathSetter = EagerReconstructionUtils.buildBlockOrInlineSetTag(
163+
CURRENT_PATH_CONTEXT_KEY,
164+
importFile.get(),
165+
interpreter
166+
);
167+
if (initialPathSetter.equals(newPathSetter)) {
168+
return resultToWrap;
169+
}
170+
return newPathSetter + resultToWrap + initialPathSetter;
130171
}
131172

132173
private String getEvaluationResultDirectly(
@@ -312,6 +353,7 @@ private boolean differentMacroWithSameNameExists(JinjavaInterpreter interpreter)
312353
return false;
313354
}
314355
MacroFunction mostRecent = context.getGlobalMacro(getName());
356+
//noinspection ErrorProne
315357
if (this != mostRecent) {
316358
return true;
317359
}

‎src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
9595
child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile);
9696
JinjavaInterpreter.pushCurrent(child);
9797
try {
98-
child.render(node);
98+
child.render(node, false);
9999
} finally {
100100
JinjavaInterpreter.popCurrent();
101101
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
102102
JinjavaInterpreter.pushCurrent(child);
103103

104104
try {
105-
child.render(node);
105+
child.render(node, false);
106106
} finally {
107107
JinjavaInterpreter.popCurrent();
108108
}

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter
7979
JinjavaInterpreter.pushCurrent(child);
8080
String output;
8181
try {
82-
output = child.render(node);
82+
output = child.render(node, false);
8383
} finally {
8484
JinjavaInterpreter.popCurrent();
8585
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter
6464
String output;
6565
try {
6666
eagerImportingStrategy.setup(child);
67-
output = child.render(node);
67+
output = child.render(node, false);
6868
} finally {
6969
JinjavaInterpreter.popCurrent();
7070
}

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/EagerImportingStrategyFactory.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ public static String getSetTagForCurrentPath(JinjavaInterpreter interpreter) {
3434
.getContext()
3535
.getCurrentPathStack()
3636
.peek()
37+
.map(c -> {
38+
interpreter
39+
.getContext()
40+
.replace(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY, c);
41+
return c;
42+
})
3743
.orElseGet(() ->
3844
(String) interpreter
3945
.getContext()

‎src/test/java/com/hubspot/jinjava/EagerTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,4 +1597,11 @@ public void prepareContext(Context context) {
15971597
"keeps-meta-context-variables-through-import/test"
15981598
);
15991599
}
1600+
1601+
@Test
1602+
public void itWrapsCurrentPathAroundMacro() {
1603+
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
1604+
"wraps-current-path-around-macro/test"
1605+
);
1606+
}
16001607
}

‎src/test/java/com/hubspot/jinjava/ExpectedTemplateInterpreter.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,12 @@ public String getFixtureTemplate(String name) {
127127

128128
private String expected(String name) {
129129
try {
130-
return Resources.toString(
131-
Resources.getResource(String.format("%s/%s.expected.jinja", path, name)),
132-
StandardCharsets.UTF_8
133-
);
130+
return Resources
131+
.toString(
132+
Resources.getResource(String.format("%s/%s.expected.jinja", path, name)),
133+
StandardCharsets.UTF_8
134+
)
135+
.replaceAll("\\\\\n\\s*", "");
134136
} catch (IOException e) {
135137
throw new RuntimeException(e);
136138
}

‎src/test/resources/eager/wraps-current-path-around-macro/dir1/macro.jinja‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{% macro foo_importer() -%}
2+
{%- import "../../../eager/wraps-current-path-around-macro/dir2/imported.jinja" -%}
3+
{%- print foo -%}
4+
{%- endmacro %}

‎src/test/resources/eager/wraps-current-path-around-macro/dir2/imported.jinja‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{% set foo = deferred %}

0 commit comments

Comments
 (0)