Skip to content

Commit 6be60f4

Browse files
hs-lsonghs-lsong
committed
Chained filters optimization.
1 parent 0b610c9 commit 6be60f4

22 files changed

Lines changed: 553 additions & 68 deletions

File tree

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

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
import com.hubspot.jinjava.interpret.DisabledException;
4+
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
5+
import com.hubspot.jinjava.interpret.TemplateError;
6+
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
7+
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
8+
import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
9+
import com.hubspot.jinjava.lib.filter.Filter;
10+
import com.hubspot.jinjava.objects.SafeString;
11+
import de.odysseus.el.tree.Bindings;
12+
import de.odysseus.el.tree.impl.ast.AstNode;
13+
import de.odysseus.el.tree.impl.ast.AstParameters;
14+
import de.odysseus.el.tree.impl.ast.AstRightValue;
15+
import java.util.ArrayList;
16+
import java.util.LinkedHashMap;
17+
import java.util.List;
18+
import java.util.Map;
19+
import java.util.Objects;
20+
import javax.el.ELContext;
21+
import javax.el.ELException;
22+
23+
/**
24+
* AST node for a chain of filters applied to an input expression.
25+
* Instead of creating nested AstMethod calls for each filter in a chain like:
26+
* filter:length.filter(filter:lower.filter(filter:trim.filter(input)))
27+
*
28+
* This node represents the entire chain as a single evaluation unit:
29+
* input|trim|lower|length
30+
*
31+
* This optimization reduces:
32+
* - Filter lookups (done once per filter instead of per AST node traversal)
33+
* - Method invocation overhead
34+
* - Object wrapping/unwrapping between filters
35+
* - Context operations
36+
*/
37+
public class AstFilterChain extends AstRightValue {
38+
39+
protected final AstNode input;
40+
protected final List<FilterSpec> filterSpecs;
41+
42+
public AstFilterChain(AstNode input, List<FilterSpec> filterSpecs) {
43+
this.input = Objects.requireNonNull(input, "Input node cannot be null");
44+
this.filterSpecs = Objects.requireNonNull(filterSpecs, "Filter specs cannot be null");
45+
if (filterSpecs.isEmpty()) {
46+
throw new IllegalArgumentException("Filter chain must have at least one filter");
47+
}
48+
}
49+
50+
public AstNode getInput() {
51+
return input;
52+
}
53+
54+
public List<FilterSpec> getFilterSpecs() {
55+
return filterSpecs;
56+
}
57+
58+
@Override
59+
public Object eval(Bindings bindings, ELContext context) {
60+
JinjavaInterpreter interpreter = getInterpreter(context);
61+
62+
if (interpreter.getContext().isValidationMode()) {
63+
return "";
64+
}
65+
66+
Object value = input.eval(bindings, context);
67+
68+
for (FilterSpec spec : filterSpecs) {
69+
String filterKey = ExtendedParser.FILTER_PREFIX + spec.getName();
70+
interpreter.getContext().addResolvedValue(filterKey);
71+
72+
Filter filter;
73+
try {
74+
filter = interpreter.getContext().getFilter(spec.getName());
75+
} catch (DisabledException e) {
76+
interpreter.addError(
77+
new TemplateError(
78+
ErrorType.FATAL,
79+
ErrorReason.DISABLED,
80+
ErrorItem.FILTER,
81+
e.getMessage(),
82+
spec.getName(),
83+
interpreter.getLineNumber(),
84+
-1,
85+
e
86+
)
87+
);
88+
return null;
89+
}
90+
if (filter == null) {
91+
continue;
92+
}
93+
94+
Object[] args = evaluateFilterArgs(spec, bindings, context);
95+
Map<String, Object> kwargs = extractNamedParams(args);
96+
Object[] positionalArgs = extractPositionalArgs(args);
97+
98+
boolean wasSafeString = value instanceof SafeString;
99+
if (wasSafeString) {
100+
value = value.toString();
101+
}
102+
103+
try {
104+
value = filter.filter(value, interpreter, positionalArgs, kwargs);
105+
} catch (ELException e) {
106+
throw e;
107+
} catch (RuntimeException e) {
108+
throw new ELException(
109+
String.format("Error in filter '%s': %s", spec.getName(), e.getMessage()),
110+
e
111+
);
112+
}
113+
114+
if (wasSafeString && filter.preserveSafeString() && value instanceof String) {
115+
value = new SafeString((String) value);
116+
}
117+
}
118+
119+
return value;
120+
}
121+
122+
protected JinjavaInterpreter getInterpreter(ELContext context) {
123+
return (JinjavaInterpreter) context
124+
.getELResolver()
125+
.getValue(context, null, ExtendedParser.INTERPRETER);
126+
}
127+
128+
protected Object[] evaluateFilterArgs(
129+
FilterSpec spec,
130+
Bindings bindings,
131+
ELContext context
132+
) {
133+
AstParameters params = spec.getParams();
134+
if (params == null || params.getCardinality() == 0) {
135+
return new Object[0];
136+
}
137+
138+
Object[] args = new Object[params.getCardinality()];
139+
for (int i = 0; i < params.getCardinality(); i++) {
140+
args[i] = params.getChild(i).eval(bindings, context);
141+
}
142+
return args;
143+
}
144+
145+
private Map<String, Object> extractNamedParams(Object[] args) {
146+
Map<String, Object> kwargs = new LinkedHashMap<>();
147+
for (Object arg : args) {
148+
if (arg instanceof NamedParameter) {
149+
NamedParameter namedParam = (NamedParameter) arg;
150+
kwargs.put(namedParam.getName(), namedParam.getValue());
151+
}
152+
}
153+
return kwargs;
154+
}
155+
156+
private Object[] extractPositionalArgs(Object[] args) {
157+
List<Object> positional = new ArrayList<>();
158+
for (Object arg : args) {
159+
if (!(arg instanceof NamedParameter)) {
160+
positional.add(arg);
161+
}
162+
}
163+
return positional.toArray();
164+
}
165+
166+
@Override
167+
public void appendStructure(StringBuilder builder, Bindings bindings) {
168+
input.appendStructure(builder, bindings);
169+
for (FilterSpec spec : filterSpecs) {
170+
builder.append('|').append(spec.getName());
171+
AstParameters params = spec.getParams();
172+
if (params != null && params.getCardinality() > 0) {
173+
builder.append('(');
174+
for (int i = 0; i < params.getCardinality(); i++) {
175+
if (i > 0) {
176+
builder.append(", ");
177+
}
178+
params.getChild(i).appendStructure(builder, bindings);
179+
}
180+
builder.append(')');
181+
}
182+
}
183+
}
184+
185+
@Override
186+
public String toString() {
187+
StringBuilder sb = new StringBuilder();
188+
sb.append(input.toString());
189+
for (FilterSpec spec : filterSpecs) {
190+
sb.append('|').append(spec.toString());
191+
}
192+
return sb.toString();
193+
}
194+
195+
@Override
196+
public int getCardinality() {
197+
return 1 + filterSpecs.size();
198+
}
199+
200+
@Override
201+
public AstNode getChild(int i) {
202+
if (i == 0) {
203+
return input;
204+
}
205+
int filterIndex = i - 1;
206+
if (filterIndex < filterSpecs.size()) {
207+
FilterSpec spec = filterSpecs.get(filterIndex);
208+
return spec.getParams();
209+
}
210+
return null;
211+
}
212+
}

