Update managers to extend factories, leaving factories as an abstract…

… interface

PiperOrigin-RevId: 592331551

common/BUILD

@@ -288,6 +288,7 @@ cc_library(
     ) + [
         "type_factory.cc",
         "type_manager.cc",
+        "type_provider.cc",
         "value_factory.cc",
         "value_interface.cc",
         "value_manager.cc",

common/type_factory.cc

@@ -19,12 +19,9 @@
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 #include "common/casting.h"
-#include "common/memory.h"
 #include "common/sized_input_view.h"
 #include "common/type.h"
 #include "common/type_kind.h"
-#include "common/types/thread_compatible_type_factory.h"
-#include "common/types/thread_safe_type_factory.h"
 #include "common/types/type_cache.h"
 #include "internal/names.h"
 
@@ -112,15 +109,4 @@ OptionalTypeView TypeFactory::GetDynOptionalType() {
   return ProcessLocalTypeCache::Get()->GetDynOptionalType();
 }
 
-Shared<TypeFactory> NewThreadCompatibleTypeFactory(
-    MemoryManagerRef memory_manager) {
-  return memory_manager
-      .MakeShared<common_internal::ThreadCompatibleTypeFactory>(memory_manager);
-}
-
-Shared<TypeFactory> NewThreadSafeTypeFactory(MemoryManagerRef memory_manager) {
-  return memory_manager.MakeShared<common_internal::ThreadSafeTypeFactory>(
-      memory_manager);
-}
-
 }  // namespace cel

common/type_factory.h

@@ -82,15 +82,6 @@ class TypeFactory {
       absl::string_view name, const SizedInputView<TypeView>& parameters) = 0;
 };
 
-// Creates a new `TypeFactory` which is thread compatible. The returned
-// `TypeFactory` and all types it creates are managed my `memory_manager`.
-Shared<TypeFactory> NewThreadCompatibleTypeFactory(
-    MemoryManagerRef memory_manager);
-
-// Creates a new `TypeFactory` which is thread safe. The returned `TypeFactory`
-// and all types it creates are managed my `memory_manager`.
-Shared<TypeFactory> NewThreadSafeTypeFactory(MemoryManagerRef memory_manager);
-
 }  // namespace cel
 
 #endif  // THIRD_PARTY_CEL_CPP_COMMON_TYPE_FACTORY_H_

common/type_factory_test.cc

@@ -22,6 +22,8 @@
 #include "absl/types/optional.h"
 #include "common/memory.h"
 #include "common/type.h"
+#include "common/type_manager.h"
+#include "common/type_provider.h"
 #include "common/types/type_cache.h"
 #include "internal/testing.h"
 
@@ -34,7 +36,6 @@ using testing::Eq;
 using testing::Ne;
 using testing::TestParamInfo;
 using testing::TestWithParam;
-using cel::internal::IsOkAndHolds;
 
 enum class ThreadSafety {
   kCompatible,
@@ -65,26 +66,29 @@ class TypeFactoryTest
     }
     switch (thread_safety()) {
       case ThreadSafety::kCompatible:
-        type_factory_ = NewThreadCompatibleTypeFactory(memory_manager());
+        type_manager_ = NewThreadCompatibleTypeManager(
+            memory_manager(),
+            NewThreadCompatibleTypeProvider(memory_manager()));
         break;
       case ThreadSafety::kSafe:
-        type_factory_ = NewThreadSafeTypeFactory(memory_manager());
+        type_manager_ = NewThreadSafeTypeManager(
+            memory_manager(), NewThreadSafeTypeProvider(memory_manager()));
         break;
     }
   }
 
   void TearDown() override { Finish(); }
 
   void Finish() {
-    type_factory_.reset();
+    type_manager_.reset();
     memory_manager_.reset();
   }
 
   MemoryManagerRef memory_manager() { return *memory_manager_; }
 
   MemoryManagement memory_management() const { return std::get<0>(GetParam()); }
 
