Skip to content

Commit a3913f0

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Add parsed-only evaluation coverage to Comprehensions Extensions
Includes a fix to preserve first encountered error message for comprehensions PiperOrigin-RevId: 899281542
1 parent 3075687 commit a3913f0

6 files changed

Lines changed: 184 additions & 190 deletions

File tree

‎extensions/src/main/java/dev/cel/extensions/CelListsExtensions.java‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,20 @@ public enum Function {
128128
"list_sort",
129129
"Sorts a list with comparable elements.",
130130
ListType.create(TypeParamType.create("T")),
131-
ListType.create(TypeParamType.create("T"))))),
131+
ListType.create(TypeParamType.create("T")))),
132+
CelFunctionBinding.from("list_sort", Collection.class, CelListsExtensions::sort)),
132133
SORT_BY(
133134
CelFunctionDecl.newFunctionDeclaration(
134135
"lists.@sortByAssociatedKeys",
135136
CelOverloadDecl.newGlobalOverload(
136137
"list_sortByAssociatedKeys",
137138
"Sorts a list by a key value. Used by the 'sortBy' macro",
138139
ListType.create(TypeParamType.create("T")),
139-
ListType.create(TypeParamType.create("T")))));
140+
ListType.create(TypeParamType.create("T")))),
141+
CelFunctionBinding.from(
142+
"list_sortByAssociatedKeys",
143+
Collection.class,
144+
CelListsExtensions::sortByAssociatedKeys));
140145

141146
private final CelFunctionDecl functionDecl;
142147
private final ImmutableSet<CelFunctionBinding> functionBindings;
@@ -147,7 +152,10 @@ String getFunction() {
147152

148153
Function(CelFunctionDecl functionDecl, CelFunctionBinding... functionBindings) {
149154
this.functionDecl = functionDecl;
150-
this.functionBindings = ImmutableSet.copyOf(functionBindings);
155+
this.functionBindings =
156+
functionBindings.length > 0
157+
? CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindings)
158+
: ImmutableSet.of();
151159
}
152160
}
153161

