Skip to content

Commit 26b6a31

Browse files
maple-buicemaple-buice
authored
Merge pull request #1248 from HubSpot/fix/hubL-nested-relative-import-resolution
Fix HubL `from` tag nested relative import resolution
2 parents 6bcb7cb + c213d7a commit 26b6a31

File tree

6 files changed

+362
-13
lines changed

6 files changed

+362
-13
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,16 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
8585
Map<String, String> imports = getImportMap(helper);
8686

8787
try {
88-
String template = interpreter.getResource(templateFile);
89-
Node node = interpreter.parse(template);
88+
Node node = ImportTag.parseTemplateAsNode(interpreter, templateFile);
9089

9190
JinjavaInterpreter child = interpreter
9291
.getConfig()
9392
.getInterpreterFactory()
9493
.newInstance(interpreter);
9594
child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile);
95+
9696
JinjavaInterpreter.pushCurrent(child);
97+
9798
try {
9899
child.render(node);
99100
} finally {
@@ -125,6 +126,7 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
125126
}
126127
} finally {
127128
interpreter.getContext().popFromStack();
129+
interpreter.getContext().getCurrentPathStack().pop();
128130
}
129131
}
130132

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.hubspot.jinjava.lib.fn.eager.EagerMacroFunction;
1010
import com.hubspot.jinjava.lib.tag.DoTag;
1111
import com.hubspot.jinjava.lib.tag.FromTag;
12+
import com.hubspot.jinjava.lib.tag.ImportTag;
1213
import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategyFactory;
1314
import com.hubspot.jinjava.tree.Node;
1415
import com.hubspot.jinjava.tree.parse.TagToken;
@@ -81,8 +82,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter
8182
String templateFile = maybeTemplateFile.get();
8283
try {
8384
try {
84-
String template = interpreter.getResource(templateFile);
85-
Node node = interpreter.parse(template);
85+
Node node = ImportTag.parseTemplateAsNode(interpreter, templateFile);
8686

8787
JinjavaInterpreter child = interpreter
8888
.getConfig()
@@ -138,6 +138,7 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter
138138
}
139139
} finally {
140140
interpreter.getContext().popFromStack();
141+
interpreter.getContext().getCurrentPathStack().pop();
141142
}
142143
}
143144

‎src/test/java/com/hubspot/jinjava/lib/tag/FromTagTest.java‎

Lines changed: 196 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package com.hubspot.jinjava.lib.tag;
22

3+
import static com.hubspot.jinjava.lib.tag.ResourceLocatorTestHelper.getTestResourceLocator;
34
import static com.hubspot.jinjava.loader.RelativePathResolver.CURRENT_PATH_CONTEXT_KEY;
45
import static org.assertj.core.api.Assertions.assertThat;
5-
import static org.junit.Assert.assertTrue;
66

