From 218d4dfeecdb535a0ff8a6a9fb22276f63b4a6db Mon Sep 17 00:00:00 2001 From: bconn98 Date: Wed, 3 Apr 2024 21:11:18 -0400 Subject: [PATCH 1/9] fix: replace deprecated API in chrono 0.4.35 for converting to time --- Cargo.toml | 2 +- .../policy/compound/trigger/time.rs | 52 +++++++++++++------ 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fbc77abd..85932203 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,7 @@ harness = false [dependencies] arc-swap = "1.6" -chrono = { version = "0.4.23", optional = true, features = ["clock"], default-features = false } +chrono = { version = "0.4.35", optional = true, features = ["clock"], default-features = false } flate2 = { version = "1.0", optional = true } fnv = "1.0" humantime = { version = "2.1", optional = true } diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 4568a524..0b97dd2f 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -2,8 +2,6 @@ //! //! Requires the `time_trigger` feature. -#[cfg(test)] -use chrono::NaiveDateTime; use chrono::{DateTime, Datelike, Duration, Local, TimeZone, Timelike}; #[cfg(test)] use mock_instant::{SystemTime, UNIX_EPOCH}; @@ -13,11 +11,18 @@ use serde::de; #[cfg(feature = "config_parsing")] use std::fmt; use std::sync::RwLock; +use thiserror::Error; use crate::append::rolling_file::{policy::compound::trigger::Trigger, LogFile}; #[cfg(feature = "config_parsing")] use crate::config::{Deserialize, Deserializers}; +macro_rules! try_from { + ($func: ident, $para: expr, $interval: expr) => { + Duration::$func($para).ok_or(TimeTrigerError::TooLargeInterval($interval))? + }; +} + #[cfg(feature = "config_parsing")] /// Configuration for the time trigger. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default, serde::Deserialize)] @@ -73,6 +78,12 @@ impl Default for TimeTriggerInterval { } } +#[derive(Debug, Error)] +enum TimeTrigerError { + #[error("The integer value {0:?} for the specified time trigger interval is too large, it must be less than 9,223,372,036,854,775,807 seconds.")] + TooLargeInterval(TimeTriggerInterval), +} + #[cfg(feature = "config_parsing")] impl<'de> serde::Deserialize<'de> for TimeTriggerInterval { fn deserialize(d: D) -> Result @@ -181,18 +192,22 @@ impl TimeTrigger { let now: std::time::Duration = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("system time before Unix epoch"); - NaiveDateTime::from_timestamp_opt(now.as_secs() as i64, now.subsec_nanos()) + DateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()) .unwrap() + .naive_local() .and_local_timezone(Local) .unwrap() }; #[cfg(not(test))] let current = Local::now(); - let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate); + let next_time = + TimeTrigger::get_next_time(current, config.interval, config.modulate).unwrap(); let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); - next_time + Duration::seconds(random_delay as i64) + next_time + + Duration::try_seconds(random_delay as i64) + .unwrap_or(Duration::try_milliseconds(i64::MAX).unwrap()) } else { next_time }; @@ -207,13 +222,13 @@ impl TimeTrigger { current: DateTime, interval: TimeTriggerInterval, modulate: bool, - ) -> DateTime { + ) -> Result, TimeTrigerError> { let year = current.year(); if let TimeTriggerInterval::Year(n) = interval { let n = n as i32; let increment = if modulate { n - year % n } else { n }; let year_new = year + increment; - return Local.with_ymd_and_hms(year_new, 1, 1, 0, 0, 0).unwrap(); + return Ok(Local.with_ymd_and_hms(year_new, 1, 1, 0, 0, 0).unwrap()); } if let TimeTriggerInterval::Month(n) = interval { @@ -224,9 +239,9 @@ impl TimeTrigger { let num_months_new = num_months + increment; let year_new = (num_months_new / 12) as i32; let month_new = (num_months_new) % 12 + 1; - return Local + return Ok(Local .with_ymd_and_hms(year_new, month_new, 1, 0, 0, 0) - .unwrap(); + .unwrap()); } let month = current.month(); @@ -236,14 +251,17 @@ impl TimeTrigger { let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - week0 % n } else { n }; - return time + Duration::weeks(increment) - Duration::days(weekday); + let dur = + try_from!(try_weeks, increment, interval) - try_from!(try_days, weekday, interval); + return Ok(time + dur); } if let TimeTriggerInterval::Day(n) = interval { let ordinal0 = current.ordinal0() as i64; let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - ordinal0 % n } else { n }; - return time + Duration::days(increment); + let dur = try_from!(try_days, increment, interval); + return Ok(time + dur); } let hour = current.hour(); @@ -252,7 +270,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, 0, 0) .unwrap(); let increment = if modulate { n - (hour as i64) % n } else { n }; - return time + Duration::hours(increment); + let dur = try_from!(try_hours, increment, interval); + return Ok(time + dur); } let min = current.minute(); @@ -261,7 +280,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, 0) .unwrap(); let increment = if modulate { n - (min as i64) % n } else { n }; - return time + Duration::minutes(increment); + let dur = try_from!(try_minutes, increment, interval); + return Ok(time + dur); } let sec = current.second(); @@ -270,7 +290,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, sec) .unwrap(); let increment = if modulate { n - (sec as i64) % n } else { n }; - return time + Duration::seconds(increment); + let dur = try_from!(try_seconds, increment, interval); + return Ok(time + dur); } panic!("Should not reach here!"); } @@ -283,8 +304,9 @@ impl Trigger for TimeTrigger { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("system time before Unix epoch"); - NaiveDateTime::from_timestamp_opt(now.as_secs() as i64, now.subsec_nanos()) + DateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()) .unwrap() + .naive_local() .and_local_timezone(Local) .unwrap() }; From 46b1779297b7dc624872d23f330c8c7c477ad9da Mon Sep 17 00:00:00 2001 From: bconn98 Date: Tue, 9 Jul 2024 21:01:11 -0400 Subject: [PATCH 2/9] chore: resolve review comments + clippy error --- .../policy/compound/trigger/time.rs | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 0b97dd2f..bc9bb453 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -17,12 +17,6 @@ use crate::append::rolling_file::{policy::compound::trigger::Trigger, LogFile}; #[cfg(feature = "config_parsing")] use crate::config::{Deserialize, Deserializers}; -macro_rules! try_from { - ($func: ident, $para: expr, $interval: expr) => { - Duration::$func($para).ok_or(TimeTrigerError::TooLargeInterval($interval))? - }; -} - #[cfg(feature = "config_parsing")] /// Configuration for the time trigger. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default, serde::Deserialize)] @@ -79,9 +73,17 @@ impl Default for TimeTriggerInterval { } #[derive(Debug, Error)] -enum TimeTrigerError { - #[error("The integer value {0:?} for the specified time trigger interval is too large, it must be less than 9,223,372,036,854,775,807 seconds.")] - TooLargeInterval(TimeTriggerInterval), +enum TimeTrigerIntervalError { + #[error("The 'Seconds' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + Second(TimeTriggerInterval), + #[error("The 'Minutes' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + Minute(TimeTriggerInterval), + #[error("The 'Hours' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + Hour(TimeTriggerInterval), + #[error("The 'Days' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + Day(TimeTriggerInterval), + #[error("The 'Weeks' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + Week(TimeTriggerInterval), } #[cfg(feature = "config_parsing")] @@ -201,10 +203,16 @@ impl TimeTrigger { #[cfg(not(test))] let current = Local::now(); - let next_time = - TimeTrigger::get_next_time(current, config.interval, config.modulate).unwrap(); + let next_time = match TimeTrigger::get_next_time(current, config.interval, config.modulate) + { + Ok(next_time) => next_time, + Err(err) => panic!("{}", err), + }; let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); + // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 + // and we're providing a known valid value. We can trust the Option will always return + // us a valid number. next_time + Duration::try_seconds(random_delay as i64) .unwrap_or(Duration::try_milliseconds(i64::MAX).unwrap()) @@ -222,7 +230,7 @@ impl TimeTrigger { current: DateTime, interval: TimeTriggerInterval, modulate: bool, - ) -> Result, TimeTrigerError> { + ) -> Result, TimeTrigerIntervalError> { let year = current.year(); if let TimeTriggerInterval::Year(n) = interval { let n = n as i32; @@ -251,8 +259,9 @@ impl TimeTrigger { let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - week0 % n } else { n }; - let dur = - try_from!(try_weeks, increment, interval) - try_from!(try_days, weekday, interval); + let dur = Duration::try_weeks(increment) + .ok_or(TimeTrigerIntervalError::Week(interval))? + - Duration::try_days(weekday).ok_or(TimeTrigerIntervalError::Day(interval))?; return Ok(time + dur); } @@ -260,7 +269,8 @@ impl TimeTrigger { let ordinal0 = current.ordinal0() as i64; let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - ordinal0 % n } else { n }; - let dur = try_from!(try_days, increment, interval); + let dur = + Duration::try_days(increment).ok_or(TimeTrigerIntervalError::Day(interval))?; return Ok(time + dur); } @@ -270,7 +280,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, 0, 0) .unwrap(); let increment = if modulate { n - (hour as i64) % n } else { n }; - let dur = try_from!(try_hours, increment, interval); + let dur = + Duration::try_hours(increment).ok_or(TimeTrigerIntervalError::Hour(interval))?; return Ok(time + dur); } @@ -280,7 +291,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, 0) .unwrap(); let increment = if modulate { n - (min as i64) % n } else { n }; - let dur = try_from!(try_minutes, increment, interval); + let dur = Duration::try_minutes(increment) + .ok_or(TimeTrigerIntervalError::Minute(interval))?; return Ok(time + dur); } @@ -290,7 +302,8 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, sec) .unwrap(); let increment = if modulate { n - (sec as i64) % n } else { n }; - let dur = try_from!(try_seconds, increment, interval); + let dur = Duration::try_seconds(increment) + .ok_or(TimeTrigerIntervalError::Second(interval))?; return Ok(time + dur); } panic!("Should not reach here!"); From 88e123e77d4b6965d09d09153a194b63fd3e91b8 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Tue, 9 Jul 2024 21:13:32 -0400 Subject: [PATCH 3/9] refactor: swap to unwrap_or on get_next_time call --- src/append/rolling_file/policy/compound/trigger/time.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index bc9bb453..efef2242 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -203,11 +203,8 @@ impl TimeTrigger { #[cfg(not(test))] let current = Local::now(); - let next_time = match TimeTrigger::get_next_time(current, config.interval, config.modulate) - { - Ok(next_time) => next_time, - Err(err) => panic!("{}", err), - }; + // In the case where bad user input results in an invalid next time, provide a valid time. + let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate).unwrap_or(i64::MAX); let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 From 303d0099661986609999a6cb55e006ed303a9bd0 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Tue, 9 Jul 2024 21:19:10 -0400 Subject: [PATCH 4/9] refactor: swap to unwrap_or to utilize duration --- src/append/rolling_file/policy/compound/trigger/time.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index efef2242..4405abbb 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -204,7 +204,8 @@ impl TimeTrigger { #[cfg(not(test))] let current = Local::now(); // In the case where bad user input results in an invalid next time, provide a valid time. - let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate).unwrap_or(i64::MAX); + let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate) + .unwrap_or(current + Duration::try_seconds(1_i64).unwrap()); let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 From 5faead751cec7c78ed3d0a3f3606f82c78c90c9c Mon Sep 17 00:00:00 2001 From: bconn98 Date: Tue, 9 Jul 2024 22:42:42 -0400 Subject: [PATCH 5/9] refactor: interval error message --- .../rolling_file/policy/compound/trigger/time.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 4405abbb..0a09ed88 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -74,15 +74,15 @@ impl Default for TimeTriggerInterval { #[derive(Debug, Error)] enum TimeTrigerIntervalError { - #[error("The 'Seconds' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + #[error("The 'Seconds' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] Second(TimeTriggerInterval), - #[error("The 'Minutes' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + #[error("The 'Minutes' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] Minute(TimeTriggerInterval), - #[error("The 'Hours' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + #[error("The 'Hours' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] Hour(TimeTriggerInterval), - #[error("The 'Days' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + #[error("The 'Days' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] Day(TimeTriggerInterval), - #[error("The 'Weeks' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{0:?}'")] + #[error("The 'Weeks' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] Week(TimeTriggerInterval), } From 83fd3ed0d42b216d46e74ea128c8d0ddf5f8ed75 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Wed, 10 Jul 2024 20:24:29 -0400 Subject: [PATCH 6/9] doc: reason that i64::MAX is appropriate --- src/append/rolling_file/policy/compound/trigger/time.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 0a09ed88..b8444fd9 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -210,7 +210,9 @@ impl TimeTrigger { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 // and we're providing a known valid value. We can trust the Option will always return - // us a valid number. + // us a valid number. It's acceptable to use i64::MAX here because it is replacement of + // a randomly generated value. If a user enters a value greater than i64::MAX, then + // i64::MAX is an acceptable value. next_time + Duration::try_seconds(random_delay as i64) .unwrap_or(Duration::try_milliseconds(i64::MAX).unwrap()) From f6bc1dac5b3df4b056d7443b2242b15774b244b3 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Fri, 12 Jul 2024 21:42:32 -0400 Subject: [PATCH 7/9] chore: cleaner error handling with Result --- .../policy/compound/trigger/time.rs | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index b8444fd9..337b8e16 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -73,17 +73,9 @@ impl Default for TimeTriggerInterval { } #[derive(Debug, Error)] -enum TimeTrigerIntervalError { - #[error("The 'Seconds' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] - Second(TimeTriggerInterval), - #[error("The 'Minutes' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] - Minute(TimeTriggerInterval), - #[error("The 'Hours' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] - Hour(TimeTriggerInterval), - #[error("The 'Days' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] - Day(TimeTriggerInterval), - #[error("The 'Weeks' time trigger interval value is out of bounds, ensure it fits within an i64: : '{0:?}'")] - Week(TimeTriggerInterval), +enum TimeTriggerIntervalError { + #[error("The '{0}' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{1:?}'")] + OutOfBounds(String, TimeTriggerInterval), } #[cfg(feature = "config_parsing")] @@ -188,7 +180,7 @@ impl<'de> serde::Deserialize<'de> for TimeTriggerInterval { impl TimeTrigger { /// Returns a new trigger which rolls the log once it has passed the /// specified time. - pub fn new(config: TimeTriggerConfig) -> TimeTrigger { + pub fn new(config: TimeTriggerConfig) -> anyhow::Result { #[cfg(test)] let current = { let now: std::time::Duration = SystemTime::now() @@ -203,9 +195,8 @@ impl TimeTrigger { #[cfg(not(test))] let current = Local::now(); - // In the case where bad user input results in an invalid next time, provide a valid time. - let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate) - .unwrap_or(current + Duration::try_seconds(1_i64).unwrap()); + // TODO: Design an implement error handling at a project level scope instead of panic + let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate)?; let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 @@ -220,17 +211,17 @@ impl TimeTrigger { next_time }; - TimeTrigger { + Ok(TimeTrigger { config, next_roll_time: RwLock::new(next_roll_time), - } + }) } fn get_next_time( current: DateTime, interval: TimeTriggerInterval, modulate: bool, - ) -> Result, TimeTrigerIntervalError> { + ) -> Result, TimeTriggerIntervalError> { let year = current.year(); if let TimeTriggerInterval::Year(n) = interval { let n = n as i32; @@ -259,9 +250,11 @@ impl TimeTrigger { let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - week0 % n } else { n }; - let dur = Duration::try_weeks(increment) - .ok_or(TimeTrigerIntervalError::Week(interval))? - - Duration::try_days(weekday).ok_or(TimeTrigerIntervalError::Day(interval))?; + let dur = Duration::try_weeks(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Weeks".to_owned(), interval), + )? - Duration::try_days(weekday).ok_or( + TimeTriggerIntervalError::OutOfBounds("Days".to_owned(), interval), + )?; return Ok(time + dur); } @@ -269,8 +262,9 @@ impl TimeTrigger { let ordinal0 = current.ordinal0() as i64; let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - ordinal0 % n } else { n }; - let dur = - Duration::try_days(increment).ok_or(TimeTrigerIntervalError::Day(interval))?; + let dur = Duration::try_days(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Days".to_owned(), interval), + )?; return Ok(time + dur); } @@ -280,8 +274,9 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, 0, 0) .unwrap(); let increment = if modulate { n - (hour as i64) % n } else { n }; - let dur = - Duration::try_hours(increment).ok_or(TimeTrigerIntervalError::Hour(interval))?; + let dur = Duration::try_hours(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Hours".to_owned(), interval), + )?; return Ok(time + dur); } @@ -291,8 +286,9 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, 0) .unwrap(); let increment = if modulate { n - (min as i64) % n } else { n }; - let dur = Duration::try_minutes(increment) - .ok_or(TimeTrigerIntervalError::Minute(interval))?; + let dur = Duration::try_minutes(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Minutes".to_owned(), interval), + )?; return Ok(time + dur); } @@ -302,8 +298,9 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, sec) .unwrap(); let increment = if modulate { n - (sec as i64) % n } else { n }; - let dur = Duration::try_seconds(increment) - .ok_or(TimeTrigerIntervalError::Second(interval))?; + let dur = Duration::try_seconds(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Seconds".to_owned(), interval), + )?; return Ok(time + dur); } panic!("Should not reach here!"); @@ -329,7 +326,7 @@ impl Trigger for TimeTrigger { let mut next_roll_time = self.next_roll_time.write().unwrap(); let is_trigger = current >= *next_roll_time; if is_trigger { - let tmp = TimeTrigger::new(self.config); + let tmp = TimeTrigger::new(self.config)?; let time_new = tmp.next_roll_time.read().unwrap(); *next_roll_time = *time_new; } @@ -368,7 +365,7 @@ impl Deserialize for TimeTriggerDeserializer { config: TimeTriggerConfig, _: &Deserializers, ) -> anyhow::Result> { - Ok(Box::new(TimeTrigger::new(config))) + Ok(Box::new(TimeTrigger::new(config)?)) } } @@ -396,7 +393,7 @@ mod test { max_random_delay: 0, }; - let trigger = TimeTrigger::new(config); + let trigger = TimeTrigger::new(config).unwrap(); MockClock::advance_system_time(Duration::from_millis(millis / 2)); let result1 = trigger.trigger(&logfile).unwrap(); @@ -526,6 +523,18 @@ mod test { max_random_delay: 0, }; let trigger = TimeTrigger::new(config); - assert!(trigger.is_pre_process()); + assert!(trigger.unwrap().is_pre_process()); + } + + #[test] + fn test_err() { + let config = TimeTriggerConfig { + interval: TimeTriggerInterval::Day(i64::MAX), + modulate: false, + max_random_delay: 0, + }; + + let trigger = TimeTrigger::new(config); + assert!(trigger.is_err()); } } From cf7f162836d374cf10e1d4c2761df0d8d3d02d11 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Fri, 12 Jul 2024 21:50:43 -0400 Subject: [PATCH 8/9] ci: bump codecov to v4 and set the CODECOV token from the secrets --- .github/workflows/coverage.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 2ca0f327..29e132fe 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -30,6 +30,8 @@ jobs: USER: "test-user" - name: Upload to codecov.io - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: fail_ci_if_error: true + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} From 9acd37304f73e2267b2d69b41936dd43d68ff595 Mon Sep 17 00:00:00 2001 From: bconn98 Date: Sat, 13 Jul 2024 13:42:47 -0400 Subject: [PATCH 9/9] chore: unwrap safe try_days call --- .../rolling_file/policy/compound/trigger/time.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 337b8e16..4d2f931a 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -247,14 +247,15 @@ impl TimeTrigger { let day = current.day(); if let TimeTriggerInterval::Week(n) = interval { let week0 = current.iso_week().week0() as i64; - let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - week0 % n } else { n }; + + // Unwrap instead of propogate the try_days call as this is a known safe value + // generated by the chrono library as a value between 0-6. + let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let dur = Duration::try_weeks(increment).ok_or( TimeTriggerIntervalError::OutOfBounds("Weeks".to_owned(), interval), - )? - Duration::try_days(weekday).ok_or( - TimeTriggerIntervalError::OutOfBounds("Days".to_owned(), interval), - )?; + )? - Duration::try_days(weekday).unwrap(); return Ok(time + dur); } @@ -527,7 +528,7 @@ mod test { } #[test] - fn test_err() { + fn test_days_overflow() { let config = TimeTriggerConfig { interval: TimeTriggerInterval::Day(i64::MAX), modulate: false,