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

DST handling #18

Open
julianbrost opened this issue Apr 4, 2023 · 3 comments
Open

DST handling #18

julianbrost opened this issue Apr 4, 2023 · 3 comments

Comments

@julianbrost
Copy link
Collaborator

Even though github.com/teambition/rrule-go looked like the most promising library for evaluating rrules, it does not handle DST changes correctly.

For an event repeating daily at 02:30 in Europe/Berlin it generates a recurrence at 2023-03-26 03:30:00 (there is no 02:30 on that day in that timezone). That's incorrect according to the standard:

Recurrence rules may generate recurrence instances with an invalid date (e.g., February 30) or nonexistent local time (e.g., 1:30 AM on a day where the local time is moved forward by an hour at 1:00 AM). Such recurrence instances MUST be ignored and MUST NOT be counted as part of the recurrence set.

Example:

package main

import (
	"fmt"
	"github.com/teambition/rrule-go"
	"time"
)

func main() {
	rule, err := rrule.NewRRule(rrule.ROption{
		Freq:    rrule.DAILY,
		Dtstart: parse("2023-01-01 02:30:00"),
	})
	if err != nil {
		panic(err)
	}
	for _, t := range rule.Between(parse("2023-03-25 00:00:00"), parse("2023-03-28 00:00:00"), true) {
		fmt.Println(t)
	}
}

func parse(s string) time.Time {
	loc, err := time.LoadLocation("Europe/Berlin")
	if err != nil {
		panic(err)
	}

	t, err := time.ParseInLocation(time.DateTime, s, loc)
	if err != nil {
		panic(err)
	}
	return t
}

Incorrect output (the second line should not exist):

2023-03-25 02:30:00 +0100 CET
2023-03-26 03:30:00 +0200 CEST
2023-03-27 02:30:00 +0200 CEST
@julianbrost julianbrost self-assigned this Sep 14, 2023
@julianbrost
Copy link
Collaborator Author

There's actually an errata regarding the very specific section I quoted, it should actually say:

Recurrence rules may generate recurrence instances with a nonexistent local time ((e.g., 1:30 AM on a day where the local time is moved forward by an hour at 1:00 AM). Such recurrence instances are handled as specified in Section 3.3.5.

And that referenced section says:

If, based on the definition of the referenced time zone, the local time described occurs more than once (when changing from daylight to standard time), the DATE-TIME value refers to the first occurrence of the referenced time. Thus, TZID=America/New_York:20071104T013000 indicates November 4, 2007 at 1:30 A.M. EDT (UTC-04:00). If the local time described does not occur (when changing from standard to daylight time), the DATE-TIME value is interpreted using the UTC offset before the gap in local times. Thus, TZID=America/New_York:20070311T023000 indicates March 11, 2007 at 3:30 A.M. EDT (UTC-04:00), one hour after 1:30 A.M. EST (UTC-05:00).

@julianbrost
Copy link
Collaborator Author

Testing both transitions (jumping forward an jumping backward):

package main

import (
	"fmt"
	"github.com/teambition/rrule-go"
	"time"
)

func main() {
	rule, err := rrule.NewRRule(rrule.ROption{
		Freq:    rrule.DAILY,
		Dtstart: parse("2023-01-01 02:30:00"),
	})
	if err != nil {
		panic(err)
	}
	for _, t := range rule.Between(parse("2023-03-25 00:00:00"), parse("2023-03-28 00:00:00"), true) {
		fmt.Println(t)
	}
	fmt.Println()
	for _, t := range rule.Between(parse("2023-10-28 00:00:00"), parse("2023-10-31 00:00:00"), true) {
		fmt.Println(t)
	}
}

func parse(s string) time.Time {
	loc, err := time.LoadLocation("Europe/Berlin")
	if err != nil {
		panic(err)
	}

	t, err := time.ParseInLocation(time.DateTime, s, loc)
	if err != nil {
		panic(err)
	}
	return t
}
2023-03-25 02:30:00 +0100 CET
2023-03-26 03:30:00 +0200 CEST
2023-03-27 02:30:00 +0200 CEST

2023-10-28 02:30:00 +0200 CEST
2023-10-29 02:30:00 +0100 CET
2023-10-30 02:30:00 +0100 CET

First one should be correct based on my understanding. However, the second one should be 2023-10-29 02:30:00 +0200 CEST because that's the first occurrence of 2:30.

Also, the library seems to rely on time.Date(), which does not guarantee the occurrence that will be returned:

In such cases, the choice of time zone, and therefore the time, is not well-defined. Date returns a time that is correct in one of the two zones involved in the transition, but it does not guarantee which.

@julianbrost
Copy link
Collaborator Author

Also, with more frequent repetitions than FREQ=DAILY, there are even more edge cases (FREQ=HOURLY can have 23, 24 or maybe 25 recurrences on the same day (I'm not entirely sure if the standard would allow 25)). Anyways, most calendar software doesn't even allow specifying that in the GUI (I've tested Thunderbird, Google/Android, Nextcloud, Outlook Web), so restricting us to daily or less frequently might be reasonable.

@julianbrost julianbrost removed their assignment Sep 15, 2023
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