Skip to content

Commit 4891843

Browse files
jasmith-hsclaude
andcommitted
Add iteratorOnlyReverseFilter LegacyOverride and fix Builder.from() bug
The |reverse filter in Jinja returns an iterator, but Jinjava 2 returned a list. The ReverseArrayIterator introduced in 3.0 breaks non-standard usage like {{ (arr|reverse)[0] }}. This override (enabled in THREE_POINT_0) keeps the iterator behavior; when disabled, the result is collected into a list for backwards compatibility. Also fixes Builder.from() which was missing keepNullableLoopValues, causing it to silently reset to false when cloning a LegacyOverrides instance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5450ee8 commit 4891843

3 files changed

Lines changed: 58 additions & 3 deletions

File tree

src/main/java/com/hubspot/jinjava/LegacyOverrides.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class LegacyOverrides {
1818
.withAllowAdjacentTextNodes(true)
1919
.withUseTrimmingForNotesAndExpressions(true)
2020
.withKeepNullableLoopValues(true)
21+
.withIteratorOnlyReverseFilter(true)
2122
.build();
2223
public static final LegacyOverrides ALL = new LegacyOverrides.Builder()
2324
.withEvaluateMapKeys(true)
@@ -29,6 +30,7 @@ public class LegacyOverrides {
2930
.withAllowAdjacentTextNodes(true)
3031
.withUseTrimmingForNotesAndExpressions(true)
3132
.withKeepNullableLoopValues(true)
33+
.withIteratorOnlyReverseFilter(true)
3234
.build();
3335
private final boolean evaluateMapKeys;
3436
private final boolean iterateOverMapKeys;
@@ -39,6 +41,7 @@ public class LegacyOverrides {
3941
private final boolean allowAdjacentTextNodes;
4042
private final boolean useTrimmingForNotesAndExpressions;
4143
private final boolean keepNullableLoopValues;
44+
private final boolean iteratorOnlyReverseFilter;
4245

4346
private LegacyOverrides(Builder builder) {
4447
evaluateMapKeys = builder.evaluateMapKeys;
@@ -50,6 +53,7 @@ private LegacyOverrides(Builder builder) {
5053
allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
5154
useTrimmingForNotesAndExpressions = builder.useTrimmingForNotesAndExpressions;
5255
keepNullableLoopValues = builder.keepNullableLoopValues;
56+
iteratorOnlyReverseFilter = builder.iteratorOnlyReverseFilter;
5357
}
5458

5559
public static Builder newBuilder() {
@@ -92,6 +96,10 @@ public boolean isKeepNullableLoopValues() {
9296
return keepNullableLoopValues;
9397
}
9498

99+
public boolean isIteratorOnlyReverseFilter() {
100+
return iteratorOnlyReverseFilter;
101+
}
102+
95103
public static class Builder {
96104

97105
private boolean evaluateMapKeys = false;
@@ -103,6 +111,7 @@ public static class Builder {
103111
private boolean allowAdjacentTextNodes = false;
104112
private boolean useTrimmingForNotesAndExpressions = false;
105113
private boolean keepNullableLoopValues = false;
114+
private boolean iteratorOnlyReverseFilter = false;
106115

107116
private Builder() {}
108117

@@ -123,7 +132,9 @@ public static Builder from(LegacyOverrides legacyOverrides) {
123132
.withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes)
124133
.withUseTrimmingForNotesAndExpressions(
125134
legacyOverrides.useTrimmingForNotesAndExpressions
126-
);
135+
)
136+
.withKeepNullableLoopValues(legacyOverrides.keepNullableLoopValues)
137+
.withIteratorOnlyReverseFilter(legacyOverrides.iteratorOnlyReverseFilter);
127138
}
128139

129140
public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -176,5 +187,10 @@ public Builder withKeepNullableLoopValues(boolean keepNullableLoopValues) {
176187
this.keepNullableLoopValues = keepNullableLoopValues;
177188
return this;
178189
}
190+
191+
public Builder withIteratorOnlyReverseFilter(boolean iteratorOnlyReverseFilter) {
192+
this.iteratorOnlyReverseFilter = iteratorOnlyReverseFilter;
193+
return this;
194+
}
179195
}
180196
}

src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
2222
import com.hubspot.jinjava.objects.collections.ArrayBacked;
2323
import java.lang.reflect.Array;
24+
import java.util.ArrayList;
2425
import java.util.Collection;
2526
import java.util.Iterator;
27+
import java.util.List;
2628
import java.util.NoSuchElementException;
2729

2830
@JinjavaDoc(
@@ -51,11 +53,14 @@ public Object filter(Object object, JinjavaInterpreter interpreter, String... ar
5153
}
5254
// collection
5355
if (object instanceof Collection) {
54-
return ReverseArrayIterator.create(((Collection<?>) object).toArray());
56+
return maybeCollectToList(
57+
ReverseArrayIterator.create(((Collection<?>) object).toArray()),
58+
interpreter
59+
);
5560
}
5661
// array
5762
if (object.getClass().isArray()) {
58-
return ReverseArrayIterator.create(object);
63+
return maybeCollectToList(ReverseArrayIterator.create(object), interpreter);
5964
}
6065
// string
6166
if (object instanceof String) {
@@ -72,6 +77,20 @@ public Object filter(Object object, JinjavaInterpreter interpreter, String... ar
7277
return object;
7378
}
7479

80+
private Object maybeCollectToList(
81+
ReverseArrayIterator iterator,
82+
JinjavaInterpreter interpreter
83+
) {
84+
if (interpreter.getConfig().getLegacyOverrides().isIteratorOnlyReverseFilter()) {
85+
return iterator;
86+
}
87+
List<Object> result = new ArrayList<>();
88+
while (iterator.hasNext()) {
89+
result.add(iterator.next());
90+
}
91+
return result;
92+
}
93+
7594
@Override
7695
public String getName() {
7796
return "reverse";

src/test/java/com/hubspot/jinjava/lib/filter/ReverseFilterTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import static org.assertj.core.api.Assertions.assertThat;
44

55
import com.hubspot.jinjava.BaseJinjavaTest;
6+
import com.hubspot.jinjava.Jinjava;
7+
import com.hubspot.jinjava.LegacyOverrides;
68
import java.util.HashMap;
79
import java.util.Map;
810
import org.junit.Test;
@@ -28,4 +30,22 @@ public void itReversesObjectArray() {
2830
)
2931
.isEqualTo("cba");
3032
}
33+
34+
@Test
35+
public void itAllowsIndexingWhenLegacyOverrideIsDisabled() {
36+
Jinjava legacyJinjava = new Jinjava(
37+
BaseJinjavaTest
38+
.newConfigBuilder()
39+
.withLegacyOverrides(
40+
LegacyOverrides.Builder
41+
.from(LegacyOverrides.THREE_POINT_0)
42+
.withIteratorOnlyReverseFilter(false)
43+
.build()
44+
)
45+
.build()
46+
);
47+
Map<String, Object> context = new HashMap<>();
48+
context.put("arr", new String[] { "a", "b", "c" });
49+
assertThat(legacyJinjava.render("{{ (arr|reverse)[0] }}", context)).isEqualTo("c");
50+
}
3151
}

0 commit comments

Comments
 (0)