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

Daylight savings time seems ignored when traversing a RRuleSet #109

Open
nospam2624 opened this issue Apr 2, 2024 · 0 comments
Open

Daylight savings time seems ignored when traversing a RRuleSet #109

nospam2624 opened this issue Apr 2, 2024 · 0 comments

Comments

@nospam2624
Copy link

TL;DR: With timezones there be dragons, and there might be more than one bug here. I originally summarized as

rust-rrule should possibly refuse to operate on timezones like CET, as it already does with CEST

when I thought the ticket should be titled:

CEST is considered an invalid timezone, while CET is valid

I have since realized that it would both be a silly suggestion and the wrong fix. One probably needs to read on. There is no TL;DR, apart from the title.

Note: I found a solution to my initial problem of having to use CEST rather than the IANA time zone name will digging into this, but details are still fishy. Thus some things are striked-out (like-this) or commented out (// in code), in hope that makes both my initial thinking and the remaining problem easy to follow.

Lets start with an example, by saying I wish to have lunch at 11:45 all year round, in local time regardless of daylight savings time. To express that with a RRuleSet, first create a project:

cargo init rrule-dst-bug; cd rrule-dst-bug
cargo add chrono iana-time-zone rrule tz-rs

Then replace src/main.rs with:

#[cfg(test)]
mod tests {
    use rrule::{RRuleError, RRuleSet};
    type TestResult = Result<(), RRuleError>;

    const DT: &str = "19700101T104500"; // January, winter in many timezones.
    const RRULE: &str = "RRULE:FREQ=DAILY";

    #[test]
    fn normal_time() -> TestResult {
        let tz_name = "CET";
        let ics = format!("DTSTART;TZID={}:{}\n{}", tz_name, DT, RRULE);
        ics.parse::<RRuleSet>().map(|_| ())
    }

    #[test]
    fn daylight_saving_time() -> TestResult {
        let tz_name = "CEST";
        let ics = format!("DTSTART;TZID={}:{}\n{}", tz_name, DT, RRULE);
        ics.parse::<RRuleSet>().map(|_| ())
    }

    #[test]
    fn resolved_timezone() -> TestResult {
        // let tz = tz::TimeZone::local().unwrap();
        // let tz_name = tz.find_current_local_time_type().unwrap()
        //     .time_zone_designation();
        // The above returns something like CET or CEST depending on which date
        // is it executed on, the below always gives a string like Europe/Copenhagen.
        let tz_name = iana_time_zone::get_timezone().unwrap();
        let ics = format!("DTSTART;TZID={}:{}\n{}", tz_name, DT, RRULE);
        ics.parse::<RRuleSet>().map(|_| ())
    }
}

Run cargo test and see that the juicy failure is:

Error: ParserError(InvalidTimezone("CEST"))

It might be fully as expected that CEST should not be valid… Adding support for CEST is likely a minor part towards a full fix, read on.

Indeed I would prefer to be able to resolve my timezone to the IANA format, but how do one even get the name of the local time zone in Rust? and now I have learned how to. Yet still there is either a bug or a grave misconception on my part. The latter is of course highly likely, even if I have read 3.8.5.3 Recurrence Rule of rfc5545.

Lets try adding the below code for a few more test-cases:

    use chrono::{DateTime,Days,Local,TimeZone,Utc};

    fn lunch_dt_in_days(days: usize) -> DateTime<Local> {
        //! Avoid repeating boilerplate the to seasonal test-cases.
        let tz_name = "Europe/Amsterdam";
        let ics = format!("DTSTART;TZID={}:{}\n{}", tz_name, DT, RRULE);
        let rrule_set = ics.parse::<RRuleSet>().unwrap();
        let rrule_lunch_utc = rrule_set.into_iter().nth(days).unwrap().naive_utc();
        Local.from_utc_datetime(&rrule_lunch_utc)
    }

    #[test]
    fn winter_lunch() {
        let rrule_lunch = lunch_dt_in_days(0); // First day of year. Winter.
        let desired_lunch = Local.with_ymd_and_hms(1970, 1, 1, 11, 45, 0).unwrap();
        assert_eq!(rrule_lunch, desired_lunch);
    }

    #[test]
    fn summer_lunch() {
        let rrule_lunch = lunch_dt_in_days(365/2); // Half-way through year. Summer.
        let desired_lunch_utc = Utc.with_ymd_and_hms(1970, 1, 1, 9, 45, 0).unwrap()
            .naive_utc().checked_add_days(Days::new(365/2)).unwrap();
        let desired_lunch = Local.from_utc_datetime(&desired_lunch_utc);
        assert_eq!(rrule_lunch, desired_lunch);
    }

    #[test]
    fn oslo_has_dst() {
        let tz_name = "Europe/Oslo"; // Føler du deg som hjemme, @fmeringdal?
        let ics = format!("DTSTART;TZID={}:{}\n{}", tz_name, DT, RRULE);
        let rrule_set = ics.parse::<RRuleSet>().unwrap();
        let lunch_summer_utc = rrule_set.into_iter().nth(365/2).unwrap().naive_utc();
        let lunch_summer = Local.from_utc_datetime(&lunch_summer_utc);
        let utc_offset = lunch_summer.offset();
        let desired_offset = chrono::offset::FixedOffset::east_opt(2*3600).unwrap();
        assert_eq!(utc_offset, &desired_offset);
    }

Running these new three test-cases is unsettling:

---- tests::summer_lunch stdout ----
thread 'tests::summer_lunch' panicked at src/main.rs:61:9:
assertion left == right failed
left: 1970-07-02T11:45:00+01:00
right: 1970-07-02T10:45:00+01:00

---- tests::oslo_has_dst stdout ----
thread 'tests::oslo_has_dst' panicked at src/main.rs:73:9:
assertion left == right failed
left: +01:00
right: +02:00

It is all well and good that winter_lunch() pass, but the other two both fail due to the RRuleSet being stuck with the original offset of the timezone. I've unfortunately stopped myself prior to attemping to provide a patch. It's been a bit of a rabbit-hole-dive even without that. Trying to suggest a way forward without knowing the codebase, I guess that one possible approach would be to replace all the fileds on the internal representation DateTimeIter with a single pub NaiveDateTime always set to UTC, keeping all in- and out-function signatures as is and thus deferring the timezone conversions to chrono. I realize that would be quite an invasive change.

No issue is complete without mentioning: Tested with v0.11.0 on FreeBSD, illumos and Debian.

This bug was filed after a brief conversation in #24. Thanks @WhyNotHugo for that initial answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant