From 606ad23df2a21cf0dcd367e2f2248d9f042d68ea Mon Sep 17 00:00:00 2001 From: Ingrid Date: Tue, 9 Apr 2024 19:10:38 +0200 Subject: [PATCH 01/14] implement iso 8601 parser for RelativeDuration --- src/relative_duration.rs | 2 + src/relative_duration/parse.rs | 94 ++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 src/relative_duration/parse.rs diff --git a/src/relative_duration.rs b/src/relative_duration.rs index 4c19f09..575b015 100644 --- a/src/relative_duration.rs +++ b/src/relative_duration.rs @@ -6,6 +6,8 @@ use chrono::{Date, DateTime, Duration, NaiveDate, NaiveDateTime, TimeZone}; use super::delta::shift_months; +mod parse; + /// Relative time duration extending Chrono's Duration. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] pub struct RelativeDuration { diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs new file mode 100644 index 0000000..3142fca --- /dev/null +++ b/src/relative_duration/parse.rs @@ -0,0 +1,94 @@ +use super::RelativeDuration; +use chrono::Duration; + +fn dhms_to_duration(days: i32, hours: i32, minutes: i32, seconds: i32) -> Duration { + Duration::seconds((((days * 24 + hours) * 60 + minutes) * 60 + seconds) as i64) +} + +fn get_terminated(input: &str, terminator: char) -> Result<(&str, i32), String> { + if let Some((int_string, remainder)) = input.split_once(terminator) { + let int = int_string + .parse::() + .map_err(|_| format!("{} is not a valid i32", int_string))?; + Ok((remainder, int)) + } else { + Ok((input, 0)) + } +} + +fn parse_datespec(datespec: &str) -> Result<(i32, i32, i32), String> { + let (remainder, years) = get_terminated(datespec, 'Y')?; + let (remainder, months) = get_terminated(remainder, 'M')?; + let (remainder, weeks) = get_terminated(remainder, 'W')?; + let (remainder, days) = get_terminated(remainder, 'D')?; + + if !remainder.is_empty() { + Err(format!( + "trailing characters: {} in datespec: {}", + remainder, datespec + )) + } else { + Ok((years, months, (weeks * 7) + days)) + } +} + +fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32), String> { + let (remainder, hours) = get_terminated(timespec, 'H')?; + let (remainder, mins) = get_terminated(remainder, 'M')?; + let (remainder, secs) = get_terminated(remainder, 'S')?; + + if !remainder.is_empty() { + Err(format!( + "trailing characters: {} in timespec: {}", + remainder, timespec + )) + } else { + Ok((hours, mins, secs)) + } +} + +impl RelativeDuration { + pub fn from_iso_8601(input: &str) -> Result { + let input = input + .strip_prefix('P') + .ok_or_else(|| "duration was not prefixed with P".to_string())?; + + let (datespec, timespec) = input.split_once('T').unwrap_or((input, "")); + + let (years, months, days) = parse_datespec(datespec)?; + let (hours, mins, secs) = parse_timespec(timespec)?; + + Ok(RelativeDuration::months(years * 12 + months) + .with_duration(dhms_to_duration(days, hours, mins, secs))) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_duration() { + [ + ( + "P1YT1S", + RelativeDuration::months(12).with_duration(Duration::seconds(1)), + ), + ( + "P2Y2M2DT2H2M2S", + RelativeDuration::months(2 * 12 + 2).with_duration(dhms_to_duration(2, 2, 2, 2)), + ), + ( + "P1M", + RelativeDuration::months(1).with_duration(Duration::zero()), + ), + ("PT10M", RelativeDuration::minutes(10)), + ("P-1M", RelativeDuration::months(-1)), + ("P1W1D", RelativeDuration::days(8)), + ] + .iter() + .for_each(|(input, expected)| { + assert_eq!(RelativeDuration::from_iso_8601(input).unwrap(), *expected) + }) + } +} From 516a7899c5471ad78acbc5a0d56c45a042691c76 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 10 Apr 2024 14:48:07 +0200 Subject: [PATCH 02/14] implement iso 8601 formatter for RelativeDuration --- src/relative_duration/parse.rs | 63 ++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index 3142fca..927374b 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -47,6 +47,15 @@ fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32), String> { } } +fn format_spec(nums: [i64; 3], chars: [char; 3]) -> String { + nums.iter() + .zip(chars) + .filter(|x| *x.0 != 0) + .map(|x| format!("{}{}", x.0, x.1)) + .reduce(|acc, x| acc + &x) + .unwrap_or_else(|| "".to_string()) +} + impl RelativeDuration { pub fn from_iso_8601(input: &str) -> Result { let input = input @@ -61,6 +70,38 @@ impl RelativeDuration { Ok(RelativeDuration::months(years * 12 + months) .with_duration(dhms_to_duration(days, hours, mins, secs))) } + + pub fn to_iso_8601(&self) -> String { + let years = self.months as i64 / 12; + let months = self.months as i64 % 12; + + let duration_seconds = self.duration.num_seconds(); + + // TODO: address this unwrap + let days = duration_seconds / (24 * 60 * 60); + let mut remaining_seconds = duration_seconds - (days * 24 * 60 * 60); + + let hours = remaining_seconds / (60 * 60); + remaining_seconds -= hours * 60 * 60; + + let minutes = remaining_seconds / 60; + remaining_seconds -= minutes * 60; + + let seconds = remaining_seconds; + + let date_spec = format_spec([years, months, days], ['Y', 'M', 'D']); + let time_spec = format_spec([hours, minutes, seconds], ['H', 'M', 'S']); + + let mut out = String::with_capacity(date_spec.len() + time_spec.len() + 2); + out.push('P'); + out.push_str(&date_spec); + if !time_spec.is_empty() { + out.push('T'); + out.push_str(&time_spec); + } + + out + } } #[cfg(test)] @@ -91,4 +132,26 @@ mod tests { assert_eq!(RelativeDuration::from_iso_8601(input).unwrap(), *expected) }) } + + #[test] + fn test_format_duration() { + [ + ( + RelativeDuration::months(12).with_duration(Duration::seconds(1)), + "P1YT1S", + ), + ( + RelativeDuration::months(2 * 12 + 2).with_duration(dhms_to_duration(2, 2, 2, 2)), + "P2Y2M2DT2H2M2S", + ), + ( + RelativeDuration::months(1).with_duration(Duration::zero()), + "P1M", + ), + (RelativeDuration::minutes(10), "PT10M"), + (RelativeDuration::months(-1), "P-1M"), + ] + .iter() + .for_each(|(input, expected)| assert_eq!(input.to_iso_8601(), *expected)) + } } From 4b89fcce82fba899125a9f913d782d8616d55394 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 10 Apr 2024 16:34:22 +0200 Subject: [PATCH 03/14] document iso 8601 parser and formatter --- src/relative_duration/parse.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index 927374b..29f61ef 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -57,6 +57,27 @@ fn format_spec(nums: [i64; 3], chars: [char; 3]) -> String { } impl RelativeDuration { + /// Parses an [ISO 8601 duration string](https://en.wikipedia.org/wiki/ISO_8601#Durations) into + /// a [`RelativeDuration`] value. + /// + /// This supports only duration strings with integer values (i.e `"P1Y"` but not `"P0.5Y"` or + /// `"P0,5Y"`), as fractional values cannot be unambiguously represented as a [`RelativeDuration`] + /// + /// # Errors + /// + /// - Invalid duration string input + /// - Fractional values in duration string + /// + /// # Example + /// + /// ``` + /// use chronoutil::RelativeDuration; + /// + /// assert_eq!( + /// RelativeDuration::from_iso_8601("P1Y").unwrap(), + /// RelativeDuration::years(1), + /// ); + /// ``` pub fn from_iso_8601(input: &str) -> Result { let input = input .strip_prefix('P') @@ -71,6 +92,19 @@ impl RelativeDuration { .with_duration(dhms_to_duration(days, hours, mins, secs))) } + /// Formats a [`RelativeDuration`] value into an + /// [ISO 8601 duration string](https://en.wikipedia.org/wiki/ISO_8601#Durations). + /// + /// # Example + /// + /// ``` + /// use chronoutil::RelativeDuration; + /// + /// assert_eq!( + /// RelativeDuration::years(1).to_iso_8601(), + /// "P1Y", + /// ); + /// ``` pub fn to_iso_8601(&self) -> String { let years = self.months as i64 / 12; let months = self.months as i64 % 12; From c59f2f9574ef3e94323a576d0a4a4f092dfd63fc Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 22:26:00 +0200 Subject: [PATCH 04/14] support nanoseconds in duration parser --- src/relative_duration/parse.rs | 60 ++++++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index 29f61ef..95f066b 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -1,8 +1,9 @@ use super::RelativeDuration; use chrono::Duration; -fn dhms_to_duration(days: i32, hours: i32, minutes: i32, seconds: i32) -> Duration { +fn dhmsn_to_duration(days: i32, hours: i32, minutes: i32, seconds: i32, nanos: i32) -> Duration { Duration::seconds((((days * 24 + hours) * 60 + minutes) * 60 + seconds) as i64) + + Duration::nanoseconds(nanos.into()) } fn get_terminated(input: &str, terminator: char) -> Result<(&str, i32), String> { @@ -16,6 +17,39 @@ fn get_terminated(input: &str, terminator: char) -> Result<(&str, i32), String> } } +fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i32, i32), String> { + if let Some((decimal_string, remainder)) = input.split_once(terminator) { + let (int_string, fraction_string) = decimal_string.split_once('.').unwrap_or_else(|| { + decimal_string + // if no '.' was found, look for ',', as both are valid decimal separators in iso 8601 + .split_once(',') + // if neither is found take the whole string as the integer part, with no fraction + .unwrap_or((decimal_string, "")) + }); + + let int = int_string + .parse::() + .map_err(|_| format!("{} is not a valid i32", int_string))?; + + let fraction = if fraction_string.is_empty() { + 0 + } else { + fraction_string + .chars() + // right pad with zeros + .chain(std::iter::repeat('0')) + // truncate to 9 chars, since we only support nanosecond resolution + .take(9) + .collect::() + .parse::() + .map_err(|_| format!("{} is not a valid i32", fraction_string))? + }; + Ok((remainder, int, fraction)) + } else { + Ok((input, 0, 0)) + } +} + fn parse_datespec(datespec: &str) -> Result<(i32, i32, i32), String> { let (remainder, years) = get_terminated(datespec, 'Y')?; let (remainder, months) = get_terminated(remainder, 'M')?; @@ -32,10 +66,10 @@ fn parse_datespec(datespec: &str) -> Result<(i32, i32, i32), String> { } } -fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32), String> { +fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32, i32), String> { let (remainder, hours) = get_terminated(timespec, 'H')?; let (remainder, mins) = get_terminated(remainder, 'M')?; - let (remainder, secs) = get_terminated(remainder, 'S')?; + let (remainder, secs, nanos) = get_terminated_decimal(remainder, 'S')?; if !remainder.is_empty() { Err(format!( @@ -43,7 +77,7 @@ fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32), String> { remainder, timespec )) } else { - Ok((hours, mins, secs)) + Ok((hours, mins, secs, nanos)) } } @@ -61,12 +95,14 @@ impl RelativeDuration { /// a [`RelativeDuration`] value. /// /// This supports only duration strings with integer values (i.e `"P1Y"` but not `"P0.5Y"` or - /// `"P0,5Y"`), as fractional values cannot be unambiguously represented as a [`RelativeDuration`] + /// `"P0,5Y"`), as fractional values cannot be unambiguously represented as a + /// [`RelativeDuration`]. The one exception to this is the seconds field, where the fractional + /// part is truncated to 9 digits, and parsed as nanoseconds. /// /// # Errors /// /// - Invalid duration string input - /// - Fractional values in duration string + /// - Fractional values (apart from seconds) in duration string /// /// # Example /// @@ -86,10 +122,10 @@ impl RelativeDuration { let (datespec, timespec) = input.split_once('T').unwrap_or((input, "")); let (years, months, days) = parse_datespec(datespec)?; - let (hours, mins, secs) = parse_timespec(timespec)?; + let (hours, mins, secs, nanos) = parse_timespec(timespec)?; Ok(RelativeDuration::months(years * 12 + months) - .with_duration(dhms_to_duration(days, hours, mins, secs))) + .with_duration(dhmsn_to_duration(days, hours, mins, secs, nanos))) } /// Formats a [`RelativeDuration`] value into an @@ -151,7 +187,8 @@ mod tests { ), ( "P2Y2M2DT2H2M2S", - RelativeDuration::months(2 * 12 + 2).with_duration(dhms_to_duration(2, 2, 2, 2)), + RelativeDuration::months(2 * 12 + 2) + .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0)), ), ( "P1M", @@ -160,6 +197,8 @@ mod tests { ("PT10M", RelativeDuration::minutes(10)), ("P-1M", RelativeDuration::months(-1)), ("P1W1D", RelativeDuration::days(8)), + ("PT0.0000000010S", RelativeDuration::nanoseconds(1)), + ("PT0.1S", RelativeDuration::nanoseconds(100_000_000)), ] .iter() .for_each(|(input, expected)| { @@ -175,7 +214,8 @@ mod tests { "P1YT1S", ), ( - RelativeDuration::months(2 * 12 + 2).with_duration(dhms_to_duration(2, 2, 2, 2)), + RelativeDuration::months(2 * 12 + 2) + .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0)), "P2Y2M2DT2H2M2S", ), ( From 54cf25cfa8f1726e1b66284200090caae11fec3a Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 22:45:16 +0200 Subject: [PATCH 05/14] use correct integer types in RelativeDuration parser --- src/relative_duration/parse.rs | 52 ++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index 95f066b..ee03046 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -1,23 +1,31 @@ use super::RelativeDuration; use chrono::Duration; -fn dhmsn_to_duration(days: i32, hours: i32, minutes: i32, seconds: i32, nanos: i32) -> Duration { - Duration::seconds((((days * 24 + hours) * 60 + minutes) * 60 + seconds) as i64) - + Duration::nanoseconds(nanos.into()) +fn dhmsn_to_duration(days: i64, hours: i64, minutes: i64, seconds: i64, nanos: i64) -> Duration { + // TODO: check for overflows here + Duration::seconds(((days * 24 + hours) * 60 + minutes) * 60 + seconds) + + Duration::nanoseconds(nanos) } -fn get_terminated(input: &str, terminator: char) -> Result<(&str, i32), String> { +fn get_terminated>( + input: &str, + terminator: char, +) -> Result<(&str, T), String> { if let Some((int_string, remainder)) = input.split_once(terminator) { - let int = int_string - .parse::() - .map_err(|_| format!("{} is not a valid i32", int_string))?; + let int = int_string.parse::().map_err(|_| { + format!( + "{} is not a valid {}", + int_string, + std::any::type_name::() + ) + })?; Ok((remainder, int)) } else { - Ok((input, 0)) + Ok((input, 0.into())) } } -fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i32, i32), String> { +fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i64, i64), String> { if let Some((decimal_string, remainder)) = input.split_once(terminator) { let (int_string, fraction_string) = decimal_string.split_once('.').unwrap_or_else(|| { decimal_string @@ -28,8 +36,8 @@ fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i32, i }); let int = int_string - .parse::() - .map_err(|_| format!("{} is not a valid i32", int_string))?; + .parse::() + .map_err(|_| format!("{} is not a valid i64", int_string))?; let fraction = if fraction_string.is_empty() { 0 @@ -41,8 +49,8 @@ fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i32, i // truncate to 9 chars, since we only support nanosecond resolution .take(9) .collect::() - .parse::() - .map_err(|_| format!("{} is not a valid i32", fraction_string))? + .parse::() + .map_err(|_| format!("{} is not a valid i64", fraction_string))? }; Ok((remainder, int, fraction)) } else { @@ -50,11 +58,11 @@ fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i32, i } } -fn parse_datespec(datespec: &str) -> Result<(i32, i32, i32), String> { - let (remainder, years) = get_terminated(datespec, 'Y')?; - let (remainder, months) = get_terminated(remainder, 'M')?; - let (remainder, weeks) = get_terminated(remainder, 'W')?; - let (remainder, days) = get_terminated(remainder, 'D')?; +fn parse_datespec(datespec: &str) -> Result<(i32, i32, i64), String> { + let (remainder, years) = get_terminated::(datespec, 'Y')?; + let (remainder, months) = get_terminated::(remainder, 'M')?; + let (remainder, weeks) = get_terminated::(remainder, 'W')?; + let (remainder, days) = get_terminated::(remainder, 'D')?; if !remainder.is_empty() { Err(format!( @@ -62,13 +70,14 @@ fn parse_datespec(datespec: &str) -> Result<(i32, i32, i32), String> { remainder, datespec )) } else { + // TODO: check for overflow here Ok((years, months, (weeks * 7) + days)) } } -fn parse_timespec(timespec: &str) -> Result<(i32, i32, i32, i32), String> { - let (remainder, hours) = get_terminated(timespec, 'H')?; - let (remainder, mins) = get_terminated(remainder, 'M')?; +fn parse_timespec(timespec: &str) -> Result<(i64, i64, i64, i64), String> { + let (remainder, hours) = get_terminated::(timespec, 'H')?; + let (remainder, mins) = get_terminated::(remainder, 'M')?; let (remainder, secs, nanos) = get_terminated_decimal(remainder, 'S')?; if !remainder.is_empty() { @@ -124,6 +133,7 @@ impl RelativeDuration { let (years, months, days) = parse_datespec(datespec)?; let (hours, mins, secs, nanos) = parse_timespec(timespec)?; + // TODO: check for overflows here Ok(RelativeDuration::months(years * 12 + months) .with_duration(dhmsn_to_duration(days, hours, mins, secs, nanos))) } From fed2796657d8a5f40bc53c06e78f4ad297421199 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 23:02:58 +0200 Subject: [PATCH 06/14] check for integer overflow in RelativeDuration parser --- src/relative_duration/parse.rs | 49 +++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index ee03046..a988bb5 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -1,10 +1,23 @@ use super::RelativeDuration; use chrono::Duration; -fn dhmsn_to_duration(days: i64, hours: i64, minutes: i64, seconds: i64, nanos: i64) -> Duration { - // TODO: check for overflows here - Duration::seconds(((days * 24 + hours) * 60 + minutes) * 60 + seconds) - + Duration::nanoseconds(nanos) +fn dhmsn_to_duration( + days: i64, + hours: i64, + minutes: i64, + seconds: i64, + nanos: i64, +) -> Option { + Some( + Duration::seconds( + days.checked_mul(24)? + .checked_add(hours)? + .checked_mul(60)? + .checked_add(minutes)? + .checked_mul(60)? + .checked_add(seconds)?, + ) + Duration::nanoseconds(nanos), + ) } fn get_terminated>( @@ -70,8 +83,14 @@ fn parse_datespec(datespec: &str) -> Result<(i32, i32, i64), String> { remainder, datespec )) } else { - // TODO: check for overflow here - Ok((years, months, (weeks * 7) + days)) + Ok(( + years, + months, + weeks + .checked_mul(7) + .and_then(|x| x.checked_add(days)) + .ok_or_else(|| "integer overflow on constructing duration".to_string())?, + )) } } @@ -133,9 +152,16 @@ impl RelativeDuration { let (years, months, days) = parse_datespec(datespec)?; let (hours, mins, secs, nanos) = parse_timespec(timespec)?; - // TODO: check for overflows here - Ok(RelativeDuration::months(years * 12 + months) - .with_duration(dhmsn_to_duration(days, hours, mins, secs, nanos))) + Ok(RelativeDuration::months( + years + .checked_mul(12) + .and_then(|x| x.checked_add(months)) + .ok_or_else(|| "integer overflow on constructing duration".to_string())?, + ) + .with_duration( + dhmsn_to_duration(days, hours, mins, secs, nanos) + .ok_or_else(|| "integer overflow on constructing duration".to_string())?, + )) } /// Formats a [`RelativeDuration`] value into an @@ -157,7 +183,6 @@ impl RelativeDuration { let duration_seconds = self.duration.num_seconds(); - // TODO: address this unwrap let days = duration_seconds / (24 * 60 * 60); let mut remaining_seconds = duration_seconds - (days * 24 * 60 * 60); @@ -198,7 +223,7 @@ mod tests { ( "P2Y2M2DT2H2M2S", RelativeDuration::months(2 * 12 + 2) - .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0)), + .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0).unwrap()), ), ( "P1M", @@ -225,7 +250,7 @@ mod tests { ), ( RelativeDuration::months(2 * 12 + 2) - .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0)), + .with_duration(dhmsn_to_duration(2, 2, 2, 2, 0).unwrap()), "P2Y2M2DT2H2M2S", ), ( From 47a66c5f1616e4e2067cf6931e93c2409ae56342 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 23:14:06 +0200 Subject: [PATCH 07/14] use u32 to represent nanos, to follow chrono::Duration::new signature --- src/relative_duration/parse.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index a988bb5..93046e0 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -6,17 +6,16 @@ fn dhmsn_to_duration( hours: i64, minutes: i64, seconds: i64, - nanos: i64, + nanos: u32, ) -> Option { - Some( - Duration::seconds( - days.checked_mul(24)? - .checked_add(hours)? - .checked_mul(60)? - .checked_add(minutes)? - .checked_mul(60)? - .checked_add(seconds)?, - ) + Duration::nanoseconds(nanos), + Duration::new( + days.checked_mul(24)? + .checked_add(hours)? + .checked_mul(60)? + .checked_add(minutes)? + .checked_mul(60)? + .checked_add(seconds)?, + nanos, ) } @@ -38,7 +37,7 @@ fn get_terminated>( } } -fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i64, i64), String> { +fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i64, u32), String> { if let Some((decimal_string, remainder)) = input.split_once(terminator) { let (int_string, fraction_string) = decimal_string.split_once('.').unwrap_or_else(|| { decimal_string @@ -62,8 +61,8 @@ fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i64, i // truncate to 9 chars, since we only support nanosecond resolution .take(9) .collect::() - .parse::() - .map_err(|_| format!("{} is not a valid i64", fraction_string))? + .parse::() + .map_err(|_| format!("{} is not a valid u32", fraction_string))? }; Ok((remainder, int, fraction)) } else { @@ -94,7 +93,7 @@ fn parse_datespec(datespec: &str) -> Result<(i32, i32, i64), String> { } } -fn parse_timespec(timespec: &str) -> Result<(i64, i64, i64, i64), String> { +fn parse_timespec(timespec: &str) -> Result<(i64, i64, i64, u32), String> { let (remainder, hours) = get_terminated::(timespec, 'H')?; let (remainder, mins) = get_terminated::(remainder, 'M')?; let (remainder, secs, nanos) = get_terminated_decimal(remainder, 'S')?; From a4299765d733329bc70787e36f76180131bb2bf8 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 23:41:59 +0200 Subject: [PATCH 08/14] support nanoseconds in RelativeDuration formatter --- src/relative_duration/parse.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index 93046e0..a0db089 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -199,9 +199,21 @@ impl RelativeDuration { let mut out = String::with_capacity(date_spec.len() + time_spec.len() + 2); out.push('P'); out.push_str(&date_spec); - if !time_spec.is_empty() { + if !time_spec.is_empty() || self.duration.subsec_nanos() != 0 { out.push('T'); - out.push_str(&time_spec); + if time_spec.is_empty() { + out.push('0'); + } else { + out.push_str(&time_spec); + } + } + if self.duration.subsec_nanos() != 0 { + out = out.trim_end_matches('S').to_string(); + let nanos_str_raw = format!("{:0>9}", self.duration.subsec_nanos()); + let nanos_str_trimmed = nanos_str_raw.trim_end_matches('0'); + out.push('.'); + out.push_str(nanos_str_trimmed); + out.push('S'); } out @@ -258,6 +270,8 @@ mod tests { ), (RelativeDuration::minutes(10), "PT10M"), (RelativeDuration::months(-1), "P-1M"), + (RelativeDuration::nanoseconds(1), "PT0.000000001S"), + (RelativeDuration::nanoseconds(100_000_000), "PT0.1S"), ] .iter() .for_each(|(input, expected)| assert_eq!(input.to_iso_8601(), *expected)) From 71adb669fa2d5dffd3d442fbd458da24b22606c6 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 23:54:40 +0200 Subject: [PATCH 09/14] simplify format_spec and improve efficiency --- src/relative_duration/parse.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index a0db089..e50c801 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -1,5 +1,6 @@ use super::RelativeDuration; use chrono::Duration; +use std::fmt::Write; fn dhmsn_to_duration( days: i64, @@ -112,9 +113,10 @@ fn format_spec(nums: [i64; 3], chars: [char; 3]) -> String { nums.iter() .zip(chars) .filter(|x| *x.0 != 0) - .map(|x| format!("{}{}", x.0, x.1)) - .reduce(|acc, x| acc + &x) - .unwrap_or_else(|| "".to_string()) + .fold(String::new(), |mut out, x| { + let _ = write!(out, "{}{}", x.0, x.1); + out + }) } impl RelativeDuration { From ccd6be771dfb55b77787869c0ce777a7ae985945 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 20 Apr 2024 23:59:38 +0200 Subject: [PATCH 10/14] make divmod clearer in RelativeDuration::to_iso_8601 --- src/relative_duration/parse.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index e50c801..d6c4fa9 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -185,13 +185,13 @@ impl RelativeDuration { let duration_seconds = self.duration.num_seconds(); let days = duration_seconds / (24 * 60 * 60); - let mut remaining_seconds = duration_seconds - (days * 24 * 60 * 60); + let mut remaining_seconds = duration_seconds % (24 * 60 * 60); let hours = remaining_seconds / (60 * 60); - remaining_seconds -= hours * 60 * 60; + remaining_seconds %= 60 * 60; let minutes = remaining_seconds / 60; - remaining_seconds -= minutes * 60; + remaining_seconds %= 60; let seconds = remaining_seconds; From 11257116ed3a14dda36803a856cfcd4be037789f Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 21 Apr 2024 11:37:50 +0200 Subject: [PATCH 11/14] more test cases for RelativeDuration iso parse/format --- src/relative_duration/parse.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index d6c4fa9..e8e610a 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -245,6 +245,12 @@ mod tests { ("PT10M", RelativeDuration::minutes(10)), ("P-1M", RelativeDuration::months(-1)), ("P1W1D", RelativeDuration::days(8)), + ( + "P1Y-10M-1W3DT3H-6M-1S", + RelativeDuration::months(2) + .with_duration(dhmsn_to_duration(-4, 3, -6, -1, 0).unwrap()), + ), + ("P-23M", RelativeDuration::months(-23)), ("PT0.0000000010S", RelativeDuration::nanoseconds(1)), ("PT0.1S", RelativeDuration::nanoseconds(100_000_000)), ] @@ -272,6 +278,7 @@ mod tests { ), (RelativeDuration::minutes(10), "PT10M"), (RelativeDuration::months(-1), "P-1M"), + (RelativeDuration::months(-23), "P-1Y-11M"), (RelativeDuration::nanoseconds(1), "PT0.000000001S"), (RelativeDuration::nanoseconds(100_000_000), "PT0.1S"), ] From d43bf91811842500486e29c20f73f75d0f096a64 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 21 Apr 2024 13:07:14 +0200 Subject: [PATCH 12/14] add benchmarks for RelativeDuration format/parse --- Cargo.toml | 4 +++ benches/relative_duration.rs | 53 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 benches/relative_duration.rs diff --git a/Cargo.toml b/Cargo.toml index da0a33d..6051bb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,3 +21,7 @@ chrono-tz = "0.8.3" [[bench]] name = "delta" harness = false + +[[bench]] +name = "relative_duration" +harness = false diff --git a/benches/relative_duration.rs b/benches/relative_duration.rs new file mode 100644 index 0000000..c0c6d36 --- /dev/null +++ b/benches/relative_duration.rs @@ -0,0 +1,53 @@ +use criterion::{black_box, criterion_group, criterion_main, Criterion}; + +use chronoutil::RelativeDuration; + +fn relative_duration_format_benchmark(c: &mut Criterion) { + let durations = [ + "P1M", + "P1Y1M1W1DT1H1M1S", + "P99999999Y11M30DT23H59M59.999999999S", + ] + .iter() + .map(|s| RelativeDuration::from_iso_8601(s).unwrap()) + .collect::>(); + + let mut g = c.benchmark_group("relative_duration_format"); + + g.bench_function("one_specifier", |b| { + b.iter(|| black_box(durations[0]).to_iso_8601()) + }); + g.bench_function("all_specifiers", |b| { + b.iter(|| black_box(durations[1]).to_iso_8601()) + }); + g.bench_function("long_specifiers", |b| { + b.iter(|| black_box(durations[2]).to_iso_8601()) + }); +} + +fn relative_duration_parse_benchmark(c: &mut Criterion) { + let durations = [ + "P1M", + "P1Y1M1W1DT1H1M1S", + "P99999999Y11M30DT23H59M59.999999999S", + ]; + + let mut g = c.benchmark_group("relative_duration_parse"); + + g.bench_function("one_specifier", |b| { + b.iter(|| RelativeDuration::from_iso_8601(black_box(durations[0]))) + }); + g.bench_function("all_specifiers", |b| { + b.iter(|| RelativeDuration::from_iso_8601(black_box(durations[1]))) + }); + g.bench_function("long_specifiers", |b| { + b.iter(|| RelativeDuration::from_iso_8601(black_box(durations[2]))) + }); +} + +criterion_group!( + benches, + relative_duration_format_benchmark, + relative_duration_parse_benchmark +); +criterion_main!(benches); From a9d49e120f19ded2c0d6687304dd970bb632f3e3 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 21 Apr 2024 18:10:58 +0200 Subject: [PATCH 13/14] fix quirks of nanosecond handling in RelativeDuration parsing/formatting --- src/relative_duration/parse.rs | 106 ++++++++++++++++++++++++--------- 1 file changed, 77 insertions(+), 29 deletions(-) diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index e8e610a..d9ef605 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -1,6 +1,6 @@ use super::RelativeDuration; use chrono::Duration; -use std::fmt::Write; +use std::{convert::TryInto, fmt::Write}; fn dhmsn_to_duration( days: i64, @@ -65,7 +65,17 @@ fn get_terminated_decimal(input: &str, terminator: char) -> Result<(&str, i64, u .parse::() .map_err(|_| format!("{} is not a valid u32", fraction_string))? }; - Ok((remainder, int, fraction)) + + // special handling for case of nonzero nanoseconds on a negative duration + if decimal_string.starts_with('-') && fraction != 0 { + Ok(( + remainder, + int - 1, + (-(fraction as i32) + 1_000_000_000).try_into().unwrap(), + )) + } else { + Ok((remainder, int, fraction)) + } } else { Ok((input, 0, 0)) } @@ -109,16 +119,6 @@ fn parse_timespec(timespec: &str) -> Result<(i64, i64, i64, u32), String> { } } -fn format_spec(nums: [i64; 3], chars: [char; 3]) -> String { - nums.iter() - .zip(chars) - .filter(|x| *x.0 != 0) - .fold(String::new(), |mut out, x| { - let _ = write!(out, "{}{}", x.0, x.1); - out - }) -} - impl RelativeDuration { /// Parses an [ISO 8601 duration string](https://en.wikipedia.org/wiki/ISO_8601#Durations) into /// a [`RelativeDuration`] value. @@ -193,28 +193,66 @@ impl RelativeDuration { let minutes = remaining_seconds / 60; remaining_seconds %= 60; - let seconds = remaining_seconds; + let subsec_nanos = self.duration.subsec_nanos(); - let date_spec = format_spec([years, months, days], ['Y', 'M', 'D']); - let time_spec = format_spec([hours, minutes, seconds], ['H', 'M', 'S']); + // This awkward handling is needed to represent nanoseconds as a fraction of seconds, + // instead of independently, since it must have no sign, and will affect the sign for + // seconds. This would be simpler if we could get the Duration.secs and Duration.nanos + // directly, but unfortunately chrono only offers Duration::num_seconds and + // Duration::subsec_nanos, both of which apply transformations before returning... + let (seconds, nanos, push_minus) = if remaining_seconds > 0 && subsec_nanos < 0 { + (remaining_seconds - 1, subsec_nanos + 1_000_000_000, false) + } else if remaining_seconds < 0 && subsec_nanos > 0 { + (remaining_seconds + 1, -subsec_nanos + 1_000_000_000, false) + } else if remaining_seconds <= 0 && subsec_nanos < 0 { + (remaining_seconds, -subsec_nanos, remaining_seconds == 0) + } else { + (remaining_seconds, subsec_nanos, false) + }; + + let mut out = String::new(); - let mut out = String::with_capacity(date_spec.len() + time_spec.len() + 2); out.push('P'); - out.push_str(&date_spec); - if !time_spec.is_empty() || self.duration.subsec_nanos() != 0 { + + [years, months, days] + .iter() + .zip(['Y', 'M', 'D']) + .filter(|x| *x.0 != 0) + .fold(&mut out, |out, x| { + let _ = write!(out, "{}{}", x.0, x.1); + out + }); + + if [hours, minutes, seconds, nanos as i64] + .iter() + .any(|x| *x != 0) + { out.push('T'); - if time_spec.is_empty() { - out.push('0'); - } else { - out.push_str(&time_spec); - } } - if self.duration.subsec_nanos() != 0 { - out = out.trim_end_matches('S').to_string(); - let nanos_str_raw = format!("{:0>9}", self.duration.subsec_nanos()); - let nanos_str_trimmed = nanos_str_raw.trim_end_matches('0'); - out.push('.'); - out.push_str(nanos_str_trimmed); + + [hours, minutes] + .iter() + .zip(['H', 'M']) + .filter(|x| *x.0 != 0) + .fold(&mut out, |out, x| { + let _ = write!(out, "{}{}", x.0, x.1); + out + }); + + if push_minus { + out.push('-'); + } + + if seconds != 0 || nanos != 0 { + let _ = write!(out, "{}", seconds); + + if nanos != 0 { + let nanos_str_raw = format!("{:0>9}", nanos); + let nanos_str_trimmed = nanos_str_raw.trim_end_matches('0'); + out.push('.'); + out.push_str(nanos_str_trimmed); + } + out.push('S'); } @@ -253,6 +291,11 @@ mod tests { ("P-23M", RelativeDuration::months(-23)), ("PT0.0000000010S", RelativeDuration::nanoseconds(1)), ("PT0.1S", RelativeDuration::nanoseconds(100_000_000)), + ( + "PT-0.999999999S", + RelativeDuration::years(0) + .with_duration(dhmsn_to_duration(0, 0, 0, -1, 1).unwrap()), + ), ] .iter() .for_each(|(input, expected)| { @@ -281,6 +324,11 @@ mod tests { (RelativeDuration::months(-23), "P-1Y-11M"), (RelativeDuration::nanoseconds(1), "PT0.000000001S"), (RelativeDuration::nanoseconds(100_000_000), "PT0.1S"), + ( + RelativeDuration::years(0) + .with_duration(dhmsn_to_duration(0, 0, 0, -1, 1).unwrap()), + "PT-0.999999999S", + ), ] .iter() .for_each(|(input, expected)| assert_eq!(input.to_iso_8601(), *expected)) From 1f25739a542a05d18f2802ac8f8d72357716b963 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 21 Apr 2024 19:15:45 +0200 Subject: [PATCH 14/14] add property tests for RelativeDuration parser and formatter --- Cargo.toml | 1 + src/relative_duration/parse.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6051bb9..33e5105 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ version = "0.2.6" [dev-dependencies] criterion = "0.3" chrono-tz = "0.8.3" +proptest = "1.4.0" [[bench]] name = "delta" diff --git a/src/relative_duration/parse.rs b/src/relative_duration/parse.rs index d9ef605..4d16031 100644 --- a/src/relative_duration/parse.rs +++ b/src/relative_duration/parse.rs @@ -262,6 +262,8 @@ impl RelativeDuration { #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; #[test] @@ -333,4 +335,28 @@ mod tests { .iter() .for_each(|(input, expected)| assert_eq!(input.to_iso_8601(), *expected)) } + + proptest! { + #[test] + fn proptest_format_and_back( + months in prop::num::i32::ANY, + secs in (i64::MIN/1000)..(i64::MAX/1000), + nanos in 0u32..1_000_000_000 + ) { + let d = RelativeDuration::months(months).with_duration(Duration::new(secs, nanos).unwrap()); + prop_assert_eq!(d, RelativeDuration::from_iso_8601(&(d.to_iso_8601())).unwrap()); + } + + #[test] + fn proptest_parse_and_back( + s in r"P(?:[1-9][0-9]{0,7}Y)?(?:(?:[1-9]|1[0-1])M)?(?:(?:[1-9]|[1-2][0-9])D)?(?:T(?:(?:[1-9]|1[0-9]|2[0-3])H)(?:(?:[1-9]|[1-5][0-9])M)(?:(?:(?:[1-9]|[1-5][0-9])|(?:(?:[0-9]|[1-5][0-9])\.[0-9]{0,8}[1-9]))S))?", + ) { + prop_assert_eq!(s.clone(), RelativeDuration::from_iso_8601(&s).unwrap().to_iso_8601()); + } + + #[test] + fn proptest_parse_doesnt_panic(s in r"//PC*") { + let _ = RelativeDuration::from_iso_8601(&s); + } + } }