Harden vdirsyncer against changing UIDs

In a strict sense not necessary since UIDs of an item must not be
changed.

CHANGELOG.rst

@@ -13,6 +13,7 @@ Version 0.5.2
 =============
 
 - Vdirsyncer now checks and corrects the permissions of status files.
+- Vdirsyncer is now more robust towards changing UIDs inside items.
 
 Version 0.5.1
 =============

tests/test_sync.py

@@ -189,6 +189,7 @@ def test_uses_get_multi(monkeypatch):
     old_get = MemoryStorage.get
 
     def get_multi(self, hrefs):
+        hrefs = list(hrefs)
         get_multi_calls.append(hrefs)
         for href in hrefs:
             item, etag = old_get(self, href)
@@ -234,6 +235,18 @@ def test_no_uids():
     assert a_items == b_items == {u'ASDF', u'FOOBAR'}
 
 
+def test_changed_uids():
+    a = MemoryStorage()
+    b = MemoryStorage()
+    href_a, etag_a = a.upload(Item(u'UID:A-ONE'))
+    href_b, etag_b = b.upload(Item(u'UID:B-ONE'))
+    status = {}
+    sync(a, b, status)
+
+    a.update(href_a, Item(u'UID:A-TWO'), etag_a)
+    sync(a, b, status)
+
+
 def test_both_readonly():
     a = MemoryStorage(read_only=True)
     b = MemoryStorage(read_only=True)

vdirsyncer/sync.py

@@ -92,35 +92,35 @@ class StorageSyncer(object):
                               for ident, (href, etag)
                               in iteritems(self.status))
 
-        hrefs_to_download = []
+        prefetch = {}
         self.idents = {}
 
         for href, etag in self.storage.list():
-            if href in href_to_status:
+            props = {'href': href, 'etag': etag}
-                ident, old_etag = href_to_status[href]
+            ident, old_etag = href_to_status.get(href, (None, None))
-                self.idents[ident] = {
+            assert etag is not None
-                    'etag': etag,
-                    'href': href,
-                    'ident': ident
-                }
-
             if etag != old_etag and not other_read_only:
-                    hrefs_to_download.append(href)
+                # Either the item is completely new, or updated
+                # In both cases we should prefetch
+                prefetch[href] = props
             else:
-                hrefs_to_download.append(href)
+                self.idents[ident] = props
 
         # Prefetch items
-        for href, item, etag in (self.storage.get_multi(hrefs_to_download) if
+        for href, item, etag in (self.storage.get_multi(prefetch)
-                                 hrefs_to_download else ()):
+                                 if prefetch else ()):
-            props = self.idents.setdefault(item.ident, {})
+            props = prefetch[href]
-            props['item'] = item
-            props['ident'] = item.ident
 
-            if props.setdefault('href', href) != href:
+            assert props['href'] == href
-                raise IdentConflict(storage=self.storage,
-                                    hrefs=[props['href'], href])
             if props.setdefault('etag', etag) != etag:
                 raise SyncError('Etag changed during sync.')
+            props['item'] = item
+            props['ident'] = ident = item.ident
+
+            if self.idents.setdefault(ident, props) is not props:
+                raise IdentConflict(storage=self.storage,
+                                    hrefs=[self.idents[ident]['href'],
+                                           href])
 
     def is_changed(self, ident):
         _, status_etag = self.status.get(ident, (None, None))