From b87abfa4c09eca2710fc52120ebedf73f7b9b812 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 14 May 2014 15:08:31 +0200 Subject: [PATCH] Syncronization without UIDs! --- tests/storage/__init__.py | 41 ++++------- tests/storage/dav/test_main.py | 24 +++---- tests/test_sync.py | 24 +++++-- vdirsyncer/storage/filesystem.py | 4 +- vdirsyncer/storage/memory.py | 2 +- vdirsyncer/sync.py | 118 +++++++++++++++---------------- 6 files changed, 105 insertions(+), 108 deletions(-) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 3b0e681..b8363c3 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -17,13 +17,12 @@ from vdirsyncer.utils import text_type class StorageTests(object): - item_template = u'UID:{uid}\nX-SOMETHING:{r}' + item_template = u'X-SOMETHING:{r}' - def _create_bogus_item(self, uid, item_template=None): + def _create_bogus_item(self, item_template=None): r = random.random() item_template = item_template or self.item_template - uid = '{}@vdirsyncer_tests'.format(uid) - return Item(item_template.format(uid=uid, r=r)) + return Item(item_template.format(r=r)) def get_storage_args(self, collection=None): raise NotImplementedError() @@ -34,7 +33,7 @@ class StorageTests(object): return s def test_generic(self): - items = map(self._create_bogus_item, range(1, 10)) + items = [self._create_bogus_item() for i in range(1, 10)] s = self._get_storage() hrefs = [] for item in items: @@ -47,7 +46,6 @@ class StorageTests(object): assert s.has(href) item, etag2 = s.get(href) assert etag == etag2 - assert 'UID:{}'.format(item.uid) in item.raw def test_empty_get_multi(self): s = self._get_storage() @@ -55,30 +53,30 @@ class StorageTests(object): def test_upload_already_existing(self): s = self._get_storage() - item = self._create_bogus_item(1) + item = self._create_bogus_item() s.upload(item) with pytest.raises(exceptions.PreconditionFailed): s.upload(item) def test_upload(self): s = self._get_storage() - item = self._create_bogus_item(1) + item = self._create_bogus_item() href, etag = s.upload(item) assert_item_equals(s.get(href)[0], item) def test_update(self): s = self._get_storage() - item = self._create_bogus_item(1) + item = self._create_bogus_item() href, etag = s.upload(item) assert_item_equals(s.get(href)[0], item) - new_item = self._create_bogus_item(1) + new_item = self._create_bogus_item() s.update(href, new_item, etag) assert_item_equals(s.get(href)[0], new_item) def test_update_nonexisting(self): s = self._get_storage() - item = self._create_bogus_item(1) + item = self._create_bogus_item() with pytest.raises(exceptions.PreconditionFailed): s.update(s._get_href(item), item, '"123"') with pytest.raises(exceptions.PreconditionFailed): @@ -86,7 +84,7 @@ class StorageTests(object): def test_wrong_etag(self): s = self._get_storage() - item = self._create_bogus_item(1) + item = self._create_bogus_item() href, etag = s.upload(item) with pytest.raises(exceptions.PreconditionFailed): s.update(href, item, '"lolnope"') @@ -95,7 +93,7 @@ class StorageTests(object): def test_delete(self): s = self._get_storage() - href, etag = s.upload(self._create_bogus_item(1)) + href, etag = s.upload(self._create_bogus_item()) s.delete(href, etag) assert not list(s.list()) @@ -107,9 +105,9 @@ class StorageTests(object): def test_list(self): s = self._get_storage() assert not list(s.list()) - s.upload(self._create_bogus_item('1')) + s.upload(self._create_bogus_item()) assert list(s.list()) - + def test_discover(self): collections = set() @@ -123,7 +121,7 @@ class StorageTests(object): **self.get_storage_args(collection=collection)) # radicale ignores empty collections during discovery - item = self._create_bogus_item(str(i)) + item = self._create_bogus_item() s.upload(item) collections.add(s.collection) @@ -161,15 +159,6 @@ class StorageTests(object): def test_has(self): s = self._get_storage() assert not s.has('asd') - href, etag = s.upload(self._create_bogus_item(1)) + href, etag = s.upload(self._create_bogus_item()) assert s.has(href) assert not s.has('asd') - - def test_upload_without_uid(self): - lines = [x for x in self._create_bogus_item('1').raw.splitlines() - if u'UID' not in x] - item = Item(u'\n'.join(lines)) - - s = self._get_storage() - href, etag = s.upload(item) - assert s.has(href) diff --git a/tests/storage/dav/test_main.py b/tests/storage/dav/test_main.py index cc7a062..edfff27 100644 --- a/tests/storage/dav/test_main.py +++ b/tests/storage/dav/test_main.py @@ -44,7 +44,6 @@ ORG:Self Employed TEL;TYPE=WORK;TYPE=VOICE:412 605 0499 TEL;TYPE=FAX:412 605 0705 URL:http://www.example.com -UID:{uid} X-SOMETHING:{r} END:VCARD''' @@ -58,7 +57,6 @@ DTSTAMP:20130730T074543Z LAST-MODIFIED;VALUE=DATE-TIME:20140122T151338Z SEQUENCE:2 SUMMARY:Book: Kowlani - Tödlicher Staub -UID:{uid} X-SOMETHING:{r} END:VTODO END:VCALENDAR''' @@ -72,7 +70,6 @@ DTSTART:19970714T170000Z DTEND:19970715T035959Z SUMMARY:Bastille Day Party X-SOMETHING:{r} -UID:{uid} END:VEVENT END:VCALENDAR''' @@ -85,7 +82,7 @@ templates = { class DavStorageTests(ServerMixin, StorageTests): def test_dav_broken_item(self): - item = Item(u'UID:1') + item = Item(u'HAHA:YES') s = self._get_storage() try: s.upload(item) @@ -110,8 +107,8 @@ class TestCaldavStorage(DavStorageTests): item_template = TASK_TEMPLATE def test_both_vtodo_and_vevent(self): - task = self._create_bogus_item(1, item_template=TASK_TEMPLATE) - event = self._create_bogus_item(2, item_template=EVENT_TEMPLATE) + task = self._create_bogus_item(item_template=TASK_TEMPLATE) + event = self._create_bogus_item(item_template=EVENT_TEMPLATE) s = self._get_storage() href_etag_task = s.upload(task) href_etag_event = s.upload(event) @@ -127,14 +124,14 @@ class TestCaldavStorage(DavStorageTests): s = self.storage_class(item_types=(item_type,), **kw) try: s.upload(self._create_bogus_item( - 1, item_template=templates[other_item_type])) + item_template=templates[other_item_type])) s.upload(self._create_bogus_item( - 5, item_template=templates[other_item_type])) + item_template=templates[other_item_type])) except (exceptions.Error, requests.exceptions.HTTPError): pass href, etag = \ s.upload(self._create_bogus_item( - 3, item_template=templates[item_type])) + item_template=templates[item_type])) ((href2, etag2),) = s.list() assert href2 == href assert etag2 == etag @@ -169,7 +166,7 @@ class TestCaldavStorage(DavStorageTests): end_date = datetime.datetime(2013, 9, 13) s = self.storage_class(start_date=start_date, end_date=end_date, **kw) - too_old_item = self._create_bogus_item('1', item_template=dedent(u''' + too_old_item = self._create_bogus_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -178,12 +175,11 @@ class TestCaldavStorage(DavStorageTests): DTEND:19970715T035959Z SUMMARY:Bastille Day Party X-SOMETHING:{r} - UID:{uid} END:VEVENT END:VCALENDAR ''').strip()) - too_new_item = self._create_bogus_item('2', item_template=dedent(u''' + too_new_item = self._create_bogus_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -192,12 +188,11 @@ class TestCaldavStorage(DavStorageTests): DTEND:20150715T035959Z SUMMARY:Another Bastille Day Party X-SOMETHING:{r} - UID:{uid} END:VEVENT END:VCALENDAR ''').strip()) - good_item = self._create_bogus_item('3', item_template=dedent(u''' + good_item = self._create_bogus_item(item_template=dedent(u''' BEGIN:VCALENDAR VERSION:2.0 PRODID:-//hacksw/handcal//NONSGML v1.0//EN @@ -206,7 +201,6 @@ class TestCaldavStorage(DavStorageTests): DTEND:20130912T035959Z SUMMARY:What's with all these Bastille Day Partys X-SOMETHING:{r} - UID:{uid} END:VEVENT END:VCALENDAR ''').strip()) diff --git a/tests/test_sync.py b/tests/test_sync.py index ec66524..2c65cb9 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -33,13 +33,13 @@ def test_missing_status(): a = MemoryStorage() b = MemoryStorage() status = {} - item = Item(u'UID:1') - a.upload(item) - b.upload(item) + item = Item(u'asdf') + href_a, _ = a.upload(item) + href_b, _ = b.upload(item) sync(a, b, status) assert len(status) == 1 - assert a.has('1.txt') - assert b.has('1.txt') + assert a.has(href_a) + assert b.has(href_b) def test_missing_status_and_different_items(): @@ -207,3 +207,17 @@ def test_empty_storage_dataloss(): with pytest.raises(StorageEmpty): sync(a, MemoryStorage(), status) + + +def test_no_uids(): + a = MemoryStorage() + b = MemoryStorage() + href_a, _ = a.upload(Item(u'ASDF')) + href_b, _ = b.upload(Item(u'FOOBAR')) + status = {} + sync(a, b, status) + + a_items = [a.get(href)[0].raw for href, etag in a.list()] + b_items = [b.get(href)[0].raw for href, etag in b.list()] + + assert a_items == b_items == [u'ASDF', u'FOOBAR'] diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index be8b470..08b0cab 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -123,7 +123,7 @@ class FilesystemStorage(Storage): href = self._get_href(item) fpath = self._get_filepath(href) if os.path.exists(fpath): - raise exceptions.AlreadyExistingError(item.uid) + raise exceptions.AlreadyExistingError(item) if not isinstance(item.raw, text_type): raise TypeError('item.raw must be a unicode string.') @@ -134,7 +134,7 @@ class FilesystemStorage(Storage): def update(self, href, item, etag): fpath = self._get_filepath(href) - if href != self._get_href(item): + if href != self._get_href(item) and item.uid: logger.warning('href != uid + fileext: href={}; uid={}' .format(href, item.uid)) if not os.path.exists(fpath): diff --git a/vdirsyncer/storage/memory.py b/vdirsyncer/storage/memory.py index 9d82f53..358330e 100644 --- a/vdirsyncer/storage/memory.py +++ b/vdirsyncer/storage/memory.py @@ -47,7 +47,7 @@ class MemoryStorage(Storage): return href, etag def update(self, href, item, etag): - if href != self._get_href(item) or href not in self.items: + if href not in self.items: raise exceptions.NotFoundError(href) actual_etag, _ = self.items[href] if etag != actual_etag: diff --git a/vdirsyncer/sync.py b/vdirsyncer/sync.py index 7298daa..044541a 100644 --- a/vdirsyncer/sync.py +++ b/vdirsyncer/sync.py @@ -50,8 +50,8 @@ def prepare_list(storage, href_to_status): for href, etag in storage.list(): props = rv[href] = {'etag': etag} if href in href_to_status: - uid, old_etag = href_to_status[href] - props['uid'] = uid + ident, old_etag = href_to_status[href] + props['ident'] = ident if etag != old_etag: download.append(href) else: @@ -61,7 +61,7 @@ def prepare_list(storage, href_to_status): for href, item, etag in storage.get_multi(download): props = rv[href] props['item'] = item - props['uid'] = item.uid + props['ident'] = item.uid or item.hash if props['etag'] != etag: raise SyncConflict('Etag changed during sync.') @@ -76,7 +76,7 @@ def sync(storage_a, storage_b, status, conflict_resolution=None, :type storage_a: :class:`vdirsyncer.storage.base.Storage` :param storage_b: The second storage :type storage_b: :class:`vdirsyncer.storage.base.Storage` - :param status: {uid: (href_a, etag_a, href_b, etag_b)} + :param status: {ident: (href_a, etag_a, href_b, etag_b)} metadata about the two storages for detection of changes. Will be modified by the function and should be passed to it at the next sync. If this is the first sync, an empty dictionary should be provided. @@ -89,27 +89,27 @@ def sync(storage_a, storage_b, status, conflict_resolution=None, measure. ''' a_href_to_status = dict( - (href_a, (uid, etag_a)) - for uid, (href_a, etag_a, href_b, etag_b) in iteritems(status) + (href_a, (ident, etag_a)) + for ident, (href_a, etag_a, href_b, etag_b) in iteritems(status) ) b_href_to_status = dict( - (href_b, (uid, etag_b)) - for uid, (href_a, etag_a, href_b, etag_b) in iteritems(status) + (href_b, (ident, etag_b)) + for ident, (href_a, etag_a, href_b, etag_b) in iteritems(status) ) - # href => {'etag': etag, 'item': optional item, 'uid': uid} + # href => {'etag': etag, 'item': optional item, 'ident': ident} list_a = prepare_list(storage_a, a_href_to_status) list_b = prepare_list(storage_b, b_href_to_status) if bool(list_a) != bool(list_b) and status and not force_delete: raise StorageEmpty(storage_b if list_a else storage_a) - a_uid_to_href = dict((x['uid'], href) for href, x in iteritems(list_a)) - b_uid_to_href = dict((x['uid'], href) for href, x in iteritems(list_b)) + a_ident_to_href = dict((x['ident'], href) for href, x in iteritems(list_a)) + b_ident_to_href = dict((x['ident'], href) for href, x in iteritems(list_b)) del a_href_to_status, b_href_to_status storages = { - 'a': (storage_a, list_a, a_uid_to_href), - 'b': (storage_b, list_b, b_uid_to_href) + 'a': (storage_a, list_a, a_ident_to_href), + 'b': (storage_b, list_b, b_ident_to_href) } actions = list(get_actions(storages, status)) @@ -118,14 +118,14 @@ def sync(storage_a, storage_b, status, conflict_resolution=None, action(storages, status, conflict_resolution) -def action_upload(uid, source, dest): +def action_upload(ident, source, dest): def inner(storages, status, conflict_resolution): - source_storage, source_list, source_uid_to_href = storages[source] - dest_storage, dest_list, dest_uid_to_href = storages[dest] + source_storage, source_list, source_ident_to_href = storages[source] + dest_storage, dest_list, dest_ident_to_href = storages[dest] sync_logger.info('Copying (uploading) item {} to {}' - .format(uid, dest_storage)) + .format(ident, dest_storage)) - source_href = source_uid_to_href[uid] + source_href = source_ident_to_href[ident] source_etag = source_list[source_href]['etag'] item = source_list[source_href]['item'] @@ -133,72 +133,72 @@ def action_upload(uid, source, dest): source_status = (source_href, source_etag) dest_status = (dest_href, dest_etag) - status[uid] = source_status + dest_status if source == 'a' else \ + status[ident] = source_status + dest_status if source == 'a' else \ dest_status + source_status return inner -def action_update(uid, source, dest): +def action_update(ident, source, dest): def inner(storages, status, conflict_resolution): - source_storage, source_list, source_uid_to_href = storages[source] - dest_storage, dest_list, dest_uid_to_href = storages[dest] + source_storage, source_list, source_ident_to_href = storages[source] + dest_storage, dest_list, dest_ident_to_href = storages[dest] sync_logger.info('Copying (updating) item {} to {}' - .format(uid, dest_storage)) - source_href = source_uid_to_href[uid] + .format(ident, dest_storage)) + source_href = source_ident_to_href[ident] source_etag = source_list[source_href]['etag'] - dest_href = dest_uid_to_href[uid] + dest_href = dest_ident_to_href[ident] old_etag = dest_list[dest_href]['etag'] item = source_list[source_href]['item'] dest_etag = dest_storage.update(dest_href, item, old_etag) source_status = (source_href, source_etag) dest_status = (dest_href, dest_etag) - status[uid] = source_status + dest_status if source == 'a' else \ + status[ident] = source_status + dest_status if source == 'a' else \ dest_status + source_status return inner -def action_delete(uid, dest): +def action_delete(ident, dest): def inner(storages, status, conflict_resolution): if dest is not None: - dest_storage, dest_list, dest_uid_to_href = storages[dest] + dest_storage, dest_list, dest_ident_to_href = storages[dest] sync_logger.info('Deleting item {} from {}' - .format(uid, dest_storage)) - dest_href = dest_uid_to_href[uid] + .format(ident, dest_storage)) + dest_href = dest_ident_to_href[ident] dest_etag = dest_list[dest_href]['etag'] dest_storage.delete(dest_href, dest_etag) else: sync_logger.info('Deleting status info for nonexisting item {}' - .format(uid)) - del status[uid] + .format(ident)) + del status[ident] return inner -def action_conflict_resolve(uid): +def action_conflict_resolve(ident): def inner(storages, status, conflict_resolution): sync_logger.info('Doing conflict resolution for item {}...' - .format(uid)) - a_storage, list_a, a_uid_to_href = storages['a'] - b_storage, list_b, b_uid_to_href = storages['b'] - a_href = a_uid_to_href[uid] - b_href = b_uid_to_href[uid] + .format(ident)) + a_storage, list_a, a_ident_to_href = storages['a'] + b_storage, list_b, b_ident_to_href = storages['b'] + a_href = a_ident_to_href[ident] + b_href = b_ident_to_href[ident] a_meta = list_a[a_href] b_meta = list_b[b_href] if a_meta['item'].raw == b_meta['item'].raw: sync_logger.info('...same content on both sides.') - status[uid] = a_href, a_meta['etag'], b_href, b_meta['etag'] + status[ident] = a_href, a_meta['etag'], b_href, b_meta['etag'] elif conflict_resolution is None: raise SyncConflict() elif conflict_resolution == 'a wins': sync_logger.info('...{} wins.'.format(a_storage)) - action_update(uid, 'a', 'b')(storages, status, conflict_resolution) + action_update(ident, 'a', 'b')(storages, status, conflict_resolution) elif conflict_resolution == 'b wins': sync_logger.info('...{} wins.'.format(b_storage)) - action_update(uid, 'b', 'a')(storages, status, conflict_resolution) + action_update(ident, 'b', 'a')(storages, status, conflict_resolution) else: raise ValueError('Invalid conflict resolution mode: {}' .format(conflict_resolution)) @@ -207,38 +207,38 @@ def action_conflict_resolve(uid): def get_actions(storages, status): - storage_a, list_a, a_uid_to_href = storages['a'] - storage_b, list_b, b_uid_to_href = storages['b'] + storage_a, list_a, a_ident_to_href = storages['a'] + storage_b, list_b, b_ident_to_href = storages['b'] handled = set() - for uid in itertools.chain(a_uid_to_href, b_uid_to_href, status): - if uid in handled: + for ident in itertools.chain(a_ident_to_href, b_ident_to_href, status): + if ident in handled: continue - handled.add(uid) + handled.add(ident) - href_a = a_uid_to_href.get(uid, None) - href_b = b_uid_to_href.get(uid, None) + href_a = a_ident_to_href.get(ident, None) + href_b = b_ident_to_href.get(ident, None) a = list_a.get(href_a, None) b = list_b.get(href_b, None) - if uid not in status: + if ident not in status: if a and b: # missing status - yield action_conflict_resolve(uid) + yield action_conflict_resolve(ident) elif a and not b: # new item was created in a - yield action_upload(uid, 'a', 'b') + yield action_upload(ident, 'a', 'b') elif not a and b: # new item was created in b - yield action_upload(uid, 'b', 'a') + yield action_upload(ident, 'b', 'a') else: - _, status_etag_a, _, status_etag_b = status[uid] + _, status_etag_a, _, status_etag_b = status[ident] if a and b: if a['etag'] != status_etag_a and b['etag'] != status_etag_b: - yield action_conflict_resolve(uid) + yield action_conflict_resolve(ident) elif a['etag'] != status_etag_a: # item was updated in a - yield action_update(uid, 'a', 'b') + yield action_update(ident, 'a', 'b') elif b['etag'] != status_etag_b: # item was updated in b - yield action_update(uid, 'b', 'a') + yield action_update(ident, 'b', 'a') elif a and not b: # was deleted from b - yield action_delete(uid, 'a') + yield action_delete(ident, 'a') elif not a and b: # was deleted from a - yield action_delete(uid, 'b') + yield action_delete(ident, 'b') elif not a and not b: # was deleted from a and b - yield action_delete(uid, None) + yield action_delete(ident, None)