diff --git a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java index bc8e50b05..ed14493a8 100644 --- a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java @@ -156,10 +156,6 @@ private static Object decodeWireField( } int typeCode = field.typeCode(); - if (typeCode == SelectField.CEL_MAP_TYPE_CODE) { - throw new UnsupportedOperationException( - "Decoding unknown map field from wire bytes is unsupported: " + field.fieldName()); - } if (typeCode == SelectField.NO_TYPE_CODE) { throw CelAttributeNotFoundException.forFieldResolution(field.fieldName()); } diff --git a/common/src/main/java/dev/cel/common/values/SelectField.java b/common/src/main/java/dev/cel/common/values/SelectField.java index 9374ebcf1..6df39c37c 100644 --- a/common/src/main/java/dev/cel/common/values/SelectField.java +++ b/common/src/main/java/dev/cel/common/values/SelectField.java @@ -155,6 +155,8 @@ public static SelectField create( long typeCode, @Nullable Object defaultValue, String protoTypeName) { + checkArgument( + typeCode != CEL_MAP_TYPE_CODE, "Map fields must be created via createMap: %s", fieldName); return newInstance( fieldNumber, fieldName, typeCode, defaultValue, protoTypeName, /* mapEntrySpec= */ null); } diff --git a/common/src/test/java/dev/cel/common/values/SelectFieldTest.java b/common/src/test/java/dev/cel/common/values/SelectFieldTest.java index 9e2fb4366..7e1dfbe4f 100644 --- a/common/src/test/java/dev/cel/common/values/SelectFieldTest.java +++ b/common/src/test/java/dev/cel/common/values/SelectFieldTest.java @@ -152,6 +152,16 @@ public void create_typeCodeGroupProto_throwsIllegalArgumentException() { assertThat(thrown).hasMessageThat().contains("Invalid protobuf type code: 10"); } + @Test + public void create_mapTypeCode_throwsIllegalArgumentException() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> SelectField.create(1L, "foo", SelectField.CEL_MAP_TYPE_CODE)); + + assertThat(thrown).hasMessageThat().contains("Map fields must be created via createMap: foo"); + } + @Test public void createMap_threeArguments_success() { SelectField.MapEntrySpec mapEntrySpec = SelectField.MapEntrySpec.create(9, 9); diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel index dae52c1f9..694dfc04e 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel @@ -122,7 +122,6 @@ java_library( ":default_optimizer_constants", "//:auto_value", "//bundle:cel", - "//checker:standard_decl", "//common:cel_ast", "//common:cel_descriptor_util", "//common:cel_descriptors", @@ -134,7 +133,6 @@ java_library( "//common/ast:mutable_expr", "//common/internal:cel_descriptor_pools", "//common/navigation:common", - "//common/navigation:expr_util", "//common/navigation:mutable_navigation", "//common/types", "//common/types:cel_types", diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java index 16142ee20..3efac648b 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java @@ -16,7 +16,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static dev.cel.optimizer.optimizers.DefaultOptimizerConstants.CEL_ATTRIBUTE_FUNCTION_NAME; import static dev.cel.optimizer.optimizers.DefaultOptimizerConstants.CEL_HAS_FIELD_FUNCTION_NAME; @@ -27,11 +26,11 @@ import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.protobuf.ByteString; +import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.EnumValueDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FileDescriptor; import dev.cel.bundle.Cel; -import dev.cel.checker.CelStandardDeclarations.StandardFunction; import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelDescriptorUtil; import dev.cel.common.CelDescriptors; @@ -51,11 +50,11 @@ import dev.cel.common.ast.CelMutableExpr.CelMutableList; import dev.cel.common.ast.CelMutableExpr.CelMutableMap; import dev.cel.common.ast.CelMutableExpr.CelMutableSelect; +import dev.cel.common.ast.CelMutableExpr.CelMutableStruct; import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.internal.CombinedDescriptorPool; import dev.cel.common.internal.DefaultDescriptorPool; // CEL-Internal-1 -import dev.cel.common.navigation.CelNavigableExprUtil; import dev.cel.common.navigation.CelNavigableMutableAst; import dev.cel.common.navigation.CelNavigableMutableExpr; import dev.cel.common.navigation.TraversalOrder; @@ -64,7 +63,6 @@ import dev.cel.common.types.ListType; import dev.cel.common.types.SimpleType; import dev.cel.common.types.TypeParamType; -import dev.cel.common.types.TypeType; import dev.cel.common.values.CelByteString; import dev.cel.optimizer.AstMutator; import dev.cel.optimizer.CelAstOptimizer; @@ -92,14 +90,18 @@ * represented as a metadata tuple: * * * + *

The 3rd argument of {@code cel.@attribute} is a typed expression that binds the result type + * {@code T}. For singular scalar leaves it is also the field's default value (e.g. {@code 0} or a + * proto2 custom default). For repeated, map, and message leaves (including {@code + * google.protobuf.Duration} and {@code google.protobuf.Timestamp}) it is a type-only dummy (e.g. + * {@code [0]}, {@code {"": 0}}, {@code Msg{}}) that the runtime never evaluates. + * *

Field presence paths ({@code cel.@hasField}) represent each step as a 2-tuple: {@code * [field_num, field_name]}. * @@ -109,9 +111,9 @@ *

Expressions are rewritten into the following forms: * *

- *   // Selection chains (user message is 3-tuple, leaf scalar is 4-tuple, leaf type is 3rd argument)
+ *   // Selection chains (qualifier hops, leaf dummy/default value in 3rd argument)
  *   request.user.age -> cel.@attribute(request,
- *       [[user_num, "user", type_code], [age_num, "age", type_code, default_val]], int)
+ *       [[user_num, "user", type_code], [age_num, "age", type_code]], 0)
  *
  *   // Presence tests (2-tuples)
  *   has(request.user.age) -> cel.@hasField(request,
@@ -144,12 +146,11 @@ public final class SelectOptimizer implements CelAstOptimizer {
   private static final TypeParamType TYPE_PARAM_T = TypeParamType.create("T");
 
   /**
-   * Declaration for {@code cel.@attribute(operand, qualifiers, typeIdent) -> T}.
+   * Declaration for {@code cel.@attribute(operand, qualifiers, dummyOrDefVal) -> T}.
    *
-   * 

The 3rd argument ({@code TypeType.create(TYPE_PARAM_T)}) binds type parameter {@code T} to - * the static type identifier of the leaf field so that type checking preserves the exact result - * type rather than erasing to {@code dyn}, and enables plan-time integrity validation between the - * leaf hop's wire type code and its static type. + *

The 3rd argument ({@code TYPE_PARAM_T}) binds type parameter {@code T} to the static type of + * the dummy/default value of the leaf field so that type checking preserves the exact result type + * rather than erasing to {@code dyn}, and provides the default fallback for leaf scalars. */ @VisibleForTesting static final CelFunctionDecl CEL_ATTRIBUTE_FUNCTION_DECL = @@ -160,7 +161,7 @@ public final class SelectOptimizer implements CelAstOptimizer { TYPE_PARAM_T, SimpleType.DYN, ListType.create(SimpleType.DYN), - TypeType.create(TYPE_PARAM_T))); + TYPE_PARAM_T)); @VisibleForTesting static final CelFunctionDecl CEL_HAS_FIELD_FUNCTION_DECL = @@ -279,28 +280,7 @@ private void rewriteSelectChain( "Optimization of Group fields is unsupported: " + field.getFullName()); } if (field.getType() == FieldDescriptor.Type.MESSAGE) { - String messageFullName = field.getMessageType().getFullName(); - if (messageFullName.equals(CelTypes.STRUCT_MESSAGE)) { - throw new UnsupportedOperationException( - "Optimization of Struct fields is currently unimplemented: " + field.getFullName()); - } - if (messageFullName.equals(CelTypes.LIST_VALUE_MESSAGE)) { - throw new UnsupportedOperationException( - "Optimization of ListValue fields is currently unimplemented: " - + field.getFullName()); - } - if (messageFullName.equals(CelTypes.VALUE_MESSAGE)) { - throw new UnsupportedOperationException( - "Optimization of Value fields is currently unimplemented: " + field.getFullName()); - } - if (messageFullName.equals(CelTypes.ANY_MESSAGE)) { - throw new UnsupportedOperationException( - "Optimization of Any fields is currently unimplemented: " + field.getFullName()); - } - if (CelTypes.isWrapperType(messageFullName)) { - throw new UnsupportedOperationException( - "Optimization of wrapper fields is currently unimplemented: " + field.getFullName()); - } + checkUnsupportedMessageType(field.getMessageType().getFullName(), field.getFullName()); } CelMutableList qualifierElements = @@ -315,7 +295,23 @@ private void rewriteSelectChain( .add( CelMutableExpr.ofConstant( idGenerator.nextExprId(), CelConstant.ofValue(resolveTypeCode(field)))); - resolveDefaultValue(field, idGenerator).ifPresent(qualifierElements.elements()::add); + if (field.isMapField()) { + Descriptor mapEntryDesc = field.getMessageType(); + FieldDescriptor keyField = mapEntryDesc.findFieldByName("key"); + FieldDescriptor valField = mapEntryDesc.findFieldByName("value"); + qualifierElements + .elements() + .add( + CelMutableExpr.ofList( + idGenerator.nextExprId(), + CelMutableList.create( + CelMutableExpr.ofConstant( + idGenerator.nextExprId(), + CelConstant.ofValue(resolveTypeCode(keyField))), + CelMutableExpr.ofConstant( + idGenerator.nextExprId(), + CelConstant.ofValue(resolveTypeCode(valField)))))); + } } qualifierLists.add(CelMutableExpr.ofList(idGenerator.nextExprId(), qualifierElements)); } @@ -327,14 +323,12 @@ private void rewriteSelectChain( .expr() .setCall(CelMutableCall.create(CEL_HAS_FIELD_FUNCTION_NAME, currentExpr, qualifiersExpr)); } else { - String typeIdent = resolveTypeIdent(topField); - assertNotShadowed(topNode, typeIdent); - CelMutableExpr typeExpr = CelMutableExpr.ofIdent(idGenerator.nextExprId(), typeIdent); + CelMutableExpr dummyOrDefExpr = resolveDummyOrDefExpr(topField, idGenerator); topNode .expr() .setCall( CelMutableCall.create( - CEL_ATTRIBUTE_FUNCTION_NAME, currentExpr, qualifiersExpr, typeExpr)); + CEL_ATTRIBUTE_FUNCTION_NAME, currentExpr, qualifiersExpr, dummyOrDefExpr)); } } @@ -345,58 +339,34 @@ private static long resolveTypeCode(FieldDescriptor field) { return field.getType().toProto().getNumber(); } - private static String resolveTypeIdent(FieldDescriptor field) { - if (field.isMapField()) { - return "map"; - } - if (field.isRepeated()) { - return "list"; - } - switch (field.getType()) { - case DOUBLE: - case FLOAT: - return "double"; - case INT64: - case SINT64: - case SFIXED64: - case INT32: - case SINT32: - case SFIXED32: - case ENUM: - return "int"; - case UINT64: - case FIXED64: - case UINT32: - case FIXED32: - return "uint"; - case BOOL: - return "bool"; - case STRING: - return "string"; - case BYTES: - return "bytes"; - case MESSAGE: - return field.getMessageType().getFullName(); - default: - throw new IllegalArgumentException("Unsupported protobuf field type: " + field.getType()); - } - } - private boolean isTopOfSelectChain(CelNavigableMutableAst navAst, CelNavigableMutableExpr node) { return getOptimizableField(navAst, node).isPresent() && !node.parent().flatMap(parent -> getOptimizableField(navAst, parent)).isPresent(); } - // TODO: Mangle comprehension variables. - private static void assertNotShadowed(CelNavigableMutableExpr node, String typeIdent) { - int dotIndex = typeIdent.indexOf('.'); - String rootSegment = dotIndex < 0 ? typeIdent : typeIdent.substring(0, dotIndex); - checkState( - !CelNavigableExprUtil.isVariableShadowed(node, rootSegment), - "cel.@attribute type identifier '%s' is shadowed by an enclosing comprehension variable" - + " '%s'. Rename the comprehension variable.", - typeIdent, - rootSegment); + // TODO: Support STRUCT_MESSAGE, LIST_VALUE_MESSAGE, VALUE_MESSAGE, ANY_MESSAGE, + // and wrapper types. + private static void checkUnsupportedMessageType(String messageFullName, String fieldFullName) { + if (messageFullName.equals(CelTypes.STRUCT_MESSAGE)) { + throw new UnsupportedOperationException( + "Optimization of Struct fields is currently unimplemented: " + fieldFullName); + } + if (messageFullName.equals(CelTypes.LIST_VALUE_MESSAGE)) { + throw new UnsupportedOperationException( + "Optimization of ListValue fields is currently unimplemented: " + fieldFullName); + } + if (messageFullName.equals(CelTypes.VALUE_MESSAGE)) { + throw new UnsupportedOperationException( + "Optimization of Value fields is currently unimplemented: " + fieldFullName); + } + if (messageFullName.equals(CelTypes.ANY_MESSAGE)) { + throw new UnsupportedOperationException( + "Optimization of Any fields is currently unimplemented: " + fieldFullName); + } + if (CelTypes.isWrapperType(messageFullName)) { + throw new UnsupportedOperationException( + "Optimization of wrapper fields is currently unimplemented: " + fieldFullName); + } } private Optional getOptimizableField( @@ -416,48 +386,70 @@ private Optional getOptimizableFieldForExpr( .map(desc -> desc.findFieldByName(select.field())); } - private static Optional resolveDefaultValue( + private static CelMutableExpr resolveDummyOrDefExpr( FieldDescriptor field, MonotonicIdGenerator idGenerator) { if (field.isMapField()) { - return Optional.of( - CelMutableExpr.ofMap(idGenerator.nextExprId(), CelMutableMap.create(ImmutableList.of()))); + Descriptor mapEntryDesc = field.getMessageType(); + FieldDescriptor keyField = mapEntryDesc.findFieldByName("key"); + FieldDescriptor valField = mapEntryDesc.findFieldByName("value"); + CelMutableExpr dummyKey = resolveDummyValue(keyField, idGenerator); + CelMutableExpr dummyVal = resolveDummyValue(valField, idGenerator); + return CelMutableExpr.ofMap( + idGenerator.nextExprId(), + CelMutableMap.create( + ImmutableList.of( + CelMutableMap.Entry.create(idGenerator.nextExprId(), dummyKey, dummyVal)))); } if (field.isRepeated()) { - return Optional.of(CelMutableExpr.ofList(idGenerator.nextExprId(), CelMutableList.create())); + return CelMutableExpr.ofList( + idGenerator.nextExprId(), CelMutableList.create(resolveDummyValue(field, idGenerator))); } + return resolveDummyValue(field, idGenerator); + } + + private static CelMutableExpr resolveDummyValue( + FieldDescriptor field, MonotonicIdGenerator idGenerator) { if (field.getType() == FieldDescriptor.Type.MESSAGE) { - String messageFullName = field.getMessageType().getFullName(); - switch (messageFullName) { - case CelTypes.DURATION_MESSAGE: - return Optional.of( - CelMutableExpr.ofCall( - idGenerator.nextExprId(), - CelMutableCall.create( - StandardFunction.DURATION.functionName(), - CelMutableExpr.ofConstant( - idGenerator.nextExprId(), CelConstant.ofValue("0s"))))); - case CelTypes.TIMESTAMP_MESSAGE: - return Optional.of( - CelMutableExpr.ofCall( - idGenerator.nextExprId(), - CelMutableCall.create( - StandardFunction.TIMESTAMP.functionName(), - CelMutableExpr.ofConstant( - idGenerator.nextExprId(), CelConstant.ofValue(0L))))); - // TODO: Support STRUCT_MESSAGE, LIST_VALUE_MESSAGE, VALUE_MESSAGE, - // ANY_MESSAGE, - // and wrapper types. - default: - // User-defined message fields omit default values (encoded as 3-tuples). - return Optional.empty(); - } + // A leading dot makes the type-checker resolve the name absolutely, bypassing container + // candidates and comprehension-local scopes that could otherwise capture it. + return CelMutableExpr.ofStruct( + idGenerator.nextExprId(), + CelMutableStruct.create("." + field.getMessageType().getFullName(), ImmutableList.of())); } - - return Optional.of( - CelMutableExpr.ofConstant(idGenerator.nextExprId(), resolveConstantDefaultValue(field))); + return CelMutableExpr.ofConstant(idGenerator.nextExprId(), resolveConstantDefaultValue(field)); } private static CelConstant resolveConstantDefaultValue(FieldDescriptor field) { + if (field.isRepeated()) { + // Repeated element dummies only drive type inference; the planner discards their values. + switch (field.getType()) { + case DOUBLE: + case FLOAT: + return CelConstant.ofValue(0.0d); + case INT64: + case SINT64: + case SFIXED64: + case INT32: + case SINT32: + case SFIXED32: + case ENUM: + return CelConstant.ofValue(0L); + case UINT64: + case FIXED64: + case UINT32: + case FIXED32: + return CelConstant.ofValue(UnsignedLong.ZERO); + case BOOL: + return CelConstant.ofValue(false); + case STRING: + return CelConstant.ofValue(""); + case BYTES: + return CelConstant.ofValue(CelByteString.EMPTY); + default: + throw new IllegalArgumentException( + "Unsupported repeated field type for scalar default: " + field.getType()); + } + } Object def = field.getDefaultValue(); switch (field.getType()) { case DOUBLE: diff --git a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java index b1cc01de4..fd1ad8787 100644 --- a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java +++ b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java @@ -35,6 +35,7 @@ import com.google.protobuf.Value; 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.bundle.CelBuilder; import dev.cel.common.CelAbstractSyntaxTree; @@ -48,6 +49,7 @@ import dev.cel.common.ast.CelExpr.ExprKind.Kind; import dev.cel.common.ast.CelReference; import dev.cel.common.navigation.CelNavigableMutableAst; +import dev.cel.common.types.ListType; import dev.cel.common.types.MapType; import dev.cel.common.types.SimpleType; import dev.cel.common.types.StructTypeReference; @@ -57,7 +59,6 @@ import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.extensions.CelExtensions; import dev.cel.optimizer.CelAstOptimizer; -import dev.cel.optimizer.CelOptimizationException; import dev.cel.optimizer.CelOptimizer; import dev.cel.optimizer.CelOptimizerFactory; import dev.cel.optimizer.optimizers.SelectOptimizer.SelectOptimizerOptions; @@ -135,32 +136,32 @@ private static Cel setupEnv(CelBuilder celBuilder) { private enum RewriteTestCase { // === Selection & Traversal === PROTO3_SINGLE_FIELD_SELECT( - "msg.single_int64", "cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int)"), + "msg.single_int64", "cel.@attribute(msg, [[2, \"single_int64\", 3]], 0)"), PROTO3_SINGLE_MESSAGE_FIELD_SELECT( "msg.single_nested_message", "cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," - + " cel.expr.conformance.proto3.TestAllTypes.NestedMessage)"), + + " cel.expr.conformance.proto3.TestAllTypes.NestedMessage{})"), PROTO3_CHAINED_FIELD_SELECT( "msg.single_nested_message.bb", - "cel.@attribute(msg, [[21, \"single_nested_message\", 11], [1, \"bb\", 5, 0]], int)"), + "cel.@attribute(msg, [[21, \"single_nested_message\", 11], [1, \"bb\", 5]], 0)"), PROTO2_SINGLE_MESSAGE_FIELD_SELECT( "proto2_msg.single_nested_message", "cel.@attribute(proto2_msg, [[21, \"single_nested_message\", 11]]," - + " cel.expr.conformance.proto2.TestAllTypes.NestedMessage)"), + + " cel.expr.conformance.proto2.TestAllTypes.NestedMessage{})"), PROTO2_CHAINED_FIELD_SELECT( "proto2_msg.single_nested_message.bb", - "cel.@attribute(proto2_msg, [[21, \"single_nested_message\", 11], [1, \"bb\", 5, 0]]," - + " int)"), + "cel.@attribute(proto2_msg, [[21, \"single_nested_message\", 11], [1, \"bb\", 5]]," + + " 0)"), PROTO2_TRIPLE_CHAINED_FIELD_SELECT( "nested_msg.child.payload.single_int64", "cel.@attribute(nested_msg, " + "[[1, \"child\", 11], " + "[2, \"payload\", 11], " - + "[2, \"single_int64\", 3, -64]], int)"), + + "[2, \"single_int64\", 3]], -64)"), PROTO2_CHAINED_MESSAGE_FIELD_SELECT( "nested_msg.child.payload", "cel.@attribute(nested_msg, [[1, \"child\", 11], [2, \"payload\", 11]]," - + " cel.expr.conformance.proto2.TestAllTypes)"), + + " cel.expr.conformance.proto2.TestAllTypes{})"), // === Presence Tests: Proto2 (Explicit Presence) vs Proto3 (Implicit/Explicit Presence) === // In proto2, scalar fields have explicit presence (has-bit). @@ -199,130 +200,142 @@ private enum RewriteTestCase { // === Default Value Divergence: Proto2 Custom Defaults vs Proto3 Zero Defaults === // Int32: proto2 has custom default -32, proto3 has 0 PROTO2_CUSTOM_INT32( - "proto2_msg.single_int32", - "cel.@attribute(proto2_msg, [[1, \"single_int32\", 5, -32]], int)"), - PROTO3_ZERO_INT32( - "msg.single_int32", "cel.@attribute(msg, [[1, \"single_int32\", 5, 0]], int)"), + "proto2_msg.single_int32", "cel.@attribute(proto2_msg, [[1, \"single_int32\", 5]], -32)"), + PROTO3_ZERO_INT32("msg.single_int32", "cel.@attribute(msg, [[1, \"single_int32\", 5]], 0)"), // Int64: proto2 has custom default -64, proto3 has 0 PROTO2_CUSTOM_INT64( - "proto2_msg.single_int64", - "cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"), - PROTO3_ZERO_INT64( - "msg.single_int64", "cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int)"), + "proto2_msg.single_int64", "cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"), + PROTO3_ZERO_INT64("msg.single_int64", "cel.@attribute(msg, [[2, \"single_int64\", 3]], 0)"), // Uint32: proto2 has custom default 32, proto3 has 0 PROTO2_CUSTOM_UINT32( "proto2_msg.single_uint32", - "cel.@attribute(proto2_msg, [[3, \"single_uint32\", 13, 32u]], uint)"), + "cel.@attribute(proto2_msg, [[3, \"single_uint32\", 13]], 32u)"), PROTO3_ZERO_UINT32( - "msg.single_uint32", "cel.@attribute(msg, [[3, \"single_uint32\", 13, 0u]], uint)"), + "msg.single_uint32", "cel.@attribute(msg, [[3, \"single_uint32\", 13]], 0u)"), // Uint64: proto2 has custom default 64, proto3 has 0 PROTO2_CUSTOM_UINT64( - "proto2_msg.single_uint64", - "cel.@attribute(proto2_msg, [[4, \"single_uint64\", 4, 64u]], uint)"), - PROTO3_ZERO_UINT64( - "msg.single_uint64", "cel.@attribute(msg, [[4, \"single_uint64\", 4, 0u]], uint)"), + "proto2_msg.single_uint64", "cel.@attribute(proto2_msg, [[4, \"single_uint64\", 4]], 64u)"), + PROTO3_ZERO_UINT64("msg.single_uint64", "cel.@attribute(msg, [[4, \"single_uint64\", 4]], 0u)"), // String: proto2 has custom default "empty", proto3 has "" PROTO2_CUSTOM_STRING( "proto2_msg.single_string", - "cel.@attribute(proto2_msg, [[14, \"single_string\", 9, \"empty\"]], string)"), + "cel.@attribute(proto2_msg, [[14, \"single_string\", 9]], \"empty\")"), PROTO3_ZERO_STRING( - "msg.single_string", "cel.@attribute(msg, [[14, \"single_string\", 9, \"\"]], string)"), + "msg.single_string", "cel.@attribute(msg, [[14, \"single_string\", 9]], \"\")"), // Bool: proto2 has custom default true, proto3 has false PROTO2_CUSTOM_BOOL( - "proto2_msg.single_bool", - "cel.@attribute(proto2_msg, [[13, \"single_bool\", 8, true]], bool)"), - PROTO3_ZERO_BOOL( - "msg.single_bool", "cel.@attribute(msg, [[13, \"single_bool\", 8, false]], bool)"), + "proto2_msg.single_bool", "cel.@attribute(proto2_msg, [[13, \"single_bool\", 8]], true)"), + PROTO3_ZERO_BOOL("msg.single_bool", "cel.@attribute(msg, [[13, \"single_bool\", 8]], false)"), // Float: proto2 has custom default 3.0, proto3 has 0.0 PROTO2_CUSTOM_FLOAT( - "proto2_msg.single_float", - "cel.@attribute(proto2_msg, [[11, \"single_float\", 2, 3.0]], double)"), - PROTO3_ZERO_FLOAT( - "msg.single_float", "cel.@attribute(msg, [[11, \"single_float\", 2, 0.0]], double)"), + "proto2_msg.single_float", "cel.@attribute(proto2_msg, [[11, \"single_float\", 2]], 3.0)"), + PROTO3_ZERO_FLOAT("msg.single_float", "cel.@attribute(msg, [[11, \"single_float\", 2]], 0.0)"), // Double: proto2 has custom default 6.4, proto3 has 0.0 PROTO2_CUSTOM_DOUBLE( "proto2_msg.single_double", - "cel.@attribute(proto2_msg, [[12, \"single_double\", 1, 6.4]], double)"), + "cel.@attribute(proto2_msg, [[12, \"single_double\", 1]], 6.4)"), PROTO3_ZERO_DOUBLE( - "msg.single_double", "cel.@attribute(msg, [[12, \"single_double\", 1, 0.0]], double)"), + "msg.single_double", "cel.@attribute(msg, [[12, \"single_double\", 1]], 0.0)"), // Bytes: proto2 has custom default "none", proto3 has "" PROTO2_CUSTOM_BYTES( "proto2_msg.single_bytes", - "cel.@attribute(proto2_msg, [[15, \"single_bytes\", 12, b\"\\156\\157\\156\\145\"]]," - + " bytes)"), + "cel.@attribute(proto2_msg, [[15, \"single_bytes\", 12]], b\"\\156\\157\\156\\145\")"), PROTO3_ZERO_BYTES( - "msg.single_bytes", "cel.@attribute(msg, [[15, \"single_bytes\", 12, b\"\"]], bytes)"), + "msg.single_bytes", "cel.@attribute(msg, [[15, \"single_bytes\", 12]], b\"\")"), // Enum: proto2 has custom default 1 (BAR), proto3 has 0 (FOO) PROTO2_CUSTOM_ENUM( "proto2_msg.single_nested_enum", - "cel.@attribute(proto2_msg, [[22, \"single_nested_enum\", 14, 1]], int)"), + "cel.@attribute(proto2_msg, [[22, \"single_nested_enum\", 14]], 1)"), PROTO3_ZERO_ENUM( - "msg.single_nested_enum", - "cel.@attribute(msg, [[22, \"single_nested_enum\", 14, 0]], int)"), + "msg.single_nested_enum", "cel.@attribute(msg, [[22, \"single_nested_enum\", 14]], 0)"), // Fixed / sfixed / sint fields - PROTO3_FIXED32( - "msg.single_fixed32", "cel.@attribute(msg, [[7, \"single_fixed32\", 7, 0u]], uint)"), - PROTO3_FIXED64( - "msg.single_fixed64", "cel.@attribute(msg, [[8, \"single_fixed64\", 6, 0u]], uint)"), + PROTO3_FIXED32("msg.single_fixed32", "cel.@attribute(msg, [[7, \"single_fixed32\", 7]], 0u)"), + PROTO3_FIXED64("msg.single_fixed64", "cel.@attribute(msg, [[8, \"single_fixed64\", 6]], 0u)"), PROTO3_SFIXED32( - "msg.single_sfixed32", "cel.@attribute(msg, [[9, \"single_sfixed32\", 15, 0]], int)"), + "msg.single_sfixed32", "cel.@attribute(msg, [[9, \"single_sfixed32\", 15]], 0)"), PROTO3_SFIXED64( - "msg.single_sfixed64", "cel.@attribute(msg, [[10, \"single_sfixed64\", 16, 0]], int)"), - PROTO3_SINT32("msg.single_sint32", "cel.@attribute(msg, [[5, \"single_sint32\", 17, 0]], int)"), - PROTO3_SINT64("msg.single_sint64", "cel.@attribute(msg, [[6, \"single_sint64\", 18, 0]], int)"), + "msg.single_sfixed64", "cel.@attribute(msg, [[10, \"single_sfixed64\", 16]], 0)"), + PROTO3_SINT32("msg.single_sint32", "cel.@attribute(msg, [[5, \"single_sint32\", 17]], 0)"), + PROTO3_SINT64("msg.single_sint64", "cel.@attribute(msg, [[6, \"single_sint64\", 18]], 0)"), - // Repeated fields: empty list default + // Repeated fields: [dummy] PROTO2_REPEATED_PRIMITIVE( "proto2_msg.repeated_int64", - "cel.@attribute(proto2_msg, [[32, \"repeated_int64\", 3, []]], list)"), + "cel.@attribute(proto2_msg, [[32, \"repeated_int64\", 3]], [0])"), PROTO3_REPEATED_PRIMITIVE( - "msg.repeated_int64", "cel.@attribute(msg, [[32, \"repeated_int64\", 3, []]], list)"), + "msg.repeated_int64", "cel.@attribute(msg, [[32, \"repeated_int64\", 3]], [0])"), PROTO3_REPEATED_MESSAGE( "msg.repeated_nested_message", - "cel.@attribute(msg, [[51, \"repeated_nested_message\", 11, []]], list)"), + "cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]]," + + " [cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}])"), + PROTO3_REPEATED_UINT32( + "msg.repeated_uint32", "cel.@attribute(msg, [[33, \"repeated_uint32\", 13]], [0u])"), + PROTO3_REPEATED_BYTES( + "msg.repeated_bytes", "cel.@attribute(msg, [[45, \"repeated_bytes\", 12]], [b\"\"])"), + PROTO3_REPEATED_ENUM( + "msg.repeated_nested_enum", + "cel.@attribute(msg, [[52, \"repeated_nested_enum\", 14]], [0])"), + PROTO3_REPEATED_DURATION( + "msg.repeated_duration", + "cel.@attribute(msg, [[121, \"repeated_duration\", 11]], [google.protobuf.Duration{}])"), // Well-known types PROTO3_TIMESTAMP( "msg.single_timestamp", - "cel.@attribute(msg, [[102, \"single_timestamp\", 11, timestamp(0)]]," - + " google.protobuf.Timestamp)"), + "cel.@attribute(msg, [[102, \"single_timestamp\", 11]], google.protobuf.Timestamp{})"), PROTO3_DURATION( "msg.single_duration", - "cel.@attribute(msg, [[101, \"single_duration\", 11, duration(\"0s\")]]," - + " google.protobuf.Duration)"), + "cel.@attribute(msg, [[101, \"single_duration\", 11]], google.protobuf.Duration{})"), PROTO3_TIMESTAMP_COMPARISON( "msg.single_timestamp > timestamp(0)", - "cel.@attribute(msg, [[102, \"single_timestamp\", 11, timestamp(0)]]," - + " google.protobuf.Timestamp) > timestamp(0)"), + "cel.@attribute(msg, [[102, \"single_timestamp\", 11]], google.protobuf.Timestamp{}) >" + + " timestamp(0)"), PROTO3_DURATION_COMPARISON( "msg.single_duration == duration(\"1h\")", - "cel.@attribute(msg, [[101, \"single_duration\", 11, duration(\"0s\")]]," - + " google.protobuf.Duration) == duration(\"1h\")"), + "cel.@attribute(msg, [[101, \"single_duration\", 11]], google.protobuf.Duration{}) ==" + + " duration(\"1h\")"), // Map selects + MAP_STRING_STRING( + "msg.map_string_string", + "cel.@attribute(msg, [[61, \"map_string_string\", -1, [9, 9]]], {\"\": \"\"})"), + MAP_BOOL_STRING( + "msg.map_bool_string", + "cel.@attribute(msg, [[64, \"map_bool_string\", -1, [8, 9]]], {false: \"\"})"), + MAP_UINT32_DOUBLE( + "msg.map_uint32_double", + "cel.@attribute(msg, [[204, \"map_uint32_double\", -1, [13, 1]]], {0u: 0.0})"), + MAP_INT64_ENUM( + "msg.map_int64_enum", + "cel.@attribute(msg, [[94, \"map_int64_enum\", -1, [3, 14]]], {0: 0})"), + MAP_STRING_DURATION( + "msg.map_string_duration", + "cel.@attribute(msg, [[243, \"map_string_duration\", -1, [9, 11]]]," + + " {\"\": google.protobuf.Duration{}})"), MAP_FIELD_INDEXING( "msg.map_int64_message[1].bb", "cel.@attribute(" - + "cel.@attribute(msg, [[95, \"map_int64_message\", -1, {}]], map)[1], " - + "[[1, \"bb\", 5, 0]], int)"), + + "cel.@attribute(msg, [[95, \"map_int64_message\", -1, [3, 11]]]," + + " {0: cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}})[1], " + + "[[1, \"bb\", 5]], 0)"), MAP_FIELD_SELECT_CHAIN_STOPS_AT_MAP_BOUNDARY( "map_var_msg.key.single_nested_message.bb", "cel.@attribute(map_var_msg.key, " + "[[21, \"single_nested_message\", 11], " - + "[1, \"bb\", 5, 0]], int)"), + + "[1, \"bb\", 5]], 0)"), MAP_FIELD_SELECT_STOPS_AT_MAP_BOUNDARY( "map_var_msg.key.single_int64", - "cel.@attribute(map_var_msg.key, [[2, \"single_int64\", 3, 0]], int)"), + "cel.@attribute(map_var_msg.key, [[2, \"single_int64\", 3]], 0)"), MAP_FIELD_HAS_STOPS_AT_MAP_BOUNDARY( "has(map_var_msg.key.single_nested_message)", "cel.@hasField(map_var_msg.key, [[21, \"single_nested_message\"]])"), @@ -332,22 +345,24 @@ private enum RewriteTestCase { PROTO_MAP_FIELD_SELECT_STOPS_AT_MAP_BOUNDARY( "msg.map_string_message.key.bb", "cel.@attribute(" - + "cel.@attribute(msg, [[227, \"map_string_message\", -1, {}]], map).key, " - + "[[1, \"bb\", 5, 0]], int)"), + + "cel.@attribute(msg, [[227, \"map_string_message\", -1, [9, 11]]]," + + " {\"\": cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}}).key, " + + "[[1, \"bb\", 5]], 0)"), PROTO_MAP_FIELD_HAS_STOPS_AT_MAP_BOUNDARY( "has(msg.map_string_message.key.bb)", "cel.@hasField(" - + "cel.@attribute(msg, [[227, \"map_string_message\", -1, {}]], map).key, " + + "cel.@attribute(msg, [[227, \"map_string_message\", -1, [9, 11]]]," + + " {\"\": cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}}).key, " + "[[1, \"bb\"]])"), MIXED_BOOLEAN_EXPRESSION( "msg.single_int64 > 0 && has(msg.single_nested_message)", - "cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int) > 0 " + "cel.@attribute(msg, [[2, \"single_int64\", 3]], 0) > 0 " + "&& cel.@hasField(msg, [[21, \"single_nested_message\"]])"), PROTO2_COMPLEX_OPERAND_SELECT( "(cond ? nested_msg_a : nested_msg_b).child.payload.single_int64", "cel.@attribute(cond ? nested_msg_a : nested_msg_b, " - + "[[1, \"child\", 11], [2, \"payload\", 11], [2, \"single_int64\", 3, -64]], int)"), + + "[[1, \"child\", 11], [2, \"payload\", 11], [2, \"single_int64\", 3]], -64)"), PROTO2_COMPLEX_OPERAND_HAS( "has((cond ? nested_msg_a : nested_msg_b).child.payload.single_int64)", "cel.@hasField(cond ? nested_msg_a : nested_msg_b, " @@ -433,7 +448,7 @@ public void optimize_withFileDescriptors_success() throws Exception { CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int)"); + .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3]], 0)"); } @Test @@ -451,19 +466,78 @@ public void optimize_withFileDescriptorsIterable_success() throws Exception { CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int)"); + .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3]], 0)"); } @Test - public void optimize_typeIdentShadowedByComprehensionVar_throws( + @TestParameters({ + "{expression: '[1].map(cel, msg.single_nested_message)', expected: '[1].map(cel," + + " cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}))'}", + "{expression: 'cel.bind(cel, 1, msg.single_nested_message)', expected: 'cel.bind(cel, 1," + + " cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}))'}", + "{expression: '[1].map(cel, [msg.single_nested_message].map(x, x.bb + 1))', expected:" + + " '[1].map(cel, [cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{})].map(x," + + " cel.@attribute(x, [[1, \"bb\", 5]], 0) + 1))'}", + "{expression: '[1].all(x, cel, msg.single_nested_message != null)', expected: '[1].all(x," + + " cel, cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}) != null)'}", + "{expression: '[[1]].map(cel, msg.repeated_nested_message)', expected: '[[1]].map(cel," + + " cel.@attribute(msg, [[51, \"repeated_nested_message\", 11]]," + + " [.cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}]))'}", + "{expression: '[[1]].map(cel, msg.map_int64_message)', expected: '[[1]].map(cel," + + " cel.@attribute(msg, [[95, \"map_int64_message\", -1, [3, 11]]]," + + " {0: .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}}))'}" + }) + public void optimize_messageNameUnderShadowingComprehensionVar_rewrites( + String expression, String expected) throws Exception { + Cel extendedCel = + cel.toCelBuilder() + .addCompilerLibraries(CelExtensions.bindings(), CelExtensions.comprehensions()) + .build(); + CelAbstractSyntaxTree ast = extendedCel.compile(expression).getAst(); + + CelAbstractSyntaxTree optimizedAst = newSelectOptimizer(extendedCel).optimize(ast); + + // The unparser rebuilds macros from SourceInfo.macro_calls, which the type-checker does not + // re-resolve, so the optimizer's absolute (leading-dot) name is retained in the output. + assertThat(CEL_UNPARSER.unparse(optimizedAst)).isEqualTo(expected); + } + + @Test + public void optimizeAndEvaluate_messageLeafUnderShadowingComprehensionVar_matchesUnoptimized( + @TestParameter({ + "[1].map(cel, msg.single_nested_message)", + "cel.bind(cel, 1, msg.single_nested_message)", + "[1].all(x, cel, msg.single_nested_message != null)", + "[[1]].map(cel, msg.repeated_nested_message)", + "[[1]].map(cel, msg.map_int64_message)", + "[msg.single_nested_message].map(cel, cel)" + }) + String expression) + throws Exception { + assumeTrue(runtimeFlavor == CelRuntimeFlavor.PLANNER); + Cel extendedCel = + cel.toCelBuilder() + .addCompilerLibraries(CelExtensions.bindings(), CelExtensions.comprehensions()) + .build(); + CelAbstractSyntaxTree ast = extendedCel.compile(expression).getAst(); + CelAbstractSyntaxTree optimizedAst = newSelectOptimizer(extendedCel).optimize(ast); + ImmutableMap input = ImmutableMap.of("msg", TestAllTypes.getDefaultInstance()); + + Object optimizedResult = extendedCel.createProgram(optimizedAst).eval(input); + + assertThat(optimizedResult).isEqualTo(extendedCel.createProgram(ast).eval(input)); + } + + @Test + public void optimize_scalarNotShadowedByComprehensionVar_rewrites( @TestParameter({ "[\"a\"].map(int, msg.single_int64)", "[true].map(string, msg.single_string)", - "[1].map(google, msg.single_duration)", - "[1].map(cel, msg.single_nested_message)", - "cel.bind(int, 1, msg.single_int64 + int)", - "[1].map(int, [2].map(x, msg.single_int64))", - "[1].map(int, [msg.single_int64].map(x, x + 1))" + "cel.bind(int, 1, msg.single_int64 + int)" }) String expression) throws Exception { @@ -471,10 +545,10 @@ public void optimize_typeIdentShadowedByComprehensionVar_throws( CelAbstractSyntaxTree ast = bindingsCel.compile(expression).getAst(); CelOptimizer optimizer = newSelectOptimizer(bindingsCel); - CelOptimizationException e = - assertThrows(CelOptimizationException.class, () -> optimizer.optimize(ast)); + CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast); - assertThat(e).hasMessageThat().contains("is shadowed by an enclosing comprehension variable"); + assertThat(optimizedAst.getSource().getExtensions()) + .contains(SelectOptimizer.SELECT_OPTIMIZATION_AST_EXTENSION_TAG); } @Test @@ -484,7 +558,7 @@ public void optimize_comprehensionVarNotShadowingTypeIdent_rewrites() throws Exc CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("[1].map(x, cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int))"); + .isEqualTo("[1].map(x, cel.@attribute(msg, [[2, \"single_int64\", 3]], 0))"); } @Test @@ -499,60 +573,51 @@ public void optimize_hasFieldInsideComprehensionShadowingFieldType_rewritesToHas } @Test - public void optimize_typeIdentInComprehensionRange_rewrites() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("[msg.single_int64].map(int, int + 1)").getAst(); + public void optimize_messageNameInComprehensionRange_rewrites() throws Exception { + CelAbstractSyntaxTree ast = cel.compile("[msg.single_nested_message].map(cel, cel)").getAst(); CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("[cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int)].map(int, int + 1)"); + .isEqualTo( + "[cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{})].map(cel, cel)"); } @Test - public void optimize_typeIdentInBindInit_rewrites() throws Exception { + public void optimize_messageNameInBindInit_rewrites() throws Exception { Cel bindingsCel = cel.toCelBuilder().addCompilerLibraries(CelExtensions.bindings()).build(); CelAbstractSyntaxTree ast = - bindingsCel.compile("cel.bind(int, msg.single_int64, int + 1)").getAst(); + bindingsCel.compile("cel.bind(cel, msg.single_nested_message, cel)").getAst(); CelAbstractSyntaxTree optimizedAst = newSelectOptimizer(bindingsCel).optimize(ast); assertThat(CEL_UNPARSER.unparse(optimizedAst)) .isEqualTo( - "cel.bind(int, cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int), int + 1)"); - } - - @Test - public void optimize_typeIdentShadowedByComprehensionIterVar2_throws() throws Exception { - Cel celWithComprehensions = - cel.toCelBuilder().addCompilerLibraries(CelExtensions.comprehensions()).build(); - CelAbstractSyntaxTree ast = - celWithComprehensions.compile("[1].all(x, int, msg.single_int64 == 0)").getAst(); - CelOptimizer optimizer = newSelectOptimizer(celWithComprehensions); - - CelOptimizationException e = - assertThrows(CelOptimizationException.class, () -> optimizer.optimize(ast)); - - assertThat(e) - .hasMessageThat() - .contains( - "cel.@attribute type identifier 'int' is shadowed by an enclosing comprehension" - + " variable 'int'"); + "cel.bind(cel, cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," + + " .cel.expr.conformance.proto3.TestAllTypes.NestedMessage{}), cel)"); } @Test - public void optimize_repeatedOrMapFieldShadowedByComprehensionVar_throws( - @TestParameter({ - "[[1]].map(list, msg.repeated_int64)", - "[[1]].map(map, msg.map_int64_message)" - }) - String expression) - throws Exception { + @TestParameters({ + "{expression: '[1].map(google, msg.single_duration)', expected: '[1].map(google," + + " cel.@attribute(msg, [[101, \"single_duration\", 11]], .google.protobuf.Duration{}))'}", + "{expression: '[1].map(duration, msg.single_duration)', expected: '[1].map(duration," + + " cel.@attribute(msg, [[101, \"single_duration\", 11]], .google.protobuf.Duration{}))'}", + "{expression: '[1].map(timestamp, msg.single_timestamp)', expected: '[1].map(timestamp," + + " cel.@attribute(msg, [[102, \"single_timestamp\", 11]]," + + " .google.protobuf.Timestamp{}))'}", + "{expression: '[1].map(duration, msg.map_string_duration)', expected: '[1].map(duration," + + " cel.@attribute(msg, [[243, \"map_string_duration\", -1, [9, 11]]]," + + " {\"\": .google.protobuf.Duration{}}))'}" + }) + public void optimize_durationOrTimestampUnderShadowingComprehensionVar_rewrites( + String expression, String expected) throws Exception { CelAbstractSyntaxTree ast = cel.compile(expression).getAst(); - CelOptimizationException e = - assertThrows(CelOptimizationException.class, () -> celOptimizer.optimize(ast)); + CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast); - assertThat(e).hasMessageThat().contains("is shadowed by an enclosing comprehension variable"); + assertThat(CEL_UNPARSER.unparse(optimizedAst)).isEqualTo(expected); } @Test @@ -569,7 +634,7 @@ public void optimize_messageTypeIdentUnderProtoPackageContainer_rewrites() throw assertThat(CEL_UNPARSER.unparse(optimizedAst)) .isEqualTo( "cel.@attribute(msg, [[21, \"single_nested_message\", 11]]," - + " cel.expr.conformance.proto3.TestAllTypes.NestedMessage)"); + + " cel.expr.conformance.proto3.TestAllTypes.NestedMessage{})"); } @Test @@ -584,7 +649,7 @@ public void newInstance_withOptionsAndFileDescriptors_preservesAddedDescriptors( CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast, cel).optimizedAst(); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"); + .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"); } @Test @@ -999,86 +1064,41 @@ public void optimize_iterationLimitReached_throws() throws Exception { assertThat(e).hasMessageThat().isEqualTo("Max iteration count reached."); } - @Test - public void optimize_structField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("msg.single_struct").getAst(); - SelectOptimizer optimizer = SelectOptimizer.newInstance(TestAllTypes.getDescriptor().getFile()); - - UnsupportedOperationException e = - assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); - - assertThat(e) - .hasMessageThat() - .contains("Optimization of Struct fields is currently unimplemented"); - } - - @Test - public void optimize_listValueField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("msg.list_value").getAst(); - SelectOptimizer optimizer = SelectOptimizer.newInstance(TestAllTypes.getDescriptor().getFile()); - - UnsupportedOperationException e = - assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); - - assertThat(e) - .hasMessageThat() - .contains("Optimization of ListValue fields is currently unimplemented"); - } - - @Test - public void optimize_valueField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("msg.single_value").getAst(); - SelectOptimizer optimizer = SelectOptimizer.newInstance(TestAllTypes.getDescriptor().getFile()); - - UnsupportedOperationException e = - assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); - - assertThat(e) - .hasMessageThat() - .contains("Optimization of Value fields is currently unimplemented"); - } + private enum UnsupportedWellKnownFieldTestCase { + STRUCT("msg.single_struct", "Optimization of Struct fields is currently unimplemented"), + LIST_VALUE("msg.list_value", "Optimization of ListValue fields is currently unimplemented"), + VALUE("msg.single_value", "Optimization of Value fields is currently unimplemented"), + ANY("msg.single_any", "Optimization of Any fields is currently unimplemented"), + WRAPPER( + "msg.single_int64_wrapper", "Optimization of wrapper fields is currently unimplemented"); - @Test - public void optimize_anyField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("msg.single_any").getAst(); - SelectOptimizer optimizer = SelectOptimizer.newInstance(TestAllTypes.getDescriptor().getFile()); - - UnsupportedOperationException e = - assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); + private final String expression; + private final String expectedMessageSubstring; - assertThat(e) - .hasMessageThat() - .contains("Optimization of Any fields is currently unimplemented"); + UnsupportedWellKnownFieldTestCase(String expression, String expectedMessageSubstring) { + this.expression = expression; + this.expectedMessageSubstring = expectedMessageSubstring; + } } @Test - public void optimize_wrapperField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("msg.single_int64_wrapper").getAst(); + public void optimize_unsupportedWellKnownField_throwsUnsupportedOperationException( + @TestParameter UnsupportedWellKnownFieldTestCase testCase) throws Exception { + CelAbstractSyntaxTree ast = cel.compile(testCase.expression).getAst(); SelectOptimizer optimizer = SelectOptimizer.newInstance(TestAllTypes.getDescriptor().getFile()); UnsupportedOperationException e = assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); - assertThat(e) - .hasMessageThat() - .contains("Optimization of wrapper fields is currently unimplemented"); - } - - @Test - public void optimize_groupField_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("proto2_msg.nestedgroup.single_id").getAst(); - SelectOptimizer optimizer = - SelectOptimizer.newInstance(PROTO2_TEST_ALL_TYPES_DESCRIPTOR.getFile()); - - UnsupportedOperationException e = - assertThrows(UnsupportedOperationException.class, () -> optimizer.optimize(ast, cel)); - - assertThat(e).hasMessageThat().contains("Optimization of Group fields is unsupported"); + assertThat(e).hasMessageThat().contains(testCase.expectedMessageSubstring); } @Test - public void optimize_groupFieldLeaf_throwsUnsupportedOperationException() throws Exception { - CelAbstractSyntaxTree ast = cel.compile("proto2_msg.nestedgroup").getAst(); + public void optimize_groupField_throwsUnsupportedOperationException( + @TestParameter({"proto2_msg.nestedgroup.single_id", "proto2_msg.nestedgroup"}) + String expression) + throws Exception { + CelAbstractSyntaxTree ast = cel.compile(expression).getAst(); SelectOptimizer optimizer = SelectOptimizer.newInstance(PROTO2_TEST_ALL_TYPES_DESCRIPTOR.getFile()); @@ -1097,7 +1117,7 @@ public void newInstance_fileDescriptorsVarargs_defaultOptions_success() throws E CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast, cel).optimizedAst(); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"); + .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"); } @Test @@ -1109,7 +1129,7 @@ public void newInstance_fileDescriptorsIterable_defaultOptions_success() throws CelAbstractSyntaxTree optimizedAst = optimizer.optimize(ast, cel).optimizedAst(); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"); + .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"); } @Test @@ -1127,7 +1147,7 @@ public void newInstance_fileDescriptorsIterable_defaultOptions_success() throws CelAbstractSyntaxTree proto3Optimized = optimizer.optimize(proto3Ast, cel).optimizedAst(); assertThat(CEL_UNPARSER.unparse(proto2Optimized)) - .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"); + .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"); assertThat(CEL_UNPARSER.unparse(proto3Optimized)).isEqualTo("msg.single_int64"); } @@ -1147,18 +1167,18 @@ public void newInstance_fileDescriptorsIterable_defaultOptions_success() throws CelAbstractSyntaxTree proto3Optimized = optimizer.optimize(proto3Ast, cel).optimizedAst(); assertThat(CEL_UNPARSER.unparse(proto2Optimized)) - .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3, -64]], int)"); + .isEqualTo("cel.@attribute(proto2_msg, [[2, \"single_int64\", 3]], -64)"); assertThat(CEL_UNPARSER.unparse(proto3Optimized)).isEqualTo("msg.single_int64"); } private enum CompilerRejectionTestCase { ATTRIBUTE_AT_SIGN( SelectOptimizer.CEL_ATTRIBUTE_FUNCTION_DECL, - "cel.@attribute(msg, [], int)", + "cel.@attribute(msg, [], 0)", "token recognition error at: '@'"), ATTRIBUTE_OVERLOAD( SelectOptimizer.CEL_ATTRIBUTE_FUNCTION_DECL, - "cel_attribute_list(msg, [], int)", + "cel_attribute_list(msg, [], 0)", "undeclared reference to 'cel_attribute_list'"), HAS_FIELD_AT_SIGN( SelectOptimizer.CEL_HAS_FIELD_FUNCTION_DECL, @@ -1255,20 +1275,14 @@ public void optimize_toParsedExpr_matchesExpectedSerializedProto() throws Except + " int64_value: 5\n" + " }\n" + " }\n" - + " elements {\n" - + " id: 12\n" - + " const_expr {\n" - + " int64_value: 0\n" - + " }\n" - + " }\n" + " }\n" + " }\n" + " }\n" + " }\n" + " args {\n" - + " id: 13\n" - + " ident_expr {\n" - + " name: \"int\"\n" + + " id: 12\n" + + " const_expr {\n" + + " int64_value: 0\n" + " }\n" + " }\n" + " }\n" @@ -1301,7 +1315,7 @@ public void optimize_toParsedExpr_matchesExpectedSerializedProto() throws Except assertThat(optimizedAst.getResultType()).isEqualTo(SimpleType.INT); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3, 0]], int) + 1"); + .isEqualTo("cel.@attribute(msg, [[2, \"single_int64\", 3]], 0) + 1"); } @Test @@ -1314,7 +1328,7 @@ public void optimize_toParsedExpr_matchesExpectedSerializedProto() throws Except assertThat(optimizedAst.getResultType()).isEqualTo(SimpleType.STRING); assertThat(CEL_UNPARSER.unparse(optimizedAst)) - .isEqualTo("cel.@attribute(msg, [[14, \"single_string\", 9, \"\"]], string) + \"suffix\""); + .isEqualTo("cel.@attribute(msg, [[14, \"single_string\", 9]], \"\") + \"suffix\""); } @Test @@ -1357,6 +1371,56 @@ public void optimize_binaryOperationBetweenOptimizedSelects_resolvesSingleOverlo assertThat(addReference.overloadIds()).containsExactly("add_int64"); } + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum StaticOverloadOnCollectionTestCase { + REPEATED_SCALAR_INDEX_ADD("msg.repeated_int64[0] + 1", SimpleType.INT, "add_int64"), + REPEATED_SUBMESSAGE_INDEX_ADD( + "msg.repeated_nested_message[0].bb + 1", SimpleType.INT, "add_int64"), + MAP_SCALAR_INDEX_CONCAT( + "msg.map_string_string['k'] + '_suffix'", SimpleType.STRING, "add_string"), + NESTED_MAP_AND_REPEATED_SUBMESSAGE_INDEX_ADD( + "msg.map_int64_nested_type[1].child.payload.repeated_nested_message[0].bb + 10", + SimpleType.INT, + "add_int64"); + + private final String expression; + private final Object expectedResultType; + private final String expectedRootOverloadId; + + StaticOverloadOnCollectionTestCase( + String expression, Object expectedResultType, String expectedRootOverloadId) { + this.expression = expression; + this.expectedResultType = expectedResultType; + this.expectedRootOverloadId = expectedRootOverloadId; + } + } + + @Test + public void + optimize_collectionIndexingAndFieldSelection_resolvesStaticOverloadsWithoutDynDispatch( + @TestParameter StaticOverloadOnCollectionTestCase testCase) throws Exception { + CelAbstractSyntaxTree ast = cel.compile(testCase.expression).getAst(); + + CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast); + + assertThat(optimizedAst.getResultType()).isEqualTo(testCase.expectedResultType); + CelReference rootRef = optimizedAst.getReferenceOrThrow(optimizedAst.getExpr().id()); + assertThat(rootRef.overloadIds()).containsExactly(testCase.expectedRootOverloadId); + assertThat(optimizedAst.getTypeMap().values()).doesNotContain(SimpleType.DYN); + } + + @Test + public void optimize_comprehensionOverRepeatedMessageField_preservesStaticElementTypesInTypeMap() + throws Exception { + CelAbstractSyntaxTree ast = + cel.compile("msg.repeated_nested_message.map(x, x.bb + 1)").getAst(); + + CelAbstractSyntaxTree optimizedAst = celOptimizer.optimize(ast); + + assertThat(optimizedAst.getResultType()).isEqualTo(ListType.create(SimpleType.INT)); + assertThat(optimizedAst.getTypeMap().values()).doesNotContain(SimpleType.DYN); + } + @Test public void optimize_optionalSelect_passesThroughUntouched() throws Exception { Cel celWithOptional = diff --git a/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java index 349b57bf7..a1687652f 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java @@ -18,17 +18,15 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.Immutable; import dev.cel.common.ast.CelConstant; import dev.cel.common.ast.CelExpr; -import dev.cel.common.ast.CelExpr.CelCall; +import dev.cel.common.ast.CelExpr.CelMap; import dev.cel.common.ast.CelExpr.ExprKind.Kind; -import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypes; -import dev.cel.common.types.SimpleType; import dev.cel.common.values.CelByteString; import dev.cel.common.values.CelValueConverter; import dev.cel.common.values.OptimizedSelectTraversal; @@ -36,11 +34,7 @@ import dev.cel.common.values.SelectField; import java.time.Duration; import java.time.Instant; -import java.util.List; -import java.util.Map; -import java.util.Objects; import java.util.Optional; -import org.jspecify.annotations.Nullable; /** * Plans optimizer-rewritten select chains ({@code cel.@attribute}) and presence tests ({@code @@ -53,11 +47,6 @@ final class OptimizedSelectPlanner { static final String CEL_ATTRIBUTE_FUNCTION_NAME = "cel.@attribute"; static final String CEL_HAS_FIELD_FUNCTION_NAME = "cel.@hasField"; - private static final String MAP_TYPE_IDENT = "map"; - private static final String LIST_TYPE_IDENT = "list"; - private static final String DURATION_TYPE_IDENT = SimpleType.DURATION.name(); - private static final String TIMESTAMP_TYPE_IDENT = SimpleType.TIMESTAMP.name(); - /** * Well-known message types whose CEL semantics (Any unpacking, JSON value conversion) are not * implemented by the optimized traversal. Wrapper types are rejected via {@link @@ -73,6 +62,11 @@ final class OptimizedSelectPlanner { private final AttributeFactory attributeFactory; private final CelValueConverter celValueConverter; + static OptimizedSelectPlanner create( + AttributeFactory attributeFactory, CelValueConverter celValueConverter) { + return new OptimizedSelectPlanner(attributeFactory, celValueConverter); + } + PlannedInterpretable plan(CelExpr expr, String functionName, OperandPlanner operandPlanner) { ImmutableList args = expr.call().args(); ImmutableList selectFields; @@ -82,8 +76,7 @@ PlannedInterpretable plan(CelExpr expr, String functionName, OperandPlanner oper { checkArgument( args.size() == 3, "Expected 3 arguments for %s, found %s", functionName, args.size()); - String typeIdent = extractQualifiedName(args.get(2)); - selectFields = unpackAttributeFields(args.get(1), typeIdent); + selectFields = unpackAttributeFields(args.get(1), args.get(2)); isPresenceTest = false; break; } @@ -114,7 +107,7 @@ PlannedInterpretable plan(CelExpr expr, String functionName, OperandPlanner oper } private static ImmutableList unpackAttributeFields( - CelExpr qualifiersExpr, String typeIdent) { + CelExpr qualifiersExpr, CelExpr dummyOrDefExpr) { ImmutableList elements = unpackQualifierHops(qualifiersExpr); ImmutableList.Builder fieldsBuilder = ImmutableList.builderWithExpectedSize(elements.size()); @@ -126,10 +119,6 @@ private static ImmutableList unpackAttributeFields( hopElements.size() == 3 || hopElements.size() == 4, "Expected qualifier hop for cel.@attribute to contain 3 or 4 elements, found: %s", hopElements.size()); - checkArgument( - isLeaf || hopElements.size() == 3, - "Non-leaf qualifier hop must not contain a default value: %s", - hopExpr); long fieldNumber = parseFieldNumber(hopElements.get(0)); String fieldName = parseFieldName(hopElements.get(1)); long rawTypeCode = parseTypeCode(hopElements.get(2)); @@ -137,33 +126,92 @@ private static ImmutableList unpackAttributeFields( SelectField.isSupportedTypeCode(rawTypeCode), "Invalid protobuf type code: %s", rawTypeCode); - checkArgument( - isLeaf || rawTypeCode == SelectField.MESSAGE_TYPE_CODE, - "Non-leaf qualifier hop must have MESSAGE type code (11), found: %s", - rawTypeCode); - Object defaultValue = - (hopElements.size() == 4) ? resolveDefaultValue(hopElements.get(3)) : null; - String protoTypeName = ""; - if (isLeaf) { - validateLeafTypeIdent((int) rawTypeCode, defaultValue, typeIdent); - // Preserve the full protobuf message name from typeIdent so descriptorless runtime - // evaluation (version skew) can identify well-known types (e.g. Duration, Timestamp) and - // populate RawProtoMessageLiteValue's type. - // Repeated message fields also use MESSAGE_TYPE_CODE (11) for their element wire type, but - // their typeIdent is "list" rather than the proto message name (map fields use - // CEL_MAP_TYPE_CODE (100) instead of 11, so they are already excluded). - boolean isSingularMessage = - rawTypeCode == SelectField.MESSAGE_TYPE_CODE && !typeIdent.equals(LIST_TYPE_IDENT); - if (isSingularMessage) { - protoTypeName = typeIdent; - } + + if (!isLeaf) { + checkArgument( + hopElements.size() == 3, + "Non-leaf qualifier hop must contain exactly 3 elements: %s", + hopExpr); + checkArgument( + rawTypeCode == SelectField.MESSAGE_TYPE_CODE, + "Non-leaf qualifier hop must have MESSAGE type code (11), found: %s", + rawTypeCode); + fieldsBuilder.add(SelectField.create(fieldNumber, fieldName, rawTypeCode)); + } else if (rawTypeCode == SelectField.CEL_MAP_TYPE_CODE) { + checkArgument( + hopElements.size() == 4, + "Map qualifier hop must contain exactly 4 elements: %s", + hopExpr); + fieldsBuilder.add(parseMapLeaf(fieldNumber, fieldName, hopElements.get(3), dummyOrDefExpr)); + } else { + checkArgument( + hopElements.size() == 3, + "Leaf qualifier hop must contain exactly 3 elements, found: %s", + hopExpr); + fieldsBuilder.add( + dummyOrDefExpr.getKind() == Kind.LIST + ? parseRepeatedLeaf(fieldNumber, fieldName, rawTypeCode, dummyOrDefExpr) + : parseSingularLeaf(fieldNumber, fieldName, rawTypeCode, dummyOrDefExpr)); } - fieldsBuilder.add( - SelectField.create(fieldNumber, fieldName, rawTypeCode, defaultValue, protoTypeName)); } return fieldsBuilder.build(); } + private static SelectField parseMapLeaf( + long fieldNumber, String fieldName, CelExpr mapEntrySpecExpr, CelExpr dummyExpr) { + SelectField.MapEntrySpec mapEntrySpec = parseMapEntrySpec(mapEntrySpecExpr); + checkArgument( + dummyExpr.getKind() == Kind.MAP && dummyExpr.map().entries().size() == 1, + "Expected map dummy/default value with a single entry, found: %s", + dummyExpr); + CelMap.Entry entry = dummyExpr.map().entries().get(0); + validateScalarDummy(mapEntrySpec.keyTypeCode(), entry.key()); + if (mapEntrySpec.valueTypeCode() == SelectField.MESSAGE_TYPE_CODE) { + // Unlike leaf fields, map values of Any, Struct, Value, and wrapper types are permitted: + // the traversal only produces the map, and indexing it applies standard CEL conversion. + return SelectField.createMap( + fieldNumber, fieldName, mapEntrySpec, parseMessageProtoTypeName(entry.value())); + } + validateScalarDummy(mapEntrySpec.valueTypeCode(), entry.value()); + return SelectField.createMap(fieldNumber, fieldName, mapEntrySpec); + } + + private static SelectField parseRepeatedLeaf( + long fieldNumber, String fieldName, long typeCode, CelExpr dummyExpr) { + checkArgument( + dummyExpr.list().elements().size() == 1, + "Expected repeated dummy/default value with a single element, found: %s", + dummyExpr); + CelExpr elemExpr = dummyExpr.list().elements().get(0); + if (typeCode == SelectField.MESSAGE_TYPE_CODE) { + return SelectField.create( + fieldNumber, + fieldName, + typeCode, + /* defaultValue= */ ImmutableList.of(), + parseLeafMessageProtoTypeName(elemExpr)); + } + validateScalarDummy(typeCode, elemExpr); + return SelectField.create( + fieldNumber, fieldName, typeCode, /* defaultValue= */ ImmutableList.of()); + } + + private static SelectField parseSingularLeaf( + long fieldNumber, String fieldName, long typeCode, CelExpr defaultExpr) { + if (typeCode != SelectField.MESSAGE_TYPE_CODE) { + return SelectField.create( + fieldNumber, fieldName, typeCode, resolveScalarDefaultValue(typeCode, defaultExpr)); + } + String protoTypeName = parseLeafMessageProtoTypeName(defaultExpr); + Object defaultValue = null; + if (protoTypeName.equals(CelTypes.DURATION_MESSAGE)) { + defaultValue = Duration.ZERO; + } else if (protoTypeName.equals(CelTypes.TIMESTAMP_MESSAGE)) { + defaultValue = Instant.EPOCH; + } + return SelectField.create(fieldNumber, fieldName, typeCode, defaultValue, protoTypeName); + } + private static ImmutableList unpackHasFieldFields(CelExpr qualifiersExpr) { ImmutableList elements = unpackQualifierHops(qualifiersExpr); ImmutableList.Builder fieldsBuilder = @@ -226,124 +274,58 @@ private static long parseTypeCode(CelExpr expr) { return expr.constant().int64Value(); } - private static void validateLeafTypeIdent( - int leafTypeCode, @Nullable Object defaultValue, String typeIdent) { + private static SelectField.MapEntrySpec parseMapEntrySpec(CelExpr expr) { checkArgument( - defaultValue != null || leafTypeCode == SelectField.MESSAGE_TYPE_CODE, - "Leaf hop with type code %s must specify a default value", - leafTypeCode); + expr.getKind() == Kind.LIST, + "Expected map entry spec to be a list, found: %s", + expr.getKind()); + ImmutableList elements = expr.list().elements(); checkArgument( - (leafTypeCode == SelectField.CEL_MAP_TYPE_CODE) == typeIdent.equals(MAP_TYPE_IDENT), - "Leaf type code %s is incompatible with typeIdent '%s'", - leafTypeCode, - typeIdent); - checkArgument( - (defaultValue instanceof Map) == typeIdent.equals(MAP_TYPE_IDENT), - "Leaf default value %s is incompatible with typeIdent '%s'", - defaultValue, - typeIdent); - checkArgument( - (defaultValue instanceof List) == typeIdent.equals(LIST_TYPE_IDENT), - "Leaf default value %s is incompatible with typeIdent '%s'", - defaultValue, - typeIdent); - if (typeIdent.equals(MAP_TYPE_IDENT) || typeIdent.equals(LIST_TYPE_IDENT)) { - return; - } - if (leafTypeCode == SelectField.MESSAGE_TYPE_CODE) { - checkArgument( - !ScalarType.isScalarTypeIdent(typeIdent), - "Leaf MESSAGE type code (11) is incompatible with scalar typeIdent '%s'", - typeIdent); - checkArgument( - !CelTypes.isWrapperType(typeIdent) - && !UNSUPPORTED_WELL_KNOWN_TYPE_IDENTS.contains(typeIdent), - "Leaf well-known type '%s' is not supported by the select-optimized runtime", - typeIdent); - if (typeIdent.equals(DURATION_TYPE_IDENT)) { - checkArgument( - Objects.equals(defaultValue, Duration.ZERO), - "Leaf default value for message type '%s' is invalid or missing: %s", - typeIdent, - defaultValue); - } else if (typeIdent.equals(TIMESTAMP_TYPE_IDENT)) { - checkArgument( - Objects.equals(defaultValue, Instant.EPOCH), - "Leaf default value for message type '%s' is invalid or missing: %s", - typeIdent, - defaultValue); - } else { - checkArgument( - defaultValue == null, - "Leaf default value for message type '%s' is invalid or missing: %s", - typeIdent, - defaultValue); - } - return; - } - ScalarType expectedScalar = ScalarType.fromTypeCode(leafTypeCode); + elements.size() == 2, + "Expected map entry spec list to contain exactly 2 elements (key_type_code, val_type_code)," + + " found: %s", + elements.size()); + long keyTypeCode = parseTypeCode(elements.get(0)); + long valTypeCode = parseTypeCode(elements.get(1)); + return SelectField.MapEntrySpec.create(keyTypeCode, valTypeCode); + } + + private static String parseMessageProtoTypeName(CelExpr expr) { checkArgument( - typeIdent.equals(expectedScalar.typeIdent), - "Leaf type code %s (expected '%s') is incompatible with typeIdent '%s'", - leafTypeCode, - expectedScalar.typeIdent, - typeIdent); + expr.getKind() == Kind.STRUCT, "Expected struct for message type, found: %s", expr); + String protoTypeName = expr.struct().messageName(); + checkArgument(!protoTypeName.isEmpty(), "Protobuf message type name must not be empty"); + return protoTypeName; + } + + private static String parseLeafMessageProtoTypeName(CelExpr expr) { + String protoTypeName = parseMessageProtoTypeName(expr); checkArgument( - expectedScalar.valueClass.isInstance(defaultValue), - "Leaf default value %s is incompatible with typeIdent '%s'", - defaultValue, - expectedScalar.typeIdent); + !CelTypes.isWrapperType(protoTypeName) + && !UNSUPPORTED_WELL_KNOWN_TYPE_IDENTS.contains(protoTypeName), + "Leaf well-known type '%s' is not supported by the select-optimized runtime", + protoTypeName); + return protoTypeName; } - private static Object resolveDefaultValue(CelExpr defaultExpr) { - switch (defaultExpr.getKind()) { - case CONSTANT: - if (defaultExpr.constant().getKind() != CelConstant.Kind.NULL_VALUE) { - return PlannerHelpers.resolveConstant(defaultExpr.constant()); - } - break; - case LIST: - if (defaultExpr.list().elements().isEmpty()) { - return ImmutableList.of(); - } - break; - case MAP: - if (defaultExpr.map().entries().isEmpty()) { - return ImmutableMap.of(); - } - break; - case CALL: - CelCall call = defaultExpr.call(); - if (call.function().equals("duration") - && call.args().size() == 1 - && call.args().get(0).getKind() == Kind.CONSTANT - && call.args().get(0).constant().getKind() == CelConstant.Kind.STRING_VALUE - && call.args().get(0).constant().stringValue().equals("0s")) { - return Duration.ZERO; - } - if (call.function().equals("timestamp") - && call.args().size() == 1 - && call.args().get(0).getKind() == Kind.CONSTANT - && call.args().get(0).constant().getKind() == CelConstant.Kind.INT64_VALUE - && call.args().get(0).constant().int64Value() == 0L) { - return Instant.EPOCH; - } - break; - default: - break; - } - throw new IllegalArgumentException("Unsupported default value expression: " + defaultExpr); + private static Object resolveScalarDefaultValue(long typeCode, CelExpr defaultExpr) { + checkArgument( + defaultExpr.getKind() == Kind.CONSTANT + && defaultExpr.constant().getKind() != CelConstant.Kind.NULL_VALUE, + "Expected non-null constant default value expression, found: %s", + defaultExpr); + Object value = PlannerHelpers.resolveConstant(defaultExpr.constant()); + ScalarType expectedScalar = ScalarType.fromTypeCode((int) typeCode); + checkArgument( + expectedScalar.valueClass.isInstance(value), + "Scalar default value %s is incompatible with type code %s", + value, + typeCode); + return value; } - private static String extractQualifiedName(CelExpr expr) { - if (expr.getKind() == Kind.IDENT) { - return expr.ident().name(); - } - if (expr.getKind() == Kind.SELECT && !expr.select().testOnly()) { - return extractQualifiedName(expr.select().operand()) + "." + expr.select().field(); - } - throw new IllegalArgumentException( - "Expected type identifier argument to be an IDENT or non-testOnly SELECT, found: " + expr); + private static void validateScalarDummy(long typeCode, CelExpr dummyExpr) { + resolveScalarDefaultValue(typeCode, dummyExpr); } private static void validateTarget(Object target) { @@ -362,30 +344,19 @@ interface OperandPlanner { } private enum ScalarType { - BOOL(SimpleType.BOOL, Boolean.class), - INT(SimpleType.INT, Long.class), - UINT(SimpleType.UINT, UnsignedLong.class), - DOUBLE(SimpleType.DOUBLE, Double.class), - STRING(SimpleType.STRING, String.class), - BYTES(SimpleType.BYTES, CelByteString.class); - - private final String typeIdent; + BOOL(Boolean.class), + INT(Long.class), + UINT(UnsignedLong.class), + DOUBLE(Double.class), + STRING(String.class), + BYTES(CelByteString.class); + private final Class valueClass; - ScalarType(CelType celType, Class valueClass) { - this.typeIdent = celType.name(); + ScalarType(Class valueClass) { this.valueClass = valueClass; } - static boolean isScalarTypeIdent(String typeIdent) { - for (ScalarType type : values()) { - if (type.typeIdent.equals(typeIdent)) { - return true; - } - } - return false; - } - static ScalarType fromTypeCode(int leafTypeCode) { switch (leafTypeCode) { case 1: // DOUBLE @@ -411,7 +382,7 @@ static ScalarType fromTypeCode(int leafTypeCode) { case 12: // BYTES return BYTES; default: - throw new IllegalStateException("Unexpected leaf type code: " + leafTypeCode); + throw new IllegalArgumentException("Unexpected leaf type code: " + leafTypeCode); } } } @@ -448,7 +419,7 @@ private static final class OptimizedSelectQualifier implements Qualifier { @Override public Object value() { - return fields.get(fields.size() - 1).fieldName(); + return Iterables.getLast(fields).fieldName(); } @Override @@ -471,11 +442,6 @@ private OptimizedSelectQualifier( } } - static OptimizedSelectPlanner create( - AttributeFactory attributeFactory, CelValueConverter celValueConverter) { - return new OptimizedSelectPlanner(attributeFactory, celValueConverter); - } - private OptimizedSelectPlanner( AttributeFactory attributeFactory, CelValueConverter celValueConverter) { this.attributeFactory = checkNotNull(attributeFactory); diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java index 13dd99172..547ac664d 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.Assert.assertThrows; import com.google.api.expr.v1alpha1.CheckedExpr; import com.google.common.collect.ImmutableList; @@ -512,14 +511,12 @@ public void select_unsetUnknownMapField_returnsBakedDefault() throws Exception { } @Test - public void select_populatedUnknownMapField_throwsEvaluationException() { + public void select_populatedUnknownMapField_decodesMapFromWire() throws Exception { TestAllTypes msg = TestAllTypes.newBuilder().putMapInt32Int32(1, 2).build(); - CelEvaluationException thrown = - assertThrows(CelEvaluationException.class, () -> eval("msg.map_int32_int32", msg)); + Object result = eval("msg.map_int32_int32", msg); - assertThat(thrown).hasCauseThat().isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown).hasCauseThat().hasMessageThat().contains("map_int32_int32"); + assertThat(result).isEqualTo(ImmutableMap.of(1L, 2L)); } @SuppressWarnings("ImmutableEnumChecker") // Test only @@ -944,17 +941,6 @@ public void celBind_unknownSubmessageWithUnknownInnerField_whenUnset_evaluatesDe assertThat(result).isEqualTo(-1L); } - @Test - public void shortCircuiting_hasGuardOnPopulatedUnknownMap_skipsMapEvaluation() throws Exception { - // msg.map_int32_int32 is populated on POPULATED_SERVER_MESSAGE and would throw - // UnsupportedOperationException if selected, but short-circuiting avoids evaluating it. - Object result = - eval( - "!has(msg.map_int32_int32) ? size(msg.map_int32_int32) : 99", POPULATED_SERVER_MESSAGE); - - assertThat(result).isEqualTo(99L); - } - @Test public void shortCircuiting_hasGuardOnUnsetUnknownMap_evaluatesEmptyMapSize() throws Exception { Object result = @@ -1450,21 +1436,17 @@ public void celBind_unknownOuterSubmessageWithKnownInnerMapFields_decodesMapEntr .setOneofType(NestedTestAllTypes.newBuilder().setPayload(POPULATED_RENAMED_MESSAGE)) .build(); - // TODO: Restore assertion once SelectOptimizer emits MapEntrySpec in cl/990623531. - CelEvaluationException thrown = - assertThrows( - CelEvaluationException.class, - () -> - eval( - "cel.bind(sub, msg.oneof_type," - + " has(sub.payload.map_string_string) &&" - + " sub.payload.map_string_string['k'] == 'v' &&" - + " has(sub.payload.map_string_string.k) &&" - + " !has(sub.payload.map_string_string.missing) &&" - + " sub.payload.map_int64_message[1].bb == 100)", - msg)); + Object result = + eval( + "cel.bind(sub, msg.oneof_type," + + " has(sub.payload.map_string_string) &&" + + " sub.payload.map_string_string['k'] == 'v' &&" + + " has(sub.payload.map_string_string.k) &&" + + " !has(sub.payload.map_string_string.missing) &&" + + " sub.payload.map_int64_message[1].bb == 100)", + msg); - assertThat(thrown).hasCauseThat().isInstanceOf(UnsupportedOperationException.class); + assertThat(result).isEqualTo(true); } @Test @@ -1495,20 +1477,16 @@ public void celBind_unknownOuterSubmessageWithKnownInnerMapFields_decodesMapEntr .setOneofType(NestedTestAllTypes.newBuilder().setPayload(innerPayload)) .build(); - // TODO: Restore assertion once SelectOptimizer emits MapEntrySpec in cl/990623531. - CelEvaluationException thrown = - assertThrows( - CelEvaluationException.class, - () -> - eval( - "cel.bind(sub, msg.oneof_type," - + " size(sub.payload.map_string_string) == 3 &&" - + " sub.payload.map_string_string['dup'] == 'second' &&" - + " sub.payload.map_string_string[''] == 'val_for_default_key' &&" - + " sub.payload.map_string_string['key_with_default_val'] == '')", - msg)); + Object result = + eval( + "cel.bind(sub, msg.oneof_type," + + " size(sub.payload.map_string_string) == 3 &&" + + " sub.payload.map_string_string['dup'] == 'second' &&" + + " sub.payload.map_string_string[''] == 'val_for_default_key' &&" + + " sub.payload.map_string_string['key_with_default_val'] == '')", + msg); - assertThat(thrown).hasCauseThat().isInstanceOf(UnsupportedOperationException.class); + assertThat(result).isEqualTo(true); } @SuppressWarnings("ImmutableEnumChecker") // Test only @@ -1692,21 +1670,6 @@ public void wellKnownTypes_negativeDurationAndPreEpochTimestampOnUnknownFields_d assertThat(result).isEqualTo(true); } - @Test - public void comprehension_shortCircuitingPreventsEvaluatingPopulatedUnknownMap() - throws Exception { - // msg.map_int32_int32 is populated on POPULATED_SERVER_MESSAGE and throws - // UnsupportedOperationException if evaluated; short-circuiting on the first element (bb == 10) - // must avoid evaluating the map branch. - String expression = - "msg.repeated_nested_message.exists(x, x.bb == 10 || size(msg.map_int32_int32) > 0) &&" - + " !msg.repeated_nested_message.all(x, x.bb > 15 && size(msg.map_int32_int32) > 0)"; - - Object result = eval(expression, POPULATED_SERVER_MESSAGE); - - assertThat(result).isEqualTo(true); - } - @Test public void heterogeneousNumericComparisons_onUnknownWireDecodedFields_evaluatesCorrectly() throws Exception { diff --git a/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java index 403ae0d33..caa8c9ad3 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java @@ -494,7 +494,7 @@ public void plan_invalidAst_malformedHop_throwsEvaluationException() { ImmutableList.of(CelExpr.ofConstant(5L, CelConstant.ofValue(1L))), ImmutableList.of())), ImmutableList.of()), - CelExpr.ofIdent(6L, "int"))), + CelExpr.ofConstant(6L, CelConstant.ofValue(0L)))), CelSource.newBuilder().build()); CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); @@ -506,7 +506,7 @@ public void plan_invalidAst_malformedHop_throwsEvaluationException() { } @Test - public void plan_invalidAst_messageTypeCodeWithScalarTypeIdent_throwsEvaluationException() { + public void plan_invalidAst_messageTypeCodeWithScalarDummy_throwsEvaluationException() { CelAbstractSyntaxTree ast = CelAbstractSyntaxTree.newParsedAst( CelExpr.ofCall( @@ -521,19 +521,18 @@ public void plan_invalidAst_messageTypeCodeWithScalarTypeIdent_throwsEvaluationE 4L, ImmutableList.of( CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), - CelExpr.ofConstant(6L, CelConstant.ofValue("single_int32")), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("single_nested_message")), CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), ImmutableList.of())), ImmutableList.of()), - CelExpr.ofIdent(8L, "int"))), + CelExpr.ofConstant(8L, CelConstant.ofValue(0L)))), CelSource.newBuilder().build()); CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - assertThat(e) - .hasMessageThat() - .contains("Leaf MESSAGE type code (11) is incompatible with scalar typeIdent 'int'"); + assertThat(e).hasMessageThat().contains("Expected struct for message type, found:"); } @Test @@ -557,7 +556,7 @@ public void plan_invalidAst_unsupportedWellKnownType_throwsEvaluationException() CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), ImmutableList.of())), ImmutableList.of()), - CelExpr.ofIdent(8L, "google.protobuf.Int64Value"))), + CelExpr.ofStruct(8L, "google.protobuf.Int64Value", ImmutableList.of()))), CelSource.newBuilder().build()); CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); @@ -568,6 +567,346 @@ public void plan_invalidAst_unsupportedWellKnownType_throwsEvaluationException() .contains("Leaf well-known type 'google.protobuf.Int64Value' is not supported"); } + @Test + public void plan_invalidAst_nonMapHopWithFourElements_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(32L)), + CelExpr.ofConstant(6L, CelConstant.ofValue("repeated_int64")), + CelExpr.ofConstant(7L, CelConstant.ofValue(3L)), + CelExpr.ofConstant(8L, CelConstant.ofValue(0L))), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofList( + 9L, + ImmutableList.of(CelExpr.ofConstant(10L, CelConstant.ofValue(0L))), + ImmutableList.of()))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e).hasMessageThat().contains("Leaf qualifier hop must contain exactly 3 elements"); + } + + @Test + public void plan_invalidAst_mapHopWithMissingEntrySpec_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("map_string_string")), + CelExpr.ofConstant(7L, CelConstant.ofValue(-1L))), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofMap( + 8L, + ImmutableList.of( + CelExpr.ofMapEntry( + 9L, + CelExpr.ofConstant(10L, CelConstant.ofValue("")), + CelExpr.ofConstant(11L, CelConstant.ofValue("")), + /* isOptionalEntry= */ false))))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e).hasMessageThat().contains("Map qualifier hop must contain exactly 4 elements"); + } + + @Test + public void plan_invalidAst_mapEntrySpecMalformed_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("map_string_string")), + CelExpr.ofConstant(7L, CelConstant.ofValue(-1L)), + CelExpr.ofList( + 8L, + ImmutableList.of( + CelExpr.ofConstant(9L, CelConstant.ofValue(9L)), + CelExpr.ofConstant(10L, CelConstant.ofValue(9L)), + CelExpr.ofConstant(11L, CelConstant.ofValue(9L))), + ImmutableList.of())), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofMap( + 12L, + ImmutableList.of( + CelExpr.ofMapEntry( + 13L, + CelExpr.ofConstant(14L, CelConstant.ofValue("")), + CelExpr.ofConstant(15L, CelConstant.ofValue("")), + /* isOptionalEntry= */ false))))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e) + .hasMessageThat() + .contains( + "Expected map entry spec list to contain exactly 2 elements (key_type_code," + + " val_type_code), found: 3"); + } + + @Test + public void plan_invalidAst_repeatedFieldWithEmptyListDummy_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(51L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("repeated_nested_message")), + CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofList(8L, ImmutableList.of(), ImmutableList.of()))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e) + .hasMessageThat() + .contains("Expected repeated dummy/default value with a single element, found:"); + } + + private enum ScalarDummyMismatchTestCase { + MAP_KEY( + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), + CelExpr.ofConstant(6L, CelConstant.ofValue("map_string_string")), + CelExpr.ofConstant(7L, CelConstant.ofValue(-1L)), + mapEntrySpec(9L, 9L)), + CelExpr.ofMap( + 20L, + ImmutableList.of( + CelExpr.ofMapEntry( + 21L, + CelExpr.ofConstant(22L, CelConstant.ofValue(0L)), + CelExpr.ofConstant(23L, CelConstant.ofValue("")), + /* isOptionalEntry= */ false)))), + MAP_VALUE( + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), + CelExpr.ofConstant(6L, CelConstant.ofValue("map_string_string")), + CelExpr.ofConstant(7L, CelConstant.ofValue(-1L)), + mapEntrySpec(9L, 9L)), + CelExpr.ofMap( + 20L, + ImmutableList.of( + CelExpr.ofMapEntry( + 21L, + CelExpr.ofConstant(22L, CelConstant.ofValue("")), + CelExpr.ofConstant(23L, CelConstant.ofValue(0L)), + /* isOptionalEntry= */ false)))), + REPEATED_ELEMENT( + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(32L)), + CelExpr.ofConstant(6L, CelConstant.ofValue("repeated_int64")), + CelExpr.ofConstant(7L, CelConstant.ofValue(3L))), + CelExpr.ofList( + 20L, + ImmutableList.of(CelExpr.ofConstant(21L, CelConstant.ofValue(""))), + ImmutableList.of())); + + private final ImmutableList hopElements; + private final CelExpr dummy; + + private static CelExpr mapEntrySpec(long keyTypeCode, long valueTypeCode) { + return CelExpr.ofList( + 8L, + ImmutableList.of( + CelExpr.ofConstant(9L, CelConstant.ofValue(keyTypeCode)), + CelExpr.ofConstant(10L, CelConstant.ofValue(valueTypeCode))), + ImmutableList.of()); + } + + ScalarDummyMismatchTestCase(ImmutableList hopElements, CelExpr dummy) { + this.hopElements = hopElements; + this.dummy = dummy; + } + } + + @Test + public void plan_invalidAst_scalarDummyIncompatibleWithTypeCode_throwsEvaluationException( + @TestParameter ScalarDummyMismatchTestCase testCase) { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList(4L, testCase.hopElements, ImmutableList.of())), + ImmutableList.of()), + testCase.dummy)), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e).hasMessageThat().contains("is incompatible with type code"); + } + + @Test + public void plan_invalidAst_mapFieldWithEmptyMapDummy_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(1L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("map_string_string")), + CelExpr.ofConstant(7L, CelConstant.ofValue(-1L)), + CelExpr.ofList( + 8L, + ImmutableList.of( + CelExpr.ofConstant(9L, CelConstant.ofValue(9L)), + CelExpr.ofConstant(10L, CelConstant.ofValue(9L))), + ImmutableList.of())), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofMap(11L, ImmutableList.of()))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e) + .hasMessageThat() + .contains("Expected map dummy/default value with a single entry, found:"); + } + + @Test + public void plan_invalidAst_repeatedMessageWithUnsupportedWellKnownType_throwsEvaluationException( + @TestParameter({"google.protobuf.Int64Value", "google.protobuf.Any"}) + String unsupportedProtoTypeName) { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(51L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("repeated_nested_message")), + CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofList( + 8L, + ImmutableList.of( + CelExpr.ofStruct(9L, unsupportedProtoTypeName, ImmutableList.of())), + ImmutableList.of()))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e) + .hasMessageThat() + .contains("Leaf well-known type '" + unsupportedProtoTypeName + "' is not supported"); + } + + @Test + public void plan_invalidAst_messageStructWithEmptyMessageName_throwsEvaluationException() { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst( + CelExpr.ofCall( + 1L, + OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME, + ImmutableList.of( + CelExpr.ofIdent(2L, "msg"), + CelExpr.ofList( + 3L, + ImmutableList.of( + CelExpr.ofList( + 4L, + ImmutableList.of( + CelExpr.ofConstant(5L, CelConstant.ofValue(51L)), + CelExpr.ofConstant( + 6L, CelConstant.ofValue("repeated_nested_message")), + CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), + ImmutableList.of())), + ImmutableList.of()), + CelExpr.ofList( + 8L, + ImmutableList.of(CelExpr.ofStruct(9L, "", ImmutableList.of())), + ImmutableList.of()))), + CelSource.newBuilder().build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); + assertThat(e).hasMessageThat().contains("Protobuf message type name must not be empty"); + } + private static CelAbstractSyntaxTree optimizeSelectAst(String expression) throws Exception { CelAbstractSyntaxTree ast = CEL.compile(expression).getAst(); CelAbstractSyntaxTree optimizedAst = SELECT_OPTIMIZER.optimize(ast);