@@ -240,32 +248,13 @@ public void setRuntimeOptions(CelRuntimeBuilder runtimeBuilder) {
240248
@Override
241249
public void setRuntimeOptions(
242250
CelRuntimeBuilder runtimeBuilder, RuntimeEquality runtimeEquality, CelOptions celOptions) {
243-
for (Function function : functions) {
244-
runtimeBuilder.addFunctionBindings(function.functionBindings);
245-
for (CelOverloadDecl overload : function.functionDecl.overloads()) {
246-
switch (overload.overloadId()) {
247-
case "list_distinct":
248-
runtimeBuilder.addFunctionBindings(
249-
CelFunctionBinding.from(
250-
"list_distinct", Collection.class, (list) -> distinct(list, runtimeEquality)));
251-
break;
252-
case "list_sort":
253-
runtimeBuilder.addFunctionBindings(
254-
CelFunctionBinding.from(
255-
"list_sort", Collection.class, (list) -> sort(list, celOptions)));
256-
break;
257-
case "list_sortByAssociatedKeys":
258-
runtimeBuilder.addFunctionBindings(
259-
CelFunctionBinding.from(
260-
"list_sortByAssociatedKeys",
261-
Collection.class,
262-
(list) -> sortByAssociatedKeys(list, celOptions)));
263-
break;
264-
default:
265-
// Nothing to add
266-
}
267-
}
268-
}
251+
functions.forEach(function -> runtimeBuilder.addFunctionBindings(function.functionBindings));
252+
253+
runtimeBuilder.addFunctionBindings(
254+
CelFunctionBinding.fromOverloads(
255+
"distinct",
256+
CelFunctionBinding.from(
257+
"list_distinct", Collection.class, (list) -> distinct(list, runtimeEquality))));
269258
}
270259

271260
private static ImmutableList<Object> slice(Collection<Object> list, long from, long to) {
@@ -369,22 +358,18 @@ private static List<Object> reverse(Collection<Object> list) {
369358
}
370359
}
371360

372-
private static ImmutableList<Object> sort(Collection<Object> objects, CelOptions options) {
373-
return ImmutableList.sortedCopyOf(
374-
new CelObjectComparator(options.enableHeterogeneousNumericComparisons()), objects);
361+
private static ImmutableList<Object> sort(Collection<Object> objects) {
362+
return ImmutableList.sortedCopyOf(new CelObjectComparator(), objects);
375363
}
376364

377365
private static class CelObjectComparator implements Comparator<Object> {
378-
private final boolean enableHeterogeneousNumericComparisons;
379366

380-
CelObjectComparator(boolean enableHeterogeneousNumericComparisons) {
381-
this.enableHeterogeneousNumericComparisons = enableHeterogeneousNumericComparisons;
382-
}
367+
CelObjectComparator() {}
383368

384369
@SuppressWarnings({"unchecked"})
385370
@Override
386371
public int compare(Object o1, Object o2) {
387-
if (o1 instanceof Number && o2 instanceof Number && enableHeterogeneousNumericComparisons) {
372+
if (o1 instanceof Number && o2 instanceof Number) {
388373
return ComparisonFunctions.numericCompare((Number) o1, (Number) o2);
389374
}
390375

@@ -444,12 +429,9 @@ private static Optional<CelExpr> sortByMacro(
444429

445430
@SuppressWarnings({"unchecked", "rawtypes"})
446431
private static ImmutableList<Object> sortByAssociatedKeys(
447-
Collection<List<Object>> keyValuePairs, CelOptions options) {
432+
Collection<List<Object>> keyValuePairs) {
448433
List<Object>[] array = keyValuePairs.toArray(new List[0]);
449-
Arrays.sort(
450-
array,
451-
new CelObjectByKeyComparator(
452-
new CelObjectComparator(options.enableHeterogeneousNumericComparisons())));
434+
Arrays.sort(array, new CelObjectByKeyComparator(new CelObjectComparator()));
453435
ImmutableList.Builder<Object> builder = ImmutableList.builderWithExpectedSize(array.length);
454436
for (List<Object> pair : array) {
455437
builder.add(pair.get(1));

‎extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java‎

Lines changed: 77 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
package dev.cel.extensions;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static com.google.common.truth.Truth.assertWithMessage;
1819
import static org.junit.Assert.assertThrows;
1920

21+
import com.google.common.base.Throwables;
22+
import com.google.common.collect.ImmutableMap;
2023
import com.google.testing.junit.testparameterinjector.TestParameter;
2124
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
2225
import com.google.testing.junit.testparameterinjector.TestParameters;
26+
import dev.cel.bundle.Cel;
2327
import dev.cel.common.CelAbstractSyntaxTree;
2428
import dev.cel.common.CelFunctionDecl;
2529
import dev.cel.common.CelOptions;
@@ -28,15 +32,15 @@
2832
import dev.cel.common.exceptions.CelIndexOutOfBoundsException;
2933
import dev.cel.common.types.SimpleType;
3034
import dev.cel.common.types.TypeParamType;
31-
import dev.cel.compiler.CelCompiler;
32-
import dev.cel.compiler.CelCompilerFactory;
3335
import dev.cel.parser.CelMacro;
3436
import dev.cel.parser.CelStandardMacro;
3537
import dev.cel.parser.CelUnparser;
3638
import dev.cel.parser.CelUnparserFactory;
3739
import dev.cel.runtime.CelEvaluationException;
38-
import dev.cel.runtime.CelRuntime;
39-
import dev.cel.runtime.CelRuntimeFactory;
40+
import dev.cel.testing.CelRuntimeFlavor;
41+
import java.util.Map;
42+
import org.junit.Assume;
43+
import org.junit.Before;
4044
import org.junit.Test;
4145
import org.junit.runner.RunWith;
4246

@@ -46,26 +50,35 @@ public class CelComprehensionsExtensionsTest {
4650

4751
private static final CelOptions CEL_OPTIONS =
4852
CelOptions.current()
53+
.enableHeterogeneousNumericComparisons(true)
4954
// Enable macro call population for unparsing
5055
.populateMacroCalls(true)
5156
.build();
5257

53-
private static final CelCompiler CEL_COMPILER =
54-
CelCompilerFactory.standardCelCompilerBuilder()
55-
.setOptions(CEL_OPTIONS)
56-
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
57-
.addLibraries(CelExtensions.comprehensions())
58-
.addLibraries(CelExtensions.lists())
59-
.addLibraries(CelExtensions.strings())
60-
.addLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings())
61-
.build();
62-
private static final CelRuntime CEL_RUNTIME =
63-
CelRuntimeFactory.standardCelRuntimeBuilder()
64-
.addLibraries(CelOptionalLibrary.INSTANCE)
65-
.addLibraries(CelExtensions.lists())
66-
.addLibraries(CelExtensions.strings())
67-
.addLibraries(CelExtensions.comprehensions())
68-
.build();
58+
@TestParameter public CelRuntimeFlavor runtimeFlavor;
59+
@TestParameter public boolean isParseOnly;
60+
61+
private Cel cel;
62+
63+
@Before
64+
public void setUp() {
65+
// Legacy runtime does not support parsed-only evaluation mode.
66+
Assume.assumeFalse(runtimeFlavor.equals(CelRuntimeFlavor.LEGACY) && isParseOnly);
67+
this.cel =
68+
runtimeFlavor
69+
.builder()
70+
.setOptions(CEL_OPTIONS)
71+
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
72+
.addCompilerLibraries(CelExtensions.comprehensions())
73+
.addCompilerLibraries(CelExtensions.lists())
74+
.addCompilerLibraries(CelExtensions.strings())
75+
.addCompilerLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings())
76+
.addRuntimeLibraries(CelOptionalLibrary.INSTANCE)
77+
.addRuntimeLibraries(CelExtensions.lists())
78+
.addRuntimeLibraries(CelExtensions.strings())
79+
.addRuntimeLibraries(CelExtensions.comprehensions())
80+
.build();
81+
}
6982

7083
private static final CelUnparser UNPARSER = CelUnparserFactory.newUnparser();
7184

@@ -101,11 +114,7 @@ public void allMacro_twoVarComprehension_success(
101114
})
102115
String expr)
103116
throws Exception {
104-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
105-
106-
Object result = CEL_RUNTIME.createProgram(ast).eval();
107-
108-
assertThat(result).isEqualTo(true);
117+
assertThat(eval(expr)).isEqualTo(true);
109118
}
110119

111120
@Test
@@ -127,11 +136,7 @@ public void existsMacro_twoVarComprehension_success(
127136
})
128137
String expr)
129138
throws Exception {
130-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
131-
132-
Object result = CEL_RUNTIME.createProgram(ast).eval();
133-
134-
assertThat(result).isEqualTo(true);
139+
assertThat(eval(expr)).isEqualTo(true);
135140
}
136141

137142
@Test
@@ -156,11 +161,7 @@ public void exists_oneMacro_twoVarComprehension_success(
156161
})
157162
String expr)
158163
throws Exception {
159-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
160-
161-
Object result = CEL_RUNTIME.createProgram(ast).eval();
162-
163-
assertThat(result).isEqualTo(true);
164+
assertThat(eval(expr)).isEqualTo(true);
164165
}
165166

166167
@Test
@@ -182,11 +183,7 @@ public void transformListMacro_twoVarComprehension_success(
182183
})
183184
String expr)
184185
throws Exception {
185-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
186-
187-
Object result = CEL_RUNTIME.createProgram(ast).eval();
188-
189-
assertThat(result).isEqualTo(true);
186+
assertThat(eval(expr)).isEqualTo(true);
190187
}
191188

