Merge pull request #31 from untitaker/caldav_speedup

Make CaldavStorage.list faster

docs/server_support.rst

@@ -15,18 +15,25 @@ take a look at `vfix <https://github.com/geier/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
-<https://github.com/Kozea/Radicale/issues/146>`_, 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
+  <https://github.com/Kozea/Radicale/issues/146>`_, 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.

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,11 @@ 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' and not s.item_types) \
+           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.')

vdirsyncer/storage/dav.py

@@ -226,6 +226,12 @@ class DavSession(object):
 class DavStorage(Storage):
 
     '''
+    .. note::
+
+        Please also see :ref:`server_support` for very important information,
+        as changing some of the default options might be very dangerous with
+        some servers.
+
     :param url: Base URL or an URL to a collection.
     :param username: Username for authentication.
     :param password: Password for authentication.
@@ -440,11 +446,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 +472,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 +492,34 @@ class CaldavStorage(DavStorage):
     @staticmethod
     def _get_list_filters(components, start, end):
 
-        if not components:
-            components = ('VTODO', 'VEVENT')
-
-        caldavfilter = '''
-            <C:comp-filter name="VCALENDAR">
-                <C:comp-filter name="{component}">
-                    {timefilter}
+        if components:
+            caldavfilter = '''
+                <C:comp-filter name="VCALENDAR">
+                    <C:comp-filter name="{component}">
+                        {timefilter}
+                    </C:comp-filter>
                 </C:comp-filter>
-            </C:comp-filter>
-            '''
+                '''
 
-        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 = ('<C:time-range start="{start}" end="{end}"/>'
-                          .format(start=start, end=end))
+                timefilter = ('<C:time-range start="{start}" end="{end}"/>'
+                              .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 '<C:comp-filter name="VCALENDAR"/>'
 
     def list(self):
         data = '''<?xml version="1.0" encoding="utf-8" ?>