Skip to content

Commit c3b56b8

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1202 from HubSpot/revert-1199-macro-current-path-fix
Revert "Wrap the current path around reconstructed macro functions"
2 parents 12bc66c + 0744ad1 commit c3b56b8

15 files changed

Lines changed: 12 additions & 140 deletions

File tree

‎src/main/java/com/hubspot/jinjava/Jinjava.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import com.hubspot.jinjava.lib.fn.ELFunctionDefinition;
3636
import com.hubspot.jinjava.lib.tag.Tag;
3737
import com.hubspot.jinjava.loader.ClasspathResourceLocator;
38-
import com.hubspot.jinjava.loader.RelativePathResolver;
3938
import com.hubspot.jinjava.loader.ResourceLocator;
4039
import de.odysseus.el.ExpressionFactoryImpl;
4140
import de.odysseus.el.misc.TypeConverter;
@@ -245,10 +244,6 @@ public RenderResult renderForResult(
245244
} else {
246245
context = new Context(copyGlobalContext(), bindings, renderConfig.getDisabled());
247246
}
248-
Object currentPath = context.get(RelativePathResolver.CURRENT_PATH_CONTEXT_KEY);
249-
if (currentPath != null) {
250-
context.getCurrentPathStack().pushWithoutCycleCheck(currentPath.toString(), 0, 0);
251-
}
252247

253248
JinjavaInterpreter interpreter = globalConfig
254249
.getInterpreterFactory()
@@ -295,9 +290,6 @@ public RenderResult renderForResult(
295290
);
296291
} finally {
297292
globalContext.reset();
298-
if (currentPath != null) {
299-
context.getCurrentPathStack().pop();
300-
}
301293
JinjavaInterpreter.popCurrent();
302294
}
303295
}

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

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

3-
import static com.hubspot.jinjava.loader.RelativePathResolver.CURRENT_PATH_CONTEXT_KEY;
4-
53
import com.google.common.annotations.Beta;
64
import com.hubspot.jinjava.el.ext.AstMacroFunction;
75
import com.hubspot.jinjava.el.ext.DeferredInvocationResolutionException;
@@ -15,7 +13,6 @@
1513
import com.hubspot.jinjava.lib.fn.MacroFunction;
1614
import com.hubspot.jinjava.lib.tag.MacroTag;
1715
import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult;
18-
import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategyFactory;
1916
import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
2017
import com.hubspot.jinjava.tree.Node;
2118
import com.hubspot.jinjava.util.EagerContextWatcher;
@@ -83,11 +80,6 @@ public Object doEvaluate(
8380
() -> getEvaluationResultDirectly(argMap, kwargMap, varArgs, interpreter),
8481
interpreter
8582
);
86-
return wrapCurrentPathSetting(
87-
interpreter,
88-
importFile,
89-
result.asTemplateString()
90-
);
9183
}
9284
return result.asTemplateString();
9385
} finally {
@@ -129,45 +121,12 @@ public Object doEvaluate(
129121
.put(
130122
tempVarName,
131123
new MacroFunctionTempVariable(
132-
wrapCurrentPathSetting(
133-
interpreter,
134-
Optional
135-
.ofNullable(localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY))
136-
.map(Object::toString),
137-
prefixToPreserveState + eagerExecutionResult.asTemplateString()
138-
)
124+
prefixToPreserveState + eagerExecutionResult.asTemplateString()
139125
)
140126
);
141127
throw new DeferredInvocationResolutionException(tempVarName);
142128
}
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;
129+
return eagerExecutionResult.getResult().toString(true);
171130
}
172131

