Skip to content

Commit 5102685

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1312 from HubSpot/fix-block-reconstruction
fix: Preserve block tags to preserve reconstruction/execution order
2 parents a127fe3 + e41a580 commit 5102685

19 files changed

Lines changed: 362 additions & 8 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,7 @@ private String render(Node root, boolean processExtendRoots, long renderLimit) {
427427
output.addNode(pathSetter);
428428
Optional<String> basePath = context.getCurrentPathStack().peek();
429429
StringBuilder ignoredOutput = new StringBuilder();
430+
boolean preserveBlocks = false;
430431
// render all extend parents, keeping the last as the root output
431432
if (processExtendRoots) {
432433
Set<String> extendPaths = new HashSet<>();
@@ -495,10 +496,23 @@ private String render(Node root, boolean processExtendRoots, long renderLimit) {
495496
basePath = Optional.of(currentPath);
496497
}
497498
}
499+
preserveBlocks = (context.getDeferredTokens().size() > numDeferredTokensBefore);
498500
}
499501

500502
int numDeferredTokensBefore = context.getDeferredTokens().size();
501503
resolveBlockStubs(output);
504+
if (preserveBlocks) {
505+
for (BlockPlaceholderOutputNode blockPlaceholder : output.getBlocks()) {
506+
blockPlaceholder.resolve(
507+
EagerReconstructionUtils.wrapInTag(
508+
blockPlaceholder.getValue(),
509+
"block %s".formatted(blockPlaceholder.getBlockName()),
510+
this,
511+
false
512+
)
513+
);
514+
}
515+
}
502516
if (context.getDeferredTokens().size() > numDeferredTokensBefore) {
503517
pathSetter.setValue(
504518
EagerReconstructionUtils.buildBlockOrInlineSetTag(

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,4 +1753,65 @@ public void itHandlesDeferredValueInRenderFilter() {
17531753
"handles-deferred-value-in-render-filter/test"
17541754
);
17551755
}
1756+
1757+
@Test
1758+
public void itHandlesDeferredUsedInMultipleBlockLevels() {
1759+
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
1760+
"handles-deferred-used-in-multiple-block-levels/test"
1761+
);
1762+
}
1763+
1764+
@Test
1765+
public void itHandlesDeferredUsedInMultipleBlockLevelsSecondPass() {
1766+
localContext.put("deferred", "resolved");
1767+
expectedTemplateInterpreter.assertExpectedOutput(
1768+
"handles-deferred-used-in-multiple-block-levels/test.expected"
1769+
);
1770+
expectedTemplateInterpreter.assertExpectedNonEagerOutput(
1771+
"handles-deferred-used-in-multiple-block-levels/test.expected"
1772+
);
1773+
}
1774+
1775+
@Test
1776+
public void itDoesNotDeferBlockWhenOnlyMiddleDefers() {
1777+
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
1778+
"does-not-defer-block-when-only-middle-defers/test"
1779+
);
1780+
}
1781+
1782+
@Test
1783+
public void itDoesNotDeferBlockWhenOnlyMiddleDefersSecondPass() {
1784+
localContext.put("deferred", "resolved");
1785+
expectedTemplateInterpreter.assertExpectedOutput(
1786+
"does-not-defer-block-when-only-middle-defers/test.expected"
1787+
);
1788+
expectedTemplateInterpreter.assertExpectedNonEagerOutput(
1789+
"does-not-defer-block-when-only-middle-defers/test.expected"
1790+
);
1791+
}
1792+
1793+
@Test
1794+
public void itPreservesBlocksForReconstructionOrder() {
1795+
expectedTemplateInterpreter.assertExpectedOutputNonIdempotent(
1796+
"preserves-blocks-for-reconstruction-order/test"
1797+
);
1798+
}
1799+
1800+
@Test
1801+
public void itPreservesBlocksForReconstructionOrderSecondPhase() {
1802+
localContext.put("deferred", "resolved");
1803+
String twoPhaseOutput = expectedTemplateInterpreter.assertExpectedOutput(
1804+
"preserves-blocks-for-reconstruction-order/test.expected"
1805+
);
1806+
expectedTemplateInterpreter.assertExpectedNonEagerOutput(
1807+
"preserves-blocks-for-reconstruction-order/test.expected"
1808+
);
1809+
// Sanity check
1810+
assertThat(twoPhaseOutput)
1811+
.isEqualToIgnoringWhitespace(
1812+
expectedTemplateInterpreter.renderTemplate(
1813+
"preserves-blocks-for-reconstruction-order/test"
1814+
)
1815+
);
1816+
}
17561817
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ private ExpectedTemplateInterpreter(
4949
}
5050

