diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 0ec4b94..00ef7c0 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -76,7 +76,10 @@ class StorageTests(object): items = [get_item() for i in range(1, 10)] hrefs = [] for item in items: - hrefs.append(s.upload(item)) + href, etag = s.upload(item) + if etag is None: + _, etag = s.get(href) + hrefs.append((href, etag)) hrefs.sort() assert hrefs == sorted(s.list()) for href, etag in hrefs: @@ -91,6 +94,8 @@ class StorageTests(object): def test_get_multi_duplicates(self, s, get_item): href, etag = s.upload(get_item()) + if etag is None: + _, etag = s.get(href) (href2, item, etag2), = s.get_multi([href] * 2) assert href2 == href assert etag2 == etag @@ -109,10 +114,14 @@ class StorageTests(object): def test_update(self, s, get_item): item = get_item() href, etag = s.upload(item) + if etag is None: + _, etag = s.get(href) assert_item_equals(s.get(href)[0], item) new_item = get_item(uid=item.uid) new_etag = s.update(href, new_item, etag) + if new_etag is None: + _, new_etag = s.get(href) # See https://github.com/pimutils/vdirsyncer/issues/48 assert isinstance(new_etag, (bytes, str)) assert_item_equals(s.get(href)[0], new_item) @@ -142,6 +151,8 @@ class StorageTests(object): def test_list(self, s, get_item): assert not list(s.list()) href, etag = s.upload(get_item()) + if etag is None: + _, etag = s.get(href) assert list(s.list()) == [(href, etag)] def test_has(self, s, get_item): @@ -153,12 +164,12 @@ class StorageTests(object): assert not s.has(href) def test_update_others_stay_the_same(self, s, get_item): - info = dict([ - s.upload(get_item()), - s.upload(get_item()), - s.upload(get_item()), - s.upload(get_item()) - ]) + info = {} + for _ in range(4): + href, etag = s.upload(get_item()) + if etag is None: + _, etag = s.get(href) + info[href] = etag assert dict( (href, etag) for href, item, etag @@ -197,8 +208,8 @@ class StorageTests(object): **self.storage_class.create_collection(**args) ) - item = s.upload(get_item()) - assert item in set(s.list()) + href = s.upload(get_item())[0] + assert href in set(href for href, etag in s.list()) def test_discover_collection_arg(self, requires_collections, get_storage_args): @@ -243,18 +254,12 @@ class StorageTests(object): href, etag = s.upload(item) item2, etag2 = s.get(href) - assert etag2 == etag - assert_item_equals(item2, item) + if etag is not None: + assert etag2 == etag + assert_item_equals(item2, item) - (href2, etag2), = s.list() - assert etag2 == etag - - # https://github.com/owncloud/contacts/issues/581 - assert href2.replace('%2B', '%20') == href - - item2, etag2 = s.get(href) - assert etag2 == etag - assert_item_equals(item2, item) + (_, etag3), = s.list() + assert etag2 == etag3 assert collection in urlunquote(s.collection) if self.storage_class.storage_name.endswith('dav'): diff --git a/tests/storage/dav/test_caldav.py b/tests/storage/dav/test_caldav.py index 167fbb4..b5a0808 100644 --- a/tests/storage/dav/test_caldav.py +++ b/tests/storage/dav/test_caldav.py @@ -108,9 +108,10 @@ class TestCalDAVStorage(DAVStorageTests): s.upload(too_old_item) s.upload(too_new_item) - href, etag = s.upload(good_item) + expected_href, _ = s.upload(good_item) - assert list(s.list()) == [(href, etag)] + (actual_href, _), = s.list() + assert actual_href == expected_href def test_invalid_resource(self, monkeypatch, get_storage_args): calls = [] @@ -133,13 +134,17 @@ class TestCalDAVStorage(DAVStorageTests): assert len(calls) == 1 def test_item_types_general(self, s): - event = s.upload(format_item(EVENT_TEMPLATE)) - task = s.upload(format_item(TASK_TEMPLATE)) + event = s.upload(format_item(EVENT_TEMPLATE))[0] + task = s.upload(format_item(TASK_TEMPLATE))[0] s.item_types = ('VTODO', 'VEVENT') - assert set(s.list()) == set([event, task]) + + def l(): + return set(href for href, etag in s.list()) + + assert l() == {event, task} s.item_types = ('VTODO',) - assert set(s.list()) == set([task]) + assert l() == {task} s.item_types = ('VEVENT',) - assert set(s.list()) == set([event]) + assert l() == {event} s.item_types = () - assert set(s.list()) == set([event, task]) + assert l() == {event, task} diff --git a/tests/storage/servers/nextcloud b/tests/storage/servers/nextcloud index de5c4c7..196714b 160000 --- a/tests/storage/servers/nextcloud +++ b/tests/storage/servers/nextcloud @@ -1 +1 @@ -Subproject commit de5c4c79f42e2e64b1eac95aec193a0c08f8065d +Subproject commit 196714b287e6045d614b58a5aa2c32acc50ab69f diff --git a/tests/test_sync.py b/tests/test_sync.py index 816bda2..eab8948 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -500,6 +500,20 @@ class SyncMachine(RuleBasedStateMachine): s.update = action_failure s.delete = action_failure + @rule(s=Storage) + def none_as_etag(self, s): + _old_upload = s.upload + _old_update = s.update + + def upload(item): + return _old_upload(item)[0], None + + def update(href, item, etag): + _old_update(href, item, etag) + + s.upload = upload + s.update = update + @rule(target=Status) def newstatus(self): return {} diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index 860171a..666fa3a 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -166,6 +166,10 @@ class Storage(metaclass=StorageMeta): def upload(self, item): '''Upload a new item. + In cases where the new etag is not known, this method may return `None` + as etag. This special case only exists because of DAV. Avoid this + situation whenever possible. + :raises: :exc:`vdirsyncer.exceptions.PreconditionFailed` if there is already an item with that href. @@ -176,6 +180,10 @@ class Storage(metaclass=StorageMeta): def update(self, href, item, etag): '''Update an item. + In cases where the new etag is not known, this method may return `None` + as etag. This special case only exists because of DAV. Avoid this + situation whenever possible. + :raises: :exc:`vdirsyncer.exceptions.PreconditionFailed` if the etag on the server doesn't match the given etag or if the item doesn't exist. diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 869e078..349f833 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -509,7 +509,7 @@ class DAVStorage(Storage): # # In such cases we return a constant etag. The next synchronization # will then detect an etag change and will download the new item. - etag = response.headers.get('etag', 'NULL') + etag = response.headers.get('etag', None) href = self._normalize_href(response.url) return href, etag diff --git a/vdirsyncer/sync.py b/vdirsyncer/sync.py index 258e726..496417b 100644 --- a/vdirsyncer/sync.py +++ b/vdirsyncer/sync.py @@ -354,7 +354,6 @@ class Update(Action): meta = self.dest.new_status[self.ident] meta.etag = \ self.dest.storage.update(meta.href, self.item, meta.etag) - assert isinstance(meta.etag, (bytes, str)) self.dest.new_status[self.ident] = meta