From fadff19752d602bb4b4c7708c6ffe454d10acd6b Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 12 Apr 2014 14:42:08 +0200 Subject: [PATCH] Make CaldavStorage.list faster --- docs/server_support.rst | 19 +++++++++---- tests/storage/dav/test_main.py | 11 +++++-- vdirsyncer/storage/dav.py | 52 ++++++++++++++++++---------------- 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/docs/server_support.rst b/docs/server_support.rst index f79bc48..9c11fa6 100644 --- a/docs/server_support.rst +++ b/docs/server_support.rst @@ -15,18 +15,25 @@ take a look at `vfix `_. Radicale ======== -vdirsyncer is tested against the git version and the latest PyPI release of +Vdirsyncer is tested against the git version and the latest PyPI release of Radicale. -Radicale doesn't `support time ranges in the calendar-query of CalDAV -`_, so setting ``start_date`` and -``end_date`` for :py:class:`vdirsyncer.storage.CaldavStorage` will have no or -unpredicted consequences. +- Radicale doesn't `support time ranges in the calendar-query of CalDAV + `_, so setting ``start_date`` + and ``end_date`` for :py:class:`vdirsyncer.storage.CaldavStorage` will have + no or unpredicted consequences. + +- Versions of Radicale older than 0.9b1 don't support the necessary + functionality for efficient querying for all items of a collection. + Vdirsyncer's defaults are supposed to deal with this situation, but if you're + using :py:class:`vdirsyncer.storage.CaldavStorage` and set ``item_types`` to + an empty value (``item_types =``), these versions of Radicale will not work + properly. ownCloud ======== -vdirsyncer is tested against the latest version of ownCloud. +Vdirsyncer is tested against the latest version of ownCloud. ownCloud uses SabreDAV, which had problems detecting collisions and race-conditions. The problems were reported and are fixed in SabreDAV's repo. diff --git a/tests/storage/dav/test_main.py b/tests/storage/dav/test_main.py index 4ef21b8..840388b 100644 --- a/tests/storage/dav/test_main.py +++ b/tests/storage/dav/test_main.py @@ -111,11 +111,14 @@ class TestCaldavStorage(DavStorageTests): ('VTODO',), ('VEVENT',), ('VTODO', 'VEVENT'), - ('VTODO', 'VEVENT', 'VJOURNAL') + ('VTODO', 'VEVENT', 'VJOURNAL'), + () ]) def test_item_types_performance(self, item_types, monkeypatch): kw = self.get_storage_args() s = self.storage_class(item_types=item_types, **kw) + item = self._create_bogus_item() + href, etag = s.upload(item) old_list = s._list calls = [] @@ -126,8 +129,10 @@ class TestCaldavStorage(DavStorageTests): monkeypatch.setattr(s, '_list', _list) - list(s.list()) - assert len(calls) == len(item_types) + rv = list(s.list()) + if dav_server != 'radicale' or item.parsed.name in s.item_types: + assert rv == [(href, etag)] + assert len(calls) == (len(item_types) or 1) @pytest.mark.xfail(dav_server == 'radicale', reason='Radicale doesn\'t support timeranges.') diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 12725e1..416841b 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -440,11 +440,8 @@ class CaldavStorage(DavStorage): :param start_date: Start date of timerange to show, default -inf. :param end_date: End date of timerange to show, default +inf. :param item_types: A tuple of collection types to show from the server. - For example, if you want to only get VEVENTs, pass ``('VEVENT',)``. - Falsy values mean "get all types". Dependent on server - functionality, no clientside validation of results. This currently - only affects the `list` method, but this shouldn't cause problems - in the normal usecase. + For example, if you want to only get VEVENTs, pass ``VEVENT``. + Dependent on server functionality, no clientside validation of results. ''' storage_name = 'caldav' @@ -469,7 +466,7 @@ class CaldavStorage(DavStorage): get_multi_data_query = '{urn:ietf:params:xml:ns:caldav}calendar-data' def __init__(self, start_date=None, end_date=None, - item_types=(), **kwargs): + item_types='VTODO, VEVENT', **kwargs): super(CaldavStorage, self).__init__(**kwargs) if isinstance(item_types, str): item_types = [x.strip() for x in item_types.split(',')] @@ -489,29 +486,34 @@ class CaldavStorage(DavStorage): @staticmethod def _get_list_filters(components, start, end): - if not components: - components = ('VTODO', 'VEVENT') - - caldavfilter = ''' - - - {timefilter} + if components: + caldavfilter = ''' + + + {timefilter} + - - ''' + ''' - if start is not None and end is not None: - start = start.strftime(CALDAV_DT_FORMAT) - end = end.strftime(CALDAV_DT_FORMAT) + if start is not None and end is not None: + start = start.strftime(CALDAV_DT_FORMAT) + end = end.strftime(CALDAV_DT_FORMAT) - timefilter = ('' - .format(start=start, end=end)) + timefilter = ('' + .format(start=start, end=end)) + else: + timefilter = '' + + for component in components: + yield caldavfilter.format(component=component, + timefilter=timefilter) else: - timefilter = '' - - for component in components: - yield caldavfilter.format(component=component, - timefilter=timefilter) + if start is not None and end is not None: + for x in CaldavStorage._get_list_filters(('VTODO', 'VEVENT'), + start, end): + yield x + else: + yield '' def list(self): data = '''