Restore Regex support for ObjectLibrary::Register, rename new APIs to…

… allow old one to be deprecated in the future (facebook#9362) Summary: In order to support old-style regex function registration, restored the original "Register<T>(string, Factory)" method using regular expressions. The PatternEntry methods were left in place but renamed to AddFactory. The goal is to allow for the deprecation of the original regex Registry method in an upcoming release. Added modes to the PatternEntry kMatchZeroOrMore and kMatchAtLeastOne to match * or +, respectively (kMatchAtLeastOne was the original behavior). Pull Request resolved: facebook#9362 Reviewed By: pdillinger Differential Revision: D33432562 Pulled By: mrambacher fbshipit-source-id: ed88ab3f9a2ad0d525c7bd1692873f9bb3209d02
hx235 · Jan 11, 2022 · 1973fcb · 1973fcb
1 parent 6bab278
commit 1973fcb
Show file tree

Hide file tree

Showing 28 changed files with 310 additions and 183 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -4,6 +4,7 @@
 * Added values to `TraceFilterType`: `kTraceFilterIteratorSeek`, `kTraceFilterIteratorSeekForPrev`, and `kTraceFilterMultiGet`. They can be set in `TraceOptions` to filter out the operation types after which they are named.
 * Added `TraceOptions::preserve_write_order`. When enabled it  guarantees write records are traced in the same order they are logged to WAL and applied to the DB. By default it is disabled (false) to match the legacy behavior and prevent regression.
 * Made the Env class extend the Customizable class.  Implementations need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method.
+* Add ObjectLibrary::AddFactory and ObjectLibrary::PatternEntry classes.  This method and associated class are the preferred mechanism for registering factories with the ObjectLibrary going forward.  The ObjectLibrary::Register method, which uses regular expressions and may be problematic, is deprecated and will be in a future release.
 
 ### Behavior Changes
 * `DB::DestroyColumnFamilyHandle()` will return Status::InvalidArgument() if called with `DB::DefaultColumnFamily()`.

diff --git a/db/compaction/sst_partitioner.cc b/db/compaction/sst_partitioner.cc
@@ -62,7 +62,7 @@ std::shared_ptr<SstPartitionerFactory> NewSstPartitionerFixedPrefixFactory(
 namespace {
 static int RegisterSstPartitionerFactories(ObjectLibrary& library,
                                            const std::string& /*arg*/) {
-  library.Register<SstPartitionerFactory>(
+  library.AddFactory<SstPartitionerFactory>(
       SstPartitionerFixedPrefixFactory::kClassName(),
       [](const std::string& /*uri*/,
          std::unique_ptr<SstPartitionerFactory>* guard,

diff --git a/env/env.cc b/env/env.cc
@@ -33,13 +33,13 @@ namespace {
 #ifndef ROCKSDB_LITE
 static int RegisterBuiltinEnvs(ObjectLibrary& library,
                                const std::string& /*arg*/) {
-  library.Register<Env>(MockEnv::kClassName(), [](const std::string& /*uri*/,
-                                                  std::unique_ptr<Env>* guard,
-                                                  std::string* /* errmsg */) {
+  library.AddFactory<Env>(MockEnv::kClassName(), [](const std::string& /*uri*/,
+                                                    std::unique_ptr<Env>* guard,
+                                                    std::string* /* errmsg */) {
     guard->reset(MockEnv::Create(Env::Default()));
     return guard->get();
   });
-  library.Register<Env>(
+  library.AddFactory<Env>(
       CompositeEnvWrapper::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<Env>* guard,
          std::string* /* errmsg */) {
@@ -1294,7 +1294,7 @@ std::string SystemClockWrapper::SerializeOptions(
 #ifndef ROCKSDB_LITE
 static int RegisterBuiltinSystemClocks(ObjectLibrary& library,
                                        const std::string& /*arg*/) {
-  library.Register<SystemClock>(
+  library.AddFactory<SystemClock>(
       EmulatedSystemClock::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<SystemClock>* guard,
          std::string* /* errmsg */) {

diff --git a/env/env_encryption.cc b/env/env_encryption.cc
@@ -1275,7 +1275,7 @@ static void RegisterEncryptionBuiltins() {
   std::call_once(once, [&]() {
     auto lib = ObjectRegistry::Default()->AddLibrary("encryption");
     // Match "CTR" or "CTR://test"
-    lib->Register<EncryptionProvider>(
+    lib->AddFactory<EncryptionProvider>(
         ObjectLibrary::PatternEntry(CTREncryptionProvider::kClassName(), true)
             .AddSuffix("://test"),
         [](const std::string& uri, std::unique_ptr<EncryptionProvider>* guard,
@@ -1290,7 +1290,7 @@ static void RegisterEncryptionBuiltins() {
           return guard->get();
         });
 
-    lib->Register<EncryptionProvider>(
+    lib->AddFactory<EncryptionProvider>(
         "1://test", [](const std::string& /*uri*/,
                        std::unique_ptr<EncryptionProvider>* guard,
                        std::string* /*errmsg*/) {
@@ -1301,7 +1301,7 @@ static void RegisterEncryptionBuiltins() {
         });
 
     // Match "ROT13" or "ROT13:[0-9]+"
-    lib->Register<BlockCipher>(
+    lib->AddFactory<BlockCipher>(
         ObjectLibrary::PatternEntry(ROT13BlockCipher::kClassName(), true)
             .AddNumber(":"),
         [](const std::string& uri, std::unique_ptr<BlockCipher>* guard,

diff --git a/env/env_test.cc b/env/env_test.cc
@@ -2492,7 +2492,7 @@ TEST_F(CreateEnvTest, CreateDefaultSystemClock) {
 TEST_F(CreateEnvTest, CreateMockSystemClock) {
   std::shared_ptr<SystemClock> mock, copy;
 
-  config_options_.registry->AddLibrary("test")->Register<SystemClock>(
+  config_options_.registry->AddLibrary("test")->AddFactory<SystemClock>(
       MockSystemClock::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<SystemClock>* guard,
          std::string* /* errmsg */) {
@@ -2939,12 +2939,13 @@ class WrappedEnv : public EnvWrapper {
   static const char* kClassName() { return "WrappedEnv"; }
   const char* Name() const override { return kClassName(); }
   static void Register(ObjectLibrary& lib, const std::string& /*arg*/) {
-    lib.Register<Env>(WrappedEnv::kClassName(), [](const std::string& /*uri*/,
-                                                   std::unique_ptr<Env>* guard,
-                                                   std::string* /* errmsg */) {
-      guard->reset(new WrappedEnv(nullptr));
-      return guard->get();
-    });
+    lib.AddFactory<Env>(
+        WrappedEnv::kClassName(),
+        [](const std::string& /*uri*/, std::unique_ptr<Env>* guard,
+           std::string* /* errmsg */) {
+          guard->reset(new WrappedEnv(nullptr));
+          return guard->get();
+        });
   }
 };
 }  // namespace

diff --git a/env/file_system.cc b/env/file_system.cc
@@ -32,21 +32,21 @@ Status FileSystem::Load(const std::string& value,
 #ifndef ROCKSDB_LITE
 static int RegisterBuiltinFileSystems(ObjectLibrary& library,
                                       const std::string& /*arg*/) {
-  library.Register<FileSystem>(
+  library.AddFactory<FileSystem>(
       TimedFileSystem::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<FileSystem>* guard,
          std::string* /* errmsg */) {
         guard->reset(new TimedFileSystem(nullptr));
         return guard->get();
       });
-  library.Register<FileSystem>(
+  library.AddFactory<FileSystem>(
       ReadOnlyFileSystem::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<FileSystem>* guard,
          std::string* /* errmsg */) {
         guard->reset(new ReadOnlyFileSystem(nullptr));
         return guard->get();
       });
-  library.Register<FileSystem>(
+  library.AddFactory<FileSystem>(
       EncryptedFileSystem::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<FileSystem>* guard,
          std::string* errmsg) {
@@ -56,15 +56,15 @@ static int RegisterBuiltinFileSystems(ObjectLibrary& library,
         }
         return guard->get();
       });
-  library.Register<FileSystem>(
+  library.AddFactory<FileSystem>(
       MockFileSystem::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<FileSystem>* guard,
          std::string* /*errmsg*/) {
         guard->reset(new MockFileSystem(SystemClock::Default()));
         return guard->get();
       });
 #ifndef OS_WIN
-  library.Register<FileSystem>(
+  library.AddFactory<FileSystem>(
       ChrootFileSystem::kClassName(),
       [](const std::string& /*uri*/, std::unique_ptr<FileSystem>* guard,
          std::string* /* errmsg */) {

diff --git a/env/fs_posix.cc b/env/fs_posix.cc
@@ -1124,8 +1124,8 @@ std::shared_ptr<FileSystem> FileSystem::Default() {
 
 #ifndef ROCKSDB_LITE
 static FactoryFunc<FileSystem> posix_filesystem_reg =
-    ObjectLibrary::Default()->Register<FileSystem>(
-        "posix://.*",
+    ObjectLibrary::Default()->AddFactory<FileSystem>(
+        ObjectLibrary::PatternEntry("posix").AddSeparator("://", false),
         [](const std::string& /* uri */, std::unique_ptr<FileSystem>* f,
            std::string* /* errmsg */) {
           f->reset(new PosixFileSystem());