Skip to content

Commit

Permalink
Bump cel-spec + googleapi submodules (#476)
Browse files Browse the repository at this point in the history
Build: maintenance + convenience changes:
* add `compileAll` task
* add IntelliJ-sync-task
* bump jandex version to make it work with Java 21
* CI: Bump bazel version from 3.5.1 to 6.4.0

Bump cel-spec + googleapi submodules:

Bump cel-spec submodule to [v0.13.0](https://github.com/google/cel-spec/releases/tag/v0.13.0), googleapi submodule to
[f2d78630d2c1d5e20041dfff963e093de9298e4d](googleapis/googleapis@f2d7863).

This was necessary due to recent build errors caused by missing (old) artifacts for the conformance bazel build.

The submodule bumps bring a bunch of new conformance tests as well, which unveiled that some functionality that was not present in CEL-Go when CEL-Java was created, is required by the CEl-Spec.

Summary of changes:
* Bugfix in `o.p.c.checker.Standard` that fixes a type resolution error for equals and not-equals functions, when both parameters need to be up-casted.
* Changes in many types (in `o.p.c.common.types.*T` classes) to automatically convert between numeric and null types.
  * equal and compare with a null and a non-null type no longer fail, but return `False`
  * equal and compare between different numeric types no longer fail, but return "the right" result
  * this includes that numeric CEL map keys can be heterogenous, e.g. an `int` can be retrieved using an `uint` or `double` key
* Fix retrieval of milliseconds from `Duration` - must only return the milliseconds within the second
* Un-ignore a bunch of conformance tests that pass fine now

A few conformance tests have been added to `InterpreterTest`, but the majority of tests is left in the conformance tests.
  • Loading branch information
snazy authored Nov 18, 2023
1 parent d219b98 commit d9c5a6d
Show file tree
Hide file tree
Showing 42 changed files with 903 additions and 237 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ jobs:
if: ${{ matrix.java-version == '11' }}
uses: jwlawson/actions-setup-bazel@v1.10.1
with:
bazel-version: '3.5.1'
bazel-version: '6.4.0'

- name: Conformance tests
if: ${{ matrix.java-version == '11' }}
Expand Down
18 changes: 18 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,21 @@ publishingHelper {
nessieRepoName.set("cel-java")
inceptionYear.set("2021")
}

idea.project.settings {
taskTriggers {
afterSync(
":cel-generated-pb:jar",
":cel-generated-pb:testJar",
":cel-generated-antlr:shadowJar"
)
}
}

subprojects.forEach {
it.tasks.register("compileAll").configure {
group = "build"
description = "Runs all compilation and jar tasks"
dependsOn(tasks.withType<AbstractCompile>(), tasks.withType<ProcessResources>())
}
}
7 changes: 5 additions & 2 deletions conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ The CEL-spec conformance test suite is written in Go and uses the bazel build to

Required tools:
* [Bazel build tool](https://bazel.build/)

On Ubuntu run `apt-get install bazel-bootstrap`

See [Bazel web site](https://bazel.build/install) for installation instructions.
Do **not** use the `bazel-bootstrap` package on Ubuntu, because it comes with
pre-compiled classes that are built with Java 17 or newer, preventing it from
running bazel using older Java versions.
* gcc

On Ubuntu run `apt-get install gcc`
Expand Down
9 changes: 8 additions & 1 deletion conformance/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ plugins {

apply<ProtobufPlugin>()

sourceSets.main { java.srcDir(layout.buildDirectory.dir("generated/source/proto/main/java")) }
sourceSets.main {
java.srcDir(layout.buildDirectory.dir("generated/source/proto/main/java"))
java.srcDir(layout.buildDirectory.dir("generated/source/proto/main/grpc"))
}

dependencies {
implementation(project(":cel-core"))
Expand Down Expand Up @@ -58,6 +61,10 @@ configure<ProtobufExtension> {
// Download from repositories
artifact = "com.google.protobuf:protoc:${libs.versions.protobuf.get()}"
}
plugins {
this.create("grpc") { artifact = "io.grpc:protoc-gen-grpc-java:${libs.versions.grpc.get()}" }
}
generateProtoTasks { all().configureEach { this.plugins.create("grpc") {} } }
}

// The protobuf-plugin should ideally do this
Expand Down
24 changes: 15 additions & 9 deletions conformance/run-conformance-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,33 @@ cel_java_skips=(
# nested protobuf-object-structure, which gets rejected during gRPC/protobuf request
# deserialization. Just skip those tests.
"--skip_test=parse/nest/message_literal"
"--skip_test=parse/repeat/index"
# Proto equality specialties don't seem to be in effect for Java
"--skip_test=comparisons/eq_wrapper/eq_proto_nan_equal"
"--skip_test=comparisons/ne_literal/ne_proto_nan_not_equal"

# TODO Actual known issux to fix, a protobuf Any returned via this test is wrapped twice (Any in Any).
# TODO Actual known issue to fix, a protobuf Any returned via this test is wrapped twice (Any in Any).
"--skip_test=dynamic/any/var"
)

cel_go_skips=(
"--skip_test=comparisons/eq_literal/not_eq_list_false_vs_types,not_eq_map_false_vs_types"
"--skip_test=comparisons/in_map_literal/key_in_mixed_key_type_map_error"
"--skip_test=comparisons/eq_literal/not_eq_list_false_vs_types,not_eq_map_false_vs_types"
"--skip_test=comparisons/in_map_literal/key_in_mixed_key_type_map_error"
"--skip_test=dynamic/int32/field_assign_proto2_range,field_assign_proto3_range"
"--skip_test=dynamic/uint32/field_assign_proto2_range,field_assign_proto3_range"
"--skip_test=dynamic/float/field_assign_proto2_range,field_assign_proto3_range"
"--skip_test=dynamic/value_null/literal_unset,field_read_proto2_unset,field_read_proto3_unset"
"--skip_test=enums/legacy_proto2/assign_standalone_int_too_big,assign_standalone_int_too_neg"
"--skip_test=enums/legacy_proto3/assign_standalone_int_too_big,assign_standalone_int_too_neg"
"--skip_test=enums/strong_proto2"
"--skip_test=enums/strong_proto3"
"--skip_test=fields/qualified_identifier_resolution/map_key_float,map_key_null,map_value_repeat_key"
# This conformance test is invalid nowadays
"--skip_test=fields/qualified_identifier_resolution/map_key_float"
# Unclear why the 'to_json_string' is expected to return a string, unlike the preceding to_json_number test.
"--skip_test=wrappers/uint64/to_json_string"
# TODO implement proper "toJson" at some point
"--skip_test=wrappers/field_mask/to_json"
"--skip_test=wrappers/timestamp/to_json"
"--skip_test=wrappers/empty/to_json"
)

test_files=(
"tests/simple/testdata/plumbing.textproto"
"tests/simple/testdata/basic.textproto"
"tests/simple/testdata/comparisons.textproto"
"tests/simple/testdata/conversions.textproto"
Expand All @@ -83,11 +86,14 @@ test_files=(
"tests/simple/testdata/macros.textproto"
"tests/simple/testdata/namespace.textproto"
"tests/simple/testdata/parse.textproto"
"tests/simple/testdata/plumbing.textproto"
"tests/simple/testdata/proto2.textproto"
"tests/simple/testdata/proto3.textproto"
"tests/simple/testdata/string.textproto"
# TODO add when implemnting the string-extensions "tests/simple/testdata/string_ext.textproto"
"tests/simple/testdata/timestamps.textproto"
"tests/simple/testdata/unknowns.textproto"
"tests/simple/testdata/wrappers.textproto"
)

bazel-bin/tests/simple/simple_test_/simple_test \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,20 @@
import static org.projectnessie.cel.common.types.UnknownT.isUnknown;
import static org.projectnessie.cel.common.types.UnknownT.unknownOf;

import com.google.api.expr.v1alpha1.CheckRequest;
import com.google.api.expr.v1alpha1.CheckResponse;
import com.google.api.expr.v1alpha1.ConformanceServiceGrpc.ConformanceServiceImplBase;
import com.google.api.expr.conformance.v1alpha1.CheckRequest;
import com.google.api.expr.conformance.v1alpha1.CheckResponse;
import com.google.api.expr.conformance.v1alpha1.ConformanceServiceGrpc.ConformanceServiceImplBase;
import com.google.api.expr.conformance.v1alpha1.EvalRequest;
import com.google.api.expr.conformance.v1alpha1.EvalResponse;
import com.google.api.expr.conformance.v1alpha1.IssueDetails;
import com.google.api.expr.conformance.v1alpha1.ParseRequest;
import com.google.api.expr.conformance.v1alpha1.ParseResponse;
import com.google.api.expr.conformance.v1alpha1.SourcePosition;
import com.google.api.expr.v1alpha1.ErrorSet;
import com.google.api.expr.v1alpha1.EvalRequest;
import com.google.api.expr.v1alpha1.EvalResponse;
import com.google.api.expr.v1alpha1.ExprValue;
import com.google.api.expr.v1alpha1.IssueDetails;
import com.google.api.expr.v1alpha1.ListValue;
import com.google.api.expr.v1alpha1.MapValue;
import com.google.api.expr.v1alpha1.MapValue.Entry;
import com.google.api.expr.v1alpha1.ParseRequest;
import com.google.api.expr.v1alpha1.ParseResponse;
import com.google.api.expr.v1alpha1.SourcePosition;
import com.google.api.expr.v1alpha1.UnknownSet;
import com.google.api.expr.v1alpha1.Value;
import com.google.protobuf.Any;
Expand Down
1 change: 1 addition & 0 deletions conformance/src/main/proto/google/api/expr/conformance
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@
import static org.projectnessie.cel.TestExpr.ExprLiteral;
import static org.projectnessie.cel.Util.mapOf;

import com.google.api.expr.v1alpha1.CheckRequest;
import com.google.api.expr.v1alpha1.CheckResponse;
import com.google.api.expr.conformance.v1alpha1.CheckRequest;
import com.google.api.expr.conformance.v1alpha1.CheckResponse;
import com.google.api.expr.conformance.v1alpha1.ConformanceServiceGrpc;
import com.google.api.expr.conformance.v1alpha1.ConformanceServiceGrpc.ConformanceServiceBlockingStub;
import com.google.api.expr.conformance.v1alpha1.EvalRequest;
import com.google.api.expr.conformance.v1alpha1.EvalResponse;
import com.google.api.expr.conformance.v1alpha1.ParseRequest;
import com.google.api.expr.conformance.v1alpha1.ParseResponse;
import com.google.api.expr.v1alpha1.CheckedExpr;
import com.google.api.expr.v1alpha1.ConformanceServiceGrpc;
import com.google.api.expr.v1alpha1.ConformanceServiceGrpc.ConformanceServiceBlockingStub;
import com.google.api.expr.v1alpha1.Constant;
import com.google.api.expr.v1alpha1.Constant.ConstantKindCase;
import com.google.api.expr.v1alpha1.EvalRequest;
import com.google.api.expr.v1alpha1.EvalResponse;
import com.google.api.expr.v1alpha1.Expr;
import com.google.api.expr.v1alpha1.Expr.Call;
import com.google.api.expr.v1alpha1.Expr.ExprKindCase;
import com.google.api.expr.v1alpha1.ExprValue;
import com.google.api.expr.v1alpha1.ExprValue.KindCase;
import com.google.api.expr.v1alpha1.ParseRequest;
import com.google.api.expr.v1alpha1.ParseResponse;
import com.google.api.expr.v1alpha1.ParsedExpr;
import com.google.api.expr.v1alpha1.SourceInfo;
import com.google.api.expr.v1alpha1.Type;
Expand Down
2 changes: 2 additions & 0 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ dependencies {

testImplementation(platform(libs.junit.bom))
testImplementation(libs.bundles.junit.testing)
testImplementation(libs.protobuf.java)
testImplementation(libs.guava)
testRuntimeOnly(libs.junit.jupiter.engine)

jmhImplementation(libs.jmh.core)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ Overloads.GreaterEqualsBytes, asList(Decls.Bytes, Decls.Bytes), Decls.Bool),
Decls.newFunction(
Operator.Equals.id,
Decls.newParameterizedOverload(
Overloads.Equals, asList(paramA, paramA), Decls.Bool, typeParamAList)));
Overloads.Equals, asList(paramA, paramB), Decls.Bool, typeParamABList)));

idents.add(
Decls.newFunction(
Operator.NotEquals.id,
Decls.newParameterizedOverload(
Overloads.NotEquals, asList(paramA, paramA), Decls.Bool, typeParamAList)));
Overloads.NotEquals, asList(paramA, paramB), Decls.Bool, typeParamABList)));