5151
public String assertExpectedOutput(String name) {
52-
String template = getFixtureTemplate(name);
53-
String output = JinjavaInterpreter.getCurrent().render(template);
52+
String output = renderTemplate(name);
5453
assertThat(JinjavaInterpreter.getCurrent().getContext().getDeferredNodes())
5554
.as("Ensure no deferred nodes were created")
5655
.isEmpty();
@@ -60,9 +59,13 @@ public String assertExpectedOutput(String name) {
6059
return output;
6160
}
6261

63-
public String assertExpectedOutputNonIdempotent(String name) {
62+
public String renderTemplate(String name) {
6463
String template = getFixtureTemplate(name);
65-
String output = JinjavaInterpreter.getCurrent().render(template);
64+
return JinjavaInterpreter.getCurrent().render(template);
65+
}
66+
67+
public String assertExpectedOutputNonIdempotent(String name) {
68+
String output = renderTemplate(name);
6669
assertThat(JinjavaInterpreter.getCurrent().getContext().getDeferredNodes())
6770
.as("Ensure no deferred nodes were created")
6871
.isEmpty();

‎src/test/resources/eager/does-not-defer-block-when-only-middle-defers/base.jinja‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{% set tracker_base = '1_base' %}
2+
3+
4+
-----Pre-First-----
5+
{% block first -%}
6+
{%- endblock %}
7+
-----Post-First-----
8+
-----Pre-Second-----
9+
{% block second -%}
10+
{%- endblock %}
11+
-----Post-Second-----
12+
We aren't deferring tracker base.{# This message WILL show up in final output #}

‎src/test/resources/eager/does-not-defer-block-when-only-middle-defers/middle.jinja‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{% extends '../../eager/does-not-defer-block-when-only-middle-defers/base.jinja' %}
2+
{% set tracker_middle = '2_middle' %}
3+
{% block first %}
4+
I WON'T SHOW UP
5+
{% endblock %}
6+
7+
{% block second %}
8+
tracker_base is '1_base': {{ tracker_base }}? {{ tracker_base == '1_base' }}
9+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
10+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
11+
{% endblock %}
12+
Deferring tracker middle.{# This message will not show up in final output #}
13+
{% set tracker_middle = deferred %}

‎src/test/resources/eager/does-not-defer-block-when-only-middle-defers/test.expected.expected.jinja‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
-----Pre-First-----
2+
3+
tracker_base is '1_base': 1_base? true
4+
tracker_middle is 'resolved': resolved? true
5+
tracker_test is 'resolved': resolved? true
6+
7+
-----Post-First-----
8+
-----Pre-Second-----
9+
10+
tracker_base is '1_base': 1_base? true
11+
tracker_middle is 'resolved': resolved? true
12+
tracker_test is 'resolved': resolved? true
13+
14+
-----Post-Second-----
15+
We aren't deferring tracker base.

‎src/test/resources/eager/does-not-defer-block-when-only-middle-defers/test.expected.jinja‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{# Start Label: ignored_output_from_extends #}{% do %}
2+
3+
4+
5+
Deferring tracker test.
6+
{% set tracker_test = deferred %}
7+
8+
9+
10+
11+
12+
Deferring tracker middle.
13+
{% set tracker_middle = deferred %}
14+
{% enddo %}\
15+
{# End Label: ignored_output_from_extends #}{% set current_path = 'eager/does-not-defer-block-when-only-middle-defers/base.jinja' %}
16+
17+
18+
-----Pre-First-----
19+
{% set __temp_meta_current_path_1008935144__,current_path = current_path,'eager/does-not-defer-block-when-only-middle-defers/test.jinja' %}
20+
tracker_base is '1_base': 1_base? true
21+
tracker_middle is 'resolved': {{ tracker_middle }}\
22+
? {{ tracker_middle == 'resolved' }}
23+
tracker_test is 'resolved': {{ tracker_test }}\
24+
? {{ tracker_test == 'resolved' }}
25+
{% set current_path,__temp_meta_current_path_1008935144__ = __temp_meta_current_path_1008935144__,null %}
26+
-----Post-First-----
27+
-----Pre-Second-----
28+
{% set __temp_meta_current_path_245328778__,current_path = current_path,'eager/does-not-defer-block-when-only-middle-defers/middle.jinja' %}
29+
tracker_base is '1_base': 1_base? true
30+
tracker_middle is 'resolved': {{ tracker_middle }}\
31+
? {{ tracker_middle == 'resolved' }}
32+
tracker_test is 'resolved': {{ tracker_test }}\
33+
? {{ tracker_test == 'resolved' }}
34+
{% set current_path,__temp_meta_current_path_245328778__ = __temp_meta_current_path_245328778__,null %}
35+
-----Post-Second-----
36+
We aren't deferring tracker base.

‎src/test/resources/eager/does-not-defer-block-when-only-middle-defers/test.jinja‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{% extends '../../eager/does-not-defer-block-when-only-middle-defers/middle.jinja' %}
2+
3+
{% set tracker_test = '3_test' %}
4+
{% block first %}
5+
tracker_base is '1_base': {{ tracker_base }}? {{ tracker_base == '1_base' }}
6+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
7+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
8+
{% endblock %}
9+
Deferring tracker test.{# This message will not show up in final output #}
10+
{% set tracker_test = deferred %}

‎src/test/resources/eager/handles-deferred-used-in-multiple-block-levels/base.jinja‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{% set tracker_base = '1_base' %}
2+
3+
tracker_base is '1_base': {{ tracker_base }}? {{ tracker_base == '1_base' }}
4+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
5+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
6+
-----Pre-First-----
7+
{% block first -%}
8+
tracker_base is 'resolved': {{ tracker_base }}? {{ tracker_base == 'resolved' }}
9+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
10+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
11+
{%- endblock %}
12+
-----Post-First-----
13+
tracker_base is '1_base': {{ tracker_base }}? {{ tracker_base == '1_base' }}
14+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
15+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
16+
-----Pre-Second-----
17+
{% block second -%}
18+
tracker_base is 'resolved': {{ tracker_base }}? {{ tracker_base == 'resolved' }}
19+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
20+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
21+
{%- endblock %}
22+
-----Post-Second-----
23+
Deferring tracker base.{# This message WILL show up in final output #}
24+
{% set tracker_base = deferred %}
25+
tracker_base is 'resolved': {{ tracker_base }}? {{ tracker_base == 'resolved' }}
26+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
27+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}

‎src/test/resources/eager/handles-deferred-used-in-multiple-block-levels/middle.jinja‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{% extends '../../eager/handles-deferred-used-in-multiple-block-levels/base.jinja' %}
2+
{% set tracker_middle = '2_middle' %}
3+
{% block first %}
4+
I WON'T SHOW UP
5+
{% endblock %}
6+
7+
{% block second %}
8+
tracker_base is 'resolved': {{ tracker_base }}? {{ tracker_base == 'resolved' }}
9+
tracker_middle is 'resolved': {{ tracker_middle }}? {{ tracker_middle == 'resolved' }}
10+
tracker_test is 'resolved': {{ tracker_test }}? {{ tracker_test == 'resolved' }}
11+
{% endblock %}
12+
Deferring tracker middle.{# This message will not show up in final output #}
13+
{% set tracker_middle = deferred %}

0 commit comments

Comments
 (0)