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 71a7aed38..5cd15f818 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/SelectOptimizer.java @@ -62,7 +62,6 @@ import dev.cel.common.navigation.TraversalOrder; import dev.cel.common.types.CelKind; import dev.cel.common.types.CelType; -import dev.cel.common.types.CelTypes; import dev.cel.common.types.ListType; import dev.cel.common.types.OptionalType; import dev.cel.common.types.SimpleType; @@ -139,6 +138,10 @@ * from {@code google.protobuf.Any}, or evaluated via classless wire payloads ({@code * RawProtoMessageLiteValue}), no cross-check between the embedded field number and field name * is performed at runtime. + *
  • Selection through {@code google.protobuf.Any}: The optimized chain ends at an {@code + * Any} field, whose packed type is only known at runtime. Fields selected past it ({@code + * msg.any_field.foo}) are resolved by name, so they are not rename-resilient and {@code + * CelLiteRuntime} needs the packed message's {@code CelLiteDescriptor}. *
  • Oneofs: The lite runtime reads fields from the wire, so it reports every serialized * member of a oneof as present. Protobuf serializes at most one, unless an older-schema * binary merges or mutates the message, or the bytes are concatenated or altered by hand. @@ -323,12 +326,6 @@ private static CelMutableExpr newFieldQualifier( throw new UnsupportedOperationException( "Optimization of Group fields is unsupported: " + field.getFullName()); } - // TODO: Support Any. - if (field.getType() == FieldDescriptor.Type.MESSAGE - && field.getMessageType().getFullName().equals(CelTypes.ANY_MESSAGE)) { - throw new UnsupportedOperationException( - "Optimization of Any fields is currently unimplemented: " + field.getFullName()); - } CelMutableList qualifierElements = CelMutableList.create( 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 ce5472ebf..85dfcb613 100644 --- a/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java +++ b/optimizer/src/test/java/dev/cel/optimizer/optimizers/SelectOptimizerTest.java @@ -417,6 +417,16 @@ private enum RewriteTestCase { "msg.map_string_any['k'].value", "cel.@attribute(msg, [[311, \"map_string_any\", -1, [9, 11]]]," + " {\"\": google.protobuf.Any{}})[\"k\"].value"), + ANY_FIELD( + "msg.single_any", + "cel.@attribute(msg, [[100, \"single_any\", 11]], google.protobuf.Any{})"), + ANY_FIELD_HAS("has(msg.single_any)", "cel.@hasField(msg, [[100, \"single_any\"]])"), + REPEATED_ANY_FIELD( + "msg.repeated_any", + "cel.@attribute(msg, [[120, \"repeated_any\", 11]], [google.protobuf.Any{}])"), + ANY_FIELD_SELECT_STOPS_AT_ANY_BOUNDARY( + "msg.single_any.single_int64", + "cel.@attribute(msg, [[100, \"single_any\", 11]], google.protobuf.Any{}).single_int64"), MIXED_BOOLEAN_EXPRESSION( "msg.single_int64 > 0 && has(msg.single_nested_message)", @@ -877,6 +887,26 @@ private enum NativeSelectEvaluationTestCase { "msg", TestAllTypes.newBuilder().putMapStringAny("k", Any.pack(Int64Value.of(5))).build()), 5L), + PROTO3_ANY_UNPACKS_VALUE( + "msg.single_any", + ImmutableMap.of( + "msg", TestAllTypes.newBuilder().setSingleAny(Any.pack(Int64Value.of(5))).build()), + 5L), + PROTO3_REPEATED_ANY_UNPACKS_VALUES( + "msg.repeated_any", + ImmutableMap.of( + "msg", TestAllTypes.newBuilder().addRepeatedAny(Any.pack(Int64Value.of(5))).build()), + ImmutableList.of(5L)), + PROTO3_UNSET_ANY_HAS_IS_FALSE( + "has(msg.single_any)", ImmutableMap.of("msg", TestAllTypes.getDefaultInstance()), false), + PROTO3_ANY_PACKED_MESSAGE_SELECTS_BY_NAME( + "msg.single_any.single_int64", + ImmutableMap.of( + "msg", + TestAllTypes.newBuilder() + .setSingleAny(Any.pack(TestAllTypes.newBuilder().setSingleInt64(7L).build())) + .build()), + 7L), PROTO3_MAP_OF_WRAPPER_UNWRAPS_MAP_VALUES( "msg.map_string_int64_wrapper", ImmutableMap.of( @@ -1200,8 +1230,9 @@ public void optimize_constantIndexThroughCustomOverload_leavesIndexNested() thro } @Test - public void optimizeAndEvaluate_missingIndexedElement_throwsSameErrorAsUnoptimized( + public void optimizeAndEvaluate_evaluationError_throwsSameErrorAsUnoptimized( @TestParameter({ + "msg.single_any", "msg.repeated_nested_message[1].bb", "msg.repeated_nested_message[5000000000].bb", "msg.map_int64_message[2].bb", @@ -1317,20 +1348,6 @@ public void optimize_iterationLimitReached_throws() throws Exception { assertThat(e).hasMessageThat().isEqualTo("Max iteration count reached."); } - @Test - public void optimize_unsupportedWellKnownField_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)); - - assertThat(e) - .hasMessageThat() - .contains("Optimization of Any fields is currently unimplemented"); - } - @Test public void optimize_groupField_throwsUnsupportedOperationException( @TestParameter({"proto2_msg.nestedgroup.single_id", "proto2_msg.nestedgroup"}) 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 033ab1f48..6b6cd7559 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/OptimizedSelectPlanner.java @@ -192,8 +192,6 @@ private static SelectField parseMapLeaf( 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 unsupported well-known 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())); } @@ -233,7 +231,7 @@ private static SelectField parseRepeatedLeaf( fieldName, typeCode, /* defaultValue= */ ImmutableList.of(), - parseLeafMessageProtoTypeName(elemExpr)); + parseMessageProtoTypeName(elemExpr)); } validateScalarDummy(typeCode, elemExpr); return SelectField.create( @@ -246,7 +244,7 @@ private static SelectField parseSingularLeaf( return SelectField.create( fieldNumber, fieldName, typeCode, resolveScalarDefaultValue(typeCode, defaultExpr)); } - String protoTypeName = parseLeafMessageProtoTypeName(defaultExpr); + String protoTypeName = parseMessageProtoTypeName(defaultExpr); Object defaultValue = null; if (protoTypeName.equals(CelTypes.DURATION_MESSAGE)) { defaultValue = Duration.ZERO; @@ -341,16 +339,6 @@ private static String parseMessageProtoTypeName(CelExpr expr) { return protoTypeName; } - private static String parseLeafMessageProtoTypeName(CelExpr expr) { - String protoTypeName = parseMessageProtoTypeName(expr); - // TODO: Support Any. - checkArgument( - !protoTypeName.equals(CelTypes.ANY_MESSAGE), - "Leaf well-known type '%s' is not supported by the select-optimized runtime", - protoTypeName); - return protoTypeName; - } - private static Object resolveScalarDefaultValue(long typeCode, CelExpr defaultExpr) { checkArgument( defaultExpr.getKind() == Kind.CONSTANT diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java index 9b33582c9..eed9a88b7 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java @@ -2300,6 +2300,23 @@ private enum DescriptorlessEvaluationTestCase { .addRepeatedValue(Value.newBuilder().setNullValue(NullValue.NULL_VALUE)) .build(), true), + ANY_FIELDS( + "msg.single_any == 5 && msg.repeated_any == [5] && has(msg.single_any)" + + " && msg.map_string_any['k'] == 5 && msg.oneof_type.payload.single_any == 5" + + " && has(msg.oneof_type.payload.single_any)", + TestAllTypes.newBuilder() + .setSingleAny(Any.pack(Int64Value.of(5))) + .addRepeatedAny(Any.pack(Int64Value.of(5))) + .putMapStringAny("k", Any.pack(Int64Value.of(5))) + .setOneofType( + NestedTestAllTypes.newBuilder() + .setPayload(TestAllTypes.newBuilder().setSingleAny(Any.pack(Int64Value.of(5))))) + .build(), + true), + UNSET_ANY_FIELDS( + "!has(msg.single_any) && msg.repeated_any == [] && !has(msg.oneof_type.payload.single_any)", + TestAllTypes.getDefaultInstance(), + true), ; private final String expression; @@ -2340,6 +2357,42 @@ public void descriptorlessRuntime_rootAny_evaluatesFromPackedWireBytes( assertThat(result).isEqualTo(testCase.expectedResult); } + @Test + public void descriptorlessRuntime_selectThroughAnyField_throwsFieldSelectionByNameError() + throws Exception { + CelLiteRuntime descriptorlessRuntime = newDescriptorlessRuntime(); + CelAbstractSyntaxTree optimizedAst = + serverOptimizer.optimize(serverCompiler.compile("msg.single_any.single_int64").getAst()); + Program program = descriptorlessRuntime.createProgram(optimizedAst); + TestAllTypes msg = + TestAllTypes.newBuilder() + .setSingleAny(Any.pack(TestAllTypes.newBuilder().setSingleInt64(7L).build())) + .build(); + + CelEvaluationException thrown = + assertThrows(CelEvaluationException.class, () -> program.eval(ImmutableMap.of("msg", msg))); + + assertThat(thrown) + .hasCauseThat() + .hasMessageThat() + .isEqualTo( + "Error resolving field 'single_int64' on 'cel.expr.conformance.proto3.TestAllTypes'." + + " Field selection by name is not supported on raw proto wire bytes; register its" + + " CelLiteDescriptor or enable SelectOptimizer."); + } + + @Test + public void select_throughAnyFieldWithPackedTypeDescriptor_resolvesByName() throws Exception { + TestAllTypes msg = + TestAllTypes.newBuilder() + .setSingleAny(Any.pack(NestedMessage.newBuilder().setBb(7).build())) + .build(); + + Object result = eval("msg.single_any.bb", msg); + + assertThat(result).isEqualTo(7L); + } + @Test public void partialDescriptorRuntime_missingChildDescriptor_evaluatesFromWireBytes( @TestParameter DescriptorlessEvaluationTestCase testCase) 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 ecb6d2052..06e19d303 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/OptimizedSelectPlannerTest.java @@ -560,37 +560,6 @@ public void plan_invalidAst_messageTypeCodeWithScalarDummy_throwsEvaluationExcep assertThat(e).hasMessageThat().contains("Expected struct for message type, found:"); } - @Test - public void plan_invalidAst_unsupportedWellKnownType_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(100L)), - CelExpr.ofConstant(6L, CelConstant.ofValue("single_any")), - CelExpr.ofConstant(7L, CelConstant.ofValue(11L))), - ImmutableList.of())), - ImmutableList.of()), - CelExpr.ofStruct(8L, "google.protobuf.Any", ImmutableList.of()))), - SELECT_OPTIMIZED_SOURCE); - - CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); - - assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - assertThat(e) - .hasMessageThat() - .contains("Leaf well-known type 'google.protobuf.Any' is not supported"); - } - @Test public void plan_invalidAst_nonMapHopWithFourElements_throwsEvaluationException() { CelAbstractSyntaxTree ast = @@ -851,43 +820,6 @@ public void plan_invalidAst_mapFieldWithEmptyMapDummy_throwsEvaluationException( .contains("Expected map dummy/default value with a single entry, found:"); } - @Test - public void - plan_invalidAst_repeatedMessageWithUnsupportedWellKnownType_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, "google.protobuf.Any", ImmutableList.of())), - ImmutableList.of()))), - SELECT_OPTIMIZED_SOURCE); - - CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); - - assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); - assertThat(e) - .hasMessageThat() - .contains("Leaf well-known type 'google.protobuf.Any' is not supported"); - } - @Test public void plan_invalidAst_messageStructWithEmptyMessageName_throwsEvaluationException() { CelAbstractSyntaxTree ast =