diff --git a/CMakeLists.txt b/CMakeLists.txt index 0d8d6a0b7..43e1480b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -848,7 +848,6 @@ set(SOURCES util/random.cc util/rate_limiter.cc util/ribbon_config.cc - util/regex.cc util/slice.cc util/file_checksum_helper.cc util/status.cc diff --git a/HISTORY.md b/HISTORY.md index b08c4005b..092a4333e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,7 @@ * Remove HDFS support from main repo. * Remove librados support from main repo. * Remove deprecated API DB::AddFile from main repo. +* Remove deprecated API ObjectLibrary::Register() and the (now obsolete) Regex public API. Use ObjectLibrary::AddFactory() with PatternEntry instead. ## 6.29.0 (01/21/2022) Note: The next release will be major release 7.0. See https://github.com/facebook/rocksdb/issues/9390 for more info. diff --git a/TARGETS b/TARGETS index 146c47a9c..325efd514 100644 --- a/TARGETS +++ b/TARGETS @@ -368,7 +368,6 @@ cpp_library( "util/murmurhash.cc", "util/random.cc", "util/rate_limiter.cc", - "util/regex.cc", "util/ribbon_config.cc", "util/slice.cc", "util/status.cc", @@ -697,7 +696,6 @@ cpp_library( "util/murmurhash.cc", "util/random.cc", "util/rate_limiter.cc", - "util/regex.cc", "util/ribbon_config.cc", "util/slice.cc", "util/status.cc", diff --git a/include/rocksdb/utilities/object_registry.h b/include/rocksdb/utilities/object_registry.h index a5c4b36c5..05e438bcd 100644 --- a/include/rocksdb/utilities/object_registry.h +++ b/include/rocksdb/utilities/object_registry.h @@ -16,7 +16,6 @@ #include #include "rocksdb/status.h" -#include "rocksdb/utilities/regex.h" namespace ROCKSDB_NAMESPACE { class Customizable; @@ -51,28 +50,6 @@ class ObjectLibrary { virtual const char* Name() const = 0; }; - // A class that implements an Entry based on Regex. - // - // WARNING: some regexes are problematic for std::regex; see - // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61582 for example - // - // This class is deprecated and will be removed in a future release - class RegexEntry : public Entry { - public: - explicit RegexEntry(const std::string& name) : name_(name) { - Regex::Parse(name, ®ex_).PermitUncheckedError(); - } - - bool Matches(const std::string& target) const override { - return regex_.Matches(target); - } - const char* Name() const override { return name_.c_str(); } - - private: - std::string name_; - Regex regex_; // The pattern for this entry - }; - public: // Class for matching target strings to a pattern. // Entries consist of a name that starts the pattern and attributes @@ -240,23 +217,6 @@ class ObjectLibrary { void Dump(Logger* logger) const; - // Registers the factory with the library for the regular expression pattern. - // If the pattern matches, the factory may be used to create a new object. - // - // WARNING: some regexes are problematic for std::regex; see - // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61582 for example - // - // Deprecated. Will be removed in a major release. Code should use AddFactory - // instead. - template - const FactoryFunc& Register(const std::string& pattern, - const FactoryFunc& factory) { - std::unique_ptr entry( - new FactoryEntry(new RegexEntry(pattern), factory)); - AddFactoryEntry(T::Type(), std::move(entry)); - return factory; - } - // Registers the factory with the library for the name. // If name==target, the factory may be used to create a new object. template @@ -272,6 +232,7 @@ class ObjectLibrary { // If the entry matches the target, the factory may be used to create a new // object. // @see PatternEntry for the matching rules. + // NOTE: This function replaces the old ObjectLibrary::Register() template const FactoryFunc& AddFactory(const PatternEntry& entry, const FactoryFunc& func) { diff --git a/include/rocksdb/utilities/regex.h b/include/rocksdb/utilities/regex.h deleted file mode 100644 index ac2431237..000000000 --- a/include/rocksdb/utilities/regex.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. -// This source code is licensed under both the GPLv2 (found in the -// COPYING file in the root directory) and Apache 2.0 License -// (found in the LICENSE.Apache file in the root directory). - -#pragma once - -#ifndef ROCKSDB_LITE - -#include -#include - -#include "rocksdb/status.h" - -namespace ROCKSDB_NAMESPACE { - -// A wrapper for parsed regular expressions. The regex syntax and matching is -// compatible with std::regex. -// -// !!!!!! WARNING !!!!!!: The implementation currently uses std::regex, which -// has terrible performance in some cases, including possible crash due to -// stack overflow. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61582 -// for example. Avoid use in production as much as possible. -// -// Internal note: see also TestRegex -class Regex { - public: - // Note: Cannot be constructed with a pattern, so that syntax errors can - // be handled without using exceptions. - - // Parse returns OK and saves to `out` when the pattern is valid regex - // syntax (modified ECMAScript), or else returns InvalidArgument. - // See https://en.cppreference.com/w/cpp/regex/ecmascript - static Status Parse(const char *pattern, Regex *out); - static Status Parse(const std::string &pattern, Regex *out); - - // Checks that the whole of str is matched by this regex. If called on a - // default-constructed Regex, will trigger assertion failure in DEBUG build - // or return false in release build. - bool Matches(const std::string &str) const; - - private: - class Impl; - std::shared_ptr impl_; // shared_ptr for simple implementation -}; -} // namespace ROCKSDB_NAMESPACE - -#endif // ROCKSDB_LITE diff --git a/src.mk b/src.mk index a450d0b82..a0c0191b6 100644 --- a/src.mk +++ b/src.mk @@ -223,7 +223,6 @@ LIB_SOURCES = \ util/random.cc \ util/rate_limiter.cc \ util/ribbon_config.cc \ - util/regex.cc \ util/slice.cc \ util/file_checksum_helper.cc \ util/status.cc \ diff --git a/test_util/testharness.cc b/test_util/testharness.cc index 899d39cc7..32d8a07d7 100644 --- a/test_util/testharness.cc +++ b/test_util/testharness.cc @@ -69,8 +69,6 @@ TestRegex::TestRegex(const char* pattern) const std::string& TestRegex::GetPattern() const { return pattern_; } -// Sorry about code duplication with regex.cc, but it doesn't support LITE -// due to exception handling class TestRegex::Impl : public std::regex { public: using std::regex::basic_regex; diff --git a/test_util/testharness.h b/test_util/testharness.h index e5db00dc6..8576eea62 100644 --- a/test_util/testharness.h +++ b/test_util/testharness.h @@ -14,7 +14,6 @@ #else #include #endif -#include "rocksdb/utilities/regex.h" // A "skipped" test has a specific meaning in Facebook infrastructure: the // test is in good shape and should be run, but something about the diff --git a/util/regex.cc b/util/regex.cc deleted file mode 100644 index 7aa4c52ae..000000000 --- a/util/regex.cc +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. -// This source code is licensed under both the GPLv2 (found in the -// COPYING file in the root directory) and Apache 2.0 License -// (found in the LICENSE.Apache file in the root directory). - -// LITE not supported here in part because of exception handling -#ifndef ROCKSDB_LITE - -#include "rocksdb/utilities/regex.h" - -#include -#include - -namespace ROCKSDB_NAMESPACE { - -// This section would change for alternate underlying implementations other -// than std::regex. -#if 1 -class Regex::Impl : public std::regex { - public: - using std::regex::basic_regex; -}; - -bool Regex::Matches(const std::string &str) const { - if (impl_) { - return std::regex_match(str, *impl_); - } else { - // Should not call Matches on unset Regex - assert(false); - return false; - } -} - -Status Regex::Parse(const std::string &pattern, Regex *out) { - try { - out->impl_.reset(new Impl(pattern)); - return Status::OK(); - } catch (const std::regex_error &e) { - return Status::InvalidArgument(e.what()); - } -} -#endif - -Status Regex::Parse(const char *pattern, Regex *out) { - return Parse(std::string(pattern), out); -} - -} // namespace ROCKSDB_NAMESPACE - -#endif // ROCKSDB_LITE diff --git a/util/slice_test.cc b/util/slice_test.cc index e3997ca33..6fea7e622 100644 --- a/util/slice_test.cc +++ b/util/slice_test.cc @@ -11,7 +11,6 @@ #include "port/stack_trace.h" #include "rocksdb/data_structure.h" #include "rocksdb/types.h" -#include "rocksdb/utilities/regex.h" #include "test_util/testharness.h" #include "test_util/testutil.h" @@ -177,35 +176,6 @@ TEST_F(SmallEnumSetTest, SmallSetTest) { ASSERT_FALSE(fs.Contains(FileType::kDBLockFile)); } -// ***************************************************************** // -// Unit test for Regex -#ifndef ROCKSDB_LITE -TEST(RegexTest, ParseEtc) { - Regex r; - ASSERT_OK(Regex::Parse("[abc]{5}", &r)); - ASSERT_TRUE(r.Matches("abcba")); - ASSERT_FALSE(r.Matches("abcb")); // too short - ASSERT_FALSE(r.Matches("abcbaa")); // too long - - ASSERT_OK(Regex::Parse(".*foo.*", &r)); - ASSERT_TRUE(r.Matches("123forfoodie456")); - ASSERT_FALSE(r.Matches("123forfodie456")); - // Ensure copy operator - Regex r2; - r2 = r; - ASSERT_TRUE(r2.Matches("123forfoodie456")); - ASSERT_FALSE(r2.Matches("123forfodie456")); - // Ensure copy constructor - Regex r3{r}; - ASSERT_TRUE(r3.Matches("123forfoodie456")); - ASSERT_FALSE(r3.Matches("123forfodie456")); - - ASSERT_TRUE(Regex::Parse("*foo.*", &r).IsInvalidArgument()); - ASSERT_TRUE(Regex::Parse("[abc", &r).IsInvalidArgument()); - ASSERT_TRUE(Regex::Parse("[abc]{1", &r).IsInvalidArgument()); -} -#endif // ROCKSDB_LITE - } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/utilities/object_registry_test.cc b/utilities/object_registry_test.cc index 77bf2ec1c..7880703f0 100644 --- a/utilities/object_registry_test.cc +++ b/utilities/object_registry_test.cc @@ -8,7 +8,6 @@ #include "rocksdb/utilities/object_registry.h" #include "rocksdb/customizable.h" -#include "rocksdb/utilities/regex.h" #include "test_util/testharness.h" namespace ROCKSDB_NAMESPACE { @@ -443,32 +442,6 @@ TEST_F(ObjRegistryTest, TestGetOrCreateManagedObject) { ASSERT_EQ(2, obj.use_count()); } -TEST_F(ObjRegistryTest, TestDeprecatedRegex) { - Regex regex; - Env* env = nullptr; - auto registry = ObjectRegistry::NewInstance(); - if (Regex::Parse("XYZ", ®ex).ok()) { - registry->AddLibrary("XYZ")->Register( - "XYZ", - [](const std::string& /*uri*/, std::unique_ptr* /*env_guard*/, - std::string* /* errmsg */) { return Env::Default(); }); - ASSERT_NOK(registry->NewStaticObject("X", &env)); - ASSERT_OK(registry->NewStaticObject("XYZ", &env)); - ASSERT_EQ(env, Env::Default()); - } - if (Regex::Parse("ABC://.*", ®ex).ok()) { - registry->AddLibrary("ABC")->Register( - "ABC://.*", - [](const std::string& /*uri*/, std::unique_ptr* /*env_guard*/, - std::string* /* errmsg */) { return Env::Default(); }); - ASSERT_NOK(registry->NewStaticObject("ABC", &env)); - ASSERT_OK(registry->NewStaticObject("ABC://123", &env)); - ASSERT_EQ(env, Env::Default()); - ASSERT_OK(registry->NewStaticObject("ABC://", &env)); - ASSERT_EQ(env, Env::Default()); - } -} - class PatternEntryTest : public testing::Test {}; TEST_F(PatternEntryTest, TestSimpleEntry) {