Skip to content

Commit 0dd0b0b

Browse files
jnthntatumcopybara-github
authored andcommitted
Update the LegacyTypeProvider to proxy to the normal implementation for field lookups.
PiperOrigin-RevId: 959940867
1 parent a2265a8 commit 0dd0b0b

23 files changed

Lines changed: 139 additions & 778 deletions

eval/compiler/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ cc_library(
139139
"//runtime/internal:issue_collector",
140140
"//runtime/internal:runtime_env",
141141
"@com_google_absl//absl/algorithm:container",
142-
"@com_google_absl//absl/base:core_headers",
143142
"@com_google_absl//absl/base:nullability",
144143
"@com_google_absl//absl/container:flat_hash_map",
145144
"@com_google_absl//absl/container:flat_hash_set",
@@ -150,7 +149,6 @@ cc_library(
150149
"@com_google_absl//absl/status",
151150
"@com_google_absl//absl/status:statusor",
152151
"@com_google_absl//absl/strings",
153-
"@com_google_absl//absl/types:optional",
154152
"@com_google_absl//absl/types:span",
155153
"@com_google_absl//absl/types:variant",
156154
"@com_google_protobuf//:protobuf",

eval/compiler/flat_expr_builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ absl::StatusOr<FlatExpression> FlatExprBuilder::CreateExpressionImpl(
26602660
const cel::TypeProvider& FlatExprBuilder::GetTypeProvider() const {
26612661
return use_legacy_type_provider_
26622662
? static_cast<const cel::TypeProvider&>(
2663-
*GetLegacyRuntimeTypeProvider(type_registry_))
2663+
GetLegacyRuntimeTypeProvider(type_registry_))
26642664
: GetRuntimeTypeProvider(type_registry_);
26652665
}
26662666

eval/eval/create_struct_step_test.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ namespace {
5858

5959
using ::absl_testing::IsOk;
6060
using ::absl_testing::StatusIs;
61-
using ::cel::Expr;
6261
using ::cel::TypeProvider;
6362
using ::cel::internal::test::EqualsProto;
6463
using ::cel::runtime_internal::NewTestingRuntimeEnv;
@@ -200,7 +199,7 @@ TEST_P(CreateCreateStructStepTest, TestEmptyMessageCreation) {
200199

201200
auto adapter = env_->legacy_type_registry.FindTypeAdapter(
202201
"google.api.expr.runtime.TestMessage");
203-
ASSERT_TRUE(adapter.has_value() && adapter->mutation_apis() != nullptr);
202+
ASSERT_TRUE(adapter.has_value() && adapter->access_apis() != nullptr);
204203

205204
ASSERT_OK_AND_ASSIGN(auto maybe_type,
206205
env_->type_registry.GetComposedTypeProvider().FindType(

eval/public/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,7 @@ cc_library(
813813
"//eval/public/structs:legacy_type_provider",
814814
"//eval/public/structs:protobuf_descriptor_type_provider",
815815
"//runtime:type_registry",
816+
"@com_google_absl//absl/base",
816817
"@com_google_absl//absl/base:nullability",
817818
"@com_google_absl//absl/container:flat_hash_map",
818819
"@com_google_absl//absl/container:flat_hash_set",

eval/public/cel_type_registry.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <utility>
2121
#include <vector>
2222

23+
#include "absl/base/call_once.h"
2324
#include "absl/base/nullability.h"
2425
#include "absl/container/flat_hash_map.h"
2526
#include "absl/container/flat_hash_set.h"
@@ -28,6 +29,7 @@
2829
#include "base/type_provider.h"
2930
#include "eval/public/structs/legacy_type_adapter.h"
3031
#include "eval/public/structs/legacy_type_provider.h"
32+
#include "eval/public/structs/protobuf_descriptor_type_provider.h"
3133
#include "runtime/type_registry.h"
3234
#include "google/protobuf/descriptor.h"
3335
#include "google/protobuf/message.h"
@@ -60,7 +62,9 @@ class CelTypeRegistry {
6062

6163
CelTypeRegistry(const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
6264
google::protobuf::MessageFactory* absl_nullable message_factory)
63-
: modern_type_registry_(descriptor_pool, message_factory) {}
65+
: descriptor_pool_(descriptor_pool),
66+
message_factory_(message_factory),
67+
modern_type_registry_(descriptor_pool, message_factory) {}
6468

6569
~CelTypeRegistry() = default;
6670

@@ -77,8 +81,12 @@ class CelTypeRegistry {
7781

7882
// Get the first registered type provider.
7983
std::shared_ptr<const LegacyTypeProvider> GetFirstTypeProvider() const {
80-
return cel::runtime_internal::GetLegacyRuntimeTypeProvider(
81-
modern_type_registry_);
84+
absl::call_once(legacy_type_provider_once_, [&]() {
85+
this->legacy_type_provider_ = std::make_shared<
86+
google::api::expr::runtime::ProtobufDescriptorProvider>(
87+
descriptor_pool_, message_factory_);
88+
});
89+
return legacy_type_provider_;
8290
}
8391

8492
// Returns the effective type provider that has been configured with the
@@ -136,6 +144,13 @@ class CelTypeRegistry {
136144
}
137145

138146
private:
147+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool_;
148+
google::protobuf::MessageFactory* absl_nullable message_factory_;
149+
150+
// Legacy type provider. This is now disconnected from the actual type
151+
// resolution, but preserved for legacy clients that used it directly.
152+
mutable absl::once_flag legacy_type_provider_once_;
153+
mutable std::shared_ptr<LegacyTypeProvider> legacy_type_provider_;
139154
// Internal modern registry.
140155
cel::TypeRegistry modern_type_registry_;
141156
};

eval/public/cel_type_registry_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class TestTypeProvider : public LegacyTypeProvider {
3636
absl::string_view name) const override {
3737
for (const auto& type : types_) {
3838
if (name == type) {
39-
return LegacyTypeAdapter(/*access=*/nullptr, /*mutation=*/nullptr);
39+
return LegacyTypeAdapter(/*access=*/nullptr);
4040
}
4141
}
4242
return std::nullopt;

eval/public/structs/BUILD

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ cc_library(
273273
srcs = ["proto_message_type_adapter.cc"],
274274
hdrs = ["proto_message_type_adapter.h"],
275275
deps = [
276-
":cel_proto_wrap_util",
277276
":field_access_impl",
278277
":legacy_type_adapter",
279278
":legacy_type_info_apis",
@@ -311,12 +310,9 @@ cc_test(
311310
"//common:value_testing",
312311
"//eval/public:cel_value",
313312
"//eval/public:message_wrapper",
314-
"//eval/public/containers:container_backed_list_impl",
315-
"//eval/public/containers:container_backed_map_impl",
316313
"//eval/public/testing:matchers",
317314
"//eval/testutil:test_message_cc_proto",
318315
"//extensions/protobuf:memory_manager",
319-
"//internal:proto_matchers",
320316
"//internal:testing",
321317
"//runtime:runtime_options",
322318
"@com_google_absl//absl/status",

eval/public/structs/legacy_type_adapter.h

Lines changed: 7 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#ifndef THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_LEGACY_TYPE_ADPATER_H_
1919
#define THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_STRUCTS_LEGACY_TYPE_ADPATER_H_
2020

21-
#include <cstdint>
21+
#include <cstddef>
2222
#include <vector>
2323

2424
#include "absl/status/status.h"
@@ -36,54 +36,6 @@ namespace google::api::expr::runtime {
3636
class DucktypedMessageAdapter;
3737
class ProtoMessageTypeAdapter;
3838

39-
// Interface for mutation apis.
40-
// Note: in the new type system, a type provider represents this by returning
41-
// a cel::Type and cel::ValueManager for the type.
42-
class LegacyTypeMutationApis {
43-
public:
44-
virtual ~LegacyTypeMutationApis() = default;
45-
46-
// Return whether the type defines the given field.
47-
// TODO(uncreated-issue/3): This is only used to eagerly fail during the planning
48-
// phase. Check if it's safe to remove this behavior and fail at runtime.
49-
virtual bool DefinesField(absl::string_view field_name) const = 0;
50-
51-
// Create a new empty instance of the type.
52-
// May return a status if the type is not possible to create.
53-
virtual absl::StatusOr<CelValue::MessageWrapper::Builder> NewInstance(
54-
cel::MemoryManagerRef memory_manager) const = 0;
55-
56-
// Normalize special types to a native CEL value after building.
57-
// The interpreter guarantees that instance is uniquely owned by the
58-
// interpreter, and can be safely mutated.
59-
virtual absl::StatusOr<CelValue> AdaptFromWellKnownType(
60-
cel::MemoryManagerRef memory_manager,
61-
CelValue::MessageWrapper::Builder instance) const = 0;
62-
63-
// Set field on instance to value.
64-
// The interpreter guarantees that instance is uniquely owned by the
65-
// interpreter, and can be safely mutated.
66-
virtual absl::Status SetField(
67-
absl::string_view field_name, const CelValue& value,
68-
cel::MemoryManagerRef memory_manager,
69-
CelValue::MessageWrapper::Builder& instance) const = 0;
70-
71-
virtual absl::Status SetFieldByNumber(
72-
int64_t field_number [[maybe_unused]],
73-
const CelValue& value [[maybe_unused]],
74-
cel::MemoryManagerRef memory_manager [[maybe_unused]],
75-
CelValue::MessageWrapper::Builder& instance [[maybe_unused]]) const {
76-
return absl::UnimplementedError("SetFieldByNumber is not yet implemented");
77-
}
78-
79-
private:
80-
// This class should only be implemented by CEL. Custom structs are only
81-
// supported using the cel::Value APIs.
82-
friend class ProtoMessageTypeAdapter;
83-
84-
LegacyTypeMutationApis() = default;
85-
};
86-
8739
// Interface for access apis.
8840
// Note: in new type system this is integrated into the StructValue (via
8941
// dynamic dispatch to concrete implementations).
@@ -162,9 +114,6 @@ class LegacyTypeAccessApis {
162114
// Type information about a legacy Struct type.
163115
// Provides methods to the interpreter for interacting with a custom type.
164116
//
165-
// mutation_apis() provide equivalent behavior to a cel::Type and
166-
// cel::ValueManager (resolved from a type name).
167-
//
168117
// access_apis() provide equivalent behavior to cel::StructValue accessors
169118
// (virtual dispatch to a concrete implementation for accessing underlying
170119
// values).
@@ -174,21 +123,18 @@ class LegacyTypeAccessApis {
174123
// the type provider that returned this object.
175124
class LegacyTypeAdapter {
176125
public:
177-
LegacyTypeAdapter(const LegacyTypeAccessApis* access,
178-
const LegacyTypeMutationApis* mutation)
179-
: access_apis_(access), mutation_apis_(mutation) {}
126+
explicit LegacyTypeAdapter(const LegacyTypeAccessApis* access)
127+
: access_apis_(access) {}
128+
// Temporary constructor to support fakes in client tests.
129+
LegacyTypeAdapter(const LegacyTypeAccessApis* access, std::nullptr_t)
130+
: access_apis_(access) {}
180131

181132
// Apis for access for the represented type.
182133
// If null, access is not supported (this is an opaque type).
183-
const LegacyTypeAccessApis* access_apis() { return access_apis_; }
184-
185-
// Apis for mutation for the represented type.
186-
// If null, mutation is not supported (this type cannot be created).
187-
const LegacyTypeMutationApis* mutation_apis() { return mutation_apis_; }
134+
const LegacyTypeAccessApis* access_apis() const { return access_apis_; }
188135

189136
private:
190137
const LegacyTypeAccessApis* access_apis_;
191-
const LegacyTypeMutationApis* mutation_apis_;
192138
};
193139

194140
} // namespace google::api::expr::runtime

eval/public/structs/legacy_type_adapter_test.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,9 @@ namespace {
2323

2424
TEST(LegacyTypeAdapter, Basic) {
2525
ProtoMessageTypeAdapter adapter(TestMessage::descriptor(), nullptr);
26-
LegacyTypeAdapter type_adapter(&adapter, &adapter);
26+
LegacyTypeAdapter type_adapter(&adapter);
2727

2828
EXPECT_EQ(type_adapter.access_apis(), &adapter);
29-
EXPECT_EQ(type_adapter.mutation_apis(), &adapter);
3029
}
3130

3231
} // namespace

eval/public/structs/legacy_type_info_apis.h

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ namespace google::api::expr::runtime {
2727

2828
// Forward declared to resolve cyclic dependency.
2929
class LegacyTypeAccessApis;
30-
class LegacyTypeMutationApis;
3130

3231
// Forward declare permitted subclasses.
3332
class DucktypedMessageAdapter;
@@ -40,8 +39,8 @@ class TrivialTypeInfo;
4039
// Provides ability to obtain field access apis, type info, and debug
4140
// representation of a message.
4241
//
43-
// The message parameter may wrap a nullptr to request generic accessors /
44-
// mutators for the TypeInfo instance if it is available.
42+
// The message parameter may wrap a nullptr to request generic accessors for
43+
// the TypeInfo instance if it is available.
4544
//
4645
// This is implemented as a separate class from LegacyTypeAccessApis to resolve
4746
// cyclic dependency between CelValue (which needs to access these apis to
@@ -87,17 +86,6 @@ class LegacyTypeInfoApis {
8786
virtual const LegacyTypeAccessApis* GetAccessApis(
8887
const MessageWrapper& wrapped_message) const = 0;
8988

90-
// Return a pointer to the wrapped message's mutation api implementation.
91-
//
92-
// The CEL interpreter assumes that the returned pointer is owned externally
93-
// and will outlive any CelValues created by the interpreter.
94-
//
95-
// Nullptr signals that the value does not provide mutation apis.
96-
virtual const LegacyTypeMutationApis* GetMutationApis(
97-
const MessageWrapper& wrapped_message [[maybe_unused]]) const {
98-
return nullptr;
99-
}
100-
10189
// Return a description of the underlying field if defined.
10290
//
10391
// The underlying string is expected to remain valid as long as the

0 commit comments

Comments
 (0)