-  TypeFactory& type_factory() const { return **type_factory_; }
+  TypeFactory& type_factory() const { return **type_manager_; }
 
   ThreadSafety thread_safety() const { return std::get<1>(GetParam()); }
 
@@ -97,7 +101,7 @@ class TypeFactoryTest
 
  private:
   absl::optional<MemoryManager> memory_manager_;
-  absl::optional<Shared<TypeFactory>> type_factory_;
+  absl::optional<Shared<TypeManager>> type_manager_;
 };
 
 TEST_P(TypeFactoryTest, ListType) {

common/type_manager.cc

@@ -17,51 +17,23 @@
 #include <utility>
 
 #include "common/memory.h"
-#include "common/type_factory.h"
 #include "common/type_provider.h"
+#include "common/types/thread_compatible_type_manager.h"
+#include "common/types/thread_safe_type_manager.h"
 
 namespace cel {
 
-namespace {
-
-// Currently `TypeManagerImpl` is really a wrapper around `TypeFactory` and
-// `TypeProvider`, so the same implementation is used regardless of thread
-// safety.
-class TypeManagerImpl final : public TypeManager {
- public:
-  TypeManagerImpl(MemoryManagerRef memory_manager,
-                  Shared<TypeFactory> type_factory,
-                  Shared<TypeProvider> type_provider)
-      : memory_manager_(memory_manager),
-        type_factory_(std::move(type_factory)),
-        type_provider_(std::move(type_provider)) {}
-
-  MemoryManagerRef GetMemoryManager() const override { return memory_manager_; }
-
-  TypeFactory& GetTypeFactory() const override { return *type_factory_; }
-
-  TypeProvider& GetTypeProvider() const override { return *type_provider_; }
-
- private:
-  MemoryManagerRef memory_manager_;
-  Shared<TypeFactory> type_factory_;
-  Shared<TypeProvider> type_provider_;
-};
-
 Shared<TypeManager> NewThreadCompatibleTypeManager(
-    MemoryManagerRef memory_manager, Shared<TypeFactory> type_factory,
-    Shared<TypeProvider> type_provider) {
-  return memory_manager.MakeShared<TypeManagerImpl>(
-      memory_manager, std::move(type_factory), std::move(type_provider));
+    MemoryManagerRef memory_manager, Shared<TypeProvider> type_provider) {
+  return memory_manager
+      .MakeShared<common_internal::ThreadCompatibleTypeManager>(
+          memory_manager, std::move(type_provider));
 }
 
 Shared<TypeManager> NewThreadSafeTypeManager(
-    MemoryManagerRef memory_manager, Shared<TypeFactory> type_factory,
-    Shared<TypeProvider> type_provider) {
-  return memory_manager.MakeShared<TypeManagerImpl>(
-      memory_manager, std::move(type_factory), std::move(type_provider));
+    MemoryManagerRef memory_manager, Shared<TypeProvider> type_provider) {
+  return memory_manager.MakeShared<common_internal::ThreadSafeTypeManager>(
+      memory_manager, std::move(type_provider));
 }
 
-}  // namespace
-
 }  // namespace cel

common/type_manager.h

@@ -27,49 +27,44 @@ namespace cel {
 
 // `TypeManager` is an additional layer on top of `TypeFactory` and
 // `TypeProvider` which combines the two and adds additional functionality.
-class TypeManager {
+class TypeManager : public virtual TypeFactory {
  public:
   virtual ~TypeManager() = default;
 
-  virtual MemoryManagerRef GetMemoryManager() const = 0;
-
-  virtual TypeFactory& GetTypeFactory() const = 0;
-
-  virtual TypeProvider& GetTypeProvider() const = 0;
-
   // See `TypeProvider::FindType`.
   absl::StatusOr<TypeView> FindType(
       absl::string_view name, Type& scratch ABSL_ATTRIBUTE_LIFETIME_BOUND) {
-    return GetTypeProvider().FindType(GetTypeFactory(), name, scratch);
+    return GetTypeProvider().FindType(*this, name, scratch);
   }
 
   // See `TypeProvider::FindStructTypeFieldByName`.
   absl::StatusOr<StructTypeFieldView> FindStructTypeFieldByName(
       absl::string_view type, absl::string_view name,
       StructTypeField& scratch ABSL_ATTRIBUTE_LIFETIME_BOUND) {
-    return GetTypeProvider().FindStructTypeFieldByName(GetTypeFactory(), type,
-                                                       name, scratch);
+    return GetTypeProvider().FindStructTypeFieldByName(*this, type, name,
+                                                       scratch);
   }
 
   // See `TypeProvider::FindStructTypeFieldByName`.
   absl::StatusOr<StructTypeFieldView> FindStructTypeFieldByName(
       StructTypeView type, absl::string_view name,
       StructTypeField& scratch ABSL_ATTRIBUTE_LIFETIME_BOUND) {
-    return GetTypeProvider().FindStructTypeFieldByName(GetTypeFactory(), type,
-                                                       name, scratch);
+    return GetTypeProvider().FindStructTypeFieldByName(*this, type, name,
+                                                       scratch);
   }
+
+ protected:
+  virtual TypeProvider& GetTypeProvider() const = 0;
 };
 
 // Creates a new `TypeManager` which is thread compatible.
 Shared<TypeManager> NewThreadCompatibleTypeManager(
-    MemoryManagerRef memory_manager, Shared<TypeFactory> type_factory,
-    Shared<TypeProvider> type_provider);
+    MemoryManagerRef memory_manager, Shared<TypeProvider> type_provider);
 
 // Creates a new `TypeManager` which is thread safe if and only if the provided
 // `TypeFactory` and `TypeProvider` are also thread safe.
 Shared<TypeManager> NewThreadSafeTypeManager(
-    MemoryManagerRef memory_manager, Shared<TypeFactory> type_factory,
-    Shared<TypeProvider> type_provider);
+    MemoryManagerRef memory_manager, Shared<TypeProvider> type_provider);
 
 }  // namespace cel
 

