From b98a6dd3efefcd95b50ba6acbb2f20059055cf22 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Thu, 18 Sep 2025 10:53:15 -0700 Subject: [PATCH] Enable evaluateCanonicalTypesToNativeValues by default PiperOrigin-RevId: 808650209 --- .../test/java/dev/cel/bundle/CelImplTest.java | 12 ++++++++---- codelab/README.md | 4 ++-- codelab/src/test/codelab/Exercise8Test.java | 4 ++-- .../test/codelab/solutions/Exercise8Test.java | 4 ++-- .../main/java/dev/cel/common/CelOptions.java | 3 +++ .../cel/common/internal/ProtoAdapterTest.java | 17 +++++++++-------- .../values/ProtoMessageValueProviderTest.java | 8 ++++---- .../validators/DurationLiteralValidator.java | 2 +- .../validators/TimestampLiteralValidator.java | 4 ++-- .../DurationLiteralValidatorTest.java | 4 ++-- .../TimestampLiteralValidatorTest.java | 13 ++++++------- 11 files changed, 41 insertions(+), 34 deletions(-) diff --git a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java index d781c80b4..2d337e4cf 100644 --- a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java +++ b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java @@ -77,7 +77,6 @@ import dev.cel.common.CelVarDecl; import dev.cel.common.ast.CelExpr; import dev.cel.common.ast.CelExpr.CelList; -import dev.cel.common.internal.ProtoTimeUtils; import dev.cel.common.testing.RepeatedTestProvider; import dev.cel.common.types.CelKind; import dev.cel.common.types.CelProtoMessageTypes; @@ -114,6 +113,7 @@ import dev.cel.runtime.CelVariableResolver; import dev.cel.runtime.UnknownContext; import dev.cel.testing.testdata.proto3.StandaloneGlobalEnum; +import java.time.Instant; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -817,6 +817,7 @@ public void program_messageConstruction() throws Exception { public void program_duplicateTypeDescriptor() throws Exception { Cel cel = standardCelBuilderWithMacros() + .setOptions(CelOptions.current().evaluateCanonicalTypesToNativeValues(true).build()) .addMessageTypes(Timestamp.getDescriptor()) .addMessageTypes(ImmutableList.of(Timestamp.getDescriptor())) .setContainer(CelContainer.ofName("google")) @@ -824,20 +825,22 @@ public void program_duplicateTypeDescriptor() throws Exception { .build(); CelRuntime.Program program = cel.createProgram(cel.compile("protobuf.Timestamp{seconds: 12}").getAst()); - assertThat(program.eval()).isEqualTo(ProtoTimeUtils.fromSecondsToTimestamp(12)); + + assertThat(program.eval()).isEqualTo(Instant.ofEpochSecond(12)); } @Test public void program_hermeticDescriptors_wellKnownProtobuf() throws Exception { Cel cel = standardCelBuilderWithMacros() + .setOptions(CelOptions.current().evaluateCanonicalTypesToNativeValues(true).build()) .addMessageTypes(Timestamp.getDescriptor()) .setContainer(CelContainer.ofName("google")) .setResultType(SimpleType.TIMESTAMP) .build(); CelRuntime.Program program = cel.createProgram(cel.compile("protobuf.Timestamp{seconds: 12}").getAst()); - assertThat(program.eval()).isEqualTo(ProtoTimeUtils.fromSecondsToTimestamp(12)); + assertThat(program.eval()).isEqualTo(Instant.ofEpochSecond(12)); } @Test @@ -959,6 +962,7 @@ public void program_deepTypeResolutionDisabledForRuntime_fails() throws Exceptio public void program_typeProvider() throws Exception { Cel cel = standardCelBuilderWithMacros() + .setOptions(CelOptions.current().evaluateCanonicalTypesToNativeValues(true).build()) .setTypeProvider( new DescriptorTypeProvider(ImmutableList.of(Timestamp.getDescriptor()))) .setContainer(CelContainer.ofName("google")) @@ -966,7 +970,7 @@ public void program_typeProvider() throws Exception { .build(); CelRuntime.Program program = cel.createProgram(cel.compile("protobuf.Timestamp{seconds: 12}").getAst()); - assertThat(program.eval()).isEqualTo(ProtoTimeUtils.fromSecondsToTimestamp(12)); + assertThat(program.eval()).isEqualTo(Instant.ofEpochSecond(12)); } @Test diff --git a/codelab/README.md b/codelab/README.md index 6de1a2fb0..f7d248b13 100644 --- a/codelab/README.md +++ b/codelab/README.md @@ -1065,8 +1065,8 @@ public void validate_invalidTimestampLiteral_returnsError() throws Exception { assertThat(validationResult.hasError()).isTrue(); assertThat(validationResult.getErrorString()) .isEqualTo( - "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Failed to" - + " parse timestamp: invalid timestamp \"bad\"\n" + "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Text 'bad'" + + " could not be parsed at index 0\n" + " | timestamp('bad')\n" + " | ..........^"); } diff --git a/codelab/src/test/codelab/Exercise8Test.java b/codelab/src/test/codelab/Exercise8Test.java index 9ebbc505a..36fef248c 100644 --- a/codelab/src/test/codelab/Exercise8Test.java +++ b/codelab/src/test/codelab/Exercise8Test.java @@ -43,8 +43,8 @@ public void validate_invalidTimestampLiteral_returnsError() throws Exception { assertThat(validationResult.hasError()).isTrue(); assertThat(validationResult.getErrorString()) .isEqualTo( - "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Failed to" - + " parse timestamp: invalid timestamp \"bad\"\n" + "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Text 'bad'" + + " could not be parsed at index 0\n" + " | timestamp('bad')\n" + " | ..........^"); } diff --git a/codelab/src/test/codelab/solutions/Exercise8Test.java b/codelab/src/test/codelab/solutions/Exercise8Test.java index ac39340e0..73a0ddc91 100644 --- a/codelab/src/test/codelab/solutions/Exercise8Test.java +++ b/codelab/src/test/codelab/solutions/Exercise8Test.java @@ -43,8 +43,8 @@ public void validate_invalidTimestampLiteral_returnsError() throws Exception { assertThat(validationResult.hasError()).isTrue(); assertThat(validationResult.getErrorString()) .isEqualTo( - "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Failed to" - + " parse timestamp: invalid timestamp \"bad\"\n" + "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Text 'bad'" + + " could not be parsed at index 0\n" + " | timestamp('bad')\n" + " | ..........^"); } diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java index 23654235d..ead987ffd 100644 --- a/common/src/main/java/dev/cel/common/CelOptions.java +++ b/common/src/main/java/dev/cel/common/CelOptions.java @@ -184,6 +184,7 @@ public static Builder current() { .enableUnsignedLongs(true) .enableRegexPartialMatch(true) .errorOnDuplicateMapKeys(true) + .evaluateCanonicalTypesToNativeValues(true) .errorOnIntWrap(true) .resolveTypeDependencies(true) .disableCelStandardEquality(false); @@ -462,6 +463,8 @@ public abstract static class Builder { * com.google.protobuf.ByteString}. *
  • CEL null: {@code dev.cel.common.values.NullValue} instead of {@code * com.google.protobuf.NullValue}. + *
  • Timestamp: {@code java.time.Instant} instead of {@code com.google.protobuf.Timestamp}. + *
  • Duration: {@code java.time.Duration} instead of {@code com.google.protobuf.Duration}. * */ public abstract Builder evaluateCanonicalTypesToNativeValues(boolean value); diff --git a/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java b/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java index 3172258f4..0ce37ac1d 100644 --- a/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java +++ b/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java @@ -41,6 +41,7 @@ import com.google.type.Expr; import dev.cel.common.CelOptions; import dev.cel.common.values.CelByteString; +import java.time.Instant; import java.util.Arrays; import java.util.List; import java.util.Optional; @@ -88,11 +89,10 @@ public static List data() { {1.5D, Any.pack(DoubleValue.of(1.5D))}, {1.5D, Value.newBuilder().setNumberValue(1.5D).build()}, { - Duration.newBuilder().setSeconds(123).build(), - Duration.newBuilder().setSeconds(123).build(), + java.time.Duration.ofSeconds(123), Duration.newBuilder().setSeconds(123).build(), }, { - Duration.newBuilder().setSeconds(123).build(), + java.time.Duration.ofSeconds(123), Any.pack(Duration.newBuilder().setSeconds(123).build()), }, {1L, Int64Value.of(1L)}, @@ -132,12 +132,10 @@ public static List data() { .build(), }, { - Timestamp.newBuilder().setSeconds(123).build(), - Timestamp.newBuilder().setSeconds(123).build(), + Instant.ofEpochSecond(123), Timestamp.newBuilder().setSeconds(123).build(), }, { - Timestamp.newBuilder().setSeconds(123).build(), - Any.pack(Timestamp.newBuilder().setSeconds(123).build()), + Instant.ofEpochSecond(123), Any.pack(Timestamp.newBuilder().setSeconds(123).build()), }, {UnsignedLong.valueOf(1L), UInt64Value.of(1L)}, {UnsignedLong.valueOf(1L), Any.pack(UInt64Value.of(1L))}, @@ -152,7 +150,10 @@ public static List data() { @Test public void adaptValueToProto_bidirectionalConversion() { DynamicProto dynamicProto = DynamicProto.create(DefaultMessageFactory.INSTANCE); - ProtoAdapter protoAdapter = new ProtoAdapter(dynamicProto, CelOptions.DEFAULT); + ProtoAdapter protoAdapter = + new ProtoAdapter( + dynamicProto, + CelOptions.current().evaluateCanonicalTypesToNativeValues(true).build()); assertThat(protoAdapter.adaptValueToProto(value, proto.getDescriptorForType().getFullName())) .isEqualTo(proto); assertThat(protoAdapter.adaptProtoToValue(proto)).isEqualTo(value); diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java index 891a7d0e2..f9bee1af1 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java @@ -28,7 +28,6 @@ import dev.cel.common.internal.DefaultMessageFactory; import dev.cel.common.internal.DynamicProto; import dev.cel.common.internal.ProtoMessageFactory; -import dev.cel.common.internal.ProtoTimeUtils; import dev.cel.expr.conformance.proto2.TestAllTypes; import dev.cel.expr.conformance.proto2.TestAllTypesExtensions; import java.time.Duration; @@ -66,7 +65,8 @@ public void newValue_createEmptyProtoMessage() { @Test public void newValue_createProtoMessage_fieldsPopulated() { ProtoMessageValueProvider protoMessageValueProvider = - ProtoMessageValueProvider.newInstance(CelOptions.DEFAULT, DYNAMIC_PROTO); + ProtoMessageValueProvider.newInstance( + CelOptions.current().evaluateCanonicalTypesToNativeValues(true).build(), DYNAMIC_PROTO); ProtoMessageValue protoMessageValue = (ProtoMessageValue) @@ -89,9 +89,9 @@ public void newValue_createProtoMessage_fieldsPopulated() { "single_string", "hello", "single_timestamp", - ProtoTimeUtils.fromSecondsToTimestamp(50), + Instant.ofEpochSecond(50), "single_duration", - ProtoTimeUtils.fromSecondsToDuration(100))) + Duration.ofSeconds(100))) .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); diff --git a/validator/src/main/java/dev/cel/validator/validators/DurationLiteralValidator.java b/validator/src/main/java/dev/cel/validator/validators/DurationLiteralValidator.java index 349374ca2..f0220e6ab 100644 --- a/validator/src/main/java/dev/cel/validator/validators/DurationLiteralValidator.java +++ b/validator/src/main/java/dev/cel/validator/validators/DurationLiteralValidator.java @@ -14,7 +14,7 @@ package dev.cel.validator.validators; -import com.google.protobuf.Duration; +import java.time.Duration; /** DurationLiteralValidator ensures that duration literal arguments are valid. */ public final class DurationLiteralValidator extends LiteralValidator { diff --git a/validator/src/main/java/dev/cel/validator/validators/TimestampLiteralValidator.java b/validator/src/main/java/dev/cel/validator/validators/TimestampLiteralValidator.java index 4f6a5209e..f1167f4eb 100644 --- a/validator/src/main/java/dev/cel/validator/validators/TimestampLiteralValidator.java +++ b/validator/src/main/java/dev/cel/validator/validators/TimestampLiteralValidator.java @@ -14,12 +14,12 @@ package dev.cel.validator.validators; -import com.google.protobuf.Timestamp; +import java.time.Instant; /** TimestampLiteralValidator ensures that timestamp literal arguments are valid. */ public final class TimestampLiteralValidator extends LiteralValidator { public static final TimestampLiteralValidator INSTANCE = - new TimestampLiteralValidator("timestamp", Timestamp.class); + new TimestampLiteralValidator("timestamp", Instant.class); private TimestampLiteralValidator(String functionName, Class expectedResultType) { super(functionName, expectedResultType); diff --git a/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java index e89e5ce35..1ec3ef4aa 100644 --- a/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableMap; -import com.google.protobuf.Duration; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.bundle.Cel; @@ -35,6 +34,7 @@ import dev.cel.validator.CelValidator; import dev.cel.validator.CelValidatorFactory; import java.text.ParseException; +import java.time.Duration; import org.junit.Test; import org.junit.runner.RunWith; @@ -174,7 +174,7 @@ public void duration_unexpectedResultType_throws() throws Exception { assertThat(result.getAllIssues().get(0).toDisplayString(ast.getSource())) .isEqualTo( "ERROR: :1:10: duration validation failed. Reason: Expected" - + " com.google.protobuf.Duration type but got java.lang.Integer instead\n" + + " java.time.Duration type but got java.lang.Integer instead\n" + " | duration('1h')\n" + " | .........^"); } diff --git a/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java index 65b7d593a..d263fc6bf 100644 --- a/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableMap; -import com.google.protobuf.Timestamp; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.bundle.Cel; @@ -36,6 +35,7 @@ import dev.cel.validator.CelValidator; import dev.cel.validator.CelValidatorFactory; import java.text.ParseException; +import java.time.Instant; import org.junit.Test; import org.junit.runner.RunWith; @@ -63,7 +63,7 @@ public void timestamp_validFormat(String source) throws Exception { assertThat(result.hasError()).isFalse(); assertThat(result.getAllIssues()).isEmpty(); - assertThat(CEL.createProgram(ast).eval()).isInstanceOf(Timestamp.class); + assertThat(CEL.createProgram(ast).eval()).isInstanceOf(Instant.class); } @Test @@ -100,8 +100,7 @@ public void timestamp_withVariable_noOp() throws Exception { () -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "bad"))); assertThat(e) .hasMessageThat() - .contains( - "evaluation error at :9: Failed to parse timestamp: invalid timestamp \"bad\""); + .contains("evaluation error at :9: Text 'bad' could not be parsed at index 0"); } @Test @@ -153,8 +152,8 @@ public void timestamp_invalidFormat() throws Exception { assertThat(result.getAllIssues().get(0).getSeverity()).isEqualTo(Severity.ERROR); assertThat(result.getAllIssues().get(0).toDisplayString(ast.getSource())) .isEqualTo( - "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Failed to" - + " parse timestamp: invalid timestamp \"bad\"\n" + "ERROR: :1:11: timestamp validation failed. Reason: evaluation error: Text 'bad'" + + " could not be parsed at index 0\n" + " | timestamp('bad')\n" + " | ..........^"); } @@ -185,7 +184,7 @@ public void timestamp_unexpectedResultType_throws() throws Exception { assertThat(result.getAllIssues().get(0).toDisplayString(ast.getSource())) .isEqualTo( "ERROR: :1:11: timestamp validation failed. Reason: Expected" - + " com.google.protobuf.Timestamp type but got java.lang.Integer instead\n" + + " java.time.Instant type but got java.lang.Integer instead\n" + " | timestamp(0)\n" + " | ..........^"); }