Allow CreateFromString to work on complex URIs (#8547)

Summary:
Some URIs for creating instances (ala SecondaryCache) use complex URIs like (cache://name;prop=value).  These URIs were treated as name-value properties.  With this change, if the URI does not contain an "id=XX" setting, it will be treated as a single string value (and not an ID and map of name-value properties).

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8547

Reviewed By: anand1976

Differential Revision: D29741386

Pulled By: mrambacher

fbshipit-source-id: 0621f62bec3a6699a7b66c7c0b5634b2856653aa

options/configurable.cc

@@ -747,15 +747,20 @@ Status ConfigurableHelper::GetOptionsMap(
 #ifndef ROCKSDB_LITE
   } else {
     status = StringToMap(value, props);
-    if (status.ok()) {
+    if (!status.ok()) {       // There was an error creating the map.
+      *id = value;            // Treat the value as id
+      props->clear();         // Clear the properties
+      status = Status::OK();  // and ignore the error
+    } else {
       auto iter = props->find(ConfigurableHelper::kIdPropName);
       if (iter != props->end()) {
         *id = iter->second;
         props->erase(iter);
-      } else if (default_id.empty()) {  // Should this be an error??
-        status = Status::InvalidArgument("Name property is missing");
-      } else {
+      } else if (!default_id.empty()) {
         *id = default_id;
+      } else {           // No id property and no default
+        *id = value;     // Treat the value as id
+        props->clear();  // Clear the properties
       }
     }
 #else

options/customizable_test.cc

@@ -785,6 +785,23 @@ TEST_F(CustomizableTest, FactoryFunctionTest) {
   ASSERT_EQ(pointer, nullptr);
 }
 
+TEST_F(CustomizableTest, URLFactoryTest) {
+  std::unique_ptr<TestCustomizable> unique;
+  ConfigOptions ignore = config_options_;
+  ignore.ignore_unsupported_options = false;
+  ignore.ignore_unsupported_options = false;
+  ASSERT_OK(TestCustomizable::CreateFromString(ignore, "A=1;x=y", &unique));
+  ASSERT_NE(unique, nullptr);
+  ASSERT_EQ(unique->GetId(), "A=1;x=y");
+  ASSERT_OK(TestCustomizable::CreateFromString(ignore, "A;x=y", &unique));
+  ASSERT_NE(unique, nullptr);
+  ASSERT_EQ(unique->GetId(), "A;x=y");
+  unique.reset();
+  ASSERT_OK(TestCustomizable::CreateFromString(ignore, "A=1?x=y", &unique));
+  ASSERT_NE(unique, nullptr);
+  ASSERT_EQ(unique->GetId(), "A=1?x=y");
+}
+
 TEST_F(CustomizableTest, MutableOptionsTest) {
   static std::unordered_map<std::string, OptionTypeInfo> mutable_option_info = {
       {"mutable",

options/options_helper.cc

@@ -644,10 +644,13 @@ Status StringToMap(const std::string& opts_str,
   }
 
   while (pos < opts.size()) {
-    size_t eq_pos = opts.find('=', pos);
+    size_t eq_pos = opts.find_first_of("={};", pos);
     if (eq_pos == std::string::npos) {
       return Status::InvalidArgument("Mismatched key value pair, '=' expected");
+    } else if (opts[eq_pos] != '=') {
+      return Status::InvalidArgument("Unexpected char in key");
     }
+
     std::string key = trim(opts.substr(pos, eq_pos - pos));
     if (key.empty()) {
       return Status::InvalidArgument("Empty key found");