Return back to None as non-ETag

nextCloud now returns no etag on upload, which is why we're forced to
adapt the tests accordingly. So now we need to specify a fixed value for
"no etag returned" such that the tests can act accordingly. We also need
to test that the sync algorithm works properly with None.

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([
+        info = {}
-            s.upload(get_item()),
+        for _ in range(4):
-            s.upload(get_item()),
+            href, etag = s.upload(get_item())
-            s.upload(get_item()),
+            if etag is None:
-            s.upload(get_item())
+                _, 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())
+        href = s.upload(get_item())[0]
-        assert item in set(s.list())
+        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
+        if etag is not None:
-        assert_item_equals(item2, item)
+            assert etag2 == etag
+            assert_item_equals(item2, item)
 
-        (href2, etag2), = s.list()
+        (_, etag3), = s.list()
-        assert etag2 == etag
+        assert etag2 == etag3
-
-        # 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)
 
         assert collection in urlunquote(s.collection)
         if self.storage_class.storage_name.endswith('dav'):

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))
+        event = s.upload(format_item(EVENT_TEMPLATE))[0]
-        task = s.upload(format_item(TASK_TEMPLATE))
+        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}

tests/storage/servers/nextcloud

@@ -1 +1 @@
-Subproject commit de5c4c79f42e2e64b1eac95aec193a0c08f8065d
+Subproject commit 196714b287e6045d614b58a5aa2c32acc50ab69f

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 {}

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.

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
 

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