diff --git a/eval/eval/BUILD b/eval/eval/BUILD index 2597563a4..507ff6f0e 100644 --- a/eval/eval/BUILD +++ b/eval/eval/BUILD @@ -789,9 +789,7 @@ cc_test( deps = [ ":attribute_trail", ":cel_expression_flat_impl", - ":compiler_constant_step", ":const_value_step", - ":create_map_step", ":evaluator_core", ":ident_step", ":select_step", @@ -810,9 +808,7 @@ cc_test( "//eval/public:unknown_set", "//eval/public/containers:container_backed_map_impl", "//eval/public/structs:cel_proto_wrapper", - "//eval/public/structs:legacy_type_adapter", "//eval/public/structs:trivial_legacy_type_info", - "//eval/public/testing:matchers", "//eval/testutil:test_extensions_cc_proto", "//eval/testutil:test_message_cc_proto", "//extensions/protobuf:value", diff --git a/eval/eval/select_step_test.cc b/eval/eval/select_step_test.cc index 1580472ba..494def40f 100644 --- a/eval/eval/select_step_test.cc +++ b/eval/eval/select_step_test.cc @@ -24,9 +24,7 @@ #include "common/value_testing.h" #include "eval/eval/attribute_trail.h" #include "eval/eval/cel_expression_flat_impl.h" -#include "eval/eval/compiler_constant_step.h" #include "eval/eval/const_value_step.h" -#include "eval/eval/create_map_step.h" #include "eval/eval/evaluator_core.h" #include "eval/eval/ident_step.h" #include "eval/public/activation.h" @@ -34,9 +32,7 @@ #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_map_impl.h" #include "eval/public/structs/cel_proto_wrapper.h" -#include "eval/public/structs/legacy_type_adapter.h" #include "eval/public/structs/trivial_legacy_type_info.h" -#include "eval/public/testing/matchers.h" #include "eval/public/unknown_attribute_set.h" #include "eval/public/unknown_set.h" #include "eval/testutil/test_extensions.pb.h" @@ -81,10 +77,8 @@ using ::cel::internal::test::EqualsProto; using ::cel::runtime_internal::NewTestingRuntimeEnv; using ::cel::runtime_internal::RuntimeEnv; using ::cel::test::IntValueIs; -using ::testing::_; using ::testing::Eq; using ::testing::HasSubstr; -using ::testing::Return; using ::testing::UnorderedElementsAre; struct RunExpressionOptions { @@ -92,32 +86,6 @@ struct RunExpressionOptions { bool enable_wrapper_type_null_unboxing = false; }; -// Simple implementation LegacyTypeAccessApis / LegacyTypeInfoApis that allows -// mocking for getters/setters. -class MockAccessor : public LegacyTypeAccessApis, public LegacyTypeInfoApis { - public: - MOCK_METHOD(absl::StatusOr, HasField, - (absl::string_view field_name, - const CelValue::MessageWrapper& value), - (const, override)); - MOCK_METHOD(absl::StatusOr, GetField, - (absl::string_view field_name, - const CelValue::MessageWrapper& instance, - ProtoWrapperTypeOptions unboxing_option, - cel::MemoryManagerRef memory_manager), - (const, override)); - MOCK_METHOD(absl::string_view, GetTypename, - (const CelValue::MessageWrapper& instance), (const, override)); - MOCK_METHOD(std::string, DebugString, - (const CelValue::MessageWrapper& instance), (const, override)); - MOCK_METHOD(std::vector, ListFields, - (const CelValue::MessageWrapper& value), (const, override)); - const LegacyTypeAccessApis* GetAccessApis( - const CelValue::MessageWrapper& instance) const override { - return this; - } -}; - class SelectStepTest : public testing::Test { public: SelectStepTest() : env_(NewTestingRuntimeEnv()) {} @@ -702,68 +670,6 @@ TEST_P(SelectStepConformanceTest, NullMessageAccessor) { EXPECT_THAT(*result.ErrorOrDie(), StatusIs(absl::StatusCode::kNotFound)); } -TEST_P(SelectStepConformanceTest, CustomAccessor) { - TestMessage message; - TestMessage* message2 = message.mutable_message_value(); - message2->set_int32_value(1); - message2->set_string_value("test"); - RunExpressionOptions options; - options.enable_unknowns = GetParam(); - testing::NiceMock accessor; - CelValue value = CelValue::CreateMessageWrapper( - CelValue::MessageWrapper(&message, &accessor)); - - ON_CALL(accessor, GetField(_, _, _, _)) - .WillByDefault(Return(CelValue::CreateInt64(2))); - ON_CALL(accessor, HasField(_, _)).WillByDefault(Return(false)); - - ASSERT_OK_AND_ASSIGN(CelValue result, - RunExpression(value, "message_value", - /*test=*/false, - /*unknown_path=*/"", options)); - - EXPECT_THAT(result, test::IsCelInt64(2)); - - // testonly select (has) - ASSERT_OK_AND_ASSIGN(result, RunExpression(value, "message_value", - /*test=*/true, - /*unknown_path=*/"", options)); - - EXPECT_THAT(result, test::IsCelBool(false)); -} - -TEST_P(SelectStepConformanceTest, CustomAccessorErrorHandling) { - TestMessage message; - TestMessage* message2 = message.mutable_message_value(); - message2->set_int32_value(1); - message2->set_string_value("test"); - RunExpressionOptions options; - options.enable_unknowns = GetParam(); - testing::NiceMock accessor; - CelValue value = CelValue::CreateMessageWrapper( - CelValue::MessageWrapper(&message, &accessor)); - - ON_CALL(accessor, GetField(_, _, _, _)) - .WillByDefault(Return(absl::InternalError("bad data"))); - ON_CALL(accessor, HasField(_, _)) - .WillByDefault(Return(absl::NotFoundError("not found"))); - - // For get field, implementation may return an error-type cel value or a - // status (e.g. broken assumption using a core type). - ASSERT_OK_AND_ASSIGN(CelValue result, - RunExpression(value, "message_value", - /*test=*/false, - /*unknown_path=*/"", options)); - EXPECT_THAT(result, test::IsCelError(StatusIs(absl::StatusCode::kInternal))); - - // testonly select (has) errors are coerced to CelError. - ASSERT_OK_AND_ASSIGN(result, RunExpression(value, "message_value", - /*test=*/true, - /*unknown_path=*/"", options)); - - EXPECT_THAT(result, test::IsCelError(StatusIs(absl::StatusCode::kNotFound))); -} - TEST_P(SelectStepConformanceTest, SimpleEnumTest) { TestMessage message; message.set_enum_value(TestMessage::TEST_ENUM_1); diff --git a/eval/public/structs/BUILD b/eval/public/structs/BUILD index 4e4d5481c..75b4685a0 100644 --- a/eval/public/structs/BUILD +++ b/eval/public/structs/BUILD @@ -272,14 +272,9 @@ cc_test( srcs = ["legacy_type_adapter_test.cc"], deps = [ ":legacy_type_adapter", - ":trivial_legacy_type_info", - "//eval/public:cel_value", - "//eval/public/testing:matchers", + ":proto_message_type_adapter", "//eval/testutil:test_message_cc_proto", - "//extensions/protobuf:memory_manager", - "//internal:status_macros", "//internal:testing", - "@com_google_protobuf//:protobuf", ], ) @@ -301,7 +296,6 @@ cc_library( "//eval/public/containers:internal_field_backed_map_impl", "//extensions/protobuf:memory_manager", "//extensions/protobuf/internal:qualify", - "//internal:casts", "//internal:status_macros", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/base:nullability", @@ -309,7 +303,6 @@ cc_library( "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", - "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", "@com_google_protobuf//:differencer", "@com_google_protobuf//:protobuf", @@ -386,7 +379,6 @@ cc_library( deps = [ "//eval/public:message_wrapper", "@com_google_absl//absl/base:nullability", - "@com_google_absl//absl/status", "@com_google_absl//absl/strings:string_view", "@com_google_protobuf//:protobuf", ], @@ -418,11 +410,14 @@ cc_test( name = "legacy_type_provider_test", srcs = ["legacy_type_provider_test.cc"], deps = [ + ":legacy_type_adapter", ":legacy_type_info_apis", ":legacy_type_provider", + ":proto_message_type_adapter", + ":trivial_legacy_type_info", "//common:type", + "//eval/testutil:test_message_cc_proto", "//internal:testing", - "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/strings:string_view", ], ) diff --git a/eval/public/structs/legacy_type_adapter.h b/eval/public/structs/legacy_type_adapter.h index dc7a3ab1b..05fbc9a92 100644 --- a/eval/public/structs/legacy_type_adapter.h +++ b/eval/public/structs/legacy_type_adapter.h @@ -32,6 +32,10 @@ namespace google::api::expr::runtime { +// Forward declare permitted subclasses. +class DucktypedMessageAdapter; +class ProtoMessageTypeAdapter; + // Interface for mutation apis. // Note: in the new type system, a type provider represents this by returning // a cel::Type and cel::ValueManager for the type. @@ -71,6 +75,13 @@ class LegacyTypeMutationApis { CelValue::MessageWrapper::Builder& instance [[maybe_unused]]) const { return absl::UnimplementedError("SetFieldByNumber is not yet implemented"); } + + private: + // This class should only be implemented by CEL. Custom structs are only + // supported using the cel::Value APIs. + friend class ProtoMessageTypeAdapter; + + LegacyTypeMutationApis() = default; }; // Interface for access apis. @@ -138,6 +149,14 @@ class LegacyTypeAccessApis { virtual std::vector ListFields( const CelValue::MessageWrapper& instance) const = 0; + + private: + // This class should only be implemented by CEL. Custom structs are only + // supported using the cel::Value APIs. + friend class DucktypedMessageAdapter; + friend class ProtoMessageTypeAdapter; + + LegacyTypeAccessApis() = default; }; // Type information about a legacy Struct type. diff --git a/eval/public/structs/legacy_type_adapter_test.cc b/eval/public/structs/legacy_type_adapter_test.cc index 4c16a59ad..1dabac7d7 100644 --- a/eval/public/structs/legacy_type_adapter_test.cc +++ b/eval/public/structs/legacy_type_adapter_test.cc @@ -14,49 +14,19 @@ #include "eval/public/structs/legacy_type_adapter.h" -#include - -#include "eval/public/cel_value.h" -#include "eval/public/structs/trivial_legacy_type_info.h" -#include "eval/public/testing/matchers.h" +#include "eval/public/structs/proto_message_type_adapter.h" #include "eval/testutil/test_message.pb.h" -#include "extensions/protobuf/memory_manager.h" -#include "internal/status_macros.h" #include "internal/testing.h" namespace google::api::expr::runtime { namespace { -class TestAccessApiImpl : public LegacyTypeAccessApis { - public: - TestAccessApiImpl() {} - absl::StatusOr HasField( - absl::string_view field_name, - const CelValue::MessageWrapper& value) const override { - return absl::UnimplementedError("Not implemented"); - } - - absl::StatusOr GetField( - absl::string_view field_name, const CelValue::MessageWrapper& instance, - ProtoWrapperTypeOptions unboxing_option, - cel::MemoryManagerRef memory_manager) const override { - return absl::UnimplementedError("Not implemented"); - } - - std::vector ListFields( - const CelValue::MessageWrapper& instance) const override { - return std::vector(); - } -}; - -TEST(LegacyTypeAdapterAccessApis, DefaultAlwaysInequal) { - TestMessage message; - MessageWrapper wrapper(&message, nullptr); - MessageWrapper wrapper2(&message, nullptr); - - TestAccessApiImpl impl; +TEST(LegacyTypeAdapter, Basic) { + ProtoMessageTypeAdapter adapter(TestMessage::descriptor(), nullptr); + LegacyTypeAdapter type_adapter(&adapter, &adapter); - EXPECT_FALSE(impl.IsEqualTo(wrapper, wrapper2)); + EXPECT_EQ(type_adapter.access_apis(), &adapter); + EXPECT_EQ(type_adapter.mutation_apis(), &adapter); } } // namespace diff --git a/eval/public/structs/legacy_type_info_apis.h b/eval/public/structs/legacy_type_info_apis.h index e470ac566..90e2f3710 100644 --- a/eval/public/structs/legacy_type_info_apis.h +++ b/eval/public/structs/legacy_type_info_apis.h @@ -15,10 +15,10 @@ #ifndef THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_LEGACY_TYPE_INFO_APIS_H_ #define THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_LEGACY_TYPE_INFO_APIS_H_ +#include #include #include "absl/base/nullability.h" -#include "absl/status/status.h" #include "absl/strings/string_view.h" #include "eval/public/message_wrapper.h" #include "google/protobuf/descriptor.h" @@ -29,6 +29,11 @@ namespace google::api::expr::runtime { class LegacyTypeAccessApis; class LegacyTypeMutationApis; +// Forward declare permitted subclasses. +class DucktypedMessageAdapter; +class ProtoMessageTypeAdapter; +class TrivialTypeInfo; + // Interface for providing type info from a user defined type (represented as a // message). // @@ -97,10 +102,19 @@ class LegacyTypeInfoApis { // // The underlying string is expected to remain valid as long as the // LegacyTypeInfoApis instance. - virtual absl::optional FindFieldByName( + virtual std::optional FindFieldByName( absl::string_view name [[maybe_unused]]) const { - return absl::nullopt; + return std::nullopt; } + + private: + // This class should only be implemented by CEL. Custom structs are only + // supported using the cel::Value APIs. + friend class DucktypedMessageAdapter; + friend class ProtoMessageTypeAdapter; + friend class TrivialTypeInfo; + + LegacyTypeInfoApis() = default; }; } // namespace google::api::expr::runtime diff --git a/eval/public/structs/legacy_type_provider.h b/eval/public/structs/legacy_type_provider.h index e2e67411c..6148caee7 100644 --- a/eval/public/structs/legacy_type_provider.h +++ b/eval/public/structs/legacy_type_provider.h @@ -15,11 +15,12 @@ #ifndef THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_TYPE_PROVIDER_H_ #define THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_TYPE_PROVIDER_H_ +#include + #include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "absl/types/optional.h" #include "common/type.h" #include "common/type_reflector.h" #include "common/value.h" @@ -45,7 +46,7 @@ class LegacyTypeProvider : public cel::TypeReflector { // Returned non-null pointers from the adapter implemententation must remain // valid as long as the type provider. // TODO(uncreated-issue/3): add alternative for new type system. - virtual absl::optional ProvideLegacyType( + virtual std::optional ProvideLegacyType( absl::string_view name) const = 0; // Return LegacyTypeInfoApis for the fully qualified type name if available. @@ -55,9 +56,9 @@ class LegacyTypeProvider : public cel::TypeReflector { // Since custom type providers should create values compatible with evaluator // created ones, the TypeInfoApis returned from this method should be the same // as the ones used in value creation. - virtual absl::optional ProvideLegacyTypeInfo( + virtual std::optional ProvideLegacyTypeInfo( ABSL_ATTRIBUTE_UNUSED absl::string_view name) const { - return absl::nullopt; + return std::nullopt; } absl::StatusOr NewValueBuilder( @@ -66,10 +67,10 @@ class LegacyTypeProvider : public cel::TypeReflector { google::protobuf::Arena* absl_nonnull arena) const final; protected: - absl::StatusOr> FindTypeImpl( + absl::StatusOr> FindTypeImpl( absl::string_view name) const final; - absl::StatusOr> + absl::StatusOr> FindStructTypeFieldByNameImpl(absl::string_view type, absl::string_view name) const final; }; diff --git a/eval/public/structs/legacy_type_provider_test.cc b/eval/public/structs/legacy_type_provider_test.cc index 2da45e69d..cd683bb50 100644 --- a/eval/public/structs/legacy_type_provider_test.cc +++ b/eval/public/structs/legacy_type_provider_test.cc @@ -15,19 +15,19 @@ #include "eval/public/structs/legacy_type_provider.h" #include -#include -#include "absl/status/status_matchers.h" #include "absl/strings/string_view.h" #include "common/type.h" +#include "eval/public/structs/legacy_type_adapter.h" #include "eval/public/structs/legacy_type_info_apis.h" +#include "eval/public/structs/proto_message_type_adapter.h" +#include "eval/public/structs/trivial_legacy_type_info.h" +#include "eval/testutil/test_message.pb.h" #include "internal/testing.h" namespace google::api::expr::runtime { namespace { -using ::absl_testing::IsOk; - class LegacyTypeProviderTestEmpty : public LegacyTypeProvider { public: absl::optional ProvideLegacyType( @@ -36,32 +36,6 @@ class LegacyTypeProviderTestEmpty : public LegacyTypeProvider { } }; -class LegacyTypeInfoApisEmpty : public LegacyTypeInfoApis { - public: - std::string DebugString( - const MessageWrapper& wrapped_message) const override { - return ""; - } - absl::string_view GetTypename( - const MessageWrapper& wrapped_message) const override { - return test_string_; - } - const LegacyTypeAccessApis* GetAccessApis( - const MessageWrapper& wrapped_message) const override { - return nullptr; - } - absl::optional FindFieldByName( - absl::string_view name) const override { - if (name == "field1") { - return FieldDescription{1, "field1"}; - } - return absl::nullopt; - } - - private: - const std::string test_string_ = "test"; -}; - class LegacyTypeProviderTestImpl : public LegacyTypeProvider { public: explicit LegacyTypeProviderTestImpl(const LegacyTypeInfoApis* test_type_info) @@ -92,8 +66,7 @@ TEST(LegacyTypeProviderTest, EmptyTypeProviderHasProvideTypeInfo) { } TEST(LegacyTypeProviderTest, NonEmptyTypeProviderProvidesSomeTypes) { - LegacyTypeInfoApisEmpty test_type_info; - LegacyTypeProviderTestImpl provider(&test_type_info); + LegacyTypeProviderTestImpl provider(TrivialTypeInfo::GetInstance()); EXPECT_TRUE(provider.ProvideLegacyType("test").has_value()); EXPECT_TRUE(provider.ProvideLegacyTypeInfo("test").has_value()); EXPECT_EQ(provider.ProvideLegacyType("other"), std::nullopt); @@ -101,15 +74,16 @@ TEST(LegacyTypeProviderTest, NonEmptyTypeProviderProvidesSomeTypes) { } TEST(LegacyTypeProviderTest, FindStructTypeFieldByName) { - LegacyTypeInfoApisEmpty test_type_info; - LegacyTypeProviderTestImpl provider(&test_type_info); + ProtoMessageTypeAdapter adapter(TestMessage::descriptor(), nullptr); + LegacyTypeProviderTestImpl provider(&adapter); - ASSERT_OK_AND_ASSIGN(absl::optional field, - provider.FindStructTypeFieldByName("test", "field1")); + ASSERT_OK_AND_ASSIGN( + absl::optional field, + provider.FindStructTypeFieldByName("test", "int32_value")); ASSERT_TRUE(field.has_value()); - EXPECT_EQ(field->name(), "field1"); + EXPECT_EQ(field->name(), "int32_value"); EXPECT_EQ(field->number(), 1); - EXPECT_EQ(field->GetType(), cel::DynType()); + EXPECT_EQ(field->GetType(), cel::IntType()); ASSERT_OK_AND_ASSIGN( absl::optional not_found_field, diff --git a/eval/public/structs/proto_message_type_adapter.cc b/eval/public/structs/proto_message_type_adapter.cc index a8bb852fc..db5f3c6e2 100644 --- a/eval/public/structs/proto_message_type_adapter.cc +++ b/eval/public/structs/proto_message_type_adapter.cc @@ -15,7 +15,7 @@ #include "eval/public/structs/proto_message_type_adapter.h" #include -#include +#include #include #include #include @@ -27,7 +27,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" -#include "absl/types/optional.h" #include "absl/types/span.h" #include "base/attribute.h" #include "common/memory.h" @@ -42,7 +41,6 @@ #include "eval/public/structs/legacy_type_info_apis.h" #include "extensions/protobuf/internal/qualify.h" #include "extensions/protobuf/memory_manager.h" -#include "internal/casts.h" #include "internal/status_macros.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" @@ -178,7 +176,7 @@ class LegacyQualifyState final LegacyQualifyState(const LegacyQualifyState&) = delete; LegacyQualifyState& operator=(const LegacyQualifyState&) = delete; - absl::optional& result() { return result_; } + std::optional& result() { return result_; } private: void SetResultFromError(absl::Status status, @@ -221,7 +219,7 @@ class LegacyQualifyState final return absl::OkStatus(); } - absl::optional result_; + std::optional result_; }; absl::StatusOr QualifyImpl( @@ -279,8 +277,9 @@ std::vector ListFieldsImpl( return field_names; } +} // namespace + class DucktypedMessageAdapter : public LegacyTypeAccessApis, - public LegacyTypeMutationApis, public LegacyTypeInfoApis { public: // Implement field access APIs. @@ -351,46 +350,6 @@ class DucktypedMessageAdapter : public LegacyTypeAccessApis, return message->ShortDebugString(); } - bool DefinesField(absl::string_view field_name) const override { - // Pretend all our fields exist. Real errors will be returned from field - // getters and setters. - return true; - } - - absl::StatusOr NewInstance( - cel::MemoryManagerRef memory_manager) const override { - return absl::UnimplementedError("NewInstance is not implemented"); - } - - absl::StatusOr AdaptFromWellKnownType( - cel::MemoryManagerRef memory_manager, - CelValue::MessageWrapper::Builder instance) const override { - if (!instance.HasFullProto() || instance.message_ptr() == nullptr) { - return absl::UnimplementedError( - "MessageLite is not supported, descriptor is required"); - } - return ProtoMessageTypeAdapter( - static_cast(instance.message_ptr()) - ->GetDescriptor(), - nullptr) - .AdaptFromWellKnownType(memory_manager, instance); - } - - absl::Status SetField( - absl::string_view field_name, const CelValue& value, - cel::MemoryManagerRef memory_manager, - CelValue::MessageWrapper::Builder& instance) const override { - if (!instance.HasFullProto() || instance.message_ptr() == nullptr) { - return absl::UnimplementedError( - "MessageLite is not supported, descriptor is required"); - } - return ProtoMessageTypeAdapter( - static_cast(instance.message_ptr()) - ->GetDescriptor(), - nullptr) - .SetField(field_name, value, memory_manager, instance); - } - std::vector ListFields( const CelValue::MessageWrapper& instance) const override { return ListFieldsImpl(instance); @@ -403,7 +362,7 @@ class DucktypedMessageAdapter : public LegacyTypeAccessApis, const LegacyTypeMutationApis* GetMutationApis( const MessageWrapper& wrapped_message) const override { - return this; + return nullptr; } static const DucktypedMessageAdapter& GetSingleton() { @@ -412,6 +371,8 @@ class DucktypedMessageAdapter : public LegacyTypeAccessApis, } }; +namespace { + CelValue MessageCelValueFactory(const google::protobuf::Message* message) { return CelValue::CreateMessageWrapper( MessageWrapper(message, &DucktypedMessageAdapter::GetSingleton())); @@ -469,7 +430,7 @@ const LegacyTypeAccessApis* ProtoMessageTypeAdapter::GetAccessApis( return this; } -absl::optional +std::optional ProtoMessageTypeAdapter::FindFieldByName(absl::string_view field_name) const { if (descriptor_ == nullptr) { return std::nullopt; diff --git a/extensions/BUILD b/extensions/BUILD index a60943f61..faf9f08c8 100644 --- a/extensions/BUILD +++ b/extensions/BUILD @@ -377,7 +377,8 @@ cc_test( "//common:decl_proto", "//common:expr", "//common:kind", - "//common:memory", + "//common:native_type", + "//common:typeinfo", "//common:value", "//compiler", "//compiler:compiler_factory", @@ -391,8 +392,6 @@ cc_test( "//eval/public:cel_type_registry", "//eval/public:cel_value", "//eval/public/structs:cel_proto_wrapper", - "//eval/public/structs:legacy_type_adapter", - "//eval/public/structs:legacy_type_info_apis", "//extensions/protobuf:ast_converters", "//internal:number", "//internal:status_macros", @@ -415,7 +414,6 @@ cc_test( "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", - "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", "@com_google_cel_spec//proto/cel/expr:checked_cc_proto", "@com_google_cel_spec//proto/cel/expr:syntax_cc_proto", diff --git a/extensions/select_optimization_test.cc b/extensions/select_optimization_test.cc index c14c4d461..27c191738 100644 --- a/extensions/select_optimization_test.cc +++ b/extensions/select_optimization_test.cc @@ -33,7 +33,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" -#include "absl/types/optional.h" #include "absl/types/span.h" #include "base/ast.h" #include "base/attribute.h" @@ -45,8 +44,10 @@ #include "common/decl_proto.h" #include "common/expr.h" #include "common/kind.h" -#include "common/memory.h" +#include "common/native_type.h" +#include "common/typeinfo.h" #include "common/value.h" +#include "common/values/custom_struct_value.h" #include "compiler/compiler.h" #include "compiler/compiler_factory.h" #include "compiler/optional.h" @@ -59,8 +60,6 @@ #include "eval/public/cel_type_registry.h" #include "eval/public/cel_value.h" #include "eval/public/structs/cel_proto_wrapper.h" -#include "eval/public/structs/legacy_type_adapter.h" -#include "eval/public/structs/legacy_type_info_apis.h" #include "extensions/protobuf/ast_converters.h" #include "internal/number.h" #include "internal/status_macros.h" @@ -96,18 +95,10 @@ using ::google::api::expr::runtime::CelProtoWrapper; using ::google::api::expr::runtime::CelValue; using ::google::api::expr::runtime::FlatExprBuilder; using ::google::api::expr::runtime::FlatExpression; -using ::google::api::expr::runtime::LegacyTypeAccessApis; -using ::google::api::expr::runtime::LegacyTypeInfoApis; -using ::google::api::expr::runtime::LegacyTypeMutationApis; -using ::google::protobuf::Empty; -using ::testing::_; using ::testing::AllOf; -using ::testing::AnyOf; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::HasSubstr; -using ::testing::NiceMock; -using ::testing::Return; using ::testing::SizeIs; using ::testing::Truly; @@ -232,75 +223,105 @@ absl::StatusOr> CompileForTestCase( return r.ReleaseAst(); } -class MockAccessApis : public LegacyTypeInfoApis, public LegacyTypeAccessApis { +class TestPartialQualifyStruct : public CustomStructValueInterface { public: - std::string DebugString( - const MessageWrapper& wrapped_message) const override { - return "MockAccessApis"; + explicit TestPartialQualifyStruct(Value leaf_value) + : leaf_value_(std::move(leaf_value)) {} + + std::string DebugString() const override { + return "TestPartialQualifyStruct"; + } + + absl::Status SerializeTo( + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::io::ZeroCopyOutputStream* output) const override { + return absl::UnimplementedError("SerializeTo"); } - absl::string_view GetTypename( - const MessageWrapper& wrapped_message) const override { - return "MockAccessApis"; + absl::Status ConvertToJsonObject( + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::Message* json) const override { + return absl::UnimplementedError("ConvertToJsonObject"); } - const LegacyTypeAccessApis* GetAccessApis( - const MessageWrapper& wrapped_message) const override { - return this; + absl::string_view GetTypeName() const override { + return "cel.expr.conformance.proto2.NestedTestAllTypes"; } - const LegacyTypeMutationApis* GetMutationApis( - const MessageWrapper& wrapped_message) const override { - return nullptr; + bool IsZeroValue() const override { return false; } + + absl::Status GetFieldByName(absl::string_view name, + ProtoWrapperTypeOptions unboxing_options, + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::Arena* arena, + Value* result) const override { + if (name == "child" || name == "payload" || name == "standalone_message") { + *result = CustomStructValue(this, arena); + return absl::OkStatus(); + } + if (name == "bb") { + *result = leaf_value_; + return absl::OkStatus(); + } + return NoSuchFieldError(name).ToStatus(); } - std::optional< - google::api::expr::runtime::LegacyTypeInfoApis::FieldDescription> - FindFieldByName(absl::string_view field_name) const override { - return std::nullopt; + absl::Status GetFieldByNumber(int64_t number, + ProtoWrapperTypeOptions unboxing_options, + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::Arena* arena, + Value* result) const override { + return absl::UnimplementedError("GetFieldByNumber"); } - MOCK_METHOD(absl::StatusOr, GetField, - (absl::string_view field_name, - const CelValue::MessageWrapper& instance, - ProtoWrapperTypeOptions unboxing_option, - cel::MemoryManagerRef memory_manager), - (const, override)); - - MOCK_METHOD(absl::StatusOr, HasField, - (absl::string_view field_name, - const CelValue::MessageWrapper& value), - (const, override)); - - MOCK_METHOD(absl::StatusOr, - Qualify, - (absl::Span qualifiers, - const CelValue::MessageWrapper& instance, bool presence_test, - MemoryManagerRef memory_manager), - (const, override)); - - bool IsEqualTo( - const CelValue::MessageWrapper& instance, - const CelValue::MessageWrapper& other_instance) const override { + absl::StatusOr HasFieldByName(absl::string_view name) const override { + if (name == "child" || name == "payload" || name == "standalone_message" || + name == "bb") { + return true; + } return false; } - std::vector ListFields( - const CelValue::MessageWrapper& instance) const override { - return {}; + absl::StatusOr HasFieldByNumber(int64_t number) const override { + return absl::UnimplementedError("HasFieldByNumber"); } -}; -std::pair MakeMockLegacyMessage( - google::protobuf::Arena* arena) { - auto* mock_access_apis = - google::protobuf::Arena::Create>(arena); - auto* message = google::protobuf::Arena::Create(arena); + absl::Status ForEachField(ForEachFieldCallback callback, + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::Arena* arena) const override { + return absl::OkStatus(); + } - CelValue::MessageWrapper::Builder wrapper(message); - return {mock_access_apis, - CelValue::CreateMessageWrapper(wrapper.Build(mock_access_apis))}; -} + absl::Status Qualify(absl::Span qualifiers, + bool presence_test, + const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, + google::protobuf::Arena* arena, Value* result, + int* count) const override { + if (qualifiers.size() >= 3) { + *count = 3; + *result = CustomStructValue(this, arena); + return absl::OkStatus(); + } + return absl::UnimplementedError("Qualify unsupported"); + } + + CustomStructValue Clone(google::protobuf::Arena* arena) const override { + return CustomStructValue(this, arena); + } + + NativeTypeId GetNativeTypeId() const override { + return cel::TypeId(); + } + + private: + Value leaf_value_; +}; absl::Status TestBindLegacyValue(absl::string_view variable, CelValue legacy_value, google::protobuf::Arena* arena, @@ -1031,24 +1052,12 @@ INSTANTIATE_TEST_SUITE_P( "nested_test_all_types.child.payload.standalone_message.bb", {}, [](google::protobuf::Arena* arena, Activation& act) { - auto mock_pair = MakeMockLegacyMessage(arena); - MockAccessApis* mock = mock_pair.first; - CelValue mocked_value = mock_pair.second; - ON_CALL(*mock, Qualify(SizeIs(4), _, /*presence_test=*/false, _)) - .WillByDefault( - Return(LegacyTypeAccessApis::LegacyQualifyResult{ - mocked_value, 3})); - ON_CALL(*mock, GetField("bb", _, _, _)) - .WillByDefault(Return(CelValue::CreateInt64(42))); - - // Support the forced-fallback case. - ON_CALL(*mock, GetField(AnyOf(Eq("child"), Eq("payload"), - Eq("standalone_message")), - _, _, _)) - .WillByDefault(Return(mocked_value)); - - return TestBindLegacyValue("nested_test_all_types", mocked_value, - arena, act); + auto* custom_struct = + google::protobuf::Arena::Create(arena, + IntValue(42)); + act.InsertOrAssignValue("nested_test_all_types", + CustomStructValue(custom_struct, arena)); + return absl::OkStatus(); }, [](const absl::StatusOr& got) { ASSERT_OK_AND_ASSIGN(Value result, got); @@ -1061,29 +1070,12 @@ INSTANTIATE_TEST_SUITE_P( "has(nested_test_all_types.child.payload.standalone_message.bb)", {}, [](google::protobuf::Arena* arena, Activation& act) { - auto mock_pair = MakeMockLegacyMessage(arena); - MockAccessApis* mock = mock_pair.first; - CelValue mocked_value = mock_pair.second; - ON_CALL(*mock, Qualify(SizeIs(4), _, /*presence_test=*/true, _)) - .WillByDefault( - Return(LegacyTypeAccessApis::LegacyQualifyResult{ - mocked_value, 3})); - ON_CALL(*mock, HasField("bb", _)).WillByDefault(Return(true)); - ON_CALL(*mock, GetField("bb", _, _, _)) - .WillByDefault(Return(CelValue::CreateInt64(42))); - - // Support the forced-fallback case. - ON_CALL(*mock, GetField(AnyOf(Eq("child"), Eq("payload"), - Eq("standalone_message")), - _, _, _)) - .WillByDefault(Return(mocked_value)); - ON_CALL(*mock, HasField(AnyOf(Eq("child"), Eq("payload"), - Eq("standalone_message")), - _)) - .WillByDefault(Return(true)); - - return TestBindLegacyValue("nested_test_all_types", mocked_value, - arena, act); + auto* custom_struct = + google::protobuf::Arena::Create(arena, + IntValue(42)); + act.InsertOrAssignValue("nested_test_all_types", + CustomStructValue(custom_struct, arena)); + return absl::OkStatus(); }, [](const absl::StatusOr& got) { ASSERT_OK_AND_ASSIGN(Value result, got);