// Algebra.

Expand Down
10 changes: 7 additions & 3 deletions core/src/main/java/org/projectnessie/cel/common/types/BoolT.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,14 @@ public Val convertToType(Type typeVal) {
/** Equal implements the ref.Val interface method. */
@Override
public Val equal(Val other) {
if (!(other instanceof BoolT)) {
return noSuchOverload(this, "equal", other);
switch (other.type().typeEnum()) {
case Bool:
return Types.boolOf(b == ((BoolT) other).b);
case Null:
return False;
default:
return noSuchOverload(this, "equal", other);
}
return Types.boolOf(b == ((BoolT) other).b);
}

/** Negate implements the traits.Negater interface method. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.projectnessie.cel.common.types;

import static org.projectnessie.cel.common.types.BoolT.False;
import static org.projectnessie.cel.common.types.Err.newErr;
import static org.projectnessie.cel.common.types.Err.newTypeConversionError;
import static org.projectnessie.cel.common.types.Err.noSuchOverload;
Expand Down Expand Up @@ -162,10 +163,14 @@ public Val convertToType(Type typeValue) {
/** Equal implements the ref.Val interface method. */
@Override
public Val equal(Val other) {
if (!(other instanceof BytesT)) {
return noSuchOverload(this, "equal", other);
switch (other.type().typeEnum()) {
case Bytes:
return boolOf(Arrays.equals(b, ((BytesT) other).b));
case Null:
return False;
default:
return noSuchOverload(this, "equal", other);
}
return boolOf(Arrays.equals(b, ((BytesT) other).b));
}

/** Size implements the traits.Sizer interface method. */
Expand Down
Loading

0 comments on commit d9c5a6d

Please sign in to comment.