[Calypso] Incorrect parsing of rrule?
Guido Günther
agx at sigxcpu.org
Mon Jan 25 22:54:55 PST 2016
On Mon, Jan 25, 2016 at 10:42:55AM +0100, Petter Reinholdtsen wrote:
> [Guido Günther]
> > Wouldn't it be simpler to exercise match_filter_element() directly
> > (introducing a new unit test class TestMatchFIlterElement). You'd then
> > have tight control on the input data and the matching rules. The
> > necessary data could be dumped out of match_filter_element() with the
> > unpatched code.
>
> Yes, this was simpler. Using match_filter() I got a working test. In
> the process I discovered that the test probably worked earlier too, as
> the query was for VTODO items while the test set had VCALENDAR an item.
> Anyway, attached are two patches to fix the handling of <time-range>.
> The first is just making the type of a return value more consistent,
> while the second actually fixes the issue.
>
> --
> Happy hacking
> Petter Reinholdtsen
> From eb2beaaecd430273133ef21bcf81eaf647fd4041 Mon Sep 17 00:00:00 2001
> From: Petter Reinholdtsen <pere at hungry.com>
> Date: Mon, 25 Jan 2016 09:30:19 +0000
> Subject: [PATCH 1/2] Make sure match_filter() return True or False, not True
> or None.
>
> This make the return consistenly a boolean, and make printing the
> result prettier.
> ---
> calypso/xmlutils.py | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/calypso/xmlutils.py b/calypso/xmlutils.py
> index e94a5e9..67290b7 100644
> --- a/calypso/xmlutils.py
> +++ b/calypso/xmlutils.py
> @@ -313,6 +313,7 @@ def match_filter(item, filter):
> for fe in filter.getchildren():
> if match_filter_element(item.object, fe):
> return True
> + return False
I checked that callers and they don't distinguish False and None so this
looks good.
>
> def report(path, xml_request, collection):
> """Read and answer REPORT requests.
> --
> 2.7.0.rc3
>
> From e8a193736c49a8d48d787679e49d2ea44fbb428a Mon Sep 17 00:00:00 2001
> From: Petter Reinholdtsen <pere at hungry.com>
> Date: Mon, 25 Jan 2016 09:32:20 +0000
> Subject: [PATCH 2/2] Allow time-range query filters to be open ended.
>
> RFC 4791 state that the start or end attribute (but not boht) can
> be skipped. Allow this in the code, and add a test case to verify
> that this work.
> ---
> calypso/xmlutils.py | 13 ++++++
> tests/data/import.vcalendar | 85 ++++++++++++++++++++++++++++++++++++++
> tests/test_matchfilterelement.py | 88 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 186 insertions(+)
> create mode 100644 tests/data/import.vcalendar
> create mode 100644 tests/test_matchfilterelement.py
>
> diff --git a/calypso/xmlutils.py b/calypso/xmlutils.py
> index 67290b7..29202c3 100644
> --- a/calypso/xmlutils.py
> +++ b/calypso/xmlutils.py
> @@ -275,6 +275,19 @@ def match_filter_element(vobject, fe):
> return False
> start = fe.get("start")
> end = fe.get("end")
> + # According to RFC 4791, one of start and stop must be set,
> + # but the other can be empty. If both are empty, the
> + # specification is violated.
> + if start is None and end is None:
> + # How to best report the error?
> + log.error("time-range missing start and stop attribute (Required by RFC 4791)")
> + return False
Since this all ends up in answering the report request from a client I
think we should set a proper response code (400) in report() for that
particular imtem of the MultiStatus repose. just as we unconditionally
set 200 atm. Does this make sense?
We don't have calypso specific exception's yet. Is it time to introduce
these or do we just raise ValueError?
> + # RFC 4791 state if start is missing, assume it is -infinity
> + if start is None:
> + start = "00010101T000000Z" # start of year one
> + # RFC 4791 state if end is missing, assume it is +infinity
> + if end is None:
> + end = "99991231T235959Z" # last date with four digit year
> if rruleset is None:
> rruleset = dateutil.rrule.rruleset()
> dtstart = vobject.dtstart.value
> diff --git a/tests/data/import.vcalendar b/tests/data/import.vcalendar
> new file mode 100644
> index 0000000..d48d9e8
> --- /dev/null
> +++ b/tests/data/import.vcalendar
> @@ -0,0 +1,85 @@
> +BEGIN:VCALENDAR
> +VERSION:2.0
> +PRODID:-//K Desktop Environment//NONSGML libkcal 4.3//EN
> +BEGIN:VTIMEZONE
> +TZID:Europe/Oslo
> +BEGIN:STANDARD
> +DTSTART:19011213T212852
> +RDATE;VALUE=DATE-TIME:19011213T212852
> +TZNAME:CET
> +TZOFFSETFROM:+0043
> +TZOFFSETTO:+0100
> +END:STANDARD
> +BEGIN:STANDARD
> +DTSTART:19800928T030000
> +RRULE:FREQ=YEARLY;COUNT=16;BYDAY=-1SU;BYMONTH=9
> +TZNAME:CET
> +TZOFFSETFROM:+0200
> +TZOFFSETTO:+0100
> +END:STANDARD
> +BEGIN:STANDARD
> +DTSTART:19961027T030000
> +RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10
> +TZNAME:CET
> +TZOFFSETFROM:+0200
> +TZOFFSETTO:+0100
> +END:STANDARD
> +BEGIN:STANDARD
> +DTSTART:19160930T000000
> +RDATE;VALUE=DATE-TIME:19160930T000000
> +RDATE;VALUE=DATE-TIME:19421102T030000
> +RDATE;VALUE=DATE-TIME:19431004T030000
> +RDATE;VALUE=DATE-TIME:19441002T030000
> +RDATE;VALUE=DATE-TIME:19451001T030000
> +RDATE;VALUE=DATE-TIME:19590920T030000
> +RDATE;VALUE=DATE-TIME:19600918T030000
> +RDATE;VALUE=DATE-TIME:19610917T030000
> +RDATE;VALUE=DATE-TIME:19620916T030000
> +RDATE;VALUE=DATE-TIME:19630915T030000
> +RDATE;VALUE=DATE-TIME:19640920T030000
> +RDATE;VALUE=DATE-TIME:19650919T030000
> +TZNAME:CET
> +TZOFFSETFROM:+0200
> +TZOFFSETTO:+0100
> +END:STANDARD
> +BEGIN:DAYLIGHT
> +DTSTART:19810329T020000
> +RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
> +TZNAME:CEST
> +TZOFFSETFROM:+0100
> +TZOFFSETTO:+0200
> +END:DAYLIGHT
> +BEGIN:DAYLIGHT
> +DTSTART:19160522T010000
> +RDATE;VALUE=DATE-TIME:19160522T010000
> +RDATE;VALUE=DATE-TIME:19400810T230000
> +RDATE;VALUE=DATE-TIME:19430329T020000
> +RDATE;VALUE=DATE-TIME:19440403T020000
> +RDATE;VALUE=DATE-TIME:19450402T020000
> +RDATE;VALUE=DATE-TIME:19590315T020000
> +RDATE;VALUE=DATE-TIME:19600320T020000
> +RDATE;VALUE=DATE-TIME:19610319T020000
> +RDATE;VALUE=DATE-TIME:19620318T020000
> +RDATE;VALUE=DATE-TIME:19630317T020000
> +RDATE;VALUE=DATE-TIME:19640315T020000
> +RDATE;VALUE=DATE-TIME:19650425T020000
> +RDATE;VALUE=DATE-TIME:19800406T020000
> +TZNAME:CEST
> +TZOFFSETFROM:+0100
> +TZOFFSETTO:+0200
> +END:DAYLIGHT
> +END:VTIMEZONE
> +BEGIN:VEVENT
> +UID:KOrganizer-1559275381.105
> +DTSTART;TZID=Europe/Oslo:20160122T130000
> +DTEND;TZID=Europe/Oslo:20160122T133000
> +CREATED:20160121T201051Z
> +DTSTAMP:20160121T202651Z
> +LAST-MODIFIED:20160121T202651Z
> +SEQUENCE:2
> +SUMMARY:Time to party
> +TRANSP:OPAQUE
> +END:VEVENT
> +X-CALYPSO-NAME:1453407071.R507.ics
> +X-KDE-ICAL-IMPLEMENTATION-VERSION:1.0
> +END:VCALENDAR
> diff --git a/tests/test_matchfilterelement.py b/tests/test_matchfilterelement.py
> new file mode 100644
> index 0000000..094a14e
> --- /dev/null
> +++ b/tests/test_matchfilterelement.py
> @@ -0,0 +1,88 @@
> +# vim: set fileencoding=utf-8 :
> +"""Test matching filter handling """
> +
> +import sys
> +import subprocess
> +import tempfile
> +import shutil
> +import unittest
> +import xml.etree.ElementTree as ET
> +
> +import calypso.config
> +from calypso.webdav import Collection
> +from calypso import xmlutils
> +
> +
> +class TestMatchFilterElement(unittest.TestCase):
> + test_vcal = "tests/data/import.vcalendar"
> +
> + def setUp(self):
> + self.tmpdir = tempfile.mkdtemp()
> + calypso.config.set('storage', 'folder', self.tmpdir)
> + subprocess.call(["git", "init", self.tmpdir]),
> +
> + def tearDown(self):
> + if self.tmpdir:
> + shutil.rmtree(self.tmpdir)
> +
> + def test_start_end(self):
> + """
> +Check that the time-range parser accept ranges where start or stop is
> +missing.
> +"""
> + xml_request1 ="""
> +<calendar-query xmlns="urn:ietf:params:xml:ns:caldav">
> + <prop xmlns="DAV:">
> + <getetag xmlns="DAV:"/>
> + <resourcetype xmlns="DAV:"/>
> + </prop>
> + <filter xmlns="urn:ietf:params:xml:ns:caldav">
> + <comp-filter xmlns="urn:ietf:params:xml:ns:caldav" name="VCALENDAR">
> + <time-range xmlns="urn:ietf:params:xml:ns:caldav" start="20151021T201004Z"/>
> + </comp-filter>
> + </filter>
> +</calendar-query>
> +"""
> + xml_request2 ="""
> +<calendar-query xmlns="urn:ietf:params:xml:ns:caldav">
> + <prop xmlns="DAV:">
> + <getetag xmlns="DAV:"/>
> + <resourcetype xmlns="DAV:"/>
> + </prop>
> + <filter xmlns="urn:ietf:params:xml:ns:caldav">
> + <comp-filter xmlns="urn:ietf:params:xml:ns:caldav" name="VCALENDAR">
> + <time-range xmlns="urn:ietf:params:xml:ns:caldav" end="20151021T201004Z"/>
> + </comp-filter>
> + </filter>
> +</calendar-query>
> +"""
> + xml_request3 ="""
> +<calendar-query xmlns="urn:ietf:params:xml:ns:caldav">
> + <prop xmlns="DAV:">
> + <getetag xmlns="DAV:"/>
> + <resourcetype xmlns="DAV:"/>
> + </prop>
> + <filter xmlns="urn:ietf:params:xml:ns:caldav">
> + <comp-filter xmlns="urn:ietf:params:xml:ns:caldav" name="VCALENDAR">
> + <time-range xmlns="urn:ietf:params:xml:ns:caldav"/>
> + </comp-filter>
> + </filter>
> +</calendar-query>
> +"""
> + collection = Collection("")
> + self.assertTrue(collection.import_file(self.test_vcal))
> + self.assertEqual(len(collection.items), 1)
> +
> + # Tried calling do_REPORT() directly, but lacked the arguments
> + # needed to get the CollectionHTTPHandler class working. Use
> + # match_filter() directly instead.
Hmm...either we want to teaat do_REPORT than we should try harder or we
just want to exercise match_filter in which case i would leave the
comment out.
Given we do the exception raising from above we should also test if we
can catch that here.
Cheers,
-- Guido
More information about the Calypso
mailing list