From e5c826ccfd8350f37d42e9cac17240bfbce167f0 Mon Sep 17 00:00:00 2001
From: Markus Unterwaditzer <markus@unterwaditzer.net>
Date: Sat, 6 Jun 2015 15:40:13 +0200
Subject: [PATCH] Harden vdirsyncer against changing UIDs

In a strict sense not necessary since UIDs of an item must not be
changed.
---
 CHANGELOG.rst      |  1 +
 tests/test_sync.py | 13 +++++++++++++
 vdirsyncer/sync.py | 40 ++++++++++++++++++++--------------------
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 6773f8e..ea72229 100644
--- a/CHANGELOG.rst
+++ b/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
 =============
diff --git a/tests/test_sync.py b/tests/test_sync.py
index 9410c0e..a4ee73a 100644
--- a/tests/test_sync.py
+++ b/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)
diff --git a/vdirsyncer/sync.py b/vdirsyncer/sync.py
index f57a821..724ed0d 100644
--- a/vdirsyncer/sync.py
+++ b/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:
-                ident, old_etag = href_to_status[href]
-                self.idents[ident] = {
-                    'etag': etag,
-                    'href': href,
-                    'ident': ident
-                }
-
-                if etag != old_etag and not other_read_only:
-                    hrefs_to_download.append(href)
+            props = {'href': href, 'etag': etag}
+            ident, old_etag = href_to_status.get(href, (None, None))
+            assert etag is not None
+            if etag != old_etag and not other_read_only:
+                # 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
-                                 hrefs_to_download else ()):
-            props = self.idents.setdefault(item.ident, {})
-            props['item'] = item
-            props['ident'] = item.ident
+        for href, item, etag in (self.storage.get_multi(prefetch)
+                                 if prefetch else ()):
+            props = prefetch[href]
 
-            if props.setdefault('href', href) != href:
-                raise IdentConflict(storage=self.storage,
-                                    hrefs=[props['href'], href])
+            assert 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))