common/options: use new parse_timespan

It's more complete and robust, and lives in ceph_time.h where it probably
belongs.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/common/options.cc b/src/common/options.cc
index 681c8a615367332307004588f5b78b33ca1a47d8..d1b7b88b957f9c7104f21bf847dd13f1e036b6ce 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -124,72 +124,6 @@ int Option::validate(const Option::value_t &new_value, std::string *err) const
   return 0;
 }
 
-namespace {
-template<class Duration>
-std::chrono::seconds
-do_parse_duration(const char* unit, string val,
-                 size_t start, size_t* new_start)
-{
-  auto found = val.find(unit, start);
-  if (found == val.npos) {
-    *new_start = start;
-    return Duration{0};
-  }
-  val[found] = '\0';
-  string err;
-  char* s = &val[start];
-  auto intervals = strict_strtoll(s, 10, &err);
-  if (!err.empty()) {
-    throw invalid_argument(s);
-  }
-  auto secs = chrono::duration_cast<chrono::seconds>(Duration{intervals});
-  *new_start = found + strlen(unit);
-  return secs;
-}
-
-std::chrono::seconds parse_duration(const std::string& s)
-{
-  using namespace std::chrono;
-  auto secs = 0s;
-  size_t start = 0;
-  size_t new_start = 0;
-  using days_t = duration<int, std::ratio<3600 * 24>>;
-  auto v = s;
-  v.erase(std::remove_if(begin(v), end(v),
-                        [](char c){ return std::isspace(c);}), end(v));
-  if (auto delta = do_parse_duration<days_t>("days", v, start, &new_start);
-      delta.count()) {
-    start = new_start;
-    secs += delta;
-  }
-  if (auto delta = do_parse_duration<hours>("hours", v, start, &new_start);
-      delta.count()) {
-    start = new_start;
-    secs += delta;
-  }
-  if (auto delta = do_parse_duration<minutes>("minutes", v, start, &new_start);
-      delta.count()) {
-    start = new_start;
-    secs += delta;
-  }
-  if (auto delta = do_parse_duration<seconds>("seconds", v, start, &new_start);
-      delta.count()) {
-    start = new_start;
-    secs += delta;
-  }
-  if (new_start == 0) {
-    string err;
-    if (auto delta = std::chrono::seconds{strict_strtoll(s.c_str(), 10, &err)};
-       err.empty()) {
-      secs += delta;
-    } else {
-      throw invalid_argument(err);
-    }
-  }
-  return secs;
-}
-} // anonymous namespace
-
 int Option::parse_value(
   const std::string& raw_val,
   value_t *out,
@@ -262,8 +196,9 @@ int Option::parse_value(
     *out = sz;
   } else if (type == Option::TYPE_SECS) {
     try {
-      *out = parse_duration(val);
-    } catch (const invalid_argument&) {
+      *out = parse_timespan(val);
+    } catch (const invalid_argument& e) {
+      *error_message = e.what();
       return -EINVAL;
     }
   } else {

diff --git a/src/test/common/test_config.cc b/src/test/common/test_config.cc
index 116ddbb3880f0da41fc36066ac8810a0dfa9e511..d72552b219924d0268f3b01e578350f5f6a0f41b 100644 (file)
--- a/src/test/common/test_config.cc
+++ b/src/test/common/test_config.cc
@@ -159,15 +159,77 @@ TEST(md_config_t, set_val)
     const string s{"1 days 2 hours 4 minutes"};
     using days_t = duration<int, std::ratio<3600 * 24>>;
     auto expected = (duration_cast<seconds>(days_t{1}) +
-                    duration_cast<seconds>(hours{2}) +
-                    duration_cast<seconds>(minutes{4}));
+                    duration_cast<seconds>(hours{2}) +
+                    duration_cast<seconds>(minutes{4}));
     EXPECT_EQ(0, conf.set_val("mgr_tick_period",
-                             "1 days 2 hours 4 minutes", nullptr));
+                             "1 days 2 hours 4 minutes", nullptr));
     EXPECT_EQ(expected.count(), conf.get_val<seconds>("mgr_tick_period").count());
     EXPECT_EQ(-EINVAL, conf.set_val("mgr_tick_period", "21 centuries", nullptr));
     EXPECT_EQ(expected.count(), conf.get_val<seconds>("mgr_tick_period").count());
   }
 
