Issue with event expansion

[dozed]

_expand expects start and end as datetime which leads to an issue with dateutil's rruleset.between(start, end) with the following iCalendar event:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//foo//bar//EN
BEGIN:VEVENT
UID:fe1da58e-0555-11ef-9770-58ce2a14e2e5
DTSTART;VALUE=DATE:20240408
DTEND;VALUE=DATE:20240409
DTSTAMP:20240428T115356Z
RRULE:FREQ=WEEKLY;INTERVAL=1
SEQUENCE:1
SUMMARY:Test
END:VEVENT
END:VCALENDAR

Radicale logs this error:

[ERROR] An exception occurred during REPORT request on '/test/c59e3c20-4436-8258-9d21-ff25649c7290/': can't compare offset-naive and offset-aware datetimes

For a minimal test case see here: https://github.com/dozed/recur-issue-1/blob/main/test_recur_issue.py#L43-L55

The recurring-ical-events library seems to be able to handle mixed date/datetime values (example).

[dozed]

recurring-ical-events makes use of dateutil btw: https://github.com/niccokunzmann/python-recurring-ical-events/blob/2ecc6cdb50d53728d47718f952b0d83b05629d57/recurring_ical_events.py#L426-L439

[da4089]

Thanks for the detailed report @dozed

[dozed]

One thing I forget to mention is that I wrote a small "hack" to fix another issue before:

Subject: [PATCH] Trying to fix event expansion
---
Index: radicale/app/report.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/radicale/app/report.py b/radicale/app/report.py
--- a/radicale/app/report.py	
+++ b/radicale/app/report.py	
@@ -243,17 +243,27 @@
     item = copy.copy(item)
     vevent = item.vobject_item.vevent
 
-    start_utc = vevent.dtstart.value.astimezone(datetime.timezone.utc)
-    vevent.dtstart = ContentLine(
-        name='DTSTART',
-        value=start_utc.strftime('%Y%m%dT%H%M%SZ'), params={})
+    if isinstance(vevent.dtstart.value, datetime.datetime):
+        start_utc = vevent.dtstart.value.astimezone(datetime.timezone.utc)
+        vevent.dtstart = ContentLine(
+            name='DTSTART',
+            value=start_utc.strftime('%Y%m%dT%H%M%SZ'), params={})
+    else:
+        vevent.dtstart = ContentLine(
+            name='DTSTART',
+            value=vevent.dtstart.value.strftime('%Y%m%d'), params={})
 
     dt_end = getattr(vevent, 'dtend', None)
     if dt_end is not None:
-        end_utc = dt_end.value.astimezone(datetime.timezone.utc)
-        vevent.dtend = ContentLine(
-            name='DTEND',
-            value=end_utc.strftime('%Y%m%dT%H%M%SZ'), params={})
+        if isinstance(dt_end.value, datetime.datetime):
+            end_utc = dt_end.astimezone(datetime.timezone.utc)
+            vevent.dtend = ContentLine(
+                name='DTEND',
+                value=end_utc.strftime('%Y%m%dT%H%M%SZ'), params={})
+        else:
+            vevent.dtend = ContentLine(
+                name='DTEND',
+                value=dt_end.value.strftime('%Y%m%d'), params={})
 
     timezones_to_remove = []
     for component in item.vobject_item.components():

[da4089]

First off, in iCalendar 2.0, it's legal to use either a DATE type (with the VALUE=DATE property) or a DATE-TIME type for the DTSTART and DTEND fields. So this is a legitimate bug.

As @dozed points out, the problem occurs when dateutil 's rruleset.between() function gets a datetime.date instance as a parameter.

start_utc and end_utc get the value of vevent.dtstart and vevent.dtend: it's returned as datetime.date if they're dates. I think an argument could be made that they should be translated by vobject and returned as a datetime (not a date).

That does however open the issue of a timezone -- without one, we'd still hit the same issue comparing naive datetime with a timezoned datetime, but a workaround of just setting it to 00:00:00 UTC isn't correct, and (depending to the schedule) might actually cause errors if there's a recurrence early in the between() window (eg. 00:00 UTC is 10:00 Australia/Sydney, so a 9am Sydney event would not be returned, even though it's on the right date).

So I think this really should be fixed in dateutil, which should have special handling for date vs. datetime in between(), and do the right thing. Otherwise, using our own implementation of the between logic is probably the next best thing?

See https://github.com/dateutil/dateutil/blob/master/src/dateutil/rrule.py#L271-L302

Thoughts?

[pbiering]

So I think this really should be fixed in dateutil, which should have special handling for date vs. datetime in between(), and do the right thing. Otherwise, using our own implementation of the between logic is probably the next best thing?

Would it be possible to create a "dateutil" version/feature depending code sniplet until it is fixed there?

If not fixed, one can't guess the version....can it be done by some kind of selftest during start....means as long as "dateutil" is not passing the selftest, use own implementation.