bug: RRULE Timezone must be in UTC format - ends in Z

[a-nicholls]

Describe the bug

I've shared a calendar from Office 365 that shares out the ics link (downloaded file below)
but running with test code reports RRULE UNTIL values must be specified in UTC when DTSTART is timezone-aware

From what I've read the UTC time format can include a trailing Z to indicate UTC time?

If I remove the Z from the RRULE UTIL value it shows the dates but with incorrect changes - I'm guessing this is because the date is not in same time zone UTC +1?)

**start 2020-05-28 duration 1 day, 0:00:00** (shouldn't be there)
start 2020-06-11 duration 1 day, 0:00:00
start 2020-06-25 duration 1 day, 0:00:00
start 2020-07-09 duration 1 day, 0:00:00
start 2020-07-23 duration 1 day, 0:00:00
start 2020-08-06 duration 1 day, 0:00:00
start 2020-08-20 duration 1 day, 0:00:00
**start 2020-09-03 duration 1 day, 0:00:00**(shouldn't be there)
start 2020-06-04 duration 1 day, 0:00:00
start 2020-06-18 duration 1 day, 0:00:00
start 2020-07-02 duration 1 day, 0:00:00
start 2020-07-16 duration 1 day, 0:00:00
start 2020-07-30 duration 1 day, 0:00:00
start 2020-08-13 duration 1 day, 0:00:00
start 2020-08-27 duration 1 day, 0:00:00
start 2020-05-29 duration 1 day, 0:00:00
start 2020-09-04 duration 1 day, 0:00:00

To Reproduce

import icalendar
import recurring_ical_events
import urllib.request

start_date = (2020, 5, 25)
end_date =   (2020, 9, 5)

file = open("test.ics", "rb") 
ical_string = file.read() 
calendar = icalendar.Calendar.from_ical(ical_string)
events = recurring_ical_events.of(calendar).between(start_date, end_date)
for event in events:
    start = event["DTSTART"].dt
    duration = event["DTEND"].dt - event["DTSTART"].dt
    print("start {} duration {}".format(start, duration))

ICS file

test.ics.txt

Expected behavior

I expect it to not show two of the dates (2020-05-28 and 2020-09-03)

Console output

Traceback (most recent call last):
  File "/icstesting/ENV/lib/python3.8/site-packages/recurring_ical_events.py", line 173, in create_rule_with_start
    return rrulestr(rule_string, dtstart=start)
  File "/icstesting/ENV/lib/python3.8/site-packages/dateutil/rrule.py", line 1730, in __call__
    return self._parse_rfc(s, **kwargs)
  File "/icstesting/ENV/lib/python3.8/site-packages/dateutil/rrule.py", line 1650, in _parse_rfc
    return self._parse_rfc_rrule(lines[0], cache=cache,
  File "/icstesting/ENV/lib/python3.8/site-packages/dateutil/rrule.py", line 1559, in _parse_rfc_rrule
    return rrule(dtstart=dtstart, cache=cache, **rrkwargs)
  File "/icstesting/ENV/lib/python3.8/site-packages/dateutil/rrule.py", line 468, in __init__
    raise ValueError(
ValueError: RRULE UNTIL values must be specified in UTC when DTSTART is timezone-aware

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test.py", line 15, in <module>
    events = recurring_ical_events.of(calendar).between(start_date, end_date)
  File "/icstesting/ENV/lib/python3.8/site-packages/recurring_ical_events.py", line 334, in between
    repetitions = self.RepeatedEvent(event, span_start)
  File "/icstesting/ENV/lib/python3.8/site-packages/recurring_ical_events.py", line 155, in __init__
    rule.rrule(self.create_rule_with_start(_rule.to_ical().decode(), self.start))
  File "/icstesting/ENV/lib/python3.8/site-packages/recurring_ical_events.py", line 178, in create_rule_with_start
    assert self.start.tzinfo is not None, "we assume the start is timezone aware but the until value is not" # see the comment above
AssertionError: we assume the start is timezone aware but the until value is not

Version:

Additional context

I'm new to python and pull requests - but happy to have a go at putting in a test

Suggested implementation

• add an ICS file, example:
  

  python-recurring-ical-events/test/calendars/issue-15-duplicated-events.ics

  Line 1 in f4a90b2

  BEGIN:VCALENDAR

• add a test, example:
  

  python-recurring-ical-events/test/test_issue_15.py

  Line 21 in f4a90b2

  def test_sequence_is_not_present(calendars):

• implement the test
• fix the test

---

We're using Polar.sh so you can upvote and help fund this issue. We receive the funding once the issue is completed & confirmed by you. Thank you in advance for helping prioritize & fund our work.

[a-nicholls]

on my limited testing - I think this maybe a potential issue with the python-dateutil module 2.8.1
dateutil/dateutil#1006

[niccokunzmann]

Hi and thanks! I somewhere read that still it might be good to test your dependencies. Since we created an issue here, we can still write a test which fails so that other people can download the test suite to their computer and run it to see if the dependencies are up-to-date. In a way - I also heard that this was the old open-source way - that code you include is code you are responsible for. On the other hand: This module includes dependencies which are installed with it. Might it be that we should change something about how the dependencies are specified in the requirements.txt file? What are your thoughts?

[a-nicholls]

not sure what you mean - new to this (as actually just user not developer of someone elses component that called this module!) , but are you referring that the requirements.txt should have a reference to python-dateutil to be at least 2.8.1 to test?

I tried to write a test script but can't figure out what I'm supposed to do in the event that another module raises the error other than it marking as an expected failure - but that would be until potentially it gets fixed in which case the test would need to be updated to remove the expected failure?

I tried something like this with the associated ics file which tests that it should have 15 not 17 events (the exceptions) which I can test if I manually remove the Z's and will correctly fail and hopefully mark it as skipping if it raises any ValueError?

import pytest

@pytest.mark.xfail(raises=ValueError)

def test_expectedamountofevents(calendars):
    events = calendars.issue_28_rrule_with_UTC_endinginZ.between((2020, 5, 25),(2020, 9, 5))
    assert len(events) == 15

[niccokunzmann]

@a-nicholls Thanks for providing so much information. I added your file and the test. Let's see how this can be fixed.
834c441

[niccokunzmann]

When I update the dateutil module to version 2.2, I get another error.

$ pip install --upgrade py-dateutil
Collecting py-dateutil
  Downloading py-dateutil-2.2.tar.gz (64 kB)
     |████████████████████████████████| 64 kB 75 kB/s 
Requirement already satisfied, skipping upgrade: six in ./ENV/lib/python3.8/site-packages (from py-dateutil) (1.14.0)
Building wheels for collected packages: py-dateutil
  Building wheel for py-dateutil (setup.py) ... done
  Created wheel for py-dateutil: filename=py_dateutil-2.2-py3-none-any.whl size=34707 sha256=1c14e086dc827e207ceebcfb107de22087caab430264b9f9e3295d30ca9025a1
  Stored in directory: /home/sabin/.cache/pip/wheels/68/10/a2/7c4fddcbdb77c09622a08eef4a6a93168dab9341edd48f7819
Successfully built py-dateutil
Installing collected packages: py-dateutil
Successfully installed py-dateutil-2.2

$ pytest
TypeError: can't compare offset-naive and offset-aware datetimes

This error occurs in your test and in the test of Issue 4.
So, I wonder, how to approach it.

[niccokunzmann]

@a-nicholls I added code and now the test fails in the number of expected events.

test/test_issue_28_timezone_with_z.py FF                                                                                       [100%]

============================================================== FAILURES ==============================================================
_____________________________________________ test_expected_amount_of_events[Calendars] ______________________________________________

calendars = <conftest.Calendars object at 0x7f15167d6310>

    def test_expected_amount_of_events(calendars):
        events = calendars.issue_28_rrule_with_UTC_endinginZ.between((2020, 5, 25),(2020, 9, 5))
>       assert len(events) == 15
E       AssertionError: assert 17 == 15
E        +  where 17 = len([VEVENT({'DESCRIPTION': vText('b'\\n''), 'UID': vText('b'040000008200E00074C5B7101A82E00800000000017E1BADC42ED60100000...b'1''), 'X-MICROSOFT-DONOTFORWARDMEETING': vText('b'FALSE''), 'X-MICROSOFT-DISALLOW-COUNTER': vText('b'FALSE'')}), ...])

test/test_issue_28_timezone_with_z.py:9: AssertionError
_________________________________________ test_expected_amount_of_events[ReversedCalendars] __________________________________________

calendars = <conftest.ReversedCalendars object at 0x7f151680e9a0>

    def test_expected_amount_of_events(calendars):
        events = calendars.issue_28_rrule_with_UTC_endinginZ.between((2020, 5, 25),(2020, 9, 5))
>       assert len(events) == 15
E       AssertionError: assert 17 == 15
E        +  where 17 = len([VEVENT({'DESCRIPTION': vText('b'\\n''), 'UID': vText('b'040000008200E00074C5B7101A82E00800000000017E1BADC42ED60100000...b'1''), 'X-MICROSOFT-DONOTFORWARDMEETING': vText('b'FALSE''), 'X-MICROSOFT-DISALLOW-COUNTER': vText('b'FALSE'')}), ...])

test/test_issue_28_timezone_with_z.py:9: AssertionError
====================================================== 2 failed in 0.09 seconds ======================================================

What do you think how we should proceed?

[a-nicholls]

Yeah looks similar to the datetime module, the problem with local time being required to be in UTC and the exceptions are saved in UTC+1 (which I believe is correct) so there should be 17 items not 19.

The comparisons in my mind should be all converted to the same time to local time to figure out which exceptions have to be removed

[niccokunzmann]

This fix is released in Version v0.1.17

[a-nicholls]

Hi!

The test the I wrote was correct in that there are only 15 events not 17 as noted in original post, if you check the ics file,

The recurring event SUMMARY:Refuse black bin with UID

040000008200E00074C5B7101A82E00800000000017E1BADC42ED601000000000000000
 010000000FBF1FBAE2E9FBC4D81F16854E2F4D51B

There's two RECURRENCE-IDs referencing the same UID - haven't changed the SUMMARY so that's the same, but it says that in effect, 20200528T000000 (28th May) change that to DTSTART;VALUE=DATE:20200529 (29th May) and
RECURRENCE-ID;TZID=GMT Standard Time:20200903T000000 (3rd September)
change that to (keeping same SUMMARY)
DTSTART;VALUE=DATE:20200904 (4th September)

I've shown how the client shows May 2020 (note no 28th May, but there is something on 29th May)

[niccokunzmann]

Should we re-open this issue or open a new one since the test is wrong?

[a-nicholls]

actually the test written did do a test against 15 not 17, so the test file will need to be changed back and it makes sense as problem was mentioned also in issue to reopen this issue if that's ok?

[niccokunzmann]

@a-nicholls Thanks again for reporting... I could fix the tests and opened an issue for the broader problem of matching recurrence-ids. #36

[niccokunzmann]

Again, let me know if you have input!

[niccokunzmann]

This fix is included in v0.1.18b.

[a-nicholls]

looks like that did it! with the test program above, should the calling of recurring_ical_events.of(calendar).between(start_date, end_date) be returning it ordered in date order or is it the calling program that should sort it if it needs to?

[niccokunzmann]

I think, it is not supposed to be ordered. I never wrote tests for that. The reason: What should it be ordered by? Start, End, Duration, ...

[a-nicholls]

ah ok - apparently the module that uses this great library is doing the sort :) no problems :)