common: str_map: have 'get_str_map' only handling plain-text

'get_str_map()' used to handle both JSON and plain-text.  In fact it
would try parsing the map as JSON on a first try and then fallback to
plain-text if it failed.  Altough useful this would pose a big issue
when we attempted to parse some values, tha we knew to be plain-text,
that had some meaning in JSON -- e.g., 'false' or 'true'.  In such case
the JSON parser would spit out an error, stating it had been able to
parse the JSON but didn't expected the type, which in fairness is
acceptable.

In its stead we now have two functions: 'get_str_map()' will only handle
plain-text, whereas 'get_json_str_map()' will keep the previous
behavior, attempting to parse a JSON string and falling back to
'get_str_map()' should it fail.

Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>

diff --git a/src/common/str_map.cc b/src/common/str_map.cc
index f8e60634ce21b4cadabef376e40153f6b21672e3..719f94fc2cc326f996da3909b78b3926fb18cb03 100644 (file)
--- a/src/common/str_map.cc
+++ b/src/common/str_map.cc
@@ -23,9 +23,11 @@
 
 using namespace std;
 
-int get_str_map(const string &str,
-                ostream &ss,
-                map<string,string> *str_map)
+int get_json_str_map(
+    const string &str,
+    ostream &ss,
+    map<string,string> *str_map,
+    bool fallback_to_plain)
 {
   json_spirit::mValue json;
   try {
@@ -46,27 +48,46 @@ int get_str_map(const string &str,
         ++i) {
       (*str_map)[i->first] = i->second.get_str();
     }
-    
   } catch (json_spirit::Error_position &e) {
-    // fallback to key=value format
-
-    list<string> pairs;
-    get_str_list(str, "\t\n ", pairs);
-    for (list<string>::iterator i = pairs.begin(); i != pairs.end(); ++i) {
-      size_t equal = i->find('=');
-      if (equal == string::npos)
-       (*str_map)[*i] = string();
-      else {
-       const string key = i->substr(0, equal);
-       equal++;
-       const string value = i->substr(equal);
-       (*str_map)[key] = value;
-      }
+    if (fallback_to_plain) {
+      // fallback to key=value format
+      get_str_map(str, "\t\n ", str_map);
+    } else {
+      return -EINVAL;
+    }
+  }
+  return 0;
+}
+
+int get_str_map(
+    const string &str,
+    const char *delims,
+    map<string,string> *str_map)
+{
+  list<string> pairs;
+  get_str_list(str, delims, pairs);
+  for (list<string>::iterator i = pairs.begin(); i != pairs.end(); ++i) {
+    size_t equal = i->find('=');
+    if (equal == string::npos)
+      (*str_map)[*i] = string();
+    else {
+      const string key = i->substr(0, equal);
+      equal++;
+      const string value = i->substr(equal);
+      (*str_map)[key] = value;
     }
   }
   return 0;
 }
 
+int get_str_map(
+    const string &str,
+    map<string,string> *str_map)
+{
+  const char *delims = ",;\t\n ";
+  return get_str_map(str, delims, str_map);
+}
+
 string get_str_map_value(
     const map<string,string> &str_map,
     const string &key,

diff --git a/src/include/str_map.h b/src/include/str_map.h
index a695f3777152a0962905c2c8c722d5c4ea29cc7d..c2a8778583f15e5f7ab4d3aa6bf6667f3f6b9cfd 100644 (file)
--- a/src/include/str_map.h
+++ b/src/include/str_map.h
@@ -31,30 +31,70 @@
  * string, integer etc. ), -EINVAL is returned and **ss** is set to
  * a human readable error message.
  *
- * If **str** is no valid JSON, it is assumed to be a string
- * containing white space separated key=value pairs. A white space is
- * either space, tab or newline. The value is optional, in which case
- * it defaults to an empty string. For example:
+ * If **str** is no valid JSON and if **fallback_to_plain** is set to true
+ * (default: true) it is assumed to be a string containing white space
+ * separated key=value pairs. A white space is either space, tab or newline.
+ * Function **get_str_map** will be leveraged to parse the plain-text
+ * key/value pairs.
  * 
- *     insert your own=political    statement=here 
+ * @param [in] str JSON or plain text key/value pairs
+ * @param [out] ss human readable message on error
+ * @param [out] str_map key/value pairs read from str
+ * @param [in] fallback_to_plain attempt parsing as plain-text if json fails
+ * @return **0** on success or a -EINVAL on error.
+ */
+extern int get_json_str_map(
+    const std::string &str,
+    std::ostream &ss,
+    std::map<std::string,std::string> *str_map,
+    bool fallback_to_plain = true);
+
+/**
+ * Parse **str** and set **str_map** with the key/value pairs read from
+ * it.  The format of **str** is a number of custom key[=value] pairs in
+ * plain text format.
+ *
+ * The string will be parsed taking **delims** as field delimiters for
+ * key/values.  The value is optional resulting in an empty string when
+ * not provided.  For example, using white space as delimiters:
+ *
+ *     insert your own=political/ideological    statement=here 
  *
  * will be parsed into:
  *
  *     { "insert": "", 
  *       "your": "", 
- *       "own": "policital",
+ *       "own": "political/ideological",
  *       "statement": "here" }
  *
- * Returns 0 on success.
+ * Alternative delimiters may be provided.  For instance, specifying
+ * "white space and slash", for the above statement, would be parsed
+ * into:
  *
- * @param [in] str JSON or plain text key/value pairs
- * @param [out] ss human readable message on error
- * @param [out] str_map key/value pairs read from str
- * @return **0** on success or a -EINVAL on error.
+ *     { "insert": "",
+ *       "your": "",
+ *       "own": "political",
+ *       "ideological": "",
+ *       "statement": "here" }
+ *
+ * See how adding '/' to the delimiters field will spawn a new key without
+ * a set value.
+ *
+ * Always returns 0, as there is no condition for failure.
+ *
+ * @param [in] str plain text key/value pairs
+ * @param [out] str_map key/value pairs parsed from str
+ * @param [in] delim field delimiters to be used for parsing str
+ * @return **0**
  */
-extern int get_str_map(const std::string &str,
-                      std::ostream &ss,
-                      std::map<std::string,std::string> *str_map);
+extern int get_str_map(
+    const std::string &str,
+    const char *delims,
+    std::map<std::string,std::string> *str_map);
+
+extern int get_str_map(
+    const std::string &str,
+    std::map<std::string,std::string> *str_map);
 
 /**
  * Returns the value of **key** in **str_map** if available.

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 0c051660afd6dfeaff0b6382708f5c7d78cee75f..fb2157f23f394e26920a9c38ff326b431f306e21 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -3295,9 +3295,9 @@ int OSDMonitor::parse_erasure_code_profile(const vector<string> &erasure_code_pr
                                           map<string,string> *erasure_code_profile_map,
                                           stringstream &ss)
 {
-  int r = get_str_map(g_conf->osd_pool_default_erasure_code_profile,
-                     ss,
-                     erasure_code_profile_map);
+  int r = get_json_str_map(g_conf->osd_pool_default_erasure_code_profile,
+                          ss,
+                          erasure_code_profile_map);
   if (r)
     return r;
   (*erasure_code_profile_map)["directory"] =

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index 3dd0c1b5dc5ff01c46de209a8fae46f213e34bb0..2746787c50a57459e169376d612a31d06bd4e5de 100644 (file)
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -2587,7 +2587,7 @@ int OSDMap::get_erasure_code_profile_default(CephContext *cct,
                                             map<string,string> &profile_map,
                                             ostream *ss)
 {
-  int r = get_str_map(cct->_conf->osd_pool_default_erasure_code_profile,
+  int r = get_json_str_map(cct->_conf->osd_pool_default_erasure_code_profile,
                      *ss,
                      &profile_map);
   profile_map["directory"] =

diff --git a/src/test/common/test_str_map.cc b/src/test/common/test_str_map.cc
index f60f0d8bd05adbefb4d4ac57c5a960fb4e7eec7f..7ca0bddfcc335f95a1ef459945db96395d6b4a2b 100644 (file)
--- a/src/test/common/test_str_map.cc
+++ b/src/test/common/test_str_map.cc
@@ -25,10 +25,10 @@ TEST(str_map, json) {
   map<string,string> str_map;
   stringstream ss;
   // well formatted
-  ASSERT_EQ(0, get_str_map("{\"key\": \"value\"}", ss, &str_map));
+  ASSERT_EQ(0, get_json_str_map("{\"key\": \"value\"}", ss, &str_map));
   ASSERT_EQ("value", str_map["key"]);
   // well formatted but not a JSON object
-  ASSERT_EQ(-EINVAL, get_str_map("\"key\"", ss, &str_map));
+  ASSERT_EQ(-EINVAL, get_json_str_map("\"key\"", ss, &str_map));
   ASSERT_NE(string::npos, ss.str().find("must be a JSON object"));
 }
 
@@ -37,7 +37,7 @@ TEST(str_map, plaintext) {
   {
     map<string,string> str_map;
     ASSERT_EQ(0, get_str_map(" foo=bar\t\nfrob=nitz   yeah right=   \n\t",
-                            ss, &str_map));
+                            &str_map));
     ASSERT_EQ(4u, str_map.size());
     ASSERT_EQ("bar", str_map["foo"]);
     ASSERT_EQ("nitz", str_map["frob"]);
@@ -46,15 +46,15 @@ TEST(str_map, plaintext) {
   }
   {
     map<string,string> str_map;
-    ASSERT_EQ(0, get_str_map("that", ss, &str_map));
+    ASSERT_EQ(0, get_str_map("that", &str_map));
     ASSERT_EQ(1u, str_map.size());
     ASSERT_EQ("", str_map["that"]);
   }
   {
     map<string,string> str_map;
-    ASSERT_EQ(0, get_str_map(" \t \n ", ss, &str_map));
+    ASSERT_EQ(0, get_str_map(" \t \n ", &str_map));
     ASSERT_EQ(0u, str_map.size());
-    ASSERT_EQ(0, get_str_map("", ss, &str_map));
+    ASSERT_EQ(0, get_str_map("", &str_map));
     ASSERT_EQ(0u, str_map.size());
   }
 }