‎src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -531,30 +531,22 @@ protected AstNode value() throws ScanException, ParseException {
531531

532532
private AstNode parseOperators(AstNode left) throws ScanException, ParseException {
533533
if ("|".equals(getToken().getImage()) && lookahead(0).getSymbol() == IDENTIFIER) {
534-
AstNode v = left;
534+
List<FilterSpec> filterSpecs = new ArrayList<>();
535535

536536
do {
537537
consumeToken(); // '|'
538538
String filterName = consumeToken().getImage();
539-
List<AstNode> filterParams = Lists.newArrayList(v, interpreter());
539+
AstParameters filterParams = null;
540540

541541
// optional filter args
542542
if (getToken().getSymbol() == Symbol.LPAREN) {
543-
AstParameters astParameters = params();
544-
for (int i = 0; i < astParameters.getCardinality(); i++) {
545-
filterParams.add(astParameters.getChild(i));
546-
}
543+
filterParams = params();
547544
}
548545

549-
AstProperty filterProperty = createAstDot(
550-
identifier(FILTER_PREFIX + filterName),
551-
"filter",
552-
true
553-
);
554-
v = createAstMethod(filterProperty, createAstParameters(filterParams)); // function("filter:" + filterName, new AstParameters(filterParams));
546+
filterSpecs.add(new FilterSpec(filterName, filterParams));
555547
} while ("|".equals(getToken().getImage()));
556548

557-
return v;
549+
return createAstFilterChain(left, filterSpecs);
558550
} else if (
559551
"is".equals(getToken().getImage()) &&
560552
"not".equals(lookahead(0).getImage()) &&
@@ -577,6 +569,13 @@ protected AstParameters createAstParameters(List<AstNode> nodes) {
577569
return new AstParameters(nodes);
578570
}
579571

572+
protected AstFilterChain createAstFilterChain(
573+
AstNode input,
574+
List<FilterSpec> filterSpecs
575+
) {
576+
return new AstFilterChain(input, filterSpecs);
577+
}
578+
580579
private boolean isPossibleExpTest(Symbol symbol) {
581580
return VALID_SYMBOLS_FOR_EXP_TEST.contains(symbol);
582581
}

‎src/main/java/com/hubspot/jinjava/el/ext/FilterSpec.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
import de.odysseus.el.tree.impl.ast.AstParameters;
4+
import java.util.Objects;
5+
6+
/**
7+
* Specification for a filter in a filter chain.
8+
* Holds the filter name and optional parameters.
9+
*/
10+
public class FilterSpec {
11+
12+
private final String name;
13+
private final AstParameters params;
14+
15+
public FilterSpec(String name, AstParameters params) {
16+
this.name = Objects.requireNonNull(name, "Filter name cannot be null");
17+
this.params = params;
18+
}
19+
20+
public String getName() {
21+
return name;
22+
}
23+
24+
public AstParameters getParams() {
25+
return params;
26+
}
27+
28+
public boolean hasParams() {
29+
return params != null && params.getCardinality() > 0;
30+
}
31+
32+
@Override
33+
public String toString() {
34+
if (hasParams()) {
35+
StringBuilder sb = new StringBuilder(name);
36+
sb.append('(');
37+
for (int i = 0; i < params.getCardinality(); i++) {
38+
if (i > 0) {
39+
sb.append(", ");
40+
}
41+
sb.append(params.getChild(i));
42+
}
43+
sb.append(')');
44+
return sb.toString();
45+
}
46+
return name;
47+
}
48+
}

0 commit comments

Comments
 (0)