173132
private String getEvaluationResultDirectly(
@@ -353,7 +312,6 @@ private boolean differentMacroWithSameNameExists(JinjavaInterpreter interpreter)
353312
return false;
354313
}
355314
MacroFunction mostRecent = context.getGlobalMacro(getName());
356-
//noinspection ErrorProne
357315
if (this != mostRecent) {
358316
return true;
359317
}

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ public void itDefersMacroInIf() {
625625

626626
@Test
627627
public void itPutsDeferredImportedMacroInOutput() {
628-
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
628+
expectedTemplateInterpreter.assertExpectedOutput(
629629
"puts-deferred-imported-macro-in-output"
630630
);
631631
}
@@ -643,7 +643,7 @@ public void itPutsDeferredImportedMacroInOutputSecondPass() {
643643

644644
@Test
645645
public void itPutsDeferredFromedMacroInOutput() {
646-
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
646+
expectedTemplateInterpreter.assertExpectedOutput(
647647
"puts-deferred-fromed-macro-in-output"
648648
);
649649
}
@@ -1597,11 +1597,4 @@ 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-
}
16071600
}

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

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ public class ExpectedTemplateInterpreter {
1515
private Jinjava jinjava;
1616
private JinjavaInterpreter interpreter;
1717
private String path;
18-
private boolean sensibleCurrentPath = false;
1918

2019
public ExpectedTemplateInterpreter(
2120
Jinjava jinjava,
@@ -27,26 +26,6 @@ public ExpectedTemplateInterpreter(
2726
this.path = path;
2827
}
2928

30-
public static ExpectedTemplateInterpreter withSensibleCurrentPath(
31-
Jinjava jinjava,
32-
JinjavaInterpreter interpreter,
33-
String path
34-
) {
35-
return new ExpectedTemplateInterpreter(jinjava, interpreter, path, true);
36-
}
37-
38-
private ExpectedTemplateInterpreter(
39-
Jinjava jinjava,
40-
JinjavaInterpreter interpreter,
41-
String path,
42-
boolean sensibleCurrentPath
43-
) {
44-
this.jinjava = jinjava;
45-
this.interpreter = interpreter;
46-
this.path = path;
47-
this.sensibleCurrentPath = sensibleCurrentPath;
48-
}
49-
5029
public String assertExpectedOutput(String name) {
5130
String template = getFixtureTemplate(name);
5231
String output = JinjavaInterpreter.getCurrent().render(template);
@@ -137,13 +116,6 @@ public String assertExpectedNonEagerOutput(String name) {
137116

138117
public String getFixtureTemplate(String name) {
139118
try {
140-
if (sensibleCurrentPath) {
141-
JinjavaInterpreter
142-
.getCurrent()
143-
.getContext()
144-
.getCurrentPathStack()
145-
.push(String.format("%s/%s.jinja", path, name), 0, 0);
146-
}
147119
return Resources.toString(
148120
Resources.getResource(String.format("%s/%s.jinja", path, name)),
149121
StandardCharsets.UTF_8
@@ -155,12 +127,10 @@ public String getFixtureTemplate(String name) {
155127

156128
private String expected(String name) {
157129
try {
158-
return Resources
159-
.toString(
160-
Resources.getResource(String.format("%s/%s.expected.jinja", path, name)),
161-
StandardCharsets.UTF_8
162-
)
163-
.replaceAll("\\\\\n\\s*", "");
130+
return Resources.toString(
131+
Resources.getResource(String.format("%s/%s.expected.jinja", path, name)),
132+
StandardCharsets.UTF_8
133+
);
164134
} catch (IOException e) {
165135
throw new RuntimeException(e);
166136
}

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public String getString(
3939
JinjavaInterpreter interpreter
4040
) throws IOException {
4141
return Resources.toString(
42-
Resources.getResource(relativePathResolver.resolve(fullName, interpreter)),
42+
Resources.getResource(String.format("tags/macrotag/%s", fullName)),
4343
StandardCharsets.UTF_8
4444
);
4545
}
@@ -66,11 +66,7 @@ public Optional<LocationResolver> getLocationResolver() {
6666
);
6767
interpreter = new JinjavaInterpreter(parentInterpreter);
6868
expectedTemplateInterpreter =
69-
ExpectedTemplateInterpreter.withSensibleCurrentPath(
70-
jinjava,
71-
interpreter,
72-
"snippets"
73-
);
69+
new ExpectedTemplateInterpreter(jinjava, interpreter, "snippets");
7470
localContext = interpreter.getContext();
7571

7672
JinjavaInterpreter.pushCurrent(interpreter);
@@ -105,11 +101,4 @@ public void itUsesLowerScopeValueInMacroEvaluation() {
105101
"uses-lower-scope-value-in-macro-evaluation"
106102
);
107103
}
108-
109-
@Test
110-
public void itFromTagDoesntStealExtends() {
111-
expectedTemplateInterpreter.assertExpectedOutput(
112-
"from-tag-doesnt-steal-extends/test"
113-
);
114-
}
115104
}

‎src/test/resources/eager/puts-deferred-fromed-macro-in-output.expected.jinja‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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__ %}
1+
{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}

‎src/test/resources/eager/puts-deferred-imported-macro-in-output.expected.jinja‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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__ %}
1+
{% set myname = deferred + 3 %}{% set __macro_getPath_331491059_temp_variable_1__ %}Hello {{ myname }}{% endset %}{% print __macro_getPath_331491059_temp_variable_1__ %}

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

Lines changed: 0 additions & 4 deletions
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)