+  using namespace std::chrono;
+
+  using days_t = duration<int, std::ratio<3600 * 24>>;
+
+  struct testcase {
+    std::string s;
+    std::chrono::seconds r;
+  };
+  std::vector<testcase> good = {
+    { "23"s, duration_cast<seconds>(seconds{23}) },
+    { " 23 "s, duration_cast<seconds>(seconds{23}) },
+    { " 23s "s, duration_cast<seconds>(seconds{23}) },
+    { " 23 s "s, duration_cast<seconds>(seconds{23}) },
+    { " 23 sec "s, duration_cast<seconds>(seconds{23}) },
+    { "23 second "s, duration_cast<seconds>(seconds{23}) },
+    { "23 seconds"s, duration_cast<seconds>(seconds{23}) },
+    { "2m5s"s,  duration_cast<seconds>(seconds{2*60+5}) },
+    { "2 m 5 s "s,  duration_cast<seconds>(seconds{2*60+5}) },
+    { "2 m5"s,  duration_cast<seconds>(seconds{2*60+5}) },
+    { "2 min5"s,  duration_cast<seconds>(seconds{2*60+5}) },
+    { "2 minutes  5"s,  duration_cast<seconds>(seconds{2*60+5}) },
+    { "1w"s, duration_cast<seconds>(seconds{3600*24*7}) },
+    { "1wk"s, duration_cast<seconds>(seconds{3600*24*7}) },
+    { "1week"s, duration_cast<seconds>(seconds{3600*24*7}) },
+    { "1weeks"s, duration_cast<seconds>(seconds{3600*24*7}) },
+    { "1month"s, duration_cast<seconds>(seconds{3600*24*30}) },
+    { "1months"s, duration_cast<seconds>(seconds{3600*24*30}) },
+    { "1M"s, duration_cast<seconds>(seconds{3600*24*30}) },
+    { "1y"s, duration_cast<seconds>(seconds{3600*24*365}) },
+    { "1yr"s, duration_cast<seconds>(seconds{3600*24*365}) },
+    { "1year"s, duration_cast<seconds>(seconds{3600*24*365}) },
+    { "1years"s, duration_cast<seconds>(seconds{3600*24*365}) },
+    { "1d2h3m4s"s,
+      duration_cast<seconds>(days_t{1}) +
+      duration_cast<seconds>(hours{2}) +
+      duration_cast<seconds>(minutes{3}) +
+      duration_cast<seconds>(seconds{4}) },
+    { "1 days 2 hours 4 minutes"s,
+      duration_cast<seconds>(days_t{1}) +
+      duration_cast<seconds>(hours{2}) +
+      duration_cast<seconds>(minutes{4}) },
+  };
+
+  for (auto& i : good) {
+    cout << "good: " << i.s << " -> " << i.r.count() << std::endl;
+    EXPECT_EQ(0, conf.set_val("mgr_tick_period", i.s, nullptr));
+    EXPECT_EQ(i.r.count(), conf.get_val<seconds>("mgr_tick_period").count());
+  }
+
+  std::vector<std::string> bad = {
+    "12x",
+    "_ 12",
+    "1 2",
+    "21 centuries",
+    "1 y m",
+  };
+  for (auto& i : bad) {
+    std::stringstream err;
+    EXPECT_EQ(-EINVAL, conf.set_val("mgr_tick_period", i, &err));
+    cout << "bad: " << i << " -> " << err.str() << std::endl;
+  }
+
   for (int i = 0; i < 100; ++i) {
     std::chrono::seconds j = std::chrono::seconds(rand());
     string s = exact_timespan_str(j);