From f4385073be5ec087e4a6bb9171483bffba304721 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 7 Sep 2023 13:51:02 +0200 Subject: [PATCH 01/35] Update author constant --- src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli.rs b/src/cli.rs index d5c203576..375909c98 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -116,7 +116,7 @@ use std::{ path::{Path, PathBuf}, }; -pub const AUTHOR: &str = "Stackable GmbH - info@stackable.de"; +pub const AUTHOR: &str = "Stackable GmbH - info@stackable.tech"; /// Framework-standardized commands /// From 1bc051ac03107c46526b45e6c3a4a915bd0fa98a Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 7 Sep 2023 13:51:20 +0200 Subject: [PATCH 02/35] Add humantime Duration --- Cargo.toml | 1 + src/duration.rs | 164 ++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 3 files changed, 166 insertions(+) create mode 100644 src/duration.rs diff --git a/Cargo.toml b/Cargo.toml index c247e62e8..df25b9226 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ const_format = "0.2.31" derivative = "2.2.0" either = "1.9.0" futures = "0.3.28" +humantime = "2.1.0" json-patch = "1.0.0" k8s-openapi = { version = "0.19.0", default-features = false, features = ["schemars", "v1_27"] } # We use rustls instead of openssl for easier portablitly, e.g. so that we can build stackablectl without the need to vendor (build from source) openssl diff --git a/src/duration.rs b/src/duration.rs new file mode 100644 index 000000000..c5cc47cb4 --- /dev/null +++ b/src/duration.rs @@ -0,0 +1,164 @@ +//! This module contains a common [`Duration`] struct which is able to parse +//! human-readable duration formats, like `2y 2h 20m 42s`, `15d 2m 2s`, or +//! `2018-01-01T12:53:00Z` defined by [RFC 3339][1]. The [`Duration`] exported +//! in this module doesn't provide the parsing logic by itself, but instead +//! uses the crate [humantime]. It additionally implements many required +//! traits, like [`Derivative`], [`JsonSchema`], [`Deserialize`], and +//! [`Serialize`]. +//! +//! Furthermore, it implements [`Deref`], which enables us to use all associated +//! functions of [`humantime::Duration`] without re-implementing the public +//! functions on our own type. +//! +//! All operators should opt for [`Duration`] instead of the plain +//! [`std::time::Duration`] when dealing with durations of any form, like +//! timeouts or retries. +//! +//! [1]: https://www.rfc-editor.org/rfc/rfc3339 + +use std::{fmt::Display, ops::Deref, str::FromStr}; + +use derivative::Derivative; +use schemars::{ + gen::SchemaGenerator, + schema::{InstanceType, Schema, SchemaObject}, + JsonSchema, +}; +use serde::{de::Visitor, Deserialize, Serialize}; + +/// A [`Duration`] which is capable of parsing human-readable duration formats, +/// like `2y 2h 20m 42s`, `15d 2m 2s`, or `2018-01-01T12:53:00Z` defined by +/// [RFC 3339][1]. It additionally provides many required trait implementations, +/// which makes it suited for use in CRDs for example. +/// +/// [1]: https://www.rfc-editor.org/rfc/rfc3339 +#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq)] +pub struct Duration(humantime::Duration); + +struct DurationVisitor; + +impl<'de> Visitor<'de> for DurationVisitor { + type Value = Duration; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a string in any of the supported formats") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + let dur = v + .parse::() + .map_err(serde::de::Error::custom)?; + + Ok(Duration(dur)) + } +} + +impl<'de> Deserialize<'de> for Duration { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_any(DurationVisitor) + } +} + +impl Serialize for Duration { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(self.0.to_string().as_str()) + } +} + +impl JsonSchema for Duration { + fn schema_name() -> String { + "Duration".into() + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + SchemaObject { + instance_type: Some(InstanceType::String.into()), + ..Default::default() + } + .into() + } +} + +impl FromStr for Duration { + type Err = humantime::DurationError; + + fn from_str(s: &str) -> Result { + Ok(Self(s.parse::()?)) + } +} + +impl PartialOrd for Duration { + fn partial_cmp(&self, other: &Self) -> Option { + self.0.partial_cmp(&other.0) + } +} + +impl Display for Duration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Deref for Duration { + type Target = humantime::Duration; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[cfg(test)] +mod test { + use super::*; + use rstest::rstest; + + #[test] + fn deref() { + let dur: Duration = "1h".parse().unwrap(); + assert_eq!(dur.as_secs(), 3600); + } + + #[rstest] + #[case("2y 2h 20m 42s", 63123642)] + #[case("15d 2m 2s", 1296122)] + #[case("1h", 3600)] + #[case("1m", 60)] + #[case("1s", 1)] + fn parse(#[case] input: &str, #[case] output: u64) { + let dur: Duration = input.parse().unwrap(); + assert_eq!(dur.as_secs(), output); + } + + #[test] + fn deserialize() { + #[derive(Deserialize)] + struct S { + dur: Duration, + } + + let s: S = serde_yaml::from_str("dur: \"15d 2m 2s\"").unwrap(); + assert_eq!(s.dur.as_secs(), 1296122); + } + + #[test] + fn serialize() { + #[derive(Serialize)] + struct S { + dur: Duration, + } + + let s = S { + dur: "15d 2m 2s".parse().unwrap(), + }; + assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15days 2m 2s\n"); + } +} diff --git a/src/lib.rs b/src/lib.rs index 73b26be83..d42e1d4dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,7 @@ pub mod commons; pub mod config; pub mod cpu; pub mod crd; +pub mod duration; pub mod error; pub mod iter; pub mod label_selector; From a3f3a5e32a6997011e0dc8aadc7923686f05944f Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 7 Sep 2023 14:01:51 +0200 Subject: [PATCH 03/35] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72703f880..785a9f93e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Add `Duration` capable of parsing human-readable duration formats ([#647]) + +[#647]: https://github.com/stackabletech/operator-rs/pull/647 + ## [0.48.0] - 2023-08-18 ### Added From ae18cc56fb8f78f5976f65130ef5b69fc8d705d8 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 8 Sep 2023 09:47:39 +0200 Subject: [PATCH 04/35] Add `from_secs` and `from_millis` helper functions --- src/duration.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/duration.rs b/src/duration.rs index c5cc47cb4..4415d71af 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -116,6 +116,18 @@ impl Deref for Duration { } } +impl Duration { + /// Creates a new [`Duration`] from the specified number of whole seconds. + pub fn from_secs(secs: u64) -> Self { + Self(std::time::Duration::from_secs(secs).into()) + } + + /// Creates a new [`Duration`] from the specified number of milliseconds. + pub fn from_millis(millis: u64) -> Self { + Self(std::time::Duration::from_millis(millis).into()) + } +} + #[cfg(test)] mod test { use super::*; @@ -161,4 +173,13 @@ mod test { }; assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15days 2m 2s\n"); } + + #[test] + fn from_impls() { + let dur = Duration::from_secs(10); + assert_eq!(dur.to_string(), "10s"); + + let dur = Duration::from_millis(1000); + assert_eq!(dur.to_string(), "1s"); + } } From a524ef4adf93a928faa7c337db2dbcff345fe27d Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 8 Sep 2023 09:49:13 +0200 Subject: [PATCH 05/35] Trim trailing whitespace using pre-commit hook --- src/commons/authentication/static_.rs | 2 +- src/commons/opa.rs | 2 +- src/product_logging/framework.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/commons/authentication/static_.rs b/src/commons/authentication/static_.rs index bafa0e1f1..18b46f544 100644 --- a/src/commons/authentication/static_.rs +++ b/src/commons/authentication/static_.rs @@ -17,7 +17,7 @@ //! The update time depends on the kubelet syncing period. //! This would need additional functionality in restart controller to white- or blacklist certain volumes. Additionally, we would need a sidecar container that //! periodically converts the secret contents to the required product format. -//! +//! //! See use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/src/commons/opa.rs b/src/commons/opa.rs index 3aa1eb2dc..80890abb9 100644 --- a/src/commons/opa.rs +++ b/src/commons/opa.rs @@ -22,7 +22,7 @@ //! )] //! #[serde(rename_all = "camelCase")] //! pub struct TestClusterSpec { -//! opa: Option +//! opa: Option //! } //! //! let cluster: TestCluster = serde_yaml::from_str( diff --git a/src/product_logging/framework.rs b/src/product_logging/framework.rs index 7151cd68e..d9c5f285f 100644 --- a/src/product_logging/framework.rs +++ b/src/product_logging/framework.rs @@ -482,7 +482,7 @@ rootLogger.appenderRef.FILE.ref = FILE"#, /// * `config` - The logging configuration for the container /// * `additional_config` - Optional unstructured parameter to add special cases that are not /// covered in the logging configuration. Must adhere to the inner logback XML schema as -/// shown in the example below. It is not parsed or checked and added as is to the `logback.xml`. +/// shown in the example below. It is not parsed or checked and added as is to the `logback.xml`. /// /// # Example /// @@ -780,7 +780,7 @@ if includes(keys, "decision_id") {{ .logger = "decision" }} else {{ .logger = "server" -}} +}} .level = upcase!(parsed_event.level) .message = string!(parsed_event.msg) From 0e5658f4259b87d01a524fd1a2601eda129d8b00 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 8 Sep 2023 09:54:22 +0200 Subject: [PATCH 06/35] Fix markdownlint errors using pre-commit hook --- .github/pull_request_template.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 643ee52d2..1c68660d9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,4 +1,4 @@ -## Description +# Description *Please add a description here. This will become the commit message of the merge request later.* @@ -7,7 +7,6 @@ - Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant - Please make sure all these things are done and tick the boxes - ```[tasklist] # Author - [ ] Changes are OpenShift compatible From 794dca8a36d75da29af8053cc1c1605f4cabbf54 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 8 Sep 2023 15:48:53 +0200 Subject: [PATCH 07/35] Implement first iteration of custom parsing --- src/duration.rs | 263 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 184 insertions(+), 79 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index 4415d71af..181a74401 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -1,22 +1,17 @@ //! This module contains a common [`Duration`] struct which is able to parse -//! human-readable duration formats, like `2y 2h 20m 42s`, `15d 2m 2s`, or -//! `2018-01-01T12:53:00Z` defined by [RFC 3339][1]. The [`Duration`] exported -//! in this module doesn't provide the parsing logic by itself, but instead -//! uses the crate [humantime]. It additionally implements many required -//! traits, like [`Derivative`], [`JsonSchema`], [`Deserialize`], and -//! [`Serialize`]. +//! human-readable duration formats, like `2y 2h 20m 42s` or`15d 2m 2s`. It +//! additionally implements many required traits, like [`Derivative`], +//! [`JsonSchema`], [`Deserialize`], and [`Serialize`]. //! //! Furthermore, it implements [`Deref`], which enables us to use all associated -//! functions of [`humantime::Duration`] without re-implementing the public +//! functions of [`std::time::Duration`] without re-implementing the public //! functions on our own type. //! //! All operators should opt for [`Duration`] instead of the plain //! [`std::time::Duration`] when dealing with durations of any form, like //! timeouts or retries. -//! -//! [1]: https://www.rfc-editor.org/rfc/rfc3339 -use std::{fmt::Display, ops::Deref, str::FromStr}; +use std::{num::ParseIntError, ops::Deref, str::FromStr}; use derivative::Derivative; use schemars::{ @@ -24,16 +19,143 @@ use schemars::{ schema::{InstanceType, Schema, SchemaObject}, JsonSchema, }; -use serde::{de::Visitor, Deserialize, Serialize}; +use serde::{de::Visitor, Deserialize}; +use strum::Display; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum DurationParseError { + #[error("failed to parse string as number")] + ParseIntError(#[from] ParseIntError), + + #[error("expected a number, found character")] + ExpectedNumber, + + #[error("expected a character, found number")] + ExpectedChar, + + #[error("found invalid character")] + InvalidInput, + + #[error("found invalid unit")] + InvalidUnit, + + #[error("number overflow")] + NumberOverflow, +} /// A [`Duration`] which is capable of parsing human-readable duration formats, -/// like `2y 2h 20m 42s`, `15d 2m 2s`, or `2018-01-01T12:53:00Z` defined by -/// [RFC 3339][1]. It additionally provides many required trait implementations, -/// which makes it suited for use in CRDs for example. -/// -/// [1]: https://www.rfc-editor.org/rfc/rfc3339 -#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq)] -pub struct Duration(humantime::Duration); +/// like `2y 2h 20m 42s` or `15d 2m 2s`. It additionally provides many required +/// trait implementations, which makes it suited for use in CRDs for example. +#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd)] +pub struct Duration(std::time::Duration); + +#[derive(Copy, Clone, Debug, Display)] +enum DurationParseState { + Value, + Space, + Init, + Unit, + End, +} + +impl FromStr for Duration { + type Err = DurationParseError; + + fn from_str(input: &str) -> Result { + let mut state = DurationParseState::Init; + let mut buffer = String::new(); + let mut iter = input.chars(); + let mut is_unit = false; + let mut cur = 0 as char; + + let mut dur = std::time::Duration::from_secs(0); + let mut val = 0; + + loop { + state = match state { + DurationParseState::Init => match iter.next() { + Some(c) => { + cur = c; + + match c { + '0'..='9' => DurationParseState::Value, + 'a'..='z' => DurationParseState::Unit, + ' ' => DurationParseState::Space, + _ => return Err(DurationParseError::InvalidInput), + } + } + None => DurationParseState::End, + }, + DurationParseState::Value => { + if is_unit { + return Err(DurationParseError::ExpectedChar); + } + + buffer.push(cur); + DurationParseState::Init + } + DurationParseState::Unit => { + if !is_unit { + is_unit = true; + + val = buffer.parse::()?; + buffer.clear(); + } + + buffer.push(cur); + DurationParseState::Init + } + DurationParseState::Space => { + if !is_unit { + return Err(DurationParseError::ExpectedChar); + } + + let factor = parse_unit(&buffer)?; + + dur = dur + .checked_add(std::time::Duration::from_secs(val * factor)) + .ok_or(DurationParseError::NumberOverflow)?; + + is_unit = false; + buffer.clear(); + + DurationParseState::Init + } + DurationParseState::End => { + if !is_unit { + return Err(DurationParseError::ExpectedChar); + } + + let factor = parse_unit(&buffer)?; + + dur = dur + .checked_add(std::time::Duration::from_secs(val * factor)) + .ok_or(DurationParseError::NumberOverflow)?; + + break; + } + } + } + + Ok(Duration(dur)) + } +} + +fn parse_unit(buffer: &str) -> Result { + let factor = match buffer { + "seconds" | "second" | "secs" | "sec" | "s" => 1, + "minutes" | "minute" | "mins" | "min" | "m" => 60, + "hours" | "hour" | "hrs" | "hr" | "h" => 3600, + "days" | "day" | "d" => 86400, + "weeks" | "week" | "w" => 86400 * 7, + "months" | "month" | "M" => 2_630_016, + "years" | "year" | "y" => 31_557_600, + _ => return Err(DurationParseError::InvalidUnit), + }; + + Ok(factor) +} struct DurationVisitor; @@ -48,11 +170,8 @@ impl<'de> Visitor<'de> for DurationVisitor { where E: serde::de::Error, { - let dur = v - .parse::() - .map_err(serde::de::Error::custom)?; - - Ok(Duration(dur)) + let dur = v.parse::().map_err(serde::de::Error::custom)?; + Ok(dur) } } @@ -65,14 +184,14 @@ impl<'de> Deserialize<'de> for Duration { } } -impl Serialize for Duration { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - serializer.serialize_str(self.0.to_string().as_str()) - } -} +// impl Serialize for Duration { +// fn serialize(&self, serializer: S) -> Result +// where +// S: serde::Serializer, +// { +// serializer.serialize_str(self.0.to_string().as_str()) +// } +// } impl JsonSchema for Duration { fn schema_name() -> String { @@ -88,43 +207,35 @@ impl JsonSchema for Duration { } } -impl FromStr for Duration { - type Err = humantime::DurationError; - - fn from_str(s: &str) -> Result { - Ok(Self(s.parse::()?)) - } -} - -impl PartialOrd for Duration { - fn partial_cmp(&self, other: &Self) -> Option { - self.0.partial_cmp(&other.0) - } -} - -impl Display for Duration { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} +// impl Display for Duration { +// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +// write!(f, "{}", self.0) +// } +// } impl Deref for Duration { - type Target = humantime::Duration; + type Target = std::time::Duration; fn deref(&self) -> &Self::Target { &self.0 } } +impl From for Duration { + fn from(value: std::time::Duration) -> Self { + Self(value) + } +} + impl Duration { /// Creates a new [`Duration`] from the specified number of whole seconds. - pub fn from_secs(secs: u64) -> Self { - Self(std::time::Duration::from_secs(secs).into()) + pub const fn from_secs(secs: u64) -> Self { + Self(std::time::Duration::from_secs(secs)) } /// Creates a new [`Duration`] from the specified number of milliseconds. - pub fn from_millis(millis: u64) -> Self { - Self(std::time::Duration::from_millis(millis).into()) + pub const fn from_millis(millis: u64) -> Self { + Self(std::time::Duration::from_millis(millis)) } } @@ -133,12 +244,6 @@ mod test { use super::*; use rstest::rstest; - #[test] - fn deref() { - let dur: Duration = "1h".parse().unwrap(); - assert_eq!(dur.as_secs(), 3600); - } - #[rstest] #[case("2y 2h 20m 42s", 63123642)] #[case("15d 2m 2s", 1296122)] @@ -161,25 +266,25 @@ mod test { assert_eq!(s.dur.as_secs(), 1296122); } - #[test] - fn serialize() { - #[derive(Serialize)] - struct S { - dur: Duration, - } + // #[test] + // fn serialize() { + // #[derive(Serialize)] + // struct S { + // dur: Duration, + // } - let s = S { - dur: "15d 2m 2s".parse().unwrap(), - }; - assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15days 2m 2s\n"); - } + // let s = S { + // dur: "15d 2m 2s".parse().unwrap(), + // }; + // assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15days 2m 2s\n"); + // } - #[test] - fn from_impls() { - let dur = Duration::from_secs(10); - assert_eq!(dur.to_string(), "10s"); + // #[test] + // fn from_impls() { + // let dur = Duration::from_secs(10); + // assert_eq!(dur.to_string(), "10s"); - let dur = Duration::from_millis(1000); - assert_eq!(dur.to_string(), "1s"); - } + // let dur = Duration::from_millis(1000); + // assert_eq!(dur.to_string(), "1s"); + // } } From 8658713b034d640f740e35c38d21f818ab4a165d Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 8 Sep 2023 15:52:25 +0200 Subject: [PATCH 08/35] Remove humantime dependency --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index df25b9226..c247e62e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,6 @@ const_format = "0.2.31" derivative = "2.2.0" either = "1.9.0" futures = "0.3.28" -humantime = "2.1.0" json-patch = "1.0.0" k8s-openapi = { version = "0.19.0", default-features = false, features = ["schemars", "v1_27"] } # We use rustls instead of openssl for easier portablitly, e.g. so that we can build stackablectl without the need to vendor (build from source) openssl From 88d4677dae384c646b7072fb1933dd7d4006daa8 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 13 Sep 2023 12:53:09 +0200 Subject: [PATCH 09/35] Add `Display` impl, add various `ops` impls --- src/duration.rs | 189 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 139 insertions(+), 50 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index 181a74401..63ffc5317 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -11,7 +11,12 @@ //! [`std::time::Duration`] when dealing with durations of any form, like //! timeouts or retries. -use std::{num::ParseIntError, ops::Deref, str::FromStr}; +use std::{ + fmt::{Display, Write}, + num::ParseIntError, + ops::{Add, AddAssign, Deref, Sub, SubAssign}, + str::FromStr, +}; use derivative::Derivative; use schemars::{ @@ -19,10 +24,21 @@ use schemars::{ schema::{InstanceType, Schema, SchemaObject}, JsonSchema, }; -use serde::{de::Visitor, Deserialize}; +use serde::{de::Visitor, Deserialize, Serialize}; use strum::Display; use thiserror::Error; +const DAYS_IN_MONTH: f64 = 30.436875; +const DAYS_IN_YEAR: f64 = 365.2425; + +const YEARS_FACTOR: u64 = (DAYS_FACTOR as f64 * DAYS_IN_YEAR) as u64; +const MONTHS_FACTOR: u64 = (DAYS_FACTOR as f64 * DAYS_IN_MONTH) as u64; +const WEEKS_FACTOR: u64 = DAYS_FACTOR * 7; +const DAYS_FACTOR: u64 = HOURS_FACTOR * 24; +const HOURS_FACTOR: u64 = MINUTES_FACTOR * 60; +const MINUTES_FACTOR: u64 = SECONDS_FACTOR * 60; +const SECONDS_FACTOR: u64 = 1; + #[derive(Debug, Error)] pub enum DurationParseError { #[error("failed to parse string as number")] @@ -47,6 +63,9 @@ pub enum DurationParseError { /// A [`Duration`] which is capable of parsing human-readable duration formats, /// like `2y 2h 20m 42s` or `15d 2m 2s`. It additionally provides many required /// trait implementations, which makes it suited for use in CRDs for example. +/// +/// The maximum granularity currently supported is **seconds**. Support for +/// milliseconds can be added, when there is the need for it. #[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd)] pub struct Duration(std::time::Duration); @@ -144,13 +163,13 @@ impl FromStr for Duration { fn parse_unit(buffer: &str) -> Result { let factor = match buffer { - "seconds" | "second" | "secs" | "sec" | "s" => 1, - "minutes" | "minute" | "mins" | "min" | "m" => 60, - "hours" | "hour" | "hrs" | "hr" | "h" => 3600, - "days" | "day" | "d" => 86400, - "weeks" | "week" | "w" => 86400 * 7, - "months" | "month" | "M" => 2_630_016, - "years" | "year" | "y" => 31_557_600, + "seconds" | "second" | "secs" | "sec" | "s" => SECONDS_FACTOR, + "minutes" | "minute" | "mins" | "min" | "m" => MINUTES_FACTOR, + "hours" | "hour" | "hrs" | "hr" | "h" => HOURS_FACTOR, + "days" | "day" | "d" => DAYS_FACTOR, + "weeks" | "week" | "w" => WEEKS_FACTOR, + "months" | "month" | "M" => MONTHS_FACTOR, + "years" | "year" | "y" => YEARS_FACTOR, _ => return Err(DurationParseError::InvalidUnit), }; @@ -184,14 +203,14 @@ impl<'de> Deserialize<'de> for Duration { } } -// impl Serialize for Duration { -// fn serialize(&self, serializer: S) -> Result -// where -// S: serde::Serializer, -// { -// serializer.serialize_str(self.0.to_string().as_str()) -// } -// } +impl Serialize for Duration { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} impl JsonSchema for Duration { fn schema_name() -> String { @@ -207,11 +226,32 @@ impl JsonSchema for Duration { } } -// impl Display for Duration { -// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { -// write!(f, "{}", self.0) -// } -// } +impl Display for Duration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut secs = self.0.as_secs(); + let mut formatted = String::new(); + + for (factor, unit) in [ + (YEARS_FACTOR, "y"), + (MONTHS_FACTOR, "M"), + (DAYS_FACTOR, "d"), + (HOURS_FACTOR, "h"), + (MINUTES_FACTOR, "m"), + (SECONDS_FACTOR, "s"), + ] { + let whole = secs / factor; + let rest = secs % factor; + + if whole > 0 { + write!(formatted, "{}{} ", whole, unit)?; + } + + secs = rest; + } + + write!(f, "{}", formatted.trim_end()) + } +} impl Deref for Duration { type Target = std::time::Duration; @@ -227,16 +267,39 @@ impl From for Duration { } } +impl Add for Duration { + type Output = Self; + + fn add(self, rhs: Self) -> Self::Output { + Self::from(self.0 + rhs.0) + } +} + +impl Sub for Duration { + type Output = Duration; + + fn sub(self, rhs: Self) -> Self::Output { + Self::from(self.0 - rhs.0) + } +} + +impl AddAssign for Duration { + fn add_assign(&mut self, rhs: Self) { + self.0.add_assign(rhs.0) + } +} + +impl SubAssign for Duration { + fn sub_assign(&mut self, rhs: Self) { + self.0.sub_assign(rhs.0) + } +} + impl Duration { /// Creates a new [`Duration`] from the specified number of whole seconds. pub const fn from_secs(secs: u64) -> Self { Self(std::time::Duration::from_secs(secs)) } - - /// Creates a new [`Duration`] from the specified number of milliseconds. - pub const fn from_millis(millis: u64) -> Self { - Self(std::time::Duration::from_millis(millis)) - } } #[cfg(test)] @@ -245,7 +308,7 @@ mod test { use rstest::rstest; #[rstest] - #[case("2y 2h 20m 42s", 63123642)] + #[case("2y 2h 20m 42s", 63122346)] #[case("15d 2m 2s", 1296122)] #[case("1h", 3600)] #[case("1m", 60)] @@ -255,6 +318,17 @@ mod test { assert_eq!(dur.as_secs(), output); } + #[rstest] + #[case("2y 2h 20m 42s")] + #[case("15d 2m 2s")] + #[case("1h")] + #[case("1m")] + #[case("1s")] + fn to_string(#[case] duration: &str) { + let dur: Duration = duration.parse().unwrap(); + assert_eq!(dur.to_string(), duration); + } + #[test] fn deserialize() { #[derive(Deserialize)] @@ -262,29 +336,44 @@ mod test { dur: Duration, } - let s: S = serde_yaml::from_str("dur: \"15d 2m 2s\"").unwrap(); + let s: S = serde_yaml::from_str("dur: 15d 2m 2s").unwrap(); assert_eq!(s.dur.as_secs(), 1296122); } - // #[test] - // fn serialize() { - // #[derive(Serialize)] - // struct S { - // dur: Duration, - // } - - // let s = S { - // dur: "15d 2m 2s".parse().unwrap(), - // }; - // assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15days 2m 2s\n"); - // } - - // #[test] - // fn from_impls() { - // let dur = Duration::from_secs(10); - // assert_eq!(dur.to_string(), "10s"); - - // let dur = Duration::from_millis(1000); - // assert_eq!(dur.to_string(), "1s"); - // } + #[test] + fn serialize() { + #[derive(Serialize)] + struct S { + dur: Duration, + } + + let s = S { + dur: "15d 2m 2s".parse().unwrap(), + }; + assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15d 2m 2s\n"); + } + + #[test] + fn add_ops() { + let mut dur1 = Duration::from_secs(20); + let dur2 = Duration::from_secs(10); + + let dur = dur1 + dur2; + assert_eq!(dur.as_secs(), 30); + + dur1 += dur2; + assert_eq!(dur1.as_secs(), 30); + } + + #[test] + fn sub_ops() { + let mut dur1 = Duration::from_secs(20); + let dur2 = Duration::from_secs(10); + + let dur = dur1 - dur2; + assert_eq!(dur.as_secs(), 10); + + dur1 -= dur2; + assert_eq!(dur1.as_secs(), 10); + } } From 128eff76f3146cd9373cacc50d62fbcb5b714f34 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 13 Sep 2023 13:05:43 +0200 Subject: [PATCH 10/35] Add negative tests --- src/duration.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index 63ffc5317..107780e45 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -39,14 +39,11 @@ const HOURS_FACTOR: u64 = MINUTES_FACTOR * 60; const MINUTES_FACTOR: u64 = SECONDS_FACTOR * 60; const SECONDS_FACTOR: u64 = 1; -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq)] pub enum DurationParseError { #[error("failed to parse string as number")] ParseIntError(#[from] ParseIntError), - #[error("expected a number, found character")] - ExpectedNumber, - #[error("expected a character, found number")] ExpectedChar, @@ -82,6 +79,11 @@ impl FromStr for Duration { type Err = DurationParseError; fn from_str(input: &str) -> Result { + let input = input.trim(); + if input.is_empty() || !input.is_ascii() { + return Err(DurationParseError::InvalidInput); + } + let mut state = DurationParseState::Init; let mut buffer = String::new(); let mut iter = input.chars(); @@ -318,6 +320,18 @@ mod test { assert_eq!(dur.as_secs(), output); } + #[rstest] + #[case("2y2", DurationParseError::ExpectedChar)] + #[case("-1y", DurationParseError::InvalidInput)] + #[case("1Y", DurationParseError::InvalidInput)] + #[case("1ä", DurationParseError::InvalidInput)] + #[case("1q", DurationParseError::InvalidUnit)] + #[case(" ", DurationParseError::InvalidInput)] + fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { + let err = Duration::from_str(input).unwrap_err(); + assert_eq!(err, expected_err) + } + #[rstest] #[case("2y 2h 20m 42s")] #[case("15d 2m 2s")] From c4d22b004a1b465be53d95334b97c3d15361b73c Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 14 Sep 2023 14:09:24 +0200 Subject: [PATCH 11/35] Rework `Duration` implementation based on @nightkr gist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reworks/simplifies the `Duration` implementation. Supported units are now clearly defned by an enum and the parsing/display mechanism is now way more explicit and defined in one place. Co-authored-by: Natalie Klestrup Röijezon --- src/duration.rs | 393 ------------------------------------- src/duration/mod.rs | 335 +++++++++++++++++++++++++++++++ src/duration/serde_impl.rs | 39 ++++ 3 files changed, 374 insertions(+), 393 deletions(-) delete mode 100644 src/duration.rs create mode 100644 src/duration/mod.rs create mode 100644 src/duration/serde_impl.rs diff --git a/src/duration.rs b/src/duration.rs deleted file mode 100644 index 107780e45..000000000 --- a/src/duration.rs +++ /dev/null @@ -1,393 +0,0 @@ -//! This module contains a common [`Duration`] struct which is able to parse -//! human-readable duration formats, like `2y 2h 20m 42s` or`15d 2m 2s`. It -//! additionally implements many required traits, like [`Derivative`], -//! [`JsonSchema`], [`Deserialize`], and [`Serialize`]. -//! -//! Furthermore, it implements [`Deref`], which enables us to use all associated -//! functions of [`std::time::Duration`] without re-implementing the public -//! functions on our own type. -//! -//! All operators should opt for [`Duration`] instead of the plain -//! [`std::time::Duration`] when dealing with durations of any form, like -//! timeouts or retries. - -use std::{ - fmt::{Display, Write}, - num::ParseIntError, - ops::{Add, AddAssign, Deref, Sub, SubAssign}, - str::FromStr, -}; - -use derivative::Derivative; -use schemars::{ - gen::SchemaGenerator, - schema::{InstanceType, Schema, SchemaObject}, - JsonSchema, -}; -use serde::{de::Visitor, Deserialize, Serialize}; -use strum::Display; -use thiserror::Error; - -const DAYS_IN_MONTH: f64 = 30.436875; -const DAYS_IN_YEAR: f64 = 365.2425; - -const YEARS_FACTOR: u64 = (DAYS_FACTOR as f64 * DAYS_IN_YEAR) as u64; -const MONTHS_FACTOR: u64 = (DAYS_FACTOR as f64 * DAYS_IN_MONTH) as u64; -const WEEKS_FACTOR: u64 = DAYS_FACTOR * 7; -const DAYS_FACTOR: u64 = HOURS_FACTOR * 24; -const HOURS_FACTOR: u64 = MINUTES_FACTOR * 60; -const MINUTES_FACTOR: u64 = SECONDS_FACTOR * 60; -const SECONDS_FACTOR: u64 = 1; - -#[derive(Debug, Error, PartialEq)] -pub enum DurationParseError { - #[error("failed to parse string as number")] - ParseIntError(#[from] ParseIntError), - - #[error("expected a character, found number")] - ExpectedChar, - - #[error("found invalid character")] - InvalidInput, - - #[error("found invalid unit")] - InvalidUnit, - - #[error("number overflow")] - NumberOverflow, -} - -/// A [`Duration`] which is capable of parsing human-readable duration formats, -/// like `2y 2h 20m 42s` or `15d 2m 2s`. It additionally provides many required -/// trait implementations, which makes it suited for use in CRDs for example. -/// -/// The maximum granularity currently supported is **seconds**. Support for -/// milliseconds can be added, when there is the need for it. -#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd)] -pub struct Duration(std::time::Duration); - -#[derive(Copy, Clone, Debug, Display)] -enum DurationParseState { - Value, - Space, - Init, - Unit, - End, -} - -impl FromStr for Duration { - type Err = DurationParseError; - - fn from_str(input: &str) -> Result { - let input = input.trim(); - if input.is_empty() || !input.is_ascii() { - return Err(DurationParseError::InvalidInput); - } - - let mut state = DurationParseState::Init; - let mut buffer = String::new(); - let mut iter = input.chars(); - let mut is_unit = false; - let mut cur = 0 as char; - - let mut dur = std::time::Duration::from_secs(0); - let mut val = 0; - - loop { - state = match state { - DurationParseState::Init => match iter.next() { - Some(c) => { - cur = c; - - match c { - '0'..='9' => DurationParseState::Value, - 'a'..='z' => DurationParseState::Unit, - ' ' => DurationParseState::Space, - _ => return Err(DurationParseError::InvalidInput), - } - } - None => DurationParseState::End, - }, - DurationParseState::Value => { - if is_unit { - return Err(DurationParseError::ExpectedChar); - } - - buffer.push(cur); - DurationParseState::Init - } - DurationParseState::Unit => { - if !is_unit { - is_unit = true; - - val = buffer.parse::()?; - buffer.clear(); - } - - buffer.push(cur); - DurationParseState::Init - } - DurationParseState::Space => { - if !is_unit { - return Err(DurationParseError::ExpectedChar); - } - - let factor = parse_unit(&buffer)?; - - dur = dur - .checked_add(std::time::Duration::from_secs(val * factor)) - .ok_or(DurationParseError::NumberOverflow)?; - - is_unit = false; - buffer.clear(); - - DurationParseState::Init - } - DurationParseState::End => { - if !is_unit { - return Err(DurationParseError::ExpectedChar); - } - - let factor = parse_unit(&buffer)?; - - dur = dur - .checked_add(std::time::Duration::from_secs(val * factor)) - .ok_or(DurationParseError::NumberOverflow)?; - - break; - } - } - } - - Ok(Duration(dur)) - } -} - -fn parse_unit(buffer: &str) -> Result { - let factor = match buffer { - "seconds" | "second" | "secs" | "sec" | "s" => SECONDS_FACTOR, - "minutes" | "minute" | "mins" | "min" | "m" => MINUTES_FACTOR, - "hours" | "hour" | "hrs" | "hr" | "h" => HOURS_FACTOR, - "days" | "day" | "d" => DAYS_FACTOR, - "weeks" | "week" | "w" => WEEKS_FACTOR, - "months" | "month" | "M" => MONTHS_FACTOR, - "years" | "year" | "y" => YEARS_FACTOR, - _ => return Err(DurationParseError::InvalidUnit), - }; - - Ok(factor) -} - -struct DurationVisitor; - -impl<'de> Visitor<'de> for DurationVisitor { - type Value = Duration; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - formatter.write_str("a string in any of the supported formats") - } - - fn visit_str(self, v: &str) -> Result - where - E: serde::de::Error, - { - let dur = v.parse::().map_err(serde::de::Error::custom)?; - Ok(dur) - } -} - -impl<'de> Deserialize<'de> for Duration { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - deserializer.deserialize_any(DurationVisitor) - } -} - -impl Serialize for Duration { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - serializer.serialize_str(&self.to_string()) - } -} - -impl JsonSchema for Duration { - fn schema_name() -> String { - "Duration".into() - } - - fn json_schema(_: &mut SchemaGenerator) -> Schema { - SchemaObject { - instance_type: Some(InstanceType::String.into()), - ..Default::default() - } - .into() - } -} - -impl Display for Duration { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut secs = self.0.as_secs(); - let mut formatted = String::new(); - - for (factor, unit) in [ - (YEARS_FACTOR, "y"), - (MONTHS_FACTOR, "M"), - (DAYS_FACTOR, "d"), - (HOURS_FACTOR, "h"), - (MINUTES_FACTOR, "m"), - (SECONDS_FACTOR, "s"), - ] { - let whole = secs / factor; - let rest = secs % factor; - - if whole > 0 { - write!(formatted, "{}{} ", whole, unit)?; - } - - secs = rest; - } - - write!(f, "{}", formatted.trim_end()) - } -} - -impl Deref for Duration { - type Target = std::time::Duration; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl From for Duration { - fn from(value: std::time::Duration) -> Self { - Self(value) - } -} - -impl Add for Duration { - type Output = Self; - - fn add(self, rhs: Self) -> Self::Output { - Self::from(self.0 + rhs.0) - } -} - -impl Sub for Duration { - type Output = Duration; - - fn sub(self, rhs: Self) -> Self::Output { - Self::from(self.0 - rhs.0) - } -} - -impl AddAssign for Duration { - fn add_assign(&mut self, rhs: Self) { - self.0.add_assign(rhs.0) - } -} - -impl SubAssign for Duration { - fn sub_assign(&mut self, rhs: Self) { - self.0.sub_assign(rhs.0) - } -} - -impl Duration { - /// Creates a new [`Duration`] from the specified number of whole seconds. - pub const fn from_secs(secs: u64) -> Self { - Self(std::time::Duration::from_secs(secs)) - } -} - -#[cfg(test)] -mod test { - use super::*; - use rstest::rstest; - - #[rstest] - #[case("2y 2h 20m 42s", 63122346)] - #[case("15d 2m 2s", 1296122)] - #[case("1h", 3600)] - #[case("1m", 60)] - #[case("1s", 1)] - fn parse(#[case] input: &str, #[case] output: u64) { - let dur: Duration = input.parse().unwrap(); - assert_eq!(dur.as_secs(), output); - } - - #[rstest] - #[case("2y2", DurationParseError::ExpectedChar)] - #[case("-1y", DurationParseError::InvalidInput)] - #[case("1Y", DurationParseError::InvalidInput)] - #[case("1ä", DurationParseError::InvalidInput)] - #[case("1q", DurationParseError::InvalidUnit)] - #[case(" ", DurationParseError::InvalidInput)] - fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { - let err = Duration::from_str(input).unwrap_err(); - assert_eq!(err, expected_err) - } - - #[rstest] - #[case("2y 2h 20m 42s")] - #[case("15d 2m 2s")] - #[case("1h")] - #[case("1m")] - #[case("1s")] - fn to_string(#[case] duration: &str) { - let dur: Duration = duration.parse().unwrap(); - assert_eq!(dur.to_string(), duration); - } - - #[test] - fn deserialize() { - #[derive(Deserialize)] - struct S { - dur: Duration, - } - - let s: S = serde_yaml::from_str("dur: 15d 2m 2s").unwrap(); - assert_eq!(s.dur.as_secs(), 1296122); - } - - #[test] - fn serialize() { - #[derive(Serialize)] - struct S { - dur: Duration, - } - - let s = S { - dur: "15d 2m 2s".parse().unwrap(), - }; - assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15d 2m 2s\n"); - } - - #[test] - fn add_ops() { - let mut dur1 = Duration::from_secs(20); - let dur2 = Duration::from_secs(10); - - let dur = dur1 + dur2; - assert_eq!(dur.as_secs(), 30); - - dur1 += dur2; - assert_eq!(dur1.as_secs(), 30); - } - - #[test] - fn sub_ops() { - let mut dur1 = Duration::from_secs(20); - let dur2 = Duration::from_secs(10); - - let dur = dur1 - dur2; - assert_eq!(dur.as_secs(), 10); - - dur1 -= dur2; - assert_eq!(dur1.as_secs(), 10); - } -} diff --git a/src/duration/mod.rs b/src/duration/mod.rs new file mode 100644 index 000000000..d667781e7 --- /dev/null +++ b/src/duration/mod.rs @@ -0,0 +1,335 @@ +//! This module contains a common [`Duration`] struct which is able to parse +//! human-readable duration formats, like `2y 2h 20m 42s` or`15d 2m 2s`. It +//! additionally implements many required traits, like [`Derivative`], +//! [`JsonSchema`], [`Deserialize`], and [`Serialize`]. +//! +//! Furthermore, it implements [`Deref`], which enables us to use all associated +//! functions of [`std::time::Duration`] without re-implementing the public +//! functions on our own type. +//! +//! All operators should opt for [`Duration`] instead of the plain +//! [`std::time::Duration`] when dealing with durations of any form, like +//! timeouts or retries. + +use std::{ + fmt::{Display, Write}, + num::ParseIntError, + ops::{Deref, DerefMut}, + str::FromStr, +}; + +use derivative::Derivative; +use schemars::JsonSchema; +use strum::IntoEnumIterator; +use thiserror::Error; + +mod serde_impl; +pub use serde_impl::*; + +#[derive(Debug, Error, PartialEq)] +pub enum DurationParseError { + #[error("invalid input, either empty or contains non-ascii characters")] + InvalidInput, + + #[error("failed to parse duration fragment")] + FragmentError(#[from] DurationFragmentParseError), +} + +#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd, JsonSchema)] +pub struct Duration(std::time::Duration); + +impl FromStr for Duration { + type Err = DurationParseError; + + fn from_str(s: &str) -> Result { + let input = s.trim(); + + // An empty or non-ascii input is invalid + if input.is_empty() || !input.is_ascii() { + return Err(DurationParseError::InvalidInput); + } + + let parts: Vec<&str> = input.split(' ').collect(); + + let values: Vec = parts + .iter() + .map(|p| p.parse::()) + .map(|r| r.map(|f| f.millis())) + .collect::, DurationFragmentParseError>>()?; + + // NOTE (Techassi): This derefernce is super weird, but + // Duration::from_millis doesn't accept a u128, but returns a u128 + // when as_millis is called. + Ok(Self(std::time::Duration::from_millis( + values.iter().fold(0, |acc, v| acc + (*v as u64)), + ))) + } +} + +impl Display for Duration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.0.is_zero() { + return write!(f, "0{}", DurationUnit::Milliseconds); + } + + let mut millis = self.0.as_millis(); + let mut formatted = String::new(); + + for unit in DurationUnit::iter() { + let whole = millis / unit.millis(); + let rest = millis % unit.millis(); + + if whole > 0 { + write!(formatted, "{}{} ", whole, unit)?; + } + + millis = rest; + } + + write!(f, "{}", formatted.trim_end()) + } +} + +impl Deref for Duration { + type Target = std::time::Duration; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for Duration { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl From for Duration { + fn from(value: std::time::Duration) -> Self { + Self(value) + } +} + +impl Duration { + /// Creates a new [`Duration`] from the specified number of whole seconds. + pub const fn from_secs(secs: u64) -> Self { + Self(std::time::Duration::from_secs(secs)) + } +} + +#[derive(Debug, strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter)] +pub enum DurationUnit { + #[strum(serialize = "d")] + Days, + + #[strum(serialize = "h")] + Hours, + + #[strum(serialize = "m")] + Minutes, + + #[strum(serialize = "s")] + Seconds, + + #[strum(serialize = "ms")] + Milliseconds, +} + +impl DurationUnit { + /// Returns the number of whole milliseconds for each supported + /// [`DurationUnit`]. + pub fn millis(&self) -> u128 { + use DurationUnit::*; + + match self { + Days => 24 * Hours.millis(), + Hours => 60 * Minutes.millis(), + Minutes => 60 * Seconds.millis(), + Seconds => 1000, + Milliseconds => 1, + } + } +} + +#[derive(Debug, Error, PartialEq)] +pub enum DurationFragmentParseError { + #[error("invalid input, either empty or contains non-ascii characters")] + InvalidInput, + + #[error("expected number, the duration fragment must start with a numeric character")] + ExpectedNumber, + + #[error("expected character, the duration fragments must end with an alphabetic character")] + ExpectedCharacter, + + #[error("failed to parse fragment value as integer")] + ParseIntError(#[from] ParseIntError), + + #[error("failed to parse fragment unit")] + UnitParseError, +} + +#[derive(Debug)] +pub struct DurationFragment { + value: u128, + unit: DurationUnit, +} + +impl FromStr for DurationFragment { + type Err = DurationFragmentParseError; + + fn from_str(s: &str) -> Result { + let input = s.trim(); + + // An empty is invalid, non-ascii characters are already ruled out by + // the Duration impl + if input.is_empty() { + return Err(DurationFragmentParseError::InvalidInput); + } + + let mut chars = input.char_indices().peekable(); + let mut end_index = 0; + + // First loop through all numeric characters + while let Some((i, _)) = chars.next_if(|(_, c)| char::is_numeric(*c)) { + end_index = i + 1; + } + + // Parse the numeric characters as a u128 + let value = if end_index != 0 { + s[0..end_index].parse::()? + } else { + return Err(DurationFragmentParseError::ExpectedNumber); + }; + + // Loop through all alphabetic characters + let start_index = end_index; + while let Some((i, _)) = chars.next_if(|(_, c)| char::is_alphabetic(*c)) { + end_index = i + 1; + } + + // Parse the alphabetic characters as a supported duration unit + let unit = if end_index != 0 { + s[start_index..end_index] + .parse::() + .map_err(|_| DurationFragmentParseError::UnitParseError)? + } else { + return Err(DurationFragmentParseError::ExpectedCharacter); + }; + + // If there are characters left which are not alphabetic, we return an + // error + if chars.peek().is_some() { + return Err(DurationFragmentParseError::ExpectedCharacter); + } + + Ok(Self { value, unit }) + } +} + +impl Display for DurationFragment { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}{}", self.value, self.unit) + } +} + +impl DurationFragment { + pub fn millis(&self) -> u128 { + self.value * self.unit.millis() + } +} + +#[cfg(test)] +mod test { + use super::*; + use rstest::rstest; + use serde::{Deserialize, Serialize}; + + #[rstest] + #[case("15d 2m 2s", 1296122)] + #[case("1h", 3600)] + #[case("1m", 60)] + #[case("1s", 1)] + fn parse(#[case] input: &str, #[case] output: u64) { + let dur: Duration = input.parse().unwrap(); + assert_eq!(dur.as_secs(), output); + } + + #[rstest] + #[case( + "2d2", + DurationParseError::FragmentError(DurationFragmentParseError::ExpectedCharacter) + )] + #[case( + "-1y", + DurationParseError::FragmentError(DurationFragmentParseError::ExpectedNumber) + )] + #[case( + "1D", + DurationParseError::FragmentError(DurationFragmentParseError::UnitParseError) + )] + #[case("1ä", DurationParseError::InvalidInput)] + #[case(" ", DurationParseError::InvalidInput)] + fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { + let err = Duration::from_str(input).unwrap_err(); + assert_eq!(err, expected_err) + } + + #[rstest] + #[case("15d 2m 2s")] + #[case("1h 20m")] + #[case("1m")] + #[case("1s")] + fn to_string(#[case] duration: &str) { + let dur: Duration = duration.parse().unwrap(); + assert_eq!(dur.to_string(), duration); + } + + #[test] + fn deserialize() { + #[derive(Deserialize)] + struct S { + dur: Duration, + } + + let s: S = serde_yaml::from_str("dur: 15d 2m 2s").unwrap(); + assert_eq!(s.dur.as_secs(), 1296122); + } + + #[test] + fn serialize() { + #[derive(Serialize)] + struct S { + dur: Duration, + } + + let s = S { + dur: "15d 2m 2s".parse().unwrap(), + }; + assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15d 2m 2s\n"); + } + + // #[test] + // fn add_ops() { + // let mut dur1 = Duration::from_secs(20); + // let dur2 = Duration::from_secs(10); + + // let dur = dur1 + dur2; + // assert_eq!(dur.as_secs(), 30); + + // dur1 += dur2; + // assert_eq!(dur1.as_secs(), 30); + // } + + // #[test] + // fn sub_ops() { + // let mut dur1 = Duration::from_secs(20); + // let dur2 = Duration::from_secs(10); + + // let dur = dur1 - dur2; + // assert_eq!(dur.as_secs(), 10); + + // dur1 -= dur2; + // assert_eq!(dur1.as_secs(), 10); + // } +} diff --git a/src/duration/serde_impl.rs b/src/duration/serde_impl.rs new file mode 100644 index 000000000..04f1b43f5 --- /dev/null +++ b/src/duration/serde_impl.rs @@ -0,0 +1,39 @@ +use serde::{de::Visitor, Deserialize, Serialize}; + +use crate::duration::Duration; + +struct DurationVisitor; + +impl<'de> Visitor<'de> for DurationVisitor { + type Value = Duration; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a string in any of the supported formats") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + let dur = v.parse::().map_err(serde::de::Error::custom)?; + Ok(dur) + } +} + +impl<'de> Deserialize<'de> for Duration { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_str(DurationVisitor) + } +} + +impl Serialize for Duration { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} From cad5cfe42f4b01a0a676465ca0514ff34c8cf734 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 14 Sep 2023 14:27:45 +0200 Subject: [PATCH 12/35] Add documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Natalie Klestrup Röijezon --- src/duration/mod.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index d667781e7..afe280981 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -49,8 +49,11 @@ impl FromStr for Duration { return Err(DurationParseError::InvalidInput); } + // Let's split up individual parts separated by a space let parts: Vec<&str> = input.split(' ').collect(); + // Parse each part as a DurationFragment and extract the final duration + // of each fragment in milliseconds let values: Vec = parts .iter() .map(|p| p.parse::()) @@ -68,6 +71,8 @@ impl FromStr for Duration { impl Display for Duration { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // If the inner Duration is zero, print out '0ms' as milliseconds + // is the base unit for our Duration. if self.0.is_zero() { return write!(f, "0{}", DurationUnit::Milliseconds); } @@ -115,8 +120,19 @@ impl Duration { pub const fn from_secs(secs: u64) -> Self { Self(std::time::Duration::from_secs(secs)) } + + /// Creates a new [`Duration`] from the specified number of whole + /// milliseconds. + pub const fn from_millis(millis: u64) -> Self { + Self(std::time::Duration::from_millis(millis)) + } } +/// Defines supported [`DurationUnit`]s. Each [`DurationFragment`] consists of +/// a numeric value followed by a [`DurationUnit`]. The order of variants +/// **MATTERS**. It is the basis for the correct transformation of the +/// [`std::time::Duration`] back to a human-readable format, which is defined +/// in the [`Display`] implementation of [`Duration`]. #[derive(Debug, strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter)] pub enum DurationUnit { #[strum(serialize = "d")] @@ -136,7 +152,7 @@ pub enum DurationUnit { } impl DurationUnit { - /// Returns the number of whole milliseconds for each supported + /// Returns the number of whole milliseconds in each supported /// [`DurationUnit`]. pub fn millis(&self) -> u128 { use DurationUnit::*; @@ -169,6 +185,8 @@ pub enum DurationFragmentParseError { UnitParseError, } +/// Each [`DurationFragment`] consists of a numeric value followed by +/// a[`DurationUnit`]. #[derive(Debug)] pub struct DurationFragment { value: u128, @@ -234,6 +252,8 @@ impl Display for DurationFragment { } impl DurationFragment { + /// Returns the amount of whole milliseconds encoded by this + /// [`DurationFragment`]. pub fn millis(&self) -> u128 { self.value * self.unit.millis() } From f63ec1abeba86bd2382a7020546e67d5d2bac795 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 14 Sep 2023 14:33:02 +0200 Subject: [PATCH 13/35] Fix module documentation links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Natalie Klestrup Röijezon --- src/duration/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index afe280981..955983838 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -1,7 +1,8 @@ //! This module contains a common [`Duration`] struct which is able to parse //! human-readable duration formats, like `2y 2h 20m 42s` or`15d 2m 2s`. It //! additionally implements many required traits, like [`Derivative`], -//! [`JsonSchema`], [`Deserialize`], and [`Serialize`]. +//! [`JsonSchema`], [`Deserialize`][serde::Deserialize], and +//! [`Serialize`][serde::Serialize]. //! //! Furthermore, it implements [`Deref`], which enables us to use all associated //! functions of [`std::time::Duration`] without re-implementing the public From 237727652f421440a03dc561dd6008c68e1d4697 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 14 Sep 2023 14:40:56 +0200 Subject: [PATCH 14/35] Fix bug We need to check if the iterator progressed based on the start index of the unit / the end index of the numeric value. --- src/duration/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 955983838..f3a3df569 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -228,7 +228,7 @@ impl FromStr for DurationFragment { } // Parse the alphabetic characters as a supported duration unit - let unit = if end_index != 0 { + let unit = if end_index != start_index { s[start_index..end_index] .parse::() .map_err(|_| DurationFragmentParseError::UnitParseError)? From 9e9257d356a467f89bcdd1ea021d4dc07f2dbd0e Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 15 Sep 2023 09:46:57 +0200 Subject: [PATCH 15/35] Update parser, add context to errors, add more tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit now parses durations in the Go format. This format doesn't use whitespaces to separate each fragment. Instead, all fragments are concated without any space between. The parser needs to be a little cleverer when trying to keep it streamlined. Additionally, the error variants now contain some more context to make it easier for the user to spot the error in the provided value. Co-authored-by: Natalie Klestrup Röijezon --- src/duration/mod.rs | 301 ++++++++++++++++++-------------------------- 1 file changed, 124 insertions(+), 177 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index f3a3df569..dd85e9c09 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -13,9 +13,9 @@ //! timeouts or retries. use std::{ - fmt::{Display, Write}, + fmt::Display, num::ParseIntError, - ops::{Deref, DerefMut}, + ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, str::FromStr, }; @@ -32,8 +32,19 @@ pub enum DurationParseError { #[error("invalid input, either empty or contains non-ascii characters")] InvalidInput, - #[error("failed to parse duration fragment")] - FragmentError(#[from] DurationFragmentParseError), + #[error( + "expected character '{0}', the duration fragments must end with an alphabetic character" + )] + ExpectedCharacter(char), + + #[error("duration fragment with value '{0} has no unit")] + NoUnit(u64), + + #[error("failed to parse fragment unit '{0}'")] + ParseUnitError(String), + + #[error("failed to parse fragment value as integer: {0}")] + ParseIntError(#[from] ParseIntError), } #[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd, JsonSchema)] @@ -50,23 +61,39 @@ impl FromStr for Duration { return Err(DurationParseError::InvalidInput); } - // Let's split up individual parts separated by a space - let parts: Vec<&str> = input.split(' ').collect(); - - // Parse each part as a DurationFragment and extract the final duration - // of each fragment in milliseconds - let values: Vec = parts - .iter() - .map(|p| p.parse::()) - .map(|r| r.map(|f| f.millis())) - .collect::, DurationFragmentParseError>>()?; - - // NOTE (Techassi): This derefernce is super weird, but - // Duration::from_millis doesn't accept a u128, but returns a u128 - // when as_millis is called. - Ok(Self(std::time::Duration::from_millis( - values.iter().fold(0, |acc, v| acc + (*v as u64)), - ))) + let mut chars = input.char_indices().peekable(); + let mut duration = std::time::Duration::ZERO; + + let mut take_group = |f: fn(char) -> bool| { + let &(from, _) = chars.peek()?; + let mut to = from; + + while let Some((i, _)) = chars.next_if(|(_, c)| f(*c)) { + to = i; + } + + Some(&input[from..=to]) + }; + + while let Some(value) = take_group(char::is_numeric) { + let value = value.parse::()?; + + let Some(unit) = take_group(char::is_alphabetic) else { + if let Some(&(_, c)) = chars.peek() { + return Err(DurationParseError::ExpectedCharacter(c)); + } else { + return Err(DurationParseError::NoUnit(value)); + } + }; + + let unit = unit + .parse::() + .map_err(|_| DurationParseError::ParseUnitError(unit.to_string()))?; + + duration += std::time::Duration::from_secs(value * unit.secs()) + } + + Ok(Self(duration)) } } @@ -75,24 +102,23 @@ impl Display for Duration { // If the inner Duration is zero, print out '0ms' as milliseconds // is the base unit for our Duration. if self.0.is_zero() { - return write!(f, "0{}", DurationUnit::Milliseconds); + return write!(f, "0{}", DurationUnit::Seconds); } - let mut millis = self.0.as_millis(); - let mut formatted = String::new(); + let mut secs = self.0.as_secs(); for unit in DurationUnit::iter() { - let whole = millis / unit.millis(); - let rest = millis % unit.millis(); + let whole = secs / unit.secs(); + let rest = secs % unit.secs(); if whole > 0 { - write!(formatted, "{}{} ", whole, unit)?; + write!(f, "{}{}", whole, unit)?; } - millis = rest; + secs = rest; } - write!(f, "{}", formatted.trim_end()) + Ok(()) } } @@ -116,17 +142,39 @@ impl From for Duration { } } +impl Add for Duration { + type Output = Self; + + fn add(self, rhs: Self) -> Self::Output { + Self(self.0 + rhs.0) + } +} + +impl AddAssign for Duration { + fn add_assign(&mut self, rhs: Self) { + self.0.add_assign(rhs.0) + } +} + +impl Sub for Duration { + type Output = Self; + + fn sub(self, rhs: Self) -> Self::Output { + Self(self.0 - rhs.0) + } +} + +impl SubAssign for Duration { + fn sub_assign(&mut self, rhs: Self) { + self.0.sub_assign(rhs.0) + } +} + impl Duration { /// Creates a new [`Duration`] from the specified number of whole seconds. pub const fn from_secs(secs: u64) -> Self { Self(std::time::Duration::from_secs(secs)) } - - /// Creates a new [`Duration`] from the specified number of whole - /// milliseconds. - pub const fn from_millis(millis: u64) -> Self { - Self(std::time::Duration::from_millis(millis)) - } } /// Defines supported [`DurationUnit`]s. Each [`DurationFragment`] consists of @@ -147,119 +195,23 @@ pub enum DurationUnit { #[strum(serialize = "s")] Seconds, - - #[strum(serialize = "ms")] - Milliseconds, } impl DurationUnit { /// Returns the number of whole milliseconds in each supported /// [`DurationUnit`]. - pub fn millis(&self) -> u128 { + pub fn secs(&self) -> u64 { use DurationUnit::*; match self { - Days => 24 * Hours.millis(), - Hours => 60 * Minutes.millis(), - Minutes => 60 * Seconds.millis(), - Seconds => 1000, - Milliseconds => 1, + Days => 24 * Hours.secs(), + Hours => 60 * Minutes.secs(), + Minutes => 60 * Seconds.secs(), + Seconds => 1, } } } -#[derive(Debug, Error, PartialEq)] -pub enum DurationFragmentParseError { - #[error("invalid input, either empty or contains non-ascii characters")] - InvalidInput, - - #[error("expected number, the duration fragment must start with a numeric character")] - ExpectedNumber, - - #[error("expected character, the duration fragments must end with an alphabetic character")] - ExpectedCharacter, - - #[error("failed to parse fragment value as integer")] - ParseIntError(#[from] ParseIntError), - - #[error("failed to parse fragment unit")] - UnitParseError, -} - -/// Each [`DurationFragment`] consists of a numeric value followed by -/// a[`DurationUnit`]. -#[derive(Debug)] -pub struct DurationFragment { - value: u128, - unit: DurationUnit, -} - -impl FromStr for DurationFragment { - type Err = DurationFragmentParseError; - - fn from_str(s: &str) -> Result { - let input = s.trim(); - - // An empty is invalid, non-ascii characters are already ruled out by - // the Duration impl - if input.is_empty() { - return Err(DurationFragmentParseError::InvalidInput); - } - - let mut chars = input.char_indices().peekable(); - let mut end_index = 0; - - // First loop through all numeric characters - while let Some((i, _)) = chars.next_if(|(_, c)| char::is_numeric(*c)) { - end_index = i + 1; - } - - // Parse the numeric characters as a u128 - let value = if end_index != 0 { - s[0..end_index].parse::()? - } else { - return Err(DurationFragmentParseError::ExpectedNumber); - }; - - // Loop through all alphabetic characters - let start_index = end_index; - while let Some((i, _)) = chars.next_if(|(_, c)| char::is_alphabetic(*c)) { - end_index = i + 1; - } - - // Parse the alphabetic characters as a supported duration unit - let unit = if end_index != start_index { - s[start_index..end_index] - .parse::() - .map_err(|_| DurationFragmentParseError::UnitParseError)? - } else { - return Err(DurationFragmentParseError::ExpectedCharacter); - }; - - // If there are characters left which are not alphabetic, we return an - // error - if chars.peek().is_some() { - return Err(DurationFragmentParseError::ExpectedCharacter); - } - - Ok(Self { value, unit }) - } -} - -impl Display for DurationFragment { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}{}", self.value, self.unit) - } -} - -impl DurationFragment { - /// Returns the amount of whole milliseconds encoded by this - /// [`DurationFragment`]. - pub fn millis(&self) -> u128 { - self.value * self.unit.millis() - } -} - #[cfg(test)] mod test { use super::*; @@ -267,7 +219,8 @@ mod test { use serde::{Deserialize, Serialize}; #[rstest] - #[case("15d 2m 2s", 1296122)] + #[case("15d2m2s", 1296122)] + #[case("70m", 4200)] #[case("1h", 3600)] #[case("1m", 60)] #[case("1s", 1)] @@ -277,33 +230,27 @@ mod test { } #[rstest] - #[case( - "2d2", - DurationParseError::FragmentError(DurationFragmentParseError::ExpectedCharacter) - )] - #[case( - "-1y", - DurationParseError::FragmentError(DurationFragmentParseError::ExpectedNumber) - )] - #[case( - "1D", - DurationParseError::FragmentError(DurationFragmentParseError::UnitParseError) - )] + #[case("1D", DurationParseError::ParseUnitError("D".into()))] #[case("1ä", DurationParseError::InvalidInput)] #[case(" ", DurationParseError::InvalidInput)] + #[case("2d2", DurationParseError::NoUnit(2))] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err) } #[rstest] - #[case("15d 2m 2s")] - #[case("1h 20m")] - #[case("1m")] - #[case("1s")] - fn to_string(#[case] duration: &str) { - let dur: Duration = duration.parse().unwrap(); - assert_eq!(dur.to_string(), duration); + #[case("70m", Some("1h10m"))] + #[case("15d2m2s", None)] + #[case("1h20m", None)] + #[case("1m", None)] + #[case("1s", None)] + fn to_string(#[case] input: &str, #[case] expected: Option<&str>) { + let dur: Duration = input.parse().unwrap(); + match expected { + Some(e) => assert_eq!(dur.to_string(), e), + None => assert_eq!(dur.to_string(), input), + } } #[test] @@ -313,7 +260,7 @@ mod test { dur: Duration, } - let s: S = serde_yaml::from_str("dur: 15d 2m 2s").unwrap(); + let s: S = serde_yaml::from_str("dur: 15d2m2s").unwrap(); assert_eq!(s.dur.as_secs(), 1296122); } @@ -325,32 +272,32 @@ mod test { } let s = S { - dur: "15d 2m 2s".parse().unwrap(), + dur: "15d2m2s".parse().unwrap(), }; - assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15d 2m 2s\n"); + assert_eq!(serde_yaml::to_string(&s).unwrap(), "dur: 15d2m2s\n"); } - // #[test] - // fn add_ops() { - // let mut dur1 = Duration::from_secs(20); - // let dur2 = Duration::from_secs(10); + #[test] + fn add_ops() { + let mut dur1 = Duration::from_str("20s").unwrap(); + let dur2 = Duration::from_secs(10); - // let dur = dur1 + dur2; - // assert_eq!(dur.as_secs(), 30); + let dur = dur1 + dur2; + assert_eq!(dur.as_secs(), 30); - // dur1 += dur2; - // assert_eq!(dur1.as_secs(), 30); - // } + dur1 += dur2; + assert_eq!(dur1.as_secs(), 30); + } - // #[test] - // fn sub_ops() { - // let mut dur1 = Duration::from_secs(20); - // let dur2 = Duration::from_secs(10); + #[test] + fn sub_ops() { + let mut dur1 = Duration::from_str("20s").unwrap(); + let dur2 = Duration::from_secs(10); - // let dur = dur1 - dur2; - // assert_eq!(dur.as_secs(), 10); + let dur = dur1 - dur2; + assert_eq!(dur.as_secs(), 10); - // dur1 -= dur2; - // assert_eq!(dur1.as_secs(), 10); - // } + dur1 -= dur2; + assert_eq!(dur1.as_secs(), 10); + } } From 96f1600e1052f825ef912589010975f8a82e66bf Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 15 Sep 2023 11:49:19 +0200 Subject: [PATCH 16/35] Update src/duration/mod.rs Co-authored-by: Sebastian Bernauer --- src/duration/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index dd85e9c09..d3f400976 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -1,5 +1,5 @@ //! This module contains a common [`Duration`] struct which is able to parse -//! human-readable duration formats, like `2y 2h 20m 42s` or`15d 2m 2s`. It +//! human-readable duration formats, like `5s`, `24h`, `2y2h20m42s` or`15d2m2s`. It //! additionally implements many required traits, like [`Derivative`], //! [`JsonSchema`], [`Deserialize`][serde::Deserialize], and //! [`Serialize`][serde::Serialize]. From 58b04c3500ea19635da664d6a0e02d168032040e Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 15 Sep 2023 15:27:00 +0200 Subject: [PATCH 17/35] Re-add millisecond support, switch to Snafu --- src/duration/mod.rs | 76 ++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index d3f400976..300f8f9bb 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -21,30 +21,31 @@ use std::{ use derivative::Derivative; use schemars::JsonSchema; +use snafu::{OptionExt, ResultExt, Snafu}; use strum::IntoEnumIterator; -use thiserror::Error; mod serde_impl; pub use serde_impl::*; -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Snafu, PartialEq)] +#[snafu(context(suffix(false)))] pub enum DurationParseError { - #[error("invalid input, either empty or contains non-ascii characters")] + #[snafu(display("invalid input, either empty or contains non-ascii characters"))] InvalidInput, - #[error( - "expected character '{0}', the duration fragments must end with an alphabetic character" - )] - ExpectedCharacter(char), + #[snafu(display( + "expected character '{expected}', the duration fragments must end with an alphabetic character" + ))] + ExpectedCharacter { expected: char }, - #[error("duration fragment with value '{0} has no unit")] - NoUnit(u64), + #[snafu(display("duration fragment with value '{value}' has no unit"))] + NoUnit { value: u128 }, - #[error("failed to parse fragment unit '{0}'")] - ParseUnitError(String), + #[snafu(display("failed to parse fragment unit '{unit}'"))] + ParseUnitError { unit: String }, - #[error("failed to parse fragment value as integer: {0}")] - ParseIntError(#[from] ParseIntError), + #[snafu(display("failed to parse fragment value as integer"))] + ParseIntError { source: ParseIntError }, } #[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd, JsonSchema)] @@ -76,21 +77,26 @@ impl FromStr for Duration { }; while let Some(value) = take_group(char::is_numeric) { - let value = value.parse::()?; + let value = value.parse::().context(ParseInt)?; let Some(unit) = take_group(char::is_alphabetic) else { if let Some(&(_, c)) = chars.peek() { - return Err(DurationParseError::ExpectedCharacter(c)); + return ExpectedCharacter {expected: c}.fail(); } else { - return Err(DurationParseError::NoUnit(value)); + return NoUnit { value }.fail(); } }; - let unit = unit - .parse::() - .map_err(|_| DurationParseError::ParseUnitError(unit.to_string()))?; + let unit = unit.parse::().ok().context(ParseUnit { + unit: unit.to_string(), + })?; - duration += std::time::Duration::from_secs(value * unit.secs()) + // This try_into is needed, as Duration::from_millis was stabilized + // in 1.3.0 but u128 was only added in 1.26.0. See + // - https://users.rust-lang.org/t/why-duration-as-from-millis-uses-different-primitives/89302 + // - https://github.com/rust-lang/rust/issues/58580 + duration += + std::time::Duration::from_millis((value * unit.millis()).try_into().unwrap()) } Ok(Self(duration)) @@ -105,17 +111,17 @@ impl Display for Duration { return write!(f, "0{}", DurationUnit::Seconds); } - let mut secs = self.0.as_secs(); + let mut millis = self.0.as_millis(); for unit in DurationUnit::iter() { - let whole = secs / unit.secs(); - let rest = secs % unit.secs(); + let whole = millis / unit.millis(); + let rest = millis % unit.millis(); if whole > 0 { write!(f, "{}{}", whole, unit)?; } - secs = rest; + millis = rest; } Ok(()) @@ -195,19 +201,23 @@ pub enum DurationUnit { #[strum(serialize = "s")] Seconds, + + #[strum(serialize = "ms")] + Milliseconds, } impl DurationUnit { /// Returns the number of whole milliseconds in each supported /// [`DurationUnit`]. - pub fn secs(&self) -> u64 { + pub fn millis(&self) -> u128 { use DurationUnit::*; match self { - Days => 24 * Hours.secs(), - Hours => 60 * Minutes.secs(), - Minutes => 60 * Seconds.secs(), - Seconds => 1, + Days => 24 * Hours.millis(), + Hours => 60 * Minutes.millis(), + Minutes => 60 * Seconds.millis(), + Seconds => 1000, + Milliseconds => 1, } } } @@ -219,21 +229,23 @@ mod test { use serde::{Deserialize, Serialize}; #[rstest] + #[case("15d2m2s1000ms", 1296123)] + #[case("15d2m2s600ms", 1296122)] #[case("15d2m2s", 1296122)] #[case("70m", 4200)] #[case("1h", 3600)] #[case("1m", 60)] #[case("1s", 1)] - fn parse(#[case] input: &str, #[case] output: u64) { + fn parse_as_secs(#[case] input: &str, #[case] output: u64) { let dur: Duration = input.parse().unwrap(); assert_eq!(dur.as_secs(), output); } #[rstest] - #[case("1D", DurationParseError::ParseUnitError("D".into()))] + #[case("1D", DurationParseError::ParseUnitError{unit: "D".into()})] + #[case("2d2", DurationParseError::NoUnit{value: 2})] #[case("1ä", DurationParseError::InvalidInput)] #[case(" ", DurationParseError::InvalidInput)] - #[case("2d2", DurationParseError::NoUnit(2))] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err) From 3282f9ecc70c76cddc91a807814af1fbae5b2e33 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 15 Sep 2023 15:32:07 +0200 Subject: [PATCH 18/35] Fix markdown link references --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25df93e29..070eb0291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,8 @@ All notable changes to this project will be documented in this file. - Add support for tls pkcs12 password to secret operator volume builder ([#645]). - Add `Duration` capable of parsing human-readable duration formats ([#647]) -[#647]: https://github.com/stackabletech/operator-rs/pull/644 -[#647]: https://github.com/stackabletech/operator-rs/pull/645 +[#644]: https://github.com/stackabletech/operator-rs/pull/644 +[#645]: https://github.com/stackabletech/operator-rs/pull/645 [#647]: https://github.com/stackabletech/operator-rs/pull/647 ### Changed From a39af93c2f03c85c31f5a2d12c39d97b1698d167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:23:46 +0200 Subject: [PATCH 19/35] duration::serde_impl exports nothing --- src/duration/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 300f8f9bb..9df2687e6 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -25,7 +25,6 @@ use snafu::{OptionExt, ResultExt, Snafu}; use strum::IntoEnumIterator; mod serde_impl; -pub use serde_impl::*; #[derive(Debug, Snafu, PartialEq)] #[snafu(context(suffix(false)))] @@ -81,7 +80,7 @@ impl FromStr for Duration { let Some(unit) = take_group(char::is_alphabetic) else { if let Some(&(_, c)) = chars.peek() { - return ExpectedCharacter {expected: c}.fail(); + return ExpectedCharacter { expected: c }.fail(); } else { return NoUnit { value }.fail(); } From 9fdb1b35860783fbcde0049661580cb75d7bfc8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:25:12 +0200 Subject: [PATCH 20/35] Split duration parsing context selectors into a submodule --- src/duration/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 9df2687e6..688f0603b 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -27,7 +27,7 @@ use strum::IntoEnumIterator; mod serde_impl; #[derive(Debug, Snafu, PartialEq)] -#[snafu(context(suffix(false)))] +#[snafu(module)] pub enum DurationParseError { #[snafu(display("invalid input, either empty or contains non-ascii characters"))] InvalidInput, @@ -54,6 +54,7 @@ impl FromStr for Duration { type Err = DurationParseError; fn from_str(s: &str) -> Result { + use duration_parse_error::*; let input = s.trim(); // An empty or non-ascii input is invalid @@ -76,17 +77,17 @@ impl FromStr for Duration { }; while let Some(value) = take_group(char::is_numeric) { - let value = value.parse::().context(ParseInt)?; + let value = value.parse::().context(ParseIntSnafu)?; let Some(unit) = take_group(char::is_alphabetic) else { if let Some(&(_, c)) = chars.peek() { - return ExpectedCharacter { expected: c }.fail(); + return ExpectedCharacterSnafu { expected: c }.fail(); } else { - return NoUnit { value }.fail(); + return NoUnitSnafu { value }.fail(); } }; - let unit = unit.parse::().ok().context(ParseUnit { + let unit = unit.parse::().ok().context(ParseUnitSnafu { unit: unit.to_string(), })?; From 191151ae33bac401066840cfc74b9ab64f7f4906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:27:37 +0200 Subject: [PATCH 21/35] typo: "expected character" -> "unexpected character" --- src/duration/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 688f0603b..068b47aa2 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -32,10 +32,8 @@ pub enum DurationParseError { #[snafu(display("invalid input, either empty or contains non-ascii characters"))] InvalidInput, - #[snafu(display( - "expected character '{expected}', the duration fragments must end with an alphabetic character" - ))] - ExpectedCharacter { expected: char }, + #[snafu(display("unexpected character '{chr}'"))] + UnexpectedCharacter { chr: char }, #[snafu(display("duration fragment with value '{value}' has no unit"))] NoUnit { value: u128 }, @@ -80,8 +78,8 @@ impl FromStr for Duration { let value = value.parse::().context(ParseIntSnafu)?; let Some(unit) = take_group(char::is_alphabetic) else { - if let Some(&(_, c)) = chars.peek() { - return ExpectedCharacterSnafu { expected: c }.fail(); + if let Some(&(_, chr)) = chars.peek() { + return UnexpectedCharacterSnafu { chr }.fail(); } else { return NoUnitSnafu { value }.fail(); } From 9dbcefe1002893d1f0461d8b0c9a44321cc36da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:29:14 +0200 Subject: [PATCH 22/35] Fail if duration buffer is non-empty at the end of parsing --- src/duration/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 068b47aa2..4f798ec09 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -97,6 +97,11 @@ impl FromStr for Duration { std::time::Duration::from_millis((value * unit.millis()).try_into().unwrap()) } + // Buffer must not contain any remaining data + if let Some(&(_, chr)) = chars.peek() { + return UnexpectedCharacterSnafu { chr }.fail(); + } + Ok(Self(duration)) } } From 705ed1f44cd76cad4e1df41d4845ddd3b109e7fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:40:57 +0200 Subject: [PATCH 23/35] Use Debug impls to quote strings in errors --- src/duration/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 4f798ec09..8bbc16e93 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -32,13 +32,13 @@ pub enum DurationParseError { #[snafu(display("invalid input, either empty or contains non-ascii characters"))] InvalidInput, - #[snafu(display("unexpected character '{chr}'"))] + #[snafu(display("unexpected character {chr:?}"))] UnexpectedCharacter { chr: char }, - #[snafu(display("duration fragment with value '{value}' has no unit"))] + #[snafu(display("duration fragment with value {value:?} has no unit"))] NoUnit { value: u128 }, - #[snafu(display("failed to parse fragment unit '{unit}'"))] + #[snafu(display("failed to parse fragment unit {unit:?}"))] ParseUnitError { unit: String }, #[snafu(display("failed to parse fragment value as integer"))] From fe1911ec60b9b3c56ae4fe4df4366a90cf8d32ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:42:18 +0200 Subject: [PATCH 24/35] Non-ASCII input will be caught anyway, because it's never a valid character --- src/duration/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 8bbc16e93..854ca5b64 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -29,8 +29,8 @@ mod serde_impl; #[derive(Debug, Snafu, PartialEq)] #[snafu(module)] pub enum DurationParseError { - #[snafu(display("invalid input, either empty or contains non-ascii characters"))] - InvalidInput, + #[snafu(display("empty input"))] + EmptyInput, #[snafu(display("unexpected character {chr:?}"))] UnexpectedCharacter { chr: char }, @@ -55,9 +55,8 @@ impl FromStr for Duration { use duration_parse_error::*; let input = s.trim(); - // An empty or non-ascii input is invalid - if input.is_empty() || !input.is_ascii() { - return Err(DurationParseError::InvalidInput); + if input.is_empty() { + return EmptyInputSnafu.fail(); } let mut chars = input.char_indices().peekable(); @@ -247,8 +246,8 @@ mod test { #[rstest] #[case("1D", DurationParseError::ParseUnitError{unit: "D".into()})] #[case("2d2", DurationParseError::NoUnit{value: 2})] - #[case("1ä", DurationParseError::InvalidInput)] - #[case(" ", DurationParseError::InvalidInput)] + #[case("1ä", DurationParseError::EmptyInput)] + #[case(" ", DurationParseError::EmptyInput)] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err) From 8ef94a7ce124c86323e0f6b42ac9c26d037d787f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 15 Sep 2023 17:43:54 +0200 Subject: [PATCH 25/35] DurationUnit doesn't need to be public --- src/duration/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 854ca5b64..b0df8d0a0 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -191,7 +191,7 @@ impl Duration { /// [`std::time::Duration`] back to a human-readable format, which is defined /// in the [`Display`] implementation of [`Duration`]. #[derive(Debug, strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter)] -pub enum DurationUnit { +enum DurationUnit { #[strum(serialize = "d")] Days, @@ -211,7 +211,7 @@ pub enum DurationUnit { impl DurationUnit { /// Returns the number of whole milliseconds in each supported /// [`DurationUnit`]. - pub fn millis(&self) -> u128 { + fn millis(&self) -> u128 { use DurationUnit::*; match self { From e7e47184697fcfa821dd5e11516d8a8edfa07746 Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 18 Sep 2023 10:38:40 +0200 Subject: [PATCH 26/35] Add check for invalid unit order and duplicate units --- src/duration/mod.rs | 59 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index b0df8d0a0..aa6729fc2 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -13,6 +13,7 @@ //! timeouts or retries. use std::{ + cmp::Ordering, fmt::Display, num::ParseIntError, ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, @@ -35,9 +36,18 @@ pub enum DurationParseError { #[snafu(display("unexpected character {chr:?}"))] UnexpectedCharacter { chr: char }, - #[snafu(display("duration fragment with value {value:?} has no unit"))] + #[snafu(display("fragment with value {value:?} has no unit"))] NoUnit { value: u128 }, + #[snafu(display("invalid fragment order, {current} must be before {previous}"))] + InvalidUnitOrdering { + previous: DurationUnit, + current: DurationUnit, + }, + + #[snafu(display("fragment unit {unit} was specified multiple times"))] + DuplicateUnit { unit: DurationUnit }, + #[snafu(display("failed to parse fragment unit {unit:?}"))] ParseUnitError { unit: String }, @@ -45,7 +55,7 @@ pub enum DurationParseError { ParseIntError { source: ParseIntError }, } -#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd, JsonSchema)] +#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, Eq, PartialOrd, Ord, JsonSchema)] pub struct Duration(std::time::Duration); impl FromStr for Duration { @@ -61,6 +71,7 @@ impl FromStr for Duration { let mut chars = input.char_indices().peekable(); let mut duration = std::time::Duration::ZERO; + let mut last_unit = None; let mut take_group = |f: fn(char) -> bool| { let &(from, _) = chars.peek()?; @@ -88,12 +99,29 @@ impl FromStr for Duration { unit: unit.to_string(), })?; + // Check that the unit is smaller than the previous one, and that + // it wasn't specified multiple times + if let Some(last_unit) = last_unit { + match unit.cmp(&last_unit) { + Ordering::Less => { + return InvalidUnitOrderingSnafu { + previous: last_unit, + current: unit, + } + .fail() + } + Ordering::Equal => return DuplicateUnitSnafu { unit }.fail(), + _ => (), + } + } + // This try_into is needed, as Duration::from_millis was stabilized // in 1.3.0 but u128 was only added in 1.26.0. See // - https://users.rust-lang.org/t/why-duration-as-from-millis-uses-different-primitives/89302 // - https://github.com/rust-lang/rust/issues/58580 duration += - std::time::Duration::from_millis((value * unit.millis()).try_into().unwrap()) + std::time::Duration::from_millis((value * unit.millis()).try_into().unwrap()); + last_unit = Some(unit); } // Buffer must not contain any remaining data @@ -190,8 +218,18 @@ impl Duration { /// **MATTERS**. It is the basis for the correct transformation of the /// [`std::time::Duration`] back to a human-readable format, which is defined /// in the [`Display`] implementation of [`Duration`]. -#[derive(Debug, strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter)] -enum DurationUnit { +#[derive( + Debug, + PartialEq, + Eq, + PartialOrd, + Ord, + strum::EnumString, + strum::Display, + strum::AsRefStr, + strum::EnumIter, +)] +pub enum DurationUnit { #[strum(serialize = "d")] Days, @@ -253,6 +291,17 @@ mod test { assert_eq!(err, expected_err) } + #[rstest] + #[case("15d2h1d", DurationParseError::InvalidUnitOrdering { previous: DurationUnit::Hours, current: DurationUnit::Days })] + #[case("15d2d", DurationParseError::DuplicateUnit { unit: DurationUnit::Days })] + fn invalid_order_or_duplicate_unit( + #[case] input: &str, + #[case] expected_err: DurationParseError, + ) { + let err = Duration::from_str(input).unwrap_err(); + assert_eq!(err, expected_err) + } + #[rstest] #[case("70m", Some("1h10m"))] #[case("15d2m2s", None)] From 2d3d32c58614aeafaa94473b702e921faa5a926d Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 18 Sep 2023 10:41:07 +0200 Subject: [PATCH 27/35] Revert "Non-ASCII input will be caught anyway, because it's never a valid character" This reverts commit fe1911ec60b9b3c56ae4fe4df4366a90cf8d32ce. --- src/duration/mod.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index aa6729fc2..d281a11ae 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -30,8 +30,8 @@ mod serde_impl; #[derive(Debug, Snafu, PartialEq)] #[snafu(module)] pub enum DurationParseError { - #[snafu(display("empty input"))] - EmptyInput, + #[snafu(display("invalid input, either empty or contains non-ascii characters"))] + InvalidInput, #[snafu(display("unexpected character {chr:?}"))] UnexpectedCharacter { chr: char }, @@ -65,8 +65,9 @@ impl FromStr for Duration { use duration_parse_error::*; let input = s.trim(); - if input.is_empty() { - return EmptyInputSnafu.fail(); + // An empty or non-ascii input is invalid + if input.is_empty() || !input.is_ascii() { + return Err(DurationParseError::InvalidInput); } let mut chars = input.char_indices().peekable(); @@ -284,8 +285,8 @@ mod test { #[rstest] #[case("1D", DurationParseError::ParseUnitError{unit: "D".into()})] #[case("2d2", DurationParseError::NoUnit{value: 2})] - #[case("1ä", DurationParseError::EmptyInput)] - #[case(" ", DurationParseError::EmptyInput)] + #[case("1ä", DurationParseError::InvalidInput)] + #[case(" ", DurationParseError::InvalidInput)] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err) From 6b882e662e3ff05fb6913ab5b242c55259039b4d Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 18 Sep 2023 10:45:00 +0200 Subject: [PATCH 28/35] Fix doc comment --- src/duration/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index d281a11ae..72fbcdc19 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -214,9 +214,9 @@ impl Duration { } } -/// Defines supported [`DurationUnit`]s. Each [`DurationFragment`] consists of -/// a numeric value followed by a [`DurationUnit`]. The order of variants -/// **MATTERS**. It is the basis for the correct transformation of the +/// Defines supported [`DurationUnit`]s. Each fragment consists of a numeric +/// value followed by a [`DurationUnit`]. The order of variants **MATTERS**. +/// It is the basis for the correct transformation of the /// [`std::time::Duration`] back to a human-readable format, which is defined /// in the [`Display`] implementation of [`Duration`]. #[derive( From 36e8035bb4d2a364c83891c62fe45c86dc3fdbac Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 18 Sep 2023 11:27:41 +0200 Subject: [PATCH 29/35] Impl Mul and Div --- src/duration/mod.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 72fbcdc19..b71d3d755 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -16,7 +16,7 @@ use std::{ cmp::Ordering, fmt::Display, num::ParseIntError, - ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, + ops::{Add, AddAssign, Deref, DerefMut, Div, Mul, Sub, SubAssign}, str::FromStr, }; @@ -201,6 +201,22 @@ impl Sub for Duration { } } +impl Mul for Duration { + type Output = Self; + + fn mul(self, rhs: u32) -> Duration { + Self(self.0 * rhs) + } +} + +impl Div for Duration { + type Output = Self; + + fn div(self, rhs: u32) -> Duration { + Self(self.0 / rhs) + } +} + impl SubAssign for Duration { fn sub_assign(&mut self, rhs: Self) { self.0.sub_assign(rhs.0) From dcb4dd01ca8fe2f3a3a203f8da2e231ba2844715 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 18 Sep 2023 11:29:50 +0200 Subject: [PATCH 30/35] Impl Mul and Div 2 --- src/duration/mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index b71d3d755..f2db623d2 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -209,6 +209,14 @@ impl Mul for Duration { } } +impl Mul for u32 { + type Output = Duration; + + fn mul(self, rhs: Duration) -> Duration { + rhs * self + } +} + impl Div for Duration { type Output = Self; @@ -217,6 +225,14 @@ impl Div for Duration { } } +impl Div for u32 { + type Output = Duration; + + fn div(self, rhs: Duration) -> Duration { + rhs / self + } +} + impl SubAssign for Duration { fn sub_assign(&mut self, rhs: Self) { self.0.sub_assign(rhs.0) From 198ae50b79064cff2a9b356b394c69c977ad94bc Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 18 Sep 2023 11:35:11 +0200 Subject: [PATCH 31/35] impl JsonSchema for Duration --- src/duration/mod.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index f2db623d2..9ae2e29ef 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -55,7 +55,7 @@ pub enum DurationParseError { ParseIntError { source: ParseIntError }, } -#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, Eq, PartialOrd, Ord, JsonSchema)] +#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct Duration(std::time::Duration); impl FromStr for Duration { @@ -134,6 +134,16 @@ impl FromStr for Duration { } } +impl JsonSchema for Duration { + fn schema_name() -> String { + "Duration".to_string() + } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + String::json_schema(gen) + } +} + impl Display for Duration { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // If the inner Duration is zero, print out '0ms' as milliseconds From 6737e5163114ac31b5d3367bb6332f5a8fa3fe0e Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 18 Sep 2023 11:45:07 +0200 Subject: [PATCH 32/35] Re-order `SubAssign` impl --- src/duration/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 9ae2e29ef..9833b6a3c 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -211,6 +211,12 @@ impl Sub for Duration { } } +impl SubAssign for Duration { + fn sub_assign(&mut self, rhs: Self) { + self.0.sub_assign(rhs.0) + } +} + impl Mul for Duration { type Output = Self; @@ -243,12 +249,6 @@ impl Div for u32 { } } -impl SubAssign for Duration { - fn sub_assign(&mut self, rhs: Self) { - self.0.sub_assign(rhs.0) - } -} - impl Duration { /// Creates a new [`Duration`] from the specified number of whole seconds. pub const fn from_secs(secs: u64) -> Self { From 29fca80f4c4fb3eb4c0069b3c7d8ef9cd9bc0505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Mon, 18 Sep 2023 13:16:15 +0200 Subject: [PATCH 33/35] Revert "Revert "Non-ASCII input will be caught anyway, because it's never a valid character"" This reverts commit 2d3d32c58614aeafaa94473b702e921faa5a926d. --- src/duration/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 9833b6a3c..72ebfacad 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -30,8 +30,8 @@ mod serde_impl; #[derive(Debug, Snafu, PartialEq)] #[snafu(module)] pub enum DurationParseError { - #[snafu(display("invalid input, either empty or contains non-ascii characters"))] - InvalidInput, + #[snafu(display("empty input"))] + EmptyInput, #[snafu(display("unexpected character {chr:?}"))] UnexpectedCharacter { chr: char }, @@ -65,9 +65,8 @@ impl FromStr for Duration { use duration_parse_error::*; let input = s.trim(); - // An empty or non-ascii input is invalid - if input.is_empty() || !input.is_ascii() { - return Err(DurationParseError::InvalidInput); + if input.is_empty() { + return EmptyInputSnafu.fail(); } let mut chars = input.char_indices().peekable(); @@ -327,8 +326,8 @@ mod test { #[rstest] #[case("1D", DurationParseError::ParseUnitError{unit: "D".into()})] #[case("2d2", DurationParseError::NoUnit{value: 2})] - #[case("1ä", DurationParseError::InvalidInput)] - #[case(" ", DurationParseError::InvalidInput)] + #[case("1ä", DurationParseError::EmptyInput)] + #[case(" ", DurationParseError::EmptyInput)] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err) From 9a5c6daf2cffad28ce0cec42ea77850a3927db9a Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 18 Sep 2023 13:29:43 +0200 Subject: [PATCH 34/35] Remove `impl Div for u32` --- src/duration/mod.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 9833b6a3c..d9ce939d5 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -241,14 +241,6 @@ impl Div for Duration { } } -impl Div for u32 { - type Output = Duration; - - fn div(self, rhs: Duration) -> Duration { - rhs / self - } -} - impl Duration { /// Creates a new [`Duration`] from the specified number of whole seconds. pub const fn from_secs(secs: u64) -> Self { From 98149d7c57a381668b6c05e07bf4f27b198992b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Mon, 18 Sep 2023 13:30:50 +0200 Subject: [PATCH 35/35] Fix scanning of multibyte chars --- src/duration/mod.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 72ebfacad..4b4575870 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -61,10 +61,8 @@ pub struct Duration(std::time::Duration); impl FromStr for Duration { type Err = DurationParseError; - fn from_str(s: &str) -> Result { + fn from_str(input: &str) -> Result { use duration_parse_error::*; - let input = s.trim(); - if input.is_empty() { return EmptyInputSnafu.fail(); } @@ -76,12 +74,15 @@ impl FromStr for Duration { let mut take_group = |f: fn(char) -> bool| { let &(from, _) = chars.peek()?; let mut to = from; + let mut last_char = None; - while let Some((i, _)) = chars.next_if(|(_, c)| f(*c)) { + while let Some((i, c)) = chars.next_if(|(_, c)| f(*c)) { to = i; + last_char = Some(c); } - Some(&input[from..=to]) + // if last_char == None then we read 0 characters => fail + Some(&input[from..(to + last_char?.len_utf8())]) }; while let Some(value) = take_group(char::is_numeric) { @@ -326,8 +327,9 @@ mod test { #[rstest] #[case("1D", DurationParseError::ParseUnitError{unit: "D".into()})] #[case("2d2", DurationParseError::NoUnit{value: 2})] - #[case("1ä", DurationParseError::EmptyInput)] - #[case(" ", DurationParseError::EmptyInput)] + #[case("1ä", DurationParseError::ParseUnitError { unit: "ä".into() })] + #[case(" ", DurationParseError::UnexpectedCharacter { chr: ' ' })] + #[case("", DurationParseError::EmptyInput)] fn parse_invalid(#[case] input: &str, #[case] expected_err: DurationParseError) { let err = Duration::from_str(input).unwrap_err(); assert_eq!(err, expected_err)