common/type_provider.cc

@@ -0,0 +1,36 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "common/type_provider.h"
+
+#include "common/memory.h"
+#include "common/types/thread_compatible_type_provider.h"
+#include "common/types/thread_safe_type_provider.h"
+
+namespace cel {
+
+Shared<TypeProvider> NewThreadCompatibleTypeProvider(
+    MemoryManagerRef memory_manager) {
+  return memory_manager
+      .MakeShared<common_internal::ThreadCompatibleTypeProvider>(
+          memory_manager);
+}
+
+Shared<TypeProvider> NewThreadSafeTypeProvider(
+    MemoryManagerRef memory_manager) {
+  return memory_manager.MakeShared<common_internal::ThreadSafeTypeProvider>(
+      memory_manager);
+}
+
+}  // namespace cel

common/type_testing.h

@@ -34,7 +34,6 @@ class ThreadCompatibleTypeTest : public ThreadCompatibleMemoryTest<Ts...> {
     Base::SetUp();
     type_manager_ = NewThreadCompatibleTypeManager(
         this->memory_manager(),
-        NewThreadCompatibleTypeFactory(this->memory_manager()),
         NewThreadCompatibleTypeProvider(this->memory_manager()));
   }
 
@@ -45,11 +44,7 @@ class ThreadCompatibleTypeTest : public ThreadCompatibleMemoryTest<Ts...> {
 
   TypeManager& type_manager() const { return **type_manager_; }
 
-  TypeFactory& type_factory() const { return type_manager().GetTypeFactory(); }
-
-  TypeProvider& type_provider() const {
-    return type_manager().GetTypeProvider();
-  }
+  TypeFactory& type_factory() const { return type_manager(); }
 
  private:
   absl::optional<Shared<TypeManager>> type_manager_;

common/types/thread_compatible_type_factory.cc → common/types/thread_compatible_type_manager.cc

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "common/types/thread_compatible_type_factory.h"
+#include "common/types/thread_compatible_type_manager.h"
 
 #include <utility>
 
@@ -23,7 +23,7 @@
 
 namespace cel::common_internal {
 
-ListType ThreadCompatibleTypeFactory::CreateListTypeImpl(TypeView element) {
+ListType ThreadCompatibleTypeManager::CreateListTypeImpl(TypeView element) {
   if (auto list_type = list_types_.find(element);
       list_type != list_types_.end()) {
     return list_type->second;
@@ -32,7 +32,7 @@ ListType ThreadCompatibleTypeFactory::CreateListTypeImpl(TypeView element) {
   return list_types_.insert({list_type.element(), list_type}).first->second;
 }
 
-MapType ThreadCompatibleTypeFactory::CreateMapTypeImpl(TypeView key,
+MapType ThreadCompatibleTypeManager::CreateMapTypeImpl(TypeView key,
                                                        TypeView value) {
   if (auto map_type = map_types_.find(std::make_pair(key, value));
       map_type != map_types_.end()) {
@@ -44,7 +44,7 @@ MapType ThreadCompatibleTypeFactory::CreateMapTypeImpl(TypeView key,
       .first->second;
 }
 
-StructType ThreadCompatibleTypeFactory::CreateStructTypeImpl(
+StructType ThreadCompatibleTypeManager::CreateStructTypeImpl(
     absl::string_view name) {
   if (auto struct_type = struct_types_.find(name);
       struct_type != struct_types_.end()) {
@@ -54,7 +54,7 @@ StructType ThreadCompatibleTypeFactory::CreateStructTypeImpl(
   return struct_types_.insert({struct_type.name(), struct_type}).first->second;
 }
 
-OpaqueType ThreadCompatibleTypeFactory::CreateOpaqueTypeImpl(
+OpaqueType ThreadCompatibleTypeManager::CreateOpaqueTypeImpl(
     absl::string_view name, const SizedInputView<TypeView>& parameters) {
   if (auto opaque_type = opaque_types_.find(
           OpaqueTypeKeyView{.name = name, .parameters = parameters});

common/types/thread_compatible_type_factory.h → common/types/thread_compatible_type_manager.h

@@ -14,25 +14,33 @@
 
 // IWYU pragma: private
 
-#ifndef THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_FACTORY_H_
-#define THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_FACTORY_H_
+#ifndef THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_
+#define THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_
+
+#include <utility>
 
 #include "absl/strings/string_view.h"
 #include "common/memory.h"
 #include "common/sized_input_view.h"
 #include "common/type.h"
-#include "common/type_factory.h"
+#include "common/type_manager.h"
+#include "common/type_provider.h"
 #include "common/types/type_cache.h"
 
 namespace cel::common_internal {
 
-class ThreadCompatibleTypeFactory : public virtual TypeFactory {
+class ThreadCompatibleTypeManager : public virtual TypeManager {
  public:
-  explicit ThreadCompatibleTypeFactory(MemoryManagerRef memory_manager)
-      : memory_manager_(memory_manager) {}
+  explicit ThreadCompatibleTypeManager(MemoryManagerRef memory_manager,
+                                       Shared<TypeProvider> type_provider)
+      : memory_manager_(memory_manager),
+        type_provider_(std::move(type_provider)) {}
 
   MemoryManagerRef GetMemoryManager() const final { return memory_manager_; }
 
+ protected:
+  TypeProvider& GetTypeProvider() const final { return *type_provider_; }
+
  private:
   ListType CreateListTypeImpl(TypeView element) final;
 
@@ -44,6 +52,7 @@ class ThreadCompatibleTypeFactory : public virtual TypeFactory {
       absl::string_view name, const SizedInputView<TypeView>& parameters) final;
 
   MemoryManagerRef memory_manager_;
+  Shared<TypeProvider> type_provider_;
   ListTypeCacheMap list_types_;
   MapTypeCacheMap map_types_;
   StructTypeCacheMap struct_types_;
@@ -52,4 +61,4 @@ class ThreadCompatibleTypeFactory : public virtual TypeFactory {
 
 }  // namespace cel::common_internal
 
-#endif  // THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_FACTORY_H_
+#endif  // THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_