Skip to content

Commit b29324f

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #617 from HubSpot/revert-616-revert-604-eager-import-path
Revert 616 revert 604 eager import path
2 parents 535c9d4 + edbcaa3 commit b29324f

File tree

14 files changed

+170
-39
lines changed

14 files changed

+170
-39
lines changed

‎src/main/java/com/hubspot/jinjava/interpret/Context.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
public class Context extends ScopeMap<String, Object> {
5252
public static final String GLOBAL_MACROS_SCOPE_KEY = "__macros__";
5353
public static final String IMPORT_RESOURCE_PATH_KEY = "import_resource_path";
54+
public static final String DEFERRED_IMPORT_RESOURCE_PATH_KEY =
55+
"deferred_import_resource_path";
56+
5457
public static final String IMPORT_RESOURCE_ALIAS_KEY = "import_resource_alias";
5558

5659
private SetMultimap<String, String> dependencies = HashMultimap.create();

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

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package com.hubspot.jinjava.lib.fn.eager;
22

3+
import com.google.common.collect.ImmutableMap;
34
import com.hubspot.jinjava.el.ext.AbstractCallableMethod;
5+
import com.hubspot.jinjava.interpret.Context;
46
import com.hubspot.jinjava.interpret.DeferredValue;
57
import com.hubspot.jinjava.interpret.DeferredValueException;
68
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
79
import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
810
import com.hubspot.jinjava.lib.fn.MacroFunction;
911
import com.hubspot.jinjava.lib.tag.MacroTag;
12+
import com.hubspot.jinjava.lib.tag.eager.EagerTagDecorator;
1013
import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
1114
import java.util.LinkedHashMap;
1215
import java.util.List;
@@ -99,21 +102,48 @@ public String getEndTag(JinjavaInterpreter interpreter) {
99102
* rendering pass.
100103
*/
101104
public String reconstructImage() {
105+
String prefix = "";
106+
String suffix = "";
107+
Optional<String> importFile = macroFunction.getImportFile(interpreter);
108+
if (importFile.isPresent()) {
109+
interpreter.getContext().getCurrentPathStack().pop();
110+
String currentDeferredImportResource = (String) interpreter
111+
.getContext()
112+
.get(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY);
113+
prefix =
114+
EagerTagDecorator.buildSetTagForDeferredInChildContext(
115+
ImmutableMap.of(
116+
Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY,
117+
PyishObjectMapper.getAsPyishString(importFile.get())
118+
),
119+
interpreter,
120+
false
121+
);
122+
suffix =
123+
EagerTagDecorator.buildSetTagForDeferredInChildContext(
124+
ImmutableMap.of(
125+
Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY,
126+
PyishObjectMapper.getAsPyishString(currentDeferredImportResource)
127+
),
128+
interpreter,
129+
false
130+
);
131+
}
132+
102133
String result;
103134
try {
104-
result =
105-
(String) evaluate(
106-
macroFunction
107-
.getArguments()
108-
.stream()
109-
.map(arg -> DeferredValue.instance())
110-
.toArray()
111-
);
135+
String evaluation = (String) evaluate(
136+
macroFunction
137+
.getArguments()
138+
.stream()
139+
.map(arg -> DeferredValue.instance())
140+
.toArray()
141+
);
142+
result = (getStartTag(interpreter) + evaluation + getEndTag(interpreter));
112143
} catch (DeferredValueException e) {
113144
// In case something not eager-supported encountered a deferred value
114-
return macroFunction.reconstructImage();
145+
result = macroFunction.reconstructImage();
115146
}
116-
117-
return (getStartTag(interpreter) + result + getEndTag(interpreter));
147+
return prefix + result + suffix;
118148
}
119149
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ public static void handleDeferredNodesDuringImport(
181181
interpreter.getContext().addGlobalMacro(macro);
182182
}
183183
childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
184-
childBindings.remove(Context.IMPORT_RESOURCE_PATH_KEY);
185184
childBindings
186185
.keySet()
187186
.forEach(key -> interpreter.getContext().put(key, DeferredValue.instance()));
@@ -195,7 +194,6 @@ public static void handleDeferredNodesDuringImport(
195194
childBindings.put(macroEntry.getKey(), macro);
196195
}
197196
childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
198-
childBindings.remove(Context.IMPORT_RESOURCE_PATH_KEY);
199197
interpreter.getContext().put(contextVar, DeferredValue.instance(childBindings));
200198
}
201199
}

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

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.hubspot.jinjava.doc.annotations.JinjavaHasCodeBody;
88
import com.hubspot.jinjava.doc.annotations.JinjavaParam;
99
import com.hubspot.jinjava.doc.annotations.JinjavaSnippet;
10+
import com.hubspot.jinjava.interpret.Context;
1011
import com.hubspot.jinjava.interpret.DeferredValue;
1112
import com.hubspot.jinjava.interpret.DeferredValueException;
1213
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
@@ -117,16 +118,34 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
117118
args,
118119
argNamesWithDefaults
119120
);
120-
121-
MacroFunction macro = new MacroFunction(
122-
tagNode.getChildren(),
123-
name,
124-
argNamesWithDefaults,
125-
false,
126-
interpreter.getContext(),
127-
interpreter.getLineNumber(),
128-
interpreter.getPosition()
129-
);
121+
MacroFunction macro;
122+
String contextImportResourcePath = (String) interpreter
123+
.getContext()
124+
.get(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, "");
125+
boolean scopeEntered = false;
126+
try {
127+
if (StringUtils.isNotEmpty(contextImportResourcePath)) {
128+
scopeEntered = true;
129+
interpreter.enterScope();
130+
interpreter
131+
.getContext()
132+
.put(Context.IMPORT_RESOURCE_PATH_KEY, contextImportResourcePath);
133+
}
134+
macro =
135+
new MacroFunction(
136+
tagNode.getChildren(),
137+
name,
138+
argNamesWithDefaults,
139+
false,
140+
interpreter.getContext(),
141+
interpreter.getLineNumber(),
142+
interpreter.getPosition()
143+
);
144+
} finally {
145+
if (scopeEntered) {
146+
interpreter.leaveScope();
147+
}
148+
}
130149
macro.setDeferred(deferred);
131150

