Skip to content

Add parsed-only evaluation coverage to Comprehensions Extensions #1018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add parsed-only evaluation coverage to Comprehensions Extensions #1018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 24 additions & 42 deletions extensions/src/main/java/dev/cel/extensions/CelListsExtensions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,20 @@ public enum Function {
"list_sort",
"Sorts a list with comparable elements.",
ListType.create(TypeParamType.create("T")),
ListType.create(TypeParamType.create("T"))))),
ListType.create(TypeParamType.create("T")))),
CelFunctionBinding.from("list_sort", Collection.class, CelListsExtensions::sort)),
SORT_BY(
CelFunctionDecl.newFunctionDeclaration(
"lists.@sortByAssociatedKeys",
CelOverloadDecl.newGlobalOverload(
"list_sortByAssociatedKeys",
"Sorts a list by a key value. Used by the 'sortBy' macro",
ListType.create(TypeParamType.create("T")),
ListType.create(TypeParamType.create("T")))));
ListType.create(TypeParamType.create("T")))),
CelFunctionBinding.from(
"list_sortByAssociatedKeys",
Collection.class,
CelListsExtensions::sortByAssociatedKeys));

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

Function(CelFunctionDecl functionDecl, CelFunctionBinding... functionBindings) {
this.functionDecl = functionDecl;
this.functionBindings = ImmutableSet.copyOf(functionBindings);
this.functionBindings =
functionBindings.length > 0
? CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindings)
: ImmutableSet.of();
}
}