192189
@Test
@@ -210,11 +207,7 @@ public void transformMapMacro_twoVarComprehension_success(
210207
})
211208
String expr)
212209
throws Exception {
213-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
214-
215-
Object result = CEL_RUNTIME.createProgram(ast).eval();
216-
217-
assertThat(result).isEqualTo(true);
210+
assertThat(eval(expr)).isEqualTo(true);
218211
}
219212

220213
@Test
@@ -238,24 +231,22 @@ public void transformMapEntryMacro_twoVarComprehension_success(
238231
})
239232
String expr)
240233
throws Exception {
241-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
242-
243-
Object result = CEL_RUNTIME.createProgram(ast).eval();
244-
245-
assertThat(result).isEqualTo(true);
234+
assertThat(eval(expr)).isEqualTo(true);
246235
}
247236

248237
@Test
249238
public void comprehension_onTypeParam_success() throws Exception {
250-
CelCompiler celCompiler =
251-
CelCompilerFactory.standardCelCompilerBuilder()
239+
Assume.assumeFalse(isParseOnly);
240+
Cel customCel =
241+
runtimeFlavor
242+
.builder()
252243
.setOptions(CEL_OPTIONS)
253244
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
254-
.addLibraries(CelExtensions.comprehensions())
245+
.addCompilerLibraries(CelExtensions.comprehensions())
255246
.addVar("items", TypeParamType.create("T"))
256247
.build();
257248

258-
CelAbstractSyntaxTree ast = celCompiler.compile("items.all(i, v, v > 0)").getAst();
249+
CelAbstractSyntaxTree ast = customCel.compile("items.all(i, v, v > 0)").getAst();
259250

260251
assertThat(ast.getResultType()).isEqualTo(SimpleType.BOOL);
261252
}
@@ -275,7 +266,7 @@ public void unparseAST_twoVarComprehension(
275266
})
276267
String expr)
277268
throws Exception {
278-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
269+
CelAbstractSyntaxTree ast = isParseOnly ? cel.parse(expr).getAst() : cel.compile(expr).getAst();
279270
String unparsed = UNPARSER.unparse(ast);
280271
assertThat(unparsed).isEqualTo(expr);
281272
}
@@ -318,8 +309,9 @@ public void unparseAST_twoVarComprehension(
318309
"{expr: \"{'hello': 'world', 'greetings': 'tacocat'}.transformMapEntry(k, v, []) == {}\","
319310
+ " err: 'no matching overload'}")
320311
public void twoVarComprehension_compilerErrors(String expr, String err) throws Exception {
312+
Assume.assumeFalse(isParseOnly);
321313
CelValidationException e =
322-
assertThrows(CelValidationException.class, () -> CEL_COMPILER.compile(expr).getAst());
314+
assertThrows(CelValidationException.class, () -> cel.compile(expr).getAst());
323315

324316
assertThat(e).hasMessageThat().contains(err);
325317
}
@@ -339,34 +331,46 @@ public void twoVarComprehension_compilerErrors(String expr, String err) throws E
339331
+ " '2.0' already exists\"}")
340332
public void twoVarComprehension_keyCollision_runtimeError(String expr, String err)
341333
throws Exception {
342-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
343-
344-
CelEvaluationException e =
345-
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());
346-
347-
assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
348-
assertThat(e).hasCauseThat().hasMessageThat().contains(err);
334+
// Planner does not allow decimals for map keys
335+
Assume.assumeFalse(runtimeFlavor.equals(CelRuntimeFlavor.PLANNER) && expr.contains("2.0"));
336+
337+
CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval(expr));
338+
Throwable cause = Throwables.getCausalChain(e).stream()
339+
.filter(IllegalArgumentException.class::isInstance)
340+
.filter(t -> t.getMessage() != null && t.getMessage().contains(err))
341+
.findFirst()
342+
.orElse(null);
343+
344+
assertWithMessage("Expected IllegalArgumentException with message containing '%s' in cause chain", err)
345+
.that(cause)
346+
.isNotNull();
349347
}
350348

