refactor: use iterator and chain to parse_duration

src/parser/promql.y

@@ -390,11 +390,11 @@ string_literal -> Result<Expr, String>:
 
 %%
 
-use std::time::{Duration, Instant};
+use std::time::Duration;
 
 use crate::parser::{lexeme_to_string, span_to_string, lexeme_to_token};
 use crate::parser::{Expr, Token};
 
-use crate::label::{self, Label, Labels, MatchOp, Matcher, Matchers, METRIC_NAME, new_matcher};
+use crate::label::{Label, Labels, MatchOp, Matcher, Matchers, METRIC_NAME, new_matcher};
 
 use crate::util::parse_duration;

src/util/duration.rs

@@ -4,11 +4,28 @@ use std::time::Duration;
 
 lazy_static! {
     static ref DURATION_RE: Regex = Regex::new(
-        r"^((?P<year>[0-9]+)y)?((?P<week>[0-9]+)w)?((?P<day>[0-9]+)d)?((?P<hour>[0-9]+)h)?((?P<minute>[0-9]+)m)?((?P<second>[0-9]+)s)?((?P<milli>[0-9]+)ms)?$",
+        r"(?x)
+^
+((?P<y>[0-9]+)y)?
+((?P<w>[0-9]+)w)?
+((?P<d>[0-9]+)d)?
+((?P<h>[0-9]+)h)?
+((?P<m>[0-9]+)m)?
+((?P<s>[0-9]+)s)?
+((?P<ms>[0-9]+)ms)?
+$",
     )
     .unwrap();
 }
 
+const MILLI_DURATION: Duration = Duration::from_millis(1);
+const SECOND_DURATION: Duration = Duration::from_secs(1);
+const MINUTE_DURATION: Duration = Duration::from_secs(60);
+const HOUR_DURATION: Duration = Duration::from_secs(60 * 60);
+const DAY_DURATION: Duration = Duration::from_secs(60 * 60 * 24);
+const WEEK_DURATION: Duration = Duration::from_secs(60 * 60 * 24 * 7);
+const YEAR_DURATION: Duration = Duration::from_secs(60 * 60 * 24 * 365);
+
 /// parses a string into a Duration, assuming that a year
 /// always has 365d, a week always has 7d, and a day always has 24h.
 ///
@@ -36,28 +53,29 @@ pub fn parse_duration(ds: &str) -> Result<Duration, String> {
     }
 
     let caps = DURATION_RE.captures(ds).unwrap();
-    let mut result = Duration::ZERO;
-
-    let mut add = |title: &str, millis: u64| {
-        if let Some(cap) = caps.name(title) {
-            let v = cap.as_str().parse::<u64>().unwrap();
-            result = result + Duration::from_millis(v * millis);
-        };
-    };
-
-    add("year", 1000 * 60 * 60 * 24 * 365); // y
-    add("week", 1000 * 60 * 60 * 24 * 7); // w
-    add("day", 1000 * 60 * 60 * 24); // d
-    add("hour", 1000 * 60 * 60); // h
-    add("minute", 1000 * 60); // m
-    add("second", 1000); // s
-    add("milli", 1); // ms
-
-    if result > Duration::MAX {
-        return Err("duration out of range".into());
-    }
-
-    Ok(result)
+    [
+        ("y", YEAR_DURATION),
+        ("w", WEEK_DURATION),
+        ("d", DAY_DURATION),
+        ("h", HOUR_DURATION),
+        ("m", MINUTE_DURATION),
+        ("s", SECOND_DURATION),
+        ("ms", MILLI_DURATION),
+    ]
+    .into_iter()
+    // map captured string to Option<Duration> iterator
+    // FIXME: None is ignored in closure. It is better to tell users which part is wrong.
+    .map(|(title, duration): (&str, Duration)| {
+        caps.name(title)
+            .and_then(|cap| cap.as_str().parse::<u32>().ok())
+            .and_then(|v| duration.checked_mul(v))
+    })
+    .fold(Ok(Duration::ZERO), |acc, x| {
+        acc.and_then(|d| {
+            d.checked_add(x.unwrap_or(Duration::ZERO))
+                .ok_or("duration overflowed".into())
+        })
+    })
 }
 
 #[cfg(test)]
@@ -89,37 +107,43 @@ mod tests {
             ("0s", Duration::ZERO),
             ("324ms", Duration::from_millis(324)),
             ("3s", Duration::from_secs(3)),
-            ("5m", Duration::from_secs(300)),
-            ("1h", Duration::from_secs(3600)),
-            ("4d", Duration::from_secs(3600 * 24 * 4)),
-            ("4d1h", Duration::from_secs(3600 * 97)),
-            ("14d", Duration::from_secs(3600 * 24 * 14)),
-            ("3w", Duration::from_secs(3600 * 24 * 21)),
-            ("3w2d1h", Duration::from_secs(3600 * (23 * 24 + 1))),
-            ("10y", Duration::from_secs(3600 * 24 * 365 * 10)),
+            ("5m", MINUTE_DURATION * 5),
+            ("1h", HOUR_DURATION),
+            ("4d", DAY_DURATION * 4),
+            ("4d1h", DAY_DURATION * 4 + HOUR_DURATION),
+            ("14d", DAY_DURATION * 14),
+            ("3w", WEEK_DURATION * 3),
+            ("3w2d1h", WEEK_DURATION * 3 + HOUR_DURATION * 49),
+            ("10y", YEAR_DURATION * 10),
         ];
 
         for (s, expect) in ds {
             let d = parse_duration(s);
             assert!(d.is_ok());
-            assert_eq!(
-                expect.as_secs(),
-                d.unwrap().as_secs(),
-                "{} and {:?} not matched",
-                s,
-                expect
-            );
+            assert_eq!(expect, d.unwrap(), "{} and {:?} not matched", s, expect);
         }
     }
 
+    // valid here but invalid in PromQL Go Version
     #[test]
-    fn test_invalid_duration() {
+    fn test_diff_with_promql() {
         let ds = vec![
-            "1", "1y1m1d", "-1w", "1.5d", "d",
-            "",
-            // these are invalid in PromQL Go Version
-            // "294y", "200y10400w", "107675d", "2584200h",
+            ("294y", YEAR_DURATION * 294),
+            ("200y10400w", YEAR_DURATION * 200 + WEEK_DURATION * 10400),
+            ("107675d", DAY_DURATION * 107675),
+            ("2584200h", HOUR_DURATION * 2584200),
         ];
+
+        for (s, expect) in ds {
+            let d = parse_duration(s);
+            assert!(d.is_ok());
+            assert_eq!(expect, d.unwrap(), "{} and {:?} not matched", s, expect);
+        }
+    }
+
+    #[test]
+    fn test_invalid_duration() {
+        let ds = vec!["1", "1y1m1d", "-1w", "1.5d", "d", ""];
         for d in ds {
             assert!(parse_duration(d).is_err(), "{} is invalid duration!", d);
         }