From 78065a4e1f321e56374594c359492d6dec0a1dc9 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 1 Mar 2014 00:51:29 +0100 Subject: [PATCH] Implement conflict resolution Fix #4 --- config.example | 3 ++ vdirsyncer/cli.py | 10 +++--- vdirsyncer/exceptions.py | 8 +++++ vdirsyncer/sync.py | 66 ++++++++++++++++++++++++++--------- vdirsyncer/tests/test_sync.py | 27 ++++++++++++++ 5 files changed, 93 insertions(+), 21 deletions(-) diff --git a/config.example b/config.example index 433e9f9..eda4e3d 100644 --- a/config.example +++ b/config.example @@ -6,6 +6,9 @@ status_path=~/.vdirsyncer/status/ # This syncronizes only a single collection/calendar/addressbook a = bob_local b = bob_remote +# conflict_resolution = None # abort when collisions occur +# conflict_resolution = a wins # assume a's items to be more up-to-date +# conflict_resolution = b wins # assume b's items to be more up-to-date [storage bob_local] # This represents only a single collection/calendar/addressbook diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 908b577..243ea46 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -56,9 +56,8 @@ def load_config(fname): elif section.startswith('pair '): name = section[len('pair '):] options = get_options(section) - pairs[name] = a, b = options.pop('a'), options.pop('b') - storages.setdefault(a, {}).update(options) - storages.setdefault(b, {}).update(options) + a, b = options.pop('a'), options.pop('b') + pairs[name] = a, b, options elif section == 'general': general = get_options(section) else: @@ -146,7 +145,7 @@ def _main(env, file_cfg): actions = [] for pair_name in pairs: try: - a, b = all_pairs[pair_name] + a, b, pair_options = all_pairs[pair_name] except KeyError: cli_logger.critical('Pair not found: {}'.format(pair_name)) cli_logger.critical('These are the pairs found: ') @@ -158,7 +157,8 @@ def _main(env, file_cfg): def x(a=a, b=b, pair_name=pair_name): cli_logger.debug('Syncing {}'.format(pair_name)) status = load_status(general['status_path'], pair_name) - sync(a, b, status) + sync(a, b, status, + pair_options.get('conflict_resolution', None)) save_status(general['status_path'], pair_name, status) actions.append(x) diff --git a/vdirsyncer/exceptions.py b/vdirsyncer/exceptions.py index 5172d0a..7301a03 100644 --- a/vdirsyncer/exceptions.py +++ b/vdirsyncer/exceptions.py @@ -43,3 +43,11 @@ class WrongEtagError(PreconditionFailed): class StorageError(Error): '''Internal or initialization errors with storage.''' + + +class SyncError(Error): + pass + + +class SyncConflict(SyncError): + pass diff --git a/vdirsyncer/sync.py b/vdirsyncer/sync.py index afb878f..5265b6f 100644 --- a/vdirsyncer/sync.py +++ b/vdirsyncer/sync.py @@ -40,7 +40,7 @@ def prefetch(storage, item_list, hrefs): item_list[href]['obj'] = obj -def sync(storage_a, storage_b, status): +def sync(storage_a, storage_b, status, conflict_resolution=None): '''Syncronizes two storages. :param storage_a: The first storage @@ -51,6 +51,9 @@ def sync(storage_a, storage_b, status): 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. + :param conflict_resolution: Either 'a wins' or 'b wins'. If none is + provided, the sync function will raise + :py:exc:`vdirsyncer.exceptions.SyncConflict`. ''' a_href_to_uid = dict( (href_a, uid) @@ -81,13 +84,16 @@ def sync(storage_a, storage_b, status): } for action in actions: - action(storages, status) + action(storages, status, conflict_resolution) def action_upload(uid, source, dest): - def inner(storages, status): + 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] + sync_logger.debug('Copying (uploading) item {} to {}' + .format(uid, dest_storage)) + source_href = source_uid_to_href[uid] source_etag = source_list[source_href]['etag'] @@ -103,9 +109,11 @@ def action_upload(uid, source, dest): def action_update(uid, source, dest): - def inner(storages, status): + 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] + sync_logger.debug('Copying (updating) item {} to {}' + .format(uid, dest_storage)) source_href = source_uid_to_href[uid] source_etag = source_list[source_href]['etag'] @@ -123,17 +131,50 @@ def action_update(uid, source, dest): def action_delete(uid, source, dest): - def inner(storages, status): + def inner(storages, status, conflict_resolution): if dest is not None: dest_storage, dest_list, dest_uid_to_href = storages[dest] + sync_logger.debug('Deleting item {} from {}' + .format(uid, dest_storage)) dest_href = dest_uid_to_href[uid] dest_etag = dest_list[dest_href]['etag'] dest_storage.delete(dest_href, dest_etag) + else: + sync_logger.debug('Deleting status info for nonexisting item {}' + .format(uid)) del status[uid] return inner +def action_conflict_resolve(uid): + def inner(storages, status, conflict_resolution): + sync_logger.debug('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] + a_meta = list_a[a_href] + b_meta = list_b[b_href] + if a_meta['obj'].raw == b_meta['obj'].raw: + sync_logger.debug('...same content on both sides.') + status[uid] = a_href, a_meta['etag'], b_href, b_meta['etag'] + elif conflict_resolution is None: + raise exceptions.SyncConflict() + elif conflict_resolution == 'a wins': + sync_logger.debug('...{} wins.'.format(a_storage)) + action_update(uid, 'a', 'b')(storages, status, conflict_resolution) + elif conflict_resolution == 'b wins': + sync_logger.debug('...{} wins.'.format(b_storage)) + action_update(uid, 'b', 'a')(storages, status, conflict_resolution) + else: + raise ValueError('Invalid conflict resolution mode: {}' + .format(conflict_resolution)) + + return inner + + def get_actions(list_a, list_b, status, a_uid_to_href, b_uid_to_href): prefetch_from_a = [] prefetch_from_b = [] @@ -148,14 +189,7 @@ def get_actions(list_a, list_b, status, a_uid_to_href, b_uid_to_href): b = list_b.get(href_b, None) if uid not in status: if uid in uids_a and uid in uids_b: # missing status - assert type(a['obj'].raw) is unicode, repr(a['obj'].raw) - assert type(b['obj'].raw) is unicode, repr(b['obj'].raw) - if a['obj'].raw != b['obj'].raw: - raise NotImplementedError( # TODO - 'Conflict. No status and ' - 'different content on both sides.' - ) - status[uid] = (href_a, a['etag'], href_b, b['etag']) + actions.append(action_conflict_resolve(uid)) # new item was created in a elif uid in uids_a and uid not in uids_b: prefetch_from_a.append(href_a) @@ -168,9 +202,9 @@ def get_actions(list_a, list_b, status, a_uid_to_href, b_uid_to_href): _, status_etag_a, _, status_etag_b = status[uid] if uid in uids_a and uid in uids_b: if a['etag'] != status_etag_a and b['etag'] != status_etag_b: - # conflict resolution TODO - raise NotImplementedError('Conflict. ' - 'New etags on both sides.') + prefetch_from_a.append(href_a) + prefetch_from_b.append(href_b) + actions.append(action_conflict_resolve(uid)) elif a['etag'] != status_etag_a: # item was updated in a prefetch_from_a.append(href_a) actions.append(action_update(uid, 'a', 'b')) diff --git a/vdirsyncer/tests/test_sync.py b/vdirsyncer/tests/test_sync.py index 90100eb..2817335 100644 --- a/vdirsyncer/tests/test_sync.py +++ b/vdirsyncer/tests/test_sync.py @@ -113,3 +113,30 @@ class SyncTests(TestCase): sync(a, b, status) assert status == old_status assert a.has('1.txt') and b.has('1.txt') + + def test_conflict_resolution_both_etags_new(self): + a = MemoryStorage() + b = MemoryStorage() + item = Item(u'UID:1') + href_a, etag_a = a.upload(item) + href_b, etag_b = b.upload(item) + status = {} + sync(a, b, status) + assert status + a.update(href_a, Item(u'UID:1\nASDASD'), etag_a) + b.update(href_b, Item(u'UID:1\nHUEHUE'), etag_b) + self.assertRaises(exceptions.SyncConflict, sync, a, b, status) + sync(a, b, status, conflict_resolution='a wins') + obj_a, _ = a.get(href_a) + obj_b, _ = b.get(href_b) + assert obj_a.raw == obj_b.raw == u'UID:1\nASDASD' + + def tset_conflict_resolution_new_etags_without_changes(self): + a = MemoryStorage() + b = MemoryStorage() + item = Item(u'UID:1') + href_a, etag_a = a.upload(item) + href_b, etag_b = b.upload(item) + status = {'1': (href_a, 'BOGUS_a', href_b, 'BOGUS_b')} + sync(a, b, status) + assert status == {'1': (href_a, etag_a, href_b, etag_b)}