77
import com.google.common.io.Resources;
88
import com.hubspot.jinjava.BaseInterpretingTest;
@@ -16,6 +16,7 @@
1616
import java.io.IOException;
1717
import java.nio.charset.Charset;
1818
import java.nio.charset.StandardCharsets;
19+
import java.util.Map;
1920
import java.util.Optional;
2021
import org.junit.Before;
2122
import org.junit.Test;
@@ -26,7 +27,8 @@ public class FromTagTest extends BaseInterpretingTest {
2627
public void setup() {
2728
jinjava.setResourceLocator(
2829
new ResourceLocator() {
29-
private RelativePathResolver relativePathResolver = new RelativePathResolver();
30+
private final RelativePathResolver relativePathResolver =
31+
new RelativePathResolver();
3032

3133
@Override
3234
public String getString(
@@ -66,25 +68,27 @@ public void itImportsAliasedMacroName() {
6668
}
6769

6870
@Test
69-
public void importedCycleDected() {
71+
public void importedCycleDetected() {
7072
fixture("from-recursion");
71-
assertTrue(
73+
assertThat(
7274
interpreter
7375
.getErrorsCopy()
7476
.stream()
7577
.anyMatch(e -> e.getCategory() == BasicTemplateErrorCategory.FROM_CYCLE_DETECTED)
76-
);
78+
)
79+
.isTrue();
7780
}
7881

7982
@Test
80-
public void importedIndirectCycleDected() {
83+
public void importedIndirectCycleDetected() {
8184
fixture("from-a-to-b");
82-
assertTrue(
85+
assertThat(
8386
interpreter
8487
.getErrorsCopy()
8588
.stream()
8689
.anyMatch(e -> e.getCategory() == BasicTemplateErrorCategory.FROM_CYCLE_DETECTED)
87-
);
90+
)
91+
.isTrue();
8892
}
8993

9094
@Test
@@ -112,6 +116,190 @@ public void itDefersImport() {
112116
assertThat(spacer.isDeferred()).isTrue();
113117
}
114118

119+
@Test
120+
public void itResolvesNestedRelativeImports() throws Exception {
121+
jinjava.setResourceLocator(
122+
getTestResourceLocator(
123+
Map.of(
124+
"level0.jinja",
125+
"{% from 'level1/nested.jinja' import macro1 %}{{ macro1() }}",
126+
"level1/nested.jinja",
127+
"{% from '../level1/deeper/macro.jinja' import macro2 %}{% macro macro1() %}L1:{{ macro2() }}{% endmacro %}",
128+
"level1/deeper/macro.jinja",
129+
"{% from '../../utils/helper.jinja' import helper %}{% macro macro2() %}L2:{{ helper() }}{% endmacro %}",
130+
"utils/helper.jinja",
131+
"{% macro helper() %}HELPER{% endmacro %}"
132+
)
133+
)
134+
);
135+
136+
interpreter.getContext().getCurrentPathStack().push("level0.jinja", 1, 0);
137+
String result = interpreter.render(interpreter.getResource("level0.jinja"));
138+
139+
assertThat(interpreter.getErrors()).isEmpty();
140+
assertThat(result.trim()).isEqualTo("L1:L2:HELPER");
141+
}
142+
143+
@Test
144+
public void itMaintainsPathStackIntegrity() throws Exception {
145+
jinjava.setResourceLocator(
146+
getTestResourceLocator(
147+
Map.of(
148+
"root.jinja",
149+
"{% from 'simple/macro.jinja' import simple_macro %}{{ simple_macro() }}",
150+
"simple/macro.jinja",
151+
"{% macro simple_macro() %}SIMPLE{% endmacro %}"
152+
)
153+
)
154+
);
155+
156+
interpreter.getContext().getCurrentPathStack().push("root.jinja", 1, 0);
157+
Optional<String> initialTopPath = interpreter
158+
.getContext()
159+
.getCurrentPathStack()
160+
.peek();
161+
162+
interpreter.render(interpreter.getResource("root.jinja"));
163+
164+
assertThat(interpreter.getContext().getCurrentPathStack().peek())
165+
.isEqualTo(initialTopPath);
166+
assertThat(interpreter.getErrors()).isEmpty();
167+
}
168+
169+
@Test
170+
public void itWorksWithIncludeAndFromTogether() throws Exception {
171+
jinjava.setResourceLocator(
172+
getTestResourceLocator(
173+
Map.of(
174+
"mixed-tags.jinja",
175+
"{% from 'macros/test.jinja' import test_macro %}{% include 'includes/content.jinja' %}{{ test_macro() }}",
176+
"macros/test.jinja",
177+
"{% from '../utils/shared.jinja' import shared %}{% macro test_macro() %}MACRO:{{ shared() }}{% endmacro %}",
178+
"includes/content.jinja",
179+
"{% from '../utils/shared.jinja' import shared %}INCLUDE:{{ shared() }}",
180+
"utils/shared.jinja",
181+
"{% macro shared() %}SHARED{% endmacro %}"
182+
)
183+
)
184+
);
185+
186+
interpreter.getContext().getCurrentPathStack().push("mixed-tags.jinja", 1, 0);
187+
String result = interpreter.render(interpreter.getResource("mixed-tags.jinja"));
188+
189+
assertThat(interpreter.getErrors()).isEmpty();
190+
assertThat(result.trim()).contains("INCLUDE:SHARED");
191+
assertThat(result.trim()).contains("MACRO:SHARED");
192+
}
193+
194+
@Test
195+
public void itResolvesUpAndAcrossDirectoryPaths() throws Exception {
196+
jinjava.setResourceLocator(
197+
getTestResourceLocator(
198+
Map.of(
199+
"theme/hubl-modules/navigation.module/module.hubl.html",
200+
"{% from '../../partials/atoms/link/link.hubl.html' import link_macro %}{{ link_macro() }}",
201+
"theme/partials/atoms/link/link.hubl.html",
202+
"{% from '../icons/icons.hubl.html' import icon_macro %}{% macro link_macro() %}LINK:{{ icon_macro() }}{% endmacro %}",
203+
"theme/partials/atoms/icons/icons.hubl.html",
204+
"{% macro icon_macro() %}ICON{% endmacro %}"
205+
)
206+
)
207+
);
208+
209+
interpreter
210+
.getContext()
211+
.getCurrentPathStack()
212+
.push("theme/hubl-modules/navigation.module/module.hubl.html", 1, 0);
213+
String result = interpreter.render(
214+
interpreter.getResource("theme/hubl-modules/navigation.module/module.hubl.html")
215+
);
216+
217+
assertThat(interpreter.getErrors()).isEmpty();
218+
assertThat(result.trim()).isEqualTo("LINK:ICON");
219+
}
220+
221+
@Test
222+
public void itResolvesProjectsAbsolutePathsWithNestedRelativeImports()
223+
throws Exception {
224+
jinjava.setResourceLocator(
225+
getTestResourceLocator(
226+
Map.of(
227+
"@projects/theme-a/modules/header/header.html",
228+
"{% from '../../components/button.html' import render_button %}{{ render_button('primary') }}",
229+
"@projects/theme-a/components/button.html",
230+
"{% from '../utils/icons.html' import get_icon %}{% macro render_button(type) %}{{ type }}-{{ get_icon() }}{% endmacro %}",
231+
"@projects/theme-a/utils/icons.html",
232+
"{% macro get_icon() %}ICON{% endmacro %}"
233+
)
234+
)
235+
);
236+
237+
interpreter
238+
.getContext()
239+
.getCurrentPathStack()
240+
.push("@projects/theme-a/modules/header/header.html", 1, 0);
241+
String result = interpreter.render(
242+
interpreter.getResource("@projects/theme-a/modules/header/header.html")
243+
);
244+
245+
assertThat(interpreter.getErrors()).isEmpty();
246+
assertThat(result.trim()).isEqualTo("primary-ICON");
247+
}
248+
249+
@Test
250+
public void itResolvesHubspotAbsolutePathsWithNestedRelativeImports() throws Exception {
251+
jinjava.setResourceLocator(
252+
getTestResourceLocator(
253+
Map.of(
254+
"@hubspot/modules/forms/contact-form.html",
255+
"{% from '../../shared/validation.html' import validate_field %}{{ validate_field('email') }}",
256+
"@hubspot/shared/validation.html",
257+
"{% from '../helpers/formatters.html' import format_error %}{% macro validate_field(field) %}{{ format_error(field) }}{% endmacro %}",
258+
"@hubspot/helpers/formatters.html",
259+
"{% macro format_error(field) %}ERROR:{{ field }}{% endmacro %}"
260+
)
261+
)
262+
);
263+
264+
interpreter
265+
.getContext()
266+
.getCurrentPathStack()
267+
.push("@hubspot/modules/forms/contact-form.html", 1, 0);
268+
String result = interpreter.render(
269+
interpreter.getResource("@hubspot/modules/forms/contact-form.html")
270+
);
271+
272+
assertThat(interpreter.getErrors()).isEmpty();
273+
assertThat(result.trim()).isEqualTo("ERROR:email");
274+
}
275+
276+
@Test
277+
public void itResolvesMixedAbsoluteAndRelativeImports() throws Exception {
278+
jinjava.setResourceLocator(
279+
getTestResourceLocator(
280+
Map.of(
281+
"@projects/mixed/module.html",
282+
"{% from '@hubspot/shared/globals.html' import global_helper %}{{ global_helper() }}",
283+
"@hubspot/shared/globals.html",
284+
"{% from '../utils/common.html' import format_text %}{% macro global_helper() %}{{ format_text('MIXED') }}{% endmacro %}",
285+
"@hubspot/utils/common.html",
286+
"{% macro format_text(text) %}FORMAT:{{ text }}{% endmacro %}"
287+
)
288+
)
289+
);
290+
291+
interpreter
292+
.getContext()
293+
.getCurrentPathStack()
294+
.push("@projects/mixed/module.html", 1, 0);
295+
String result = interpreter.render(
296+
interpreter.getResource("@projects/mixed/module.html")
297+
);
298+
299+
assertThat(interpreter.getErrors()).isEmpty();
300+
assertThat(result.trim()).isEqualTo("FORMAT:MIXED");
301+
}
302+
115303
private String fixture(String name) {
116304
return interpreter.renderFlat(fixtureText(name));
117305
}

0 commit comments

Comments
 (0)