Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for human-readable duration formats #647

Merged
merged 38 commits into from
Sep 18, 2023
Merged
Changes from 8 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
f438507
Update author constant
Techassi Sep 7, 2023
1bc051a
Add humantime Duration
Techassi Sep 7, 2023
a3f3a5e
Update changelog
Techassi Sep 7, 2023
ae18cc5
Add `from_secs` and `from_millis` helper functions
Techassi Sep 8, 2023
a524ef4
Trim trailing whitespace using pre-commit hook
Techassi Sep 8, 2023
0e5658f
Fix markdownlint errors using pre-commit hook
Techassi Sep 8, 2023
794dca8
Implement first iteration of custom parsing
Techassi Sep 8, 2023
8658713
Remove humantime dependency
Techassi Sep 8, 2023
88d4677
Add `Display` impl, add various `ops` impls
Techassi Sep 13, 2023
128eff7
Add negative tests
Techassi Sep 13, 2023
7dd912f
Merge branch 'main' into feat/humantime
Techassi Sep 13, 2023
c4d22b0
Rework `Duration` implementation based on @nightkr gist
Techassi Sep 14, 2023
cad5cfe
Add documentation
Techassi Sep 14, 2023
f63ec1a
Fix module documentation links
Techassi Sep 14, 2023
2377276
Fix bug
Techassi Sep 14, 2023
9e9257d
Update parser, add context to errors, add more tests
Techassi Sep 15, 2023
96f1600
Update src/duration/mod.rs
Techassi Sep 15, 2023
58b04c3
Re-add millisecond support, switch to Snafu
Techassi Sep 15, 2023
dbf1dce
Merge branch 'main' into feat/humantime
Techassi Sep 15, 2023
3282f9e
Fix markdown link references
Techassi Sep 15, 2023
a39af93
duration::serde_impl exports nothing
nightkr Sep 15, 2023
9fdb1b3
Split duration parsing context selectors into a submodule
nightkr Sep 15, 2023
191151a
typo: "expected character" -> "unexpected character"
nightkr Sep 15, 2023
9dbcefe
Fail if duration buffer is non-empty at the end of parsing
nightkr Sep 15, 2023
705ed1f
Use Debug impls to quote strings in errors
nightkr Sep 15, 2023
fe1911e
Non-ASCII input will be caught anyway, because it's never a valid cha…
nightkr Sep 15, 2023
8ef94a7
DurationUnit doesn't need to be public
nightkr Sep 15, 2023
e7e4718
Add check for invalid unit order and duplicate units
Techassi Sep 18, 2023
2d3d32c
Revert "Non-ASCII input will be caught anyway, because it's never a v…
Techassi Sep 18, 2023
6b882e6
Fix doc comment
Techassi Sep 18, 2023
36e8035
Impl Mul and Div
sbernauer Sep 18, 2023
dcb4dd0
Impl Mul and Div 2
sbernauer Sep 18, 2023
198ae50
impl JsonSchema for Duration
sbernauer Sep 18, 2023
6737e51
Re-order `SubAssign` impl
Techassi Sep 18, 2023
29fca80
Revert "Revert "Non-ASCII input will be caught anyway, because it's n…
nightkr Sep 18, 2023
9a5c6da
Remove `impl Div<Duration> for u32`
Techassi Sep 18, 2023
98149d7
Fix scanning of multibyte chars
nightkr Sep 18, 2023
017ca87
Merge branch 'feat/humantime' of github.com:stackabletech/operator-rs…
nightkr Sep 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 107 additions & 15 deletions src/duration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
//! timeouts or retries.

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,
};

Expand All @@ -29,23 +30,32 @@ 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 },

#[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 },

#[snafu(display("failed to parse fragment value as integer"))]
ParseIntError { source: ParseIntError },
}

#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, PartialOrd, JsonSchema)]
#[derive(Clone, Copy, Debug, Derivative, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct Duration(std::time::Duration);

impl FromStr for Duration {
Expand All @@ -55,12 +65,14 @@ 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();
let mut duration = std::time::Duration::ZERO;
let mut last_unit = None;

let mut take_group = |f: fn(char) -> bool| {
let &(from, _) = chars.peek()?;
Expand Down Expand Up @@ -88,12 +100,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
Expand All @@ -105,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
Expand Down Expand Up @@ -178,19 +217,61 @@ impl SubAssign for Duration {
}
}

impl Mul<u32> for Duration {
type Output = Self;

fn mul(self, rhs: u32) -> Duration {
Self(self.0 * rhs)
}
}

impl Mul<Duration> for u32 {
type Output = Duration;

fn mul(self, rhs: Duration) -> Duration {
rhs * self
}
}

impl Div<u32> for Duration {
type Output = Self;

fn div(self, rhs: u32) -> Duration {
Self(self.0 / rhs)
}
}

impl Div<Duration> for u32 {
type Output = Duration;

fn div(self, rhs: Duration) -> Duration {
rhs / self
}
}
nightkr marked this conversation as resolved.
Show resolved Hide resolved

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))
}
}

/// 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(Debug, strum::EnumString, strum::Display, strum::AsRefStr, strum::EnumIter)]
#[derive(
Debug,
PartialEq,
Eq,
PartialOrd,
Ord,
strum::EnumString,
strum::Display,
strum::AsRefStr,
strum::EnumIter,
)]
pub enum DurationUnit {
#[strum(serialize = "d")]
Days,
Expand All @@ -211,7 +292,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 {
Expand Down Expand Up @@ -246,13 +327,24 @@ 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)
}

#[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)]
Expand Down