Expand Down Expand Up @@ -240,32 +248,13 @@ public void setRuntimeOptions(CelRuntimeBuilder runtimeBuilder) {
@Override
public void setRuntimeOptions(
CelRuntimeBuilder runtimeBuilder, RuntimeEquality runtimeEquality, CelOptions celOptions) {
for (Function function : functions) {
runtimeBuilder.addFunctionBindings(function.functionBindings);
for (CelOverloadDecl overload : function.functionDecl.overloads()) {
switch (overload.overloadId()) {
case "list_distinct":
runtimeBuilder.addFunctionBindings(
CelFunctionBinding.from(
"list_distinct", Collection.class, (list) -> distinct(list, runtimeEquality)));
break;
case "list_sort":
runtimeBuilder.addFunctionBindings(
CelFunctionBinding.from(
"list_sort", Collection.class, (list) -> sort(list, celOptions)));
break;
case "list_sortByAssociatedKeys":
runtimeBuilder.addFunctionBindings(
CelFunctionBinding.from(
"list_sortByAssociatedKeys",
Collection.class,
(list) -> sortByAssociatedKeys(list, celOptions)));
break;
default:
// Nothing to add
}
}
}
functions.forEach(function -> runtimeBuilder.addFunctionBindings(function.functionBindings));

runtimeBuilder.addFunctionBindings(
CelFunctionBinding.fromOverloads(
"distinct",
CelFunctionBinding.from(
"list_distinct", Collection.class, (list) -> distinct(list, runtimeEquality))));
}

private static ImmutableList<Object> slice(Collection<Object> list, long from, long to) {
Expand Down Expand Up @@ -369,22 +358,18 @@ private static List<Object> reverse(Collection<Object> list) {
}
}

private static ImmutableList<Object> sort(Collection<Object> objects, CelOptions options) {
return ImmutableList.sortedCopyOf(
new CelObjectComparator(options.enableHeterogeneousNumericComparisons()), objects);
private static ImmutableList<Object> sort(Collection<Object> objects) {
return ImmutableList.sortedCopyOf(new CelObjectComparator(), objects);
}

private static class CelObjectComparator implements Comparator<Object> {
private final boolean enableHeterogeneousNumericComparisons;

CelObjectComparator(boolean enableHeterogeneousNumericComparisons) {
this.enableHeterogeneousNumericComparisons = enableHeterogeneousNumericComparisons;
}
CelObjectComparator() {}

@SuppressWarnings({"unchecked"})
@Override
public int compare(Object o1, Object o2) {
if (o1 instanceof Number && o2 instanceof Number && enableHeterogeneousNumericComparisons) {
if (o1 instanceof Number && o2 instanceof Number) {
return ComparisonFunctions.numericCompare((Number) o1, (Number) o2);
}

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

@SuppressWarnings({"unchecked", "rawtypes"})
private static ImmutableList<Object> sortByAssociatedKeys(
Collection<List<Object>> keyValuePairs, CelOptions options) {
Collection<List<Object>> keyValuePairs) {
List<Object>[] array = keyValuePairs.toArray(new List[0]);
Arrays.sort(
array,
new CelObjectByKeyComparator(
new CelObjectComparator(options.enableHeterogeneousNumericComparisons())));
Arrays.sort(array, new CelObjectByKeyComparator(new CelObjectComparator()));
ImmutableList.Builder<Object> builder = ImmutableList.builderWithExpectedSize(array.length);
for (List<Object> pair : array) {
builder.add(pair.get(1));
Expand Down
150 changes: 77 additions & 73 deletions extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
package dev.cel.extensions;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertThrows;

import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.google.testing.junit.testparameterinjector.TestParameter;
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import com.google.testing.junit.testparameterinjector.TestParameters;
import dev.cel.bundle.Cel;
import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.CelFunctionDecl;
import dev.cel.common.CelOptions;
Expand All @@ -28,15 +32,15 @@
import dev.cel.common.exceptions.CelIndexOutOfBoundsException;
import dev.cel.common.types.SimpleType;
import dev.cel.common.types.TypeParamType;
import dev.cel.compiler.CelCompiler;
import dev.cel.compiler.CelCompilerFactory;
import dev.cel.parser.CelMacro;
import dev.cel.parser.CelStandardMacro;
import dev.cel.parser.CelUnparser;
import dev.cel.parser.CelUnparserFactory;
import dev.cel.runtime.CelEvaluationException;
import dev.cel.runtime.CelRuntime;
import dev.cel.runtime.CelRuntimeFactory;
import dev.cel.testing.CelRuntimeFlavor;
import java.util.Map;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;

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

private static final CelOptions CEL_OPTIONS =
CelOptions.current()
.enableHeterogeneousNumericComparisons(true)
// Enable macro call population for unparsing
.populateMacroCalls(true)
.build();

private static final CelCompiler CEL_COMPILER =
CelCompilerFactory.standardCelCompilerBuilder()
.setOptions(CEL_OPTIONS)
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
.addLibraries(CelExtensions.comprehensions())
.addLibraries(CelExtensions.lists())
.addLibraries(CelExtensions.strings())
.addLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings())
.build();
private static final CelRuntime CEL_RUNTIME =
CelRuntimeFactory.standardCelRuntimeBuilder()
.addLibraries(CelOptionalLibrary.INSTANCE)
.addLibraries(CelExtensions.lists())
.addLibraries(CelExtensions.strings())
.addLibraries(CelExtensions.comprehensions())
.build();
@TestParameter public CelRuntimeFlavor runtimeFlavor;
@TestParameter public boolean isParseOnly;

private Cel cel;

@Before
public void setUp() {
// Legacy runtime does not support parsed-only evaluation mode.
Assume.assumeFalse(runtimeFlavor.equals(CelRuntimeFlavor.LEGACY) && isParseOnly);
this.cel =
runtimeFlavor
.builder()
.setOptions(CEL_OPTIONS)
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
.addCompilerLibraries(CelExtensions.comprehensions())
.addCompilerLibraries(CelExtensions.lists())
.addCompilerLibraries(CelExtensions.strings())
.addCompilerLibraries(CelOptionalLibrary.INSTANCE, CelExtensions.bindings())
.addRuntimeLibraries(CelOptionalLibrary.INSTANCE)
.addRuntimeLibraries(CelExtensions.lists())
.addRuntimeLibraries(CelExtensions.strings())
.addRuntimeLibraries(CelExtensions.comprehensions())
.build();
}

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

Expand Down Expand Up @@ -101,11 +114,7 @@ public void allMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
Expand All @@ -127,11 +136,7 @@ public void existsMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
Expand All @@ -156,11 +161,7 @@ public void exists_oneMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
Expand All @@ -182,11 +183,7 @@ public void transformListMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
Expand All @@ -210,11 +207,7 @@ public void transformMapMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
Expand All @@ -238,24 +231,22 @@ public void transformMapEntryMacro_twoVarComprehension_success(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

Object result = CEL_RUNTIME.createProgram(ast).eval();

assertThat(result).isEqualTo(true);
assertThat(eval(expr)).isEqualTo(true);
}

@Test
public void comprehension_onTypeParam_success() throws Exception {
CelCompiler celCompiler =
CelCompilerFactory.standardCelCompilerBuilder()
Assume.assumeFalse(isParseOnly);
Cel customCel =
runtimeFlavor
.builder()
.setOptions(CEL_OPTIONS)
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
.addLibraries(CelExtensions.comprehensions())
.addCompilerLibraries(CelExtensions.comprehensions())
.addVar("items", TypeParamType.create("T"))
.build();

CelAbstractSyntaxTree ast = celCompiler.compile("items.all(i, v, v > 0)").getAst();
CelAbstractSyntaxTree ast = customCel.compile("items.all(i, v, v > 0)").getAst();

assertThat(ast.getResultType()).isEqualTo(SimpleType.BOOL);
}
Expand All @@ -275,7 +266,7 @@ public void unparseAST_twoVarComprehension(
})
String expr)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();
CelAbstractSyntaxTree ast = isParseOnly ? cel.parse(expr).getAst() : cel.compile(expr).getAst();
String unparsed = UNPARSER.unparse(ast);
assertThat(unparsed).isEqualTo(expr);
}
Expand Down Expand Up @@ -318,8 +309,9 @@ public void unparseAST_twoVarComprehension(
"{expr: \"{'hello': 'world', 'greetings': 'tacocat'}.transformMapEntry(k, v, []) == {}\","
+ " err: 'no matching overload'}")
public void twoVarComprehension_compilerErrors(String expr, String err) throws Exception {
Assume.assumeFalse(isParseOnly);
CelValidationException e =
assertThrows(CelValidationException.class, () -> CEL_COMPILER.compile(expr).getAst());
assertThrows(CelValidationException.class, () -> cel.compile(expr).getAst());

assertThat(e).hasMessageThat().contains(err);
}
Expand All @@ -339,34 +331,46 @@ public void twoVarComprehension_compilerErrors(String expr, String err) throws E
+ " '2.0' already exists\"}")
public void twoVarComprehension_keyCollision_runtimeError(String expr, String err)
throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile(expr).getAst();

CelEvaluationException e =
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());

assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
assertThat(e).hasCauseThat().hasMessageThat().contains(err);
// Planner does not allow decimals for map keys
Assume.assumeFalse(runtimeFlavor.equals(CelRuntimeFlavor.PLANNER) && expr.contains("2.0"));

CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval(expr));
Throwable cause = Throwables.getCausalChain(e).stream()
.filter(IllegalArgumentException.class::isInstance)
.filter(t -> t.getMessage() != null && t.getMessage().contains(err))
.findFirst()
.orElse(null);