132151
if (StringUtils.isNotEmpty(parentName)) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ private static String getDoTagToPreserve(
136136
}
137137
}
138138
if (keyValueJoiner.length() > 0) {
139-
return buildDoUpdateTag(currentImportAlias, keyValueJoiner.toString(), interpreter);
139+
return buildDoUpdateTag(
140+
currentImportAlias,
141+
"{" + keyValueJoiner.toString() + "}",
142+
interpreter
143+
);
140144
}
141145
return "";
142146
}
@@ -237,7 +241,6 @@ public static void integrateChild(
237241
parent.getContext().addGlobalMacro(macro);
238242
}
239243
childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
240-
childBindings.remove(Context.IMPORT_RESOURCE_PATH_KEY);
241244
childBindings.remove(Context.IMPORT_RESOURCE_ALIAS_KEY);
242245
parent.getContext().putAll(childBindings);
243246
} else {
@@ -270,7 +273,6 @@ public static void integrateChild(
270273
.forEach(childBindings::remove);
271274
// Remove meta keys
272275
childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY);
273-
childBindings.remove(Context.IMPORT_RESOURCE_PATH_KEY);
274276
childBindings.remove(Context.IMPORT_RESOURCE_ALIAS_KEY);
275277
mapForCurrentContextAlias.putAll(childBindings);
276278
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,6 @@ private static String getUpdateString(String variables) {
132132
StringJoiner updateString = new StringJoiner(",");
133133
// Update the alias map to the value of the set variable.
134134
varList.forEach(var -> updateString.add(String.format("'%s': %s", var, var)));
135-
return updateString.toString();
135+
return "{" + updateString.toString() + "}";
136136
}
137137
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ public static String buildDoUpdateTag(
401401
return new LengthLimitingStringJoiner(interpreter.getConfig().getMaxOutputSize(), " ")
402402
.add(interpreter.getConfig().getTokenScannerSymbols().getExpressionStartWithTag())
403403
.add(DoTag.TAG_NAME)
404-
.add(String.format("%s.update({%s})", currentImportAlias, updateString))
404+
.add(String.format("%s.update(%s)", currentImportAlias, updateString))
405405
.add(interpreter.getConfig().getTokenScannerSymbols().getExpressionEndWithTag())
406406
.toString();
407407
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import com.hubspot.jinjava.JinjavaConfig;
1212
import com.hubspot.jinjava.interpret.DeferredValue;
1313
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
14-
import com.hubspot.jinjava.interpret.RenderResult;
1514
import com.hubspot.jinjava.lib.fn.MacroFunction;
1615
import com.hubspot.jinjava.tree.Node;
1716
import com.hubspot.jinjava.tree.TagNode;

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

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.hubspot.jinjava.interpret.Context;
99
import com.hubspot.jinjava.interpret.DeferredValue;
1010
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
11+
import com.hubspot.jinjava.lib.filter.Filter;
1112
import com.hubspot.jinjava.lib.tag.ImportTag;
1213
import com.hubspot.jinjava.lib.tag.ImportTagTest;
1314
import com.hubspot.jinjava.lib.tag.Tag;
@@ -31,8 +32,9 @@ public class EagerImportTagTest extends ImportTagTest {
3132
private static final String TEMPLATE_FILE = "template.jinja";
3233

3334
@Before
34-
public void eagerSetup() {
35+
public void eagerSetup() throws Exception {
3536
context.put("padding", 42);
37+
context.registerFilter(new PrintPathFilter());
3638
interpreter =
3739
new JinjavaInterpreter(
3840
jinjava,
@@ -409,9 +411,12 @@ public void itDefersTripleLayer() {
409411
// There are some extras due to deferred values copying up the context stack.
410412
assertThat(interpreter.render(result).trim())
411413
.isEqualTo(
412-
"{'b': {'foo_b': 'ba', 'a': {'foo_a': 'a', 'something': 'somn'}, 'foo_a': 'a'}, " +
413-
"'foo_c': 'cbaa', " +
414-
"'a': {'foo_a': 'a', 'something': 'somn'}, 'foo_b': 'ba', 'foo_a': 'a'}"
414+
"{'b': {'foo_b': 'ba', 'a': " +
415+
"{'foo_a': 'a', 'import_resource_path': 'import-tree-a.jinja', 'something': 'somn'}, " +
416+
"'foo_a': 'a', 'import_resource_path': 'import-tree-b.jinja'}, " +
417+
"'foo_c': 'cbaa', 'a': {'foo_a': 'a', 'import_resource_path': " +
418+
"'import-tree-a.jinja', 'something': 'somn'}, " +
419+
"'foo_b': 'ba', 'foo_a': 'a', 'import_resource_path': 'import-tree-c.jinja'}"
415420
);
416421
}
417422

@@ -440,6 +445,64 @@ public void itDefersQuadLayer() {
440445
assertThat(interpreter.render(result).trim()).isEqualTo("12345 cbaabaaba");
441446
}
442447

448+
@Test
449+
public void itCorrectlySetsAliasedPath() {
450+
setupResourceLocator();
451+
context.put("foo", "foo");
452+
String result = interpreter.render(
453+
"{% import 'import-macro.jinja' as m %}{{ m.print_path_macro(foo) }}"
454+
);
455+
assertThat(result.trim()).isEqualTo("import-macro.jinja\nfoo");
456+
}
457+
458+
@Test
459+
public void itCorrectlySetsPath() {
460+
setupResourceLocator();
461+
context.put("foo", "foo");
462+
String result = interpreter.render(
463+
"{% import 'import-macro.jinja' %}{{ print_path_macro(foo) }}"
464+
);
465+
assertThat(result.trim()).isEqualTo("import-macro.jinja\nfoo");
466+
}
467+
468+
@Test
469+
public void itCorrectlySetsAliasedPathForSecondPass() {
470+
setupResourceLocator();
471+
context.put("foo", DeferredValue.instance());
472+
String firstPassResult = interpreter.render(
473+
"{% import 'import-macro.jinja' as m %}{{ m.print_path_macro(foo) }}"
474+
);
475+
assertThat(firstPassResult)
476+
.isEqualTo(
477+
"{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro m.print_path_macro(var) %}\n" +
478+
"{{ var|print_path }}\n" +
479+
"{{ var }}\n" +
480+
"{% endmacro %}{% set deferred_import_resource_path = null %}{{ m.print_path_macro(foo) }}"
481+
);
482+
context.put("foo", "foo");
483+
assertThat(interpreter.render(firstPassResult).trim())
484+
.isEqualTo("import-macro.jinja\nfoo");
485+
}
486+
487+
@Test
488+
public void itCorrectlySetsPathForSecondPass() {
489+
setupResourceLocator();
490+
context.put("foo", DeferredValue.instance());
491+
String firstPassResult = interpreter.render(
492+
"{% import 'import-macro.jinja' %}{{ print_path_macro(foo) }}"
493+
);
494+
assertThat(firstPassResult)
495+
.isEqualTo(
496+
"{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro print_path_macro(var) %}\n" +
497+
"{{ var|print_path }}\n" +
498+
"{{ var }}\n" +
499+
"{% endmacro %}{% set deferred_import_resource_path = null %}{{ print_path_macro(foo) }}"
500+
);
501+
context.put("foo", "foo");
502+
assertThat(interpreter.render(firstPassResult).trim())
503+
.isEqualTo("import-macro.jinja\nfoo");
504+
}
505+
443506
@Test
444507
public void itImportsDoublyNamed() {
445508
setupResourceLocator();
@@ -488,6 +551,19 @@ public Optional<LocationResolver> getLocationResolver() {
488551
);
489552
}
490553

554+
public static class PrintPathFilter implements Filter {
555+
556+
@Override
557+
public Object filter(Object var, JinjavaInterpreter interpreter, String... args) {
558+
return interpreter.getContext().getCurrentPathStack().peek().orElse("/");
559+
}
560+
561+
@Override
562+
public String getName() {
563+
return "print_path";
564+
}
565+
}
566+
491567
@Test
492568
@Ignore
493569
@Override

‎src/test/resources/eager/handles-deferred-import-vars.expected.jinja‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ bar: {{ bar }}
77
---{% set myname = deferred + 7 %}
88
{% set bar = myname + 19 %}{% set simple = {} %}{% do simple.update({'bar': bar}) %}
99
Hello {{ myname }}
10-
11-
simple.foo: {% macro simple.foo() %}Hello {{ myname }}{% endmacro %}{{ simple.foo() }}
10+
{% do simple.update({'import_resource_path': 'macro-and-set.jinja'}) %}
11+
simple.foo: {% set deferred_import_resource_path = 'macro-and-set.jinja' %}{% macro simple.foo() %}Hello {{ myname }}{% endmacro %}{% set deferred_import_resource_path = null %}{{ simple.foo() }}
1212
simple.bar: {{ simple.bar }}

0 commit comments

Comments
 (0)