-
Notifications
You must be signed in to change notification settings - Fork 248
Refactor MatchTimeZone method in RecurrencePatternEvaluator
#664
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,242 @@ | ||
| // | ||
| // Copyright ical.net project maintainers and contributors. | ||
| // Licensed under the MIT license. | ||
| // | ||
|
|
||
| using System; | ||
| using System.Linq; | ||
| using Ical.Net.DataTypes; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace Ical.Net.Tests; | ||
|
|
||
| [TestFixture] | ||
| public class MatchTimeZoneTests | ||
| { | ||
| [Test, Category("Recurrence")] | ||
| public void MatchTimeZone_LocalTimeUsaWithTimeZone() | ||
| { | ||
| // DTSTART with local time and time zone reference (negative offset), UNTIL as UTC | ||
| const string ical = | ||
| """ | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example1 | ||
| SUMMARY:Event with local time and time zone | ||
| DTSTART;TZID=America/New_York:20231101T090000 | ||
| RRULE:FREQ=DAILY;UNTIL=20231105T130000Z | ||
| DTEND;TZID=America/New_York:20231101T100000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = new DateTime(2023, 11, 05, 13, 00, 00, DateTimeKind.Utc); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2023, 11, 01), new CalDateTime(2023, 11, 06)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(4)); | ||
| /* | ||
| Should have 4 occurrences: | ||
| November 1, 2023: 09:00 AM - 10:00 AM (UTC-0400) (America/New_York) | ||
| November 2, 2023: 09:00 AM - 10:00 AM (UTC-0400) (America/New_York) | ||
| November 3, 2023: 09:00 AM - 10:00 AM (UTC-0400) (America/New_York) | ||
| November 4, 2023: 09:00 AM - 10:00 AM (UTC-0400) (America/New_York) | ||
| November 5, 2023: 09:00 AM - 10:00 AM (UTC-0500) (America/New_York) | ||
| must NOT be included, because 20231105T130000Z => November 5, 2023: 08:00 AM (America/New_York) | ||
| (Daylight Saving Time in America/New_York ended on Sunday, November 5, 2023, at 2:00 AM) | ||
| */ | ||
| }); | ||
| } | ||
|
|
||
| [Test, Category("Recurrence")] | ||
| [TestCase("20241005T140000Z", 5)] | ||
| [TestCase("20241005T150000Z", 6)] | ||
| public void MatchTimeZone_LocalTimeAustraliaWithTimeZone(string inputUntil, int expectedOccurrences) | ||
| { | ||
| // DTSTART with local time and time zone reference (positive offset), UNTIL as UTC | ||
| var ical = | ||
| $""" | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example1 | ||
| SUMMARY:Event with local time and time zone | ||
| DTSTART;TZID=Australia/Sydney:20241001T010000 | ||
| RRULE:FREQ=DAILY;UNTIL={inputUntil} | ||
| DTEND;TZID=Australia/Sydney:20241001T020000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = DateTime.ParseExact(inputUntil, "yyyyMMddTHHmmssZ", | ||
| System.Globalization.CultureInfo.InvariantCulture, | ||
| System.Globalization.DateTimeStyles.AssumeUniversal | | ||
| System.Globalization.DateTimeStyles.AdjustToUniversal); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2024, 10, 01), new CalDateTime(2024, 10, 07)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(expectedOccurrences)); | ||
| /* | ||
| Should have 5 occurrences with UNTIL=20241005T140000Z... | ||
| October 1, 2024: 01:00 AM - 02:00 AM (UTC+1000) (Australia/Sydney) | ||
| October 2, 2024: 01:00 AM - 02:00 AM (UTC+1000) (Australia/Sydney) | ||
| October 3, 2024: 01:00 AM - 02:00 AM (UTC+1000) (Australia/Sydney) | ||
| October 4, 2024: 01:00 AM - 02:00 AM (UTC+1000) (Australia/Sydney) | ||
| October 5, 2024: 01:00 AM - 02:00 AM (UTC+1000) (Australia/Sydney) | ||
| ... and 6 occurrences with UNTIL=20241005T150000Z, i.e. plus one more | ||
| October 6, 2024: 01:00 AM - 02:00 AM (UTC+1100) (Australia/Sydney) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, actually I think, this should be included, because
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the conclusion came from the description I assume. It was not updated.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was looking at the comments. But now reconsidering the test, I still think its not correct. October 6, 2024: 01:00 AM Sydney time = 20241005T140000Z, so it should be included in the first test and the number of occurrences should therefore be 6 in both cases.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't UTC Time: 2024-10-05 14:00:00 => Sydney Time (AEST, UTC+10:00): 2024-10-06 00:00:00?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yes, of course, sorry for the confusion! |
||
| (Daylight Saving Time in Australia/Sydney starts on Sunday, October 6, 2024, at 2:00 AM) | ||
| */ | ||
| }); | ||
| } | ||
|
|
||
| [Test, Category("Recurrence")] | ||
| public void MatchTimeZone_UTCTime() | ||
| { | ||
| // DTSTART and UNTIL with UTC time | ||
| const string ical = | ||
| """ | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example2 | ||
| SUMMARY:Event with UTC time | ||
| DTSTART:20231101T090000Z | ||
| RRULE:FREQ=DAILY;UNTIL=20231105T090000Z | ||
| DTEND:20231101T100000Z | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = new DateTime(2023, 11, 05, 09, 00, 00, DateTimeKind.Utc); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2023, 11, 01), new CalDateTime(2023, 11, 06)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(5)); | ||
| }); | ||
| } | ||
|
|
||
| [Test, Category("Recurrence")] | ||
| public void MatchTimeZone_FloatingTime() | ||
| { | ||
| // DTSTART AND UNTIL with floating time | ||
| const string ical = | ||
| """ | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example3 | ||
| SUMMARY:Event with floating time | ||
| DTSTART:20231101T090000 | ||
| RRULE:FREQ=DAILY;UNTIL=20231105T090000 | ||
| DTEND:20231101T100000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = new DateTime(2023, 11, 05, 09, 00, 00, DateTimeKind.Unspecified); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2023, 11, 01), new CalDateTime(2023, 11, 06)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(5)); | ||
| }); | ||
|
|
||
| } | ||
|
|
||
| [Test, Category("Recurrence")] | ||
| public void MatchTimeZone_LocalTimeNoTimeZone() | ||
| { | ||
| // DTSTART with local time and no time zone reference | ||
| const string ical = | ||
| """ | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example4 | ||
| SUMMARY:Event with local time and no time zone reference | ||
| DTSTART:20231101T090000 | ||
| RRULE:FREQ=DAILY;UNTIL=20231105T090000 | ||
| DTEND:20231101T100000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = new DateTime(2023, 11, 05, 09, 00, 00, DateTimeKind.Unspecified); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2023, 11, 01), new CalDateTime(2023, 11, 06)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(5)); | ||
| }); | ||
| } | ||
|
|
||
| [Test, Category("Recurrence")] | ||
| public void MatchTimeZone_DateOnly() | ||
| { | ||
| // DTSTART with date-only value | ||
| const string ical = | ||
| """ | ||
| BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| PRODID:-//Example Corp//NONSGML Event//EN | ||
| BEGIN:VEVENT | ||
| UID:example5 | ||
| SUMMARY:Event with date-only value | ||
| DTSTART;VALUE=DATE:20231101 | ||
| RRULE:FREQ=DAILY;UNTIL=20231105 | ||
| DTEND;VALUE=DATE:20231102 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """; | ||
|
|
||
| var calendar = Calendar.Load(ical); | ||
| var evt = calendar.Events.First(); | ||
| var until = evt.RecurrenceRules.First().Until; | ||
|
|
||
| var expectedUntil = new DateTime(2023, 11, 05, 00, 00, 00, DateTimeKind.Unspecified); | ||
| var occurrences = evt.GetOccurrences(new CalDateTime(2023, 11, 01), new CalDateTime(2023, 11, 06)); | ||
|
|
||
| Assert.Multiple(() => | ||
| { | ||
| Assert.That(until, Is.EqualTo(expectedUntil)); | ||
| Assert.That(occurrences.Count, Is.EqualTo(5)); | ||
| }); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case DTSTART has a tzid but the startTime is floating. This is quite a special case and we should considere to disallow the case. Anyhow, I think it would be preferable to specify a tzid for the start/endTime too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, CalDateTime(2024, 10, 01) resolves to a DATE value, which cannot have a timezone (RFC 5545)?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you mean, it should always be a zoned DATE-TIME?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment, not too relevant. Generally it would be advisable for users of the app, not to mix DATE vs DATE-TIME and floating vs non-floating, because there's always some ambiguity that causes additional complexity. In this case it really doesn't matter, so no need to change it. But as its advisable for users, not to mix it, we generally might want to avoid mixing them in the tests either (except for those cases where we explicitly want to test mixing them).