Skip to content

Commit 42825ee

Browse files
hs-lsongclaude
andcommitted
Fix disabled filter handling: set value to null and continue chain
The previous fix used `continue` alone, but the non-chained path propagates null through subsequent filters rather than skipping. Setting value = null before continuing matches that behavior exactly. Adds parity test confirming optimized and non-chained paths produce identical output for disabled filters in a chain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b9e58ca commit 42825ee

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

src/main/java/com/hubspot/jinjava/el/ext/AstFilterChain.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public Object eval(Bindings bindings, ELContext context) {
8585
e
8686
)
8787
);
88+
value = null;
8889
continue;
8990
}
9091
if (filter == null) {

src/test/java/com/hubspot/jinjava/el/ext/AstFilterChainTest.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@
22

33
import static org.assertj.core.api.Assertions.assertThat;
44

5+
import com.google.common.collect.ImmutableMap;
6+
import com.google.common.collect.ImmutableSet;
57
import com.hubspot.jinjava.Jinjava;
68
import com.hubspot.jinjava.JinjavaConfig;
9+
import com.hubspot.jinjava.interpret.Context;
710
import com.hubspot.jinjava.interpret.RenderResult;
11+
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
12+
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
813
import com.hubspot.jinjava.objects.date.PyishDate;
914
import java.time.ZonedDateTime;
1015
import java.util.HashMap;
1116
import java.util.Map;
17+
import java.util.Set;
1218
import org.junit.Before;
1319
import org.junit.Test;
1420

@@ -106,4 +112,60 @@ public void itFallsBackToUnoptimizedForUnknownFilterParity() {
106112
.as("Optimized should match un-optimized for unknown filter in chain")
107113
.isEqualTo(unoptimizedResult.getOutput());
108114
}
115+
116+
@Test
117+
public void itSkipsDisabledFilterAndContinuesChain() {
118+
Map<Context.Library, Set<String>> disabled = ImmutableMap.of(
119+
Context.Library.FILTER,
120+
ImmutableSet.of("lower")
121+
);
122+
Jinjava jinjavaWithDisabled = new Jinjava(
123+
JinjavaConfig
124+
.newBuilder()
125+
.withEnableFilterChainOptimization(true)
126+
.withDisabled(disabled)
127+
.build()
128+
);
129+
130+
RenderResult result = jinjavaWithDisabled.renderForResult(
131+
"{{ name|trim|lower|capitalize }}",
132+
context
133+
);
134+
135+
assertThat(result.getErrors()).isNotEmpty();
136+
assertThat(result.getErrors().get(0).getItem()).isEqualTo(ErrorItem.FILTER);
137+
assertThat(result.getErrors().get(0).getReason()).isEqualTo(ErrorReason.DISABLED);
138+
assertThat(result.getErrors().get(0).getMessage()).contains("lower");
139+
}
140+
141+
@Test
142+
public void itMatchesNonChainedBehaviorForDisabledFilter() {
143+
Map<Context.Library, Set<String>> disabled = ImmutableMap.of(
144+
Context.Library.FILTER,
145+
ImmutableSet.of("lower")
146+
);
147+
String template = "{{ name|trim|lower|capitalize }}";
148+
149+
Jinjava optimized = new Jinjava(
150+
JinjavaConfig
151+
.newBuilder()
152+
.withEnableFilterChainOptimization(true)
153+
.withDisabled(disabled)
154+
.build()
155+
);
156+
Jinjava unoptimized = new Jinjava(
157+
JinjavaConfig
158+
.newBuilder()
159+
.withEnableFilterChainOptimization(false)
160+
.withDisabled(disabled)
161+
.build()
162+
);
163+
164+
RenderResult optimizedResult = optimized.renderForResult(template, context);
165+
RenderResult unoptimizedResult = unoptimized.renderForResult(template, context);
166+
167+
assertThat(optimizedResult.getOutput())
168+
.as("Optimized should match un-optimized for disabled filter in chain")
169+
.isEqualTo(unoptimizedResult.getOutput());
170+
}
109171
}

0 commit comments

Comments
 (0)