assertWithMessage("Expected IllegalArgumentException with message containing '%s' in cause chain", err)
.that(cause)
.isNotNull();
}

@Test
public void twoVarComprehension_arithmeticException_runtimeError() throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile("[0].all(i, k, i/k < k)").getAst();

CelEvaluationException e =
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());

CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval("[0].all(i, k, i/k < k)"));
assertThat(e).hasCauseThat().isInstanceOf(CelDivideByZeroException.class);
assertThat(e).hasCauseThat().hasMessageThat().contains("/ by zero");
}

@Test
public void twoVarComprehension_outOfBounds_runtimeError() throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile("[1, 2].exists(i, v, [0][v] > 0)").getAst();

CelEvaluationException e =
assertThrows(CelEvaluationException.class, () -> CEL_RUNTIME.createProgram(ast).eval());

CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval("[1, 2].exists(i, v, [0][v] > 0)"));
assertThat(e).hasCauseThat().isInstanceOf(CelIndexOutOfBoundsException.class);
assertThat(e).hasCauseThat().hasMessageThat().contains("Index out of bounds: 1");
}

private Object eval(String expression) throws Exception {
return eval(this.cel, expression, ImmutableMap.of());
}

private Object eval(Cel cel, String expression, Map<String, ?> variables) throws Exception {
CelAbstractSyntaxTree ast;
if (isParseOnly) {
ast = cel.parse(expression).getAst();
} else {
ast = cel.compile(expression).getAst();
}
return cel.createProgram(ast).eval(variables);
}
}
Loading
Loading