From ef786c3586ae6dd1a6dd1ca408524144eb2cd4d3 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 03:35:32 +0100 Subject: [PATCH 01/13] Add basic discover tests --- tests/storage/__init__.py | 4 ++++ tests/storage/conftest.py | 8 ++++++++ tests/storage/dav/__init__.py | 8 ++------ tests/storage/test_filesystem.py | 25 ++++++++++++++++++------- tests/storage/test_memory.py | 3 +++ vdirsyncer/storage/base.py | 11 +++++++++++ vdirsyncer/storage/filesystem.py | 9 ++++++++- 7 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 tests/storage/conftest.py diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 751c933..9a09dd1 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -88,3 +88,7 @@ class StorageTests(object): assert not list(s.list()) s.upload(self._create_bogus_item('1')) assert list(s.list()) + + def test_discover(self): + # Too storage specific to implement in an abstract way + raise NotImplementedError() diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py new file mode 100644 index 0000000..800e837 --- /dev/null +++ b/tests/storage/conftest.py @@ -0,0 +1,8 @@ +import pytest +import tempfile +import shutil + +@pytest.fixture +def class_tmpdir(request): + request.cls.tmpdir = x = tempfile.mkdtemp() + request.addfinalizer(lambda: shutil.rmtree(x)) diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 9a61eb7..0290c63 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -18,6 +18,7 @@ import shutil import sys import os import urlparse +import pytest import mock from werkzeug.test import Client @@ -71,16 +72,14 @@ class Response(object): raise HTTPError(str(self.status_code)) +@pytest.mark.usefixtures('class_tmpdir') class DavStorageTests(StorageTests): '''hrefs are paths without scheme or netloc''' - tmpdir = None storage_class = None radicale_path = None patcher = None def _get_storage(self, **kwargs): - self.tmpdir = tempfile.mkdtemp() - do_the_radicale_dance(self.tmpdir) from radicale import Application app = Application() @@ -103,9 +102,6 @@ class DavStorageTests(StorageTests): def teardown_method(self, method): self.app = None - if self.tmpdir is not None: - shutil.rmtree(self.tmpdir) - self.tmpdir = None if self.patcher is not None: self.patcher.stop() self.patcher = None diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index 56e70a4..75e0761 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -10,19 +10,30 @@ from unittest import TestCase import tempfile +import pytest import shutil +import os from vdirsyncer.storage.filesystem import FilesystemStorage from . import StorageTests +@pytest.mark.usefixtures('class_tmpdir') class FilesystemStorageTests(TestCase, StorageTests): - tmpdir = None def _get_storage(self, **kwargs): - path = self.tmpdir = tempfile.mkdtemp() - return FilesystemStorage(path=path, fileext='.txt', **kwargs) + return FilesystemStorage(path=self.tmpdir, fileext='.txt', **kwargs) - def teardown_method(self, method): - if self.tmpdir is not None: - shutil.rmtree(self.tmpdir) - self.tmpdir = None + def test_discover(self): + paths = set() + for i, collection in enumerate('abcd'): + p = os.path.join(self.tmpdir, collection) + os.makedirs(os.path.join(self.tmpdir, collection)) + fname = os.path.join(p, 'asdf.txt') + with open(fname, 'w+') as f: + f.write(self._create_bogus_item(i).raw) + paths.add(p) + + storages = list(FilesystemStorage.discover(path=self.tmpdir, fileext='.txt')) + assert len(storages) == 4 + for s in storages: + assert s.path in paths diff --git a/tests/storage/test_memory.py b/tests/storage/test_memory.py index d52e7e7..5d42937 100644 --- a/tests/storage/test_memory.py +++ b/tests/storage/test_memory.py @@ -17,3 +17,6 @@ class MemoryStorageTests(TestCase, StorageTests): def _get_storage(self, **kwargs): return MemoryStorage(**kwargs) + + def test_discover(self): + '''This test doesn't make any sense here.''' diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index 24b1178..229b65c 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -33,6 +33,11 @@ class Storage(object): - HREF: Per-storage identifier of item, might be UID. - ETAG: Checksum of item, or something similar that changes when the object does. + + :param collection: If None, the given URL or path is already directly + referring to a collection. Otherwise it will be treated as a basepath + to many collections (e.g. a vdir) and the given collection name will be + looked for. ''' fileext = '.txt' _repr_attributes = () @@ -40,6 +45,12 @@ class Storage(object): def __init__(self, item_class=Item): self.item_class = item_class + @classmethod + def discover(cls, **kwargs): + '''Discover collections given a basepath to many collections. + :returns: Iterable of storages.''' + raise NotImplementedError() + def _get_href(self, uid): return uid + self.fileext diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 913925c..494534b 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -25,12 +25,19 @@ class FilesystemStorage(Storage): ''' :param path: Absolute path to a *collection* inside a vdir. ''' + super(FilesystemStorage, self).__init__(**kwargs) self.path = expand_path(path) if collection is not None: self.path = os.path.join(self.path, collection) self.encoding = encoding self.fileext = fileext - super(FilesystemStorage, self).__init__(**kwargs) + + @classmethod + def discover(cls, path, **kwargs): + for collection in os.listdir(path): + s = cls(path=path, collection=collection, **kwargs) + if next(s.list(), None) is not None: + yield s def _get_filepath(self, href): return os.path.join(self.path, href) From bcc3dc560e68bd9397ed827d9b92680a47b84a55 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 03:40:24 +0100 Subject: [PATCH 02/13] Flake 8 --- tests/storage/conftest.py | 1 + tests/storage/dav/__init__.py | 2 -- tests/storage/dav/test_caldav.py | 1 - tests/storage/dav/test_carddav.py | 1 - tests/storage/test_filesystem.py | 5 ++--- tests/storage/test_http.py | 10 +++------- vdirsyncer/cli.py | 3 ++- vdirsyncer/storage/filesystem.py | 3 ++- 8 files changed, 10 insertions(+), 16 deletions(-) diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index 800e837..d102d57 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -2,6 +2,7 @@ import pytest import tempfile import shutil + @pytest.fixture def class_tmpdir(request): request.cls.tmpdir = x = tempfile.mkdtemp() diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 0290c63..96c3d46 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -13,8 +13,6 @@ :license: MIT, see LICENSE for more details. ''' -import tempfile -import shutil import sys import os import urlparse diff --git a/tests/storage/dav/test_caldav.py b/tests/storage/dav/test_caldav.py index ce9c713..fdabf48 100644 --- a/tests/storage/dav/test_caldav.py +++ b/tests/storage/dav/test_caldav.py @@ -10,7 +10,6 @@ from unittest import TestCase -from vdirsyncer.storage.base import Item from vdirsyncer.storage.dav.caldav import CaldavStorage from . import DavStorageTests diff --git a/tests/storage/dav/test_carddav.py b/tests/storage/dav/test_carddav.py index b1defa2..75707d0 100644 --- a/tests/storage/dav/test_carddav.py +++ b/tests/storage/dav/test_carddav.py @@ -10,7 +10,6 @@ from unittest import TestCase -from vdirsyncer.storage.base import Item from vdirsyncer.storage.dav.carddav import CarddavStorage from . import DavStorageTests diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index 75e0761..cc808f7 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -9,9 +9,7 @@ ''' from unittest import TestCase -import tempfile import pytest -import shutil import os from vdirsyncer.storage.filesystem import FilesystemStorage from . import StorageTests @@ -33,7 +31,8 @@ class FilesystemStorageTests(TestCase, StorageTests): f.write(self._create_bogus_item(i).raw) paths.add(p) - storages = list(FilesystemStorage.discover(path=self.tmpdir, fileext='.txt')) + storages = list(FilesystemStorage.discover(path=self.tmpdir, + fileext='.txt')) assert len(storages) == 4 for s in storages: assert s.path in paths diff --git a/tests/storage/test_http.py b/tests/storage/test_http.py index f3d0821..c4553a6 100644 --- a/tests/storage/test_http.py +++ b/tests/storage/test_http.py @@ -8,12 +8,10 @@ ''' import mock -import requests from unittest import TestCase -from . import StorageTests from .. import assert_item_equals from textwrap import dedent -from vdirsyncer.storage.http import HttpStorage, Item, split_collection +from vdirsyncer.storage.http import HttpStorage, Item class HttpStorageTests(TestCase): @@ -31,7 +29,6 @@ class HttpStorageTests(TestCase): METHOD:PUBLISH BEGIN:VEVENT UID:461092315540@example.com - ORGANIZER;CN="Alice Balder, Example Inc.":MAILTO:alice@example.com LOCATION:Somewhere SUMMARY:Eine Kurzinfo DESCRIPTION:Beschreibung des Termines @@ -48,7 +45,7 @@ class HttpStorageTests(TestCase): BEGIN:VALARM ACTION:AUDIO TRIGGER:19980403T120000 - ATTACH;FMTTYPE=audio/basic:http://host.com/pub/audio-files/ssbanner.aud + ATTACH;FMTTYPE=audio/basic:http://host.com/pub/ssbanner.aud REPEAT:4 DURATION:PT1H END:VALARM @@ -68,7 +65,6 @@ class HttpStorageTests(TestCase): assert_item_equals(item, Item(dedent(u''' BEGIN:VEVENT UID:461092315540@example.com - ORGANIZER;CN="Alice Balder, Example Inc.":MAILTO:alice@example.com LOCATION:Somewhere SUMMARY:Eine Kurzinfo DESCRIPTION:Beschreibung des Termines @@ -89,7 +85,7 @@ class HttpStorageTests(TestCase): BEGIN:VALARM ACTION:AUDIO TRIGGER:19980403T120000 - ATTACH;FMTTYPE=audio/basic:http://host.com/pub/audio-files/ssbanner.aud + ATTACH;FMTTYPE=audio/basic:http://host.com/pub/ssbanner.aud REPEAT:4 DURATION:PT1H END:VALARM diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 6ca05e2..c7c6284 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -144,7 +144,8 @@ def _main(env, file_cfg): actions = [] for pair_name in pairs: try: - a_name, b_name, pair_options, storage_defaults = all_pairs[pair_name] + a_name, b_name, pair_options, storage_defaults = \ + all_pairs[pair_name] except KeyError: cli_logger.critical('Pair not found: {}'.format(pair_name)) cli_logger.critical('These are the pairs found: ') diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 494534b..87c0a2f 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -21,7 +21,8 @@ class FilesystemStorage(Storage): _repr_attributes = ('path',) - def __init__(self, path, fileext, collection=None, encoding='utf-8', **kwargs): + def __init__(self, path, fileext, collection=None, encoding='utf-8', + **kwargs): ''' :param path: Absolute path to a *collection* inside a vdir. ''' From bef7b3e25a9625c5765dce180162e68572d87963 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 11:22:11 +0100 Subject: [PATCH 03/13] Remove unnecessary variable --- tests/storage/dav/__init__.py | 4 ++-- tests/storage/dav/test_caldav.py | 1 - tests/storage/dav/test_carddav.py | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 96c3d46..6f03b09 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -74,7 +74,6 @@ class Response(object): class DavStorageTests(StorageTests): '''hrefs are paths without scheme or netloc''' storage_class = None - radicale_path = None patcher = None def _get_storage(self, **kwargs): @@ -84,7 +83,8 @@ class DavStorageTests(StorageTests): c = Client(app, WerkzeugResponse) server = 'http://127.0.0.1' - full_url = server + self.radicale_path + fileext = self.storage_class.fileext + full_url = server + '/bob/test{}/'.format(fileext) def x(session, method, url, data=None, headers=None, **kw): path = urlparse.urlparse(url).path or self.radicale_path diff --git a/tests/storage/dav/test_caldav.py b/tests/storage/dav/test_caldav.py index fdabf48..ee1eee7 100644 --- a/tests/storage/dav/test_caldav.py +++ b/tests/storage/dav/test_caldav.py @@ -45,7 +45,6 @@ END:VCALENDAR''' class CaldavStorageTests(TestCase, DavStorageTests): storage_class = CaldavStorage - radicale_path = '/bob/test.ics/' item_template = TASK_TEMPLATE diff --git a/tests/storage/dav/test_carddav.py b/tests/storage/dav/test_carddav.py index 75707d0..940477b 100644 --- a/tests/storage/dav/test_carddav.py +++ b/tests/storage/dav/test_carddav.py @@ -16,7 +16,6 @@ from . import DavStorageTests class CarddavStorageTests(TestCase, DavStorageTests): storage_class = CarddavStorage - radicale_path = '/bob/test.vcf/' item_template = (u'BEGIN:VCARD\n' u'VERSION:3.0\n' From 03b6d11ac8276808969b56581cb06a643c25df70 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 14:02:50 +0100 Subject: [PATCH 04/13] Deduplication --- tests/storage/__init__.py | 19 ++++++++++++++++--- tests/storage/conftest.py | 7 ++----- tests/storage/dav/__init__.py | 22 ++++++++++++++-------- tests/storage/test_filesystem.py | 25 +++++++------------------ tests/storage/test_memory.py | 6 ++++-- vdirsyncer/storage/dav/base.py | 2 ++ 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 9a09dd1..4e68b25 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -21,9 +21,12 @@ class StorageTests(object): item_template = item_template or self.item_template return Item(item_template.format(uid=uid, r=r)) - def _get_storage(self, **kwargs): + def get_storage_args(self, collection=None): raise NotImplementedError() + def _get_storage(self): + return self.storage_class(**self.get_storage_args()) + def test_generic(self): items = map(self._create_bogus_item, range(1, 10)) for i, item in enumerate(items): @@ -90,5 +93,15 @@ class StorageTests(object): assert list(s.list()) def test_discover(self): - # Too storage specific to implement in an abstract way - raise NotImplementedError() + items = [] + for i in range(4): + s = self.storage_class(**self.get_storage_args(collection=str(i))) + items.append(self._create_bogus_item(str(i))) + s.upload(items[-1]) + + d = self.storage_class.discover( + **self.get_storage_args(collection=None)) + for s in d: + ((href, etag),) = s.list() + item, etag = s.get(href) + assert item.raw in set(x.raw for x in items) diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index d102d57..9a07d98 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -1,9 +1,6 @@ import pytest -import tempfile -import shutil @pytest.fixture -def class_tmpdir(request): - request.cls.tmpdir = x = tempfile.mkdtemp() - request.addfinalizer(lambda: shutil.rmtree(x)) +def class_tmpdir(request, tmpdir): + request.instance.tmpdir = str(tmpdir) diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 6f03b09..9bedcff 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -16,7 +16,8 @@ import sys import os import urlparse -import pytest +import tempfile +import shutil import mock from werkzeug.test import Client @@ -70,24 +71,22 @@ class Response(object): raise HTTPError(str(self.status_code)) -@pytest.mark.usefixtures('class_tmpdir') class DavStorageTests(StorageTests): '''hrefs are paths without scheme or netloc''' storage_class = None patcher = None + tmpdir = None - def _get_storage(self, **kwargs): + def setup_method(self, method): + self.tmpdir = tempfile.mkdtemp() do_the_radicale_dance(self.tmpdir) from radicale import Application app = Application() c = Client(app, WerkzeugResponse) - server = 'http://127.0.0.1' - fileext = self.storage_class.fileext - full_url = server + '/bob/test{}/'.format(fileext) def x(session, method, url, data=None, headers=None, **kw): - path = urlparse.urlparse(url).path or self.radicale_path + path = urlparse.urlparse(url).path assert isinstance(data, bytes) or data is None r = c.open(path=path, method=method, data=data, headers=headers) r = Response(r) @@ -96,10 +95,17 @@ class DavStorageTests(StorageTests): self.patcher = p = mock.patch('requests.Session.request', new=x) p.start() - return self.storage_class(url=full_url, **kwargs) + def get_storage_args(self, collection=None): + url = 'http://127.0.0.1/bob/' + if collection is not None: + url += '{}{}'.format(collection, self.storage_class.fileext) + return {'url': url} def teardown_method(self, method): self.app = None + if self.tmpdir is not None: + shutil.rmtree(self.tmpdir) + self.tmpdir = None if self.patcher is not None: self.patcher.stop() self.patcher = None diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index cc808f7..503f4c3 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -17,22 +17,11 @@ from . import StorageTests @pytest.mark.usefixtures('class_tmpdir') class FilesystemStorageTests(TestCase, StorageTests): + storage_class = FilesystemStorage - def _get_storage(self, **kwargs): - return FilesystemStorage(path=self.tmpdir, fileext='.txt', **kwargs) - - def test_discover(self): - paths = set() - for i, collection in enumerate('abcd'): - p = os.path.join(self.tmpdir, collection) - os.makedirs(os.path.join(self.tmpdir, collection)) - fname = os.path.join(p, 'asdf.txt') - with open(fname, 'w+') as f: - f.write(self._create_bogus_item(i).raw) - paths.add(p) - - storages = list(FilesystemStorage.discover(path=self.tmpdir, - fileext='.txt')) - assert len(storages) == 4 - for s in storages: - assert s.path in paths + def get_storage_args(self, collection=None): + path = self.tmpdir + if collection is not None: + path = os.path.join(path, collection) + os.makedirs(path) + return {'path': path, 'fileext': '.txt'} diff --git a/tests/storage/test_memory.py b/tests/storage/test_memory.py index 5d42937..29e6972 100644 --- a/tests/storage/test_memory.py +++ b/tests/storage/test_memory.py @@ -15,8 +15,10 @@ from . import StorageTests class MemoryStorageTests(TestCase, StorageTests): - def _get_storage(self, **kwargs): - return MemoryStorage(**kwargs) + storage_class = MemoryStorage + + def get_storage_args(collection=None): + return {} def test_discover(self): '''This test doesn't make any sense here.''' diff --git a/vdirsyncer/storage/dav/base.py b/vdirsyncer/storage/dav/base.py index ddc5be7..42aa759 100644 --- a/vdirsyncer/storage/dav/base.py +++ b/vdirsyncer/storage/dav/base.py @@ -177,6 +177,8 @@ class DavStorage(Storage): def update(self, href, obj, etag): href = self._normalize_href(href) + if etag is None: + raise ValueError('etag must be given and must not be None.') return self._put(href, obj, etag) def upload(self, obj): From 3a4a8e7c45642e8c893c81ab1d82837c29bc8231 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 17:11:16 +0100 Subject: [PATCH 05/13] Evaluate abs path in a better way --- vdirsyncer/storage/filesystem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 87c0a2f..5b9a8c6 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -27,9 +27,9 @@ class FilesystemStorage(Storage): :param path: Absolute path to a *collection* inside a vdir. ''' super(FilesystemStorage, self).__init__(**kwargs) - self.path = expand_path(path) if collection is not None: - self.path = os.path.join(self.path, collection) + path = os.path.join(path, collection) + self.path = expand_path(path) self.encoding = encoding self.fileext = fileext From cff730e02e3752a7948430731cf912a365c281ad Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 17:33:17 +0100 Subject: [PATCH 06/13] More documentation --- vdirsyncer/storage/base.py | 19 ++++++++++++------- vdirsyncer/storage/dav/base.py | 12 +++++++++--- vdirsyncer/storage/dav/caldav.py | 6 +++++- vdirsyncer/storage/filesystem.py | 9 ++++++++- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index 229b65c..06c88e2 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -30,9 +30,11 @@ class Storage(object): Terminology: - UID: Global identifier of the item, across storages. - - HREF: Per-storage identifier of item, might be UID. + - HREF: Per-storage identifier of item, might be UID. The reason items + aren't just referenced by their UID is because the CalDAV and CardDAV + specifications make this imperformant to implement. - ETAG: Checksum of item, or something similar that changes when the - object does. + object does. :param collection: If None, the given URL or path is already directly referring to a collection. Otherwise it will be treated as a basepath @@ -42,13 +44,16 @@ class Storage(object): fileext = '.txt' _repr_attributes = () - def __init__(self, item_class=Item): - self.item_class = item_class - @classmethod def discover(cls, **kwargs): - '''Discover collections given a basepath to many collections. - :returns: Iterable of storages.''' + ''' + Discover collections given a basepath or -URL to many collections. + :param **kwargs: Keyword arguments to additionally pass to the storage + instances returned. You shouldn't pass `collection` here, otherwise + TypeError will be raised. + :returns: Iterable of storages which represent the discovered + collections, all of which are passed kwargs during initialization. + ''' raise NotImplementedError() def _get_href(self, uid): diff --git a/vdirsyncer/storage/dav/base.py b/vdirsyncer/storage/dav/base.py index 42aa759..9bc7090 100644 --- a/vdirsyncer/storage/dav/base.py +++ b/vdirsyncer/storage/dav/base.py @@ -16,12 +16,16 @@ from lxml import etree class DavStorage(Storage): + # the file extension of items. Useful for testing against radicale. fileext = None + # mimetype of items item_mimetype = None + # The expected header for resource validation. dav_header = None + # XML to use when fetching multiple hrefs. get_multi_template = None + # The LXML query for extracting results in get_multi get_multi_data_query = None - list_xml = None _session = None _repr_attributes = ('url', 'username') @@ -29,12 +33,13 @@ class DavStorage(Storage): def __init__(self, url, username='', password='', collection=None, verify=True, auth='basic', useragent='vdirsyncer', **kwargs): ''' - :param url: Direct URL for the CalDAV collection. No autodiscovery. + :param url: Base URL or an URL to a collection. Autodiscovery should be + done via :py:meth:`DavStorage.discover`. :param username: Username for authentication. :param password: Password for authentication. :param verify: Verify SSL certificate, default True. :param auth: Authentication method, from {'basic', 'digest'}, default - 'basic'. + 'basic'. :param useragent: Default 'vdirsyncer'. ''' super(DavStorage, self).__init__(**kwargs) @@ -56,6 +61,7 @@ class DavStorage(Storage): url = urlparse.urljoin(url, collection) self.url = url.rstrip('/') + '/' self.parsed_url = urlparse.urlparse(self.url) + self.collection = collection headers = self._default_headers() headers['Depth'] = 1 diff --git a/vdirsyncer/storage/dav/caldav.py b/vdirsyncer/storage/dav/caldav.py index 2a5de6e..b7990b7 100644 --- a/vdirsyncer/storage/dav/caldav.py +++ b/vdirsyncer/storage/dav/caldav.py @@ -21,9 +21,9 @@ class CaldavStorage(DavStorage): fileext = '.ics' item_mimetype = 'text/calendar' dav_header = 'calendar-access' + start_date = None end_date = None - item_types = None get_multi_template = ''' Date: Sun, 9 Mar 2014 20:13:51 +0100 Subject: [PATCH 07/13] More tests --- tests/storage/__init__.py | 4 ++++ tests/storage/dav/__init__.py | 4 +--- tests/storage/test_filesystem.py | 5 ++--- tests/storage/test_memory.py | 7 +++++-- vdirsyncer/storage/filesystem.py | 3 +++ 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 4e68b25..fc570bc 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -105,3 +105,7 @@ class StorageTests(object): ((href, etag),) = s.list() item, etag = s.get(href) assert item.raw in set(x.raw for x in items) + + def test_collection_arg(self): + s = self.storage_class(**self.get_storage_args(collection='asd')) + assert s.collection == 'asd' diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 9bedcff..3f6e8a8 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -97,9 +97,7 @@ class DavStorageTests(StorageTests): def get_storage_args(self, collection=None): url = 'http://127.0.0.1/bob/' - if collection is not None: - url += '{}{}'.format(collection, self.storage_class.fileext) - return {'url': url} + return {'url': url, 'collection': collection} def teardown_method(self, method): self.app = None diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index 503f4c3..5d93e62 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -22,6 +22,5 @@ class FilesystemStorageTests(TestCase, StorageTests): def get_storage_args(self, collection=None): path = self.tmpdir if collection is not None: - path = os.path.join(path, collection) - os.makedirs(path) - return {'path': path, 'fileext': '.txt'} + os.makedirs(os.path.join(path, collection)) + return {'path': path, 'fileext': '.txt', 'collection': collection} diff --git a/tests/storage/test_memory.py b/tests/storage/test_memory.py index 29e6972..eca9184 100644 --- a/tests/storage/test_memory.py +++ b/tests/storage/test_memory.py @@ -17,8 +17,11 @@ class MemoryStorageTests(TestCase, StorageTests): storage_class = MemoryStorage - def get_storage_args(collection=None): - return {} + def get_storage_args(self, **kwargs): + return kwargs def test_discover(self): '''This test doesn't make any sense here.''' + + def test_collection_arg(self): + '''This test doesn't make any sense here.''' diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index eeedad3..212ce00 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -36,12 +36,15 @@ class FilesystemStorage(Storage): super(FilesystemStorage, self).__init__(**kwargs) if collection is not None: path = os.path.join(path, collection) + self.collection = collection self.path = expand_path(path) self.encoding = encoding self.fileext = fileext @classmethod def discover(cls, path, **kwargs): + if kwargs.pop('collection', None) is not None: + raise TypeError('collection argument must not be given.') for collection in os.listdir(path): s = cls(path=path, collection=collection, **kwargs) if next(s.list(), None) is not None: From 3274945391f814399491eb54795cfd752478ab74 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 21:55:20 +0100 Subject: [PATCH 08/13] Add radicale db storage as test --- .travis.yml | 6 +++-- tests/storage/dav/__init__.py | 47 +++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3b9f87e..db6a8a7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,10 @@ language: python python: "2.7" env: - - DAV_SERVER=radicale - - DAV_SERVER=radicale-git + - DAV_SERVER=radicale RADICALE_STORAGE=filesystem + - DAV_SERVER=radicale RADICALE_STORAGE=database + - DAV_SERVER=radicale-git RADICALE_STORAGE=filesystem + - DAV_SERVER=radicale-git RADICALE_STORAGE=database install: - "sh install-deps.sh" diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 3f6e8a8..1ae73fa 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -27,6 +27,38 @@ from .. import StorageTests import vdirsyncer.exceptions as exceptions from vdirsyncer.storage.base import Item +RADICALE_SCHEMA = ''' +create table collection ( + path varchar(200) not null, + parent_path varchar(200) references collection (path), + primary key (path)); + +create table item ( + name varchar(200) not null, + tag text not null, + collection_path varchar(200) references collection (path), + primary key (name)); + +create table header ( + name varchar(200) not null, + value text not null, + collection_path varchar(200) references collection (path), + primary key (name, collection_path)); + +create table line ( + name text not null, + value text not null, + item_name varchar(200) references item (name), + timestamp bigint not null, + primary key (timestamp)); + +create table property ( + name varchar(200) not null, + value text not null, + collection_path varchar(200) references collection (path), + primary key (name, collection_path)); +''' + def do_the_radicale_dance(tmpdir): # All of radicale is already global state, the cleanliness of the code and @@ -43,10 +75,21 @@ def do_the_radicale_dance(tmpdir): import radicale.config # Now we can set some basic configuration. - radicale.config.set('storage', 'type', 'filesystem') - radicale.config.set('storage', 'filesystem_folder', tmpdir) radicale.config.set('rights', 'type', 'None') + if os.environ.get('RADICALE_STORAGE', 'filesystem') == 'filesystem': + radicale.config.set('storage', 'type', 'filesystem') + radicale.config.set('storage', 'filesystem_folder', tmpdir) + else: + radicale.config.set('storage', 'type', 'database') + radicale.config.set('storage', 'database_url', 'sqlite://') + from radicale.storage import database + + s = database.Session() + for line in RADICALE_SCHEMA.split(';'): + s.execute(line) + s.commit() + # This one is particularly useful with radicale's debugging logs and # pytest-capturelog, however, it is very verbose. #import radicale.log From 75eb5a06d75f7007a0ff6ac2b586b16bca22ad55 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 22:04:54 +0100 Subject: [PATCH 09/13] Fix missing sqlalchemy dependency --- .travis.yml | 4 ++-- install-deps.sh | 28 ++++++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index db6a8a7..33ced51 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,8 @@ python: "2.7" env: - DAV_SERVER=radicale RADICALE_STORAGE=filesystem - DAV_SERVER=radicale RADICALE_STORAGE=database - - DAV_SERVER=radicale-git RADICALE_STORAGE=filesystem - - DAV_SERVER=radicale-git RADICALE_STORAGE=database + - DAV_SERVER=radicale_git RADICALE_STORAGE=filesystem + - DAV_SERVER=radicale_git RADICALE_STORAGE=database install: - "sh install-deps.sh" diff --git a/install-deps.sh b/install-deps.sh index d130e64..e2b17f0 100644 --- a/install-deps.sh +++ b/install-deps.sh @@ -1,15 +1,19 @@ pip install --use-mirrors . pip install --use-mirrors -r requirements.txt [[ -z "$DAV_SERVER" ]] && DAV_SERVER=radicale -case "$DAV_SERVER" in - "radicale") - pip install --use-mirrors radicale - ;; - "radicale-git") - pip install git+https://github.com/Kozea/Radicale.git - ;; - *) - echo "$DAV_SERVER is not known." - exit 1 - ;; -esac + +radicale_deps() { + if [[ "$RADICALE_STORAGE" == "database" ]]; then + pip install --use-mirrors sqlalchemy + fi +} + +davserver_radicale() { + pip install --use-mirrors radicale +} + +davserver_radicale_git() { + pip install git+https://github.com/Kozea/Radicale.git +} + +davserver_$DAV_SERVER From 1f6df7f93e9678e2eddc35c31192390e741325f3 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 9 Mar 2014 22:12:37 +0100 Subject: [PATCH 10/13] Goddamnit --- .travis.yml | 6 +++++- install-deps.sh | 16 ++++++++++------ requirements.txt | 1 - 3 files changed, 15 insertions(+), 8 deletions(-) mode change 100644 => 100755 install-deps.sh diff --git a/.travis.yml b/.travis.yml index 33ced51..a7e520e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,10 @@ env: - DAV_SERVER=radicale_git RADICALE_STORAGE=database install: - - "sh install-deps.sh" + - "./install-deps.sh" script: py.test + +cache: + directories: + - /home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages diff --git a/install-deps.sh b/install-deps.sh old mode 100644 new mode 100755 index e2b17f0..b0e9048 --- a/install-deps.sh +++ b/install-deps.sh @@ -1,19 +1,23 @@ +#!/bin/sh pip install --use-mirrors . pip install --use-mirrors -r requirements.txt [[ -z "$DAV_SERVER" ]] && DAV_SERVER=radicale - -radicale_deps() { - if [[ "$RADICALE_STORAGE" == "database" ]]; then - pip install --use-mirrors sqlalchemy - fi -} +[[ -z "$RADICALE_STORAGE" ]] && RADICALE_STORAGE=filesystem davserver_radicale() { pip install --use-mirrors radicale + radicale_deps } davserver_radicale_git() { pip install git+https://github.com/Kozea/Radicale.git + radicale_deps } +radicale_deps() { radicale_storage_$RADICALE_STORAGE; } + +radicale_storage_database() { pip install --use-mirrors sqlalchemy pysqlite; } +radicale_storage_filesystem() { true; } + + davserver_$DAV_SERVER diff --git a/requirements.txt b/requirements.txt index 7ec5eee..6af71ac 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,3 @@ Werkzeug pytest -pytest-capturelog mock From 51aaba19657236a1ed48b96f6355e6041bdf8af7 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 10 Mar 2014 22:09:19 +0100 Subject: [PATCH 11/13] Remove cache This is only available for paid Travis plans --- .travis.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index a7e520e..d55a82c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,3 @@ install: - "./install-deps.sh" script: py.test - -cache: - directories: - - /home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages From 9646fda248e8d008094087afb8bf2285e67d2763 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Mar 2014 18:02:23 +0100 Subject: [PATCH 12/13] First version of autodiscovery support in dav --- vdirsyncer/storage/dav/base.py | 23 +++++++++++++++++++++++ vdirsyncer/storage/dav/caldav.py | 1 + vdirsyncer/storage/dav/carddav.py | 1 + 3 files changed, 25 insertions(+) diff --git a/vdirsyncer/storage/dav/base.py b/vdirsyncer/storage/dav/base.py index 9bc7090..a3469e2 100644 --- a/vdirsyncer/storage/dav/base.py +++ b/vdirsyncer/storage/dav/base.py @@ -26,6 +26,10 @@ class DavStorage(Storage): get_multi_template = None # The LXML query for extracting results in get_multi get_multi_data_query = None + # The leif class to use for autodiscovery + # This should be the class *name* (i.e. "module attribute name") instead of + # the class, because leif is an optional dependency + leif_class = None _session = None _repr_attributes = ('url', 'username') @@ -74,6 +78,25 @@ class DavStorage(Storage): if self.dav_header not in response.headers.get('DAV', ''): raise exceptions.StorageError('URL is not a collection') + @classmethod + def discover(cls, url, **kwargs): + if kwargs.pop('collection', None) is not None: + raise TypeError('collection argument must not be given.') + from leif import leif + d = getattr(leif, cls.leif_class)( + url, + user=kwargs.get('username', None), + password=kwargs.get('password', None), + ssl_verify=kwargs.get('verify', True) + ) + for c in d.discover(): + collection = c['href'] + if collection.startswith(url): + collection = collection[len(url):] + s = cls(url=url, collection=collection, **kwargs) + s.displayname = c['displayname'] + yield s + def _normalize_href(self, href): '''Normalize the href to be a path only relative to hostname and schema.''' diff --git a/vdirsyncer/storage/dav/caldav.py b/vdirsyncer/storage/dav/caldav.py index b7990b7..7df8c4e 100644 --- a/vdirsyncer/storage/dav/caldav.py +++ b/vdirsyncer/storage/dav/caldav.py @@ -21,6 +21,7 @@ class CaldavStorage(DavStorage): fileext = '.ics' item_mimetype = 'text/calendar' dav_header = 'calendar-access' + leif_class = 'CalDiscover' start_date = None end_date = None diff --git a/vdirsyncer/storage/dav/carddav.py b/vdirsyncer/storage/dav/carddav.py index 52a94ca..2194b6b 100644 --- a/vdirsyncer/storage/dav/carddav.py +++ b/vdirsyncer/storage/dav/carddav.py @@ -17,6 +17,7 @@ class CarddavStorage(DavStorage): fileext = '.vcf' item_mimetype = 'text/vcard' dav_header = 'addressbook' + leif_class = 'CardDiscover' get_multi_template = ''' Date: Tue, 11 Mar 2014 18:02:56 +0100 Subject: [PATCH 13/13] BAM! --- tests/storage/dav/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/storage/dav/__init__.py b/tests/storage/dav/__init__.py index 1ae73fa..22b2e6c 100644 --- a/tests/storage/dav/__init__.py +++ b/tests/storage/dav/__init__.py @@ -104,6 +104,7 @@ class Response(object): self.x = x self.status_code = x.status_code self.content = x.get_data(as_text=False) + self.text = x.get_data(as_text=True) self.headers = x.headers self.encoding = x.charset