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

fix: replace deprecated API in chrono 0.4.35 for converting to time #367

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
63 changes: 48 additions & 15 deletions src/append/rolling_file/policy/compound/trigger/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -13,6 +11,7 @@ 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")]
Expand Down Expand Up @@ -73,6 +72,20 @@ 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),
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides, I remember now why I put a specific number here before. The max value of these is not i64::MAX. They should be within i64::MAX after they convert to milliseconds.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action worthy?

#[cfg(feature = "config_parsing")]
impl<'de> serde::Deserialize<'de> for TimeTriggerInterval {
fn deserialize<D>(d: D) -> Result<Self, D::Error>
Expand Down Expand Up @@ -181,18 +194,26 @@ 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);
// 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());
bconn98 marked this conversation as resolved.
Show resolved Hide resolved
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)
// 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())
bconn98 marked this conversation as resolved.
Show resolved Hide resolved
} else {
next_time
};
Expand All @@ -207,13 +228,13 @@ impl TimeTrigger {
current: DateTime<Local>,
interval: TimeTriggerInterval,
modulate: bool,
) -> DateTime<Local> {
) -> Result<DateTime<Local>, TimeTrigerIntervalError> {
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 {
Expand All @@ -224,9 +245,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();
Expand All @@ -236,14 +257,19 @@ 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 = Duration::try_weeks(increment)
.ok_or(TimeTrigerIntervalError::Week(interval))?
- Duration::try_days(weekday).ok_or(TimeTrigerIntervalError::Day(interval))?;
bconn98 marked this conversation as resolved.
Show resolved Hide resolved
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 =
Duration::try_days(increment).ok_or(TimeTrigerIntervalError::Day(interval))?;
return Ok(time + dur);
}

let hour = current.hour();
Expand All @@ -252,7 +278,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 };
return time + Duration::hours(increment);
let dur =
Duration::try_hours(increment).ok_or(TimeTrigerIntervalError::Hour(interval))?;
return Ok(time + dur);
}

let min = current.minute();
Expand All @@ -261,7 +289,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 };
return time + Duration::minutes(increment);
let dur = Duration::try_minutes(increment)
.ok_or(TimeTrigerIntervalError::Minute(interval))?;
return Ok(time + dur);
}

let sec = current.second();
Expand All @@ -270,7 +300,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 };
return time + Duration::seconds(increment);
let dur = Duration::try_seconds(increment)
.ok_or(TimeTrigerIntervalError::Second(interval))?;
return Ok(time + dur);
}
panic!("Should not reach here!");
}
Expand All @@ -283,8 +315,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()
};
Expand Down
Loading