351349
@Test
352350
public void twoVarComprehension_arithmeticException_runtimeError() throws Exception {
353-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile("[0].all(i, k, i/k < k)").getAst();
354-
355-
CelEvaluationException e =
356-
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());
357-
351+
CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval("[0].all(i, k, i/k < k)"));
358352
assertThat(e).hasCauseThat().isInstanceOf(CelDivideByZeroException.class);
359353
assertThat(e).hasCauseThat().hasMessageThat().contains("/ by zero");
360354
}
361355

362356
@Test
363357
public void twoVarComprehension_outOfBounds_runtimeError() throws Exception {
364-
CelAbstractSyntaxTree ast = CEL_COMPILER.compile("[1, 2].exists(i, v, [0][v] > 0)").getAst();
365-
366-
CelEvaluationException e =
367-
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());
368-
358+
CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval("[1, 2].exists(i, v, [0][v] > 0)"));
369359
assertThat(e).hasCauseThat().isInstanceOf(CelIndexOutOfBoundsException.class);
370360
assertThat(e).hasCauseThat().hasMessageThat().contains("Index out of bounds: 1");
371361
}
362+
363+
private Object eval(String expression) throws Exception {
364+
return eval(this.cel, expression, ImmutableMap.of());
365+
}
366+
367+
private Object eval(Cel cel, String expression, Map<String, ?> variables) throws Exception {
368+
CelAbstractSyntaxTree ast;
369+
if (isParseOnly) {
370+
ast = cel.parse(expression).getAst();
371+
} else {
372+
ast = cel.compile(expression).getAst();
373+
}
374+
return cel.createProgram(ast).eval(variables);
375+
}
372376
}

0 commit comments

Comments
 (0)