From 1427c60d7dff3fcfa6c30e16fd9815fd4787b458 Mon Sep 17 00:00:00 2001 From: drebs Date: Wed, 28 May 2014 15:24:34 -0300 Subject: Fix stuff from kali's review. --- client/src/leap/soledad/client/sync.py | 15 +++--- client/src/leap/soledad/client/target.py | 4 +- .../ddocs/syncs/views/changes_to_return/map.js | 1 + server/src/leap/soledad/server/sync.py | 57 ++++------------------ 4 files changed, 20 insertions(+), 57 deletions(-) diff --git a/client/src/leap/soledad/client/sync.py b/client/src/leap/soledad/client/sync.py index 6e9e23fa..c29dd769 100644 --- a/client/src/leap/soledad/client/sync.py +++ b/client/src/leap/soledad/client/sync.py @@ -52,7 +52,7 @@ class ClientSyncState(object): @property def _public_attr_keys(self): - return [k for k in self._public_attrs] + return self._public_attrs.keys() def __init__(self, db=None): """ @@ -113,15 +113,16 @@ class ClientSyncState(object): def has_stored_info(self): """ - Return wether there is any sync state info stored on the database. + Return whether there is any sync state info stored on the database. - :return: Wether there's any sync state info store on db. + :return: Whether there's any sync state info store on db. :rtype: bool """ return self._db is not None and self._db.sync_state is not None def __str__(self): - ', '.join(['%s: %s' % (k, getattr(self, k)) for k in self._public_attr_keys]) + return 'ClientSyncState: %s' % ', '.join( + ['%s: %s' % (k, getattr(self, k)) for k in self._public_attr_keys]) class Synchronizer(U1DBSynchronizer): """ @@ -140,7 +141,7 @@ class Synchronizer(U1DBSynchronizer): """ Synchronize documents between source and target. - :param autocreate: Wether the target replica should be created or not. + :param autocreate: Whether the target replica should be created or not. :type autocreate: bool """ sync_target = self.sync_target @@ -173,8 +174,8 @@ class Synchronizer(U1DBSynchronizer): raise # will try to ask sync_exchange() to create the db self.target_replica_uid = None - target_gen, target_trans_id = 0, '' - target_my_gen, target_my_trans_id = 0, '' + target_gen, target_trans_id = (0, '') + target_my_gen, target_my_trans_id = (0, '') # make sure we'll have access to target replica uid once it exists if self.target_replica_uid is None: diff --git a/client/src/leap/soledad/client/target.py b/client/src/leap/soledad/client/target.py index a3b8ed00..93de98d3 100644 --- a/client/src/leap/soledad/client/target.py +++ b/client/src/leap/soledad/client/target.py @@ -703,9 +703,9 @@ class SoledadSyncTarget(HTTPSyncTarget, TokenBasedAuth): @property def stopped(self): """ - Return wether this sync session is stopped. + Return whether this sync session is stopped. - :return: Wether this sync session is stopped. + :return: Whether this sync session is stopped. :rtype: bool """ with self._stop_lock: diff --git a/common/src/leap/soledad/common/ddocs/syncs/views/changes_to_return/map.js b/common/src/leap/soledad/common/ddocs/syncs/views/changes_to_return/map.js index 220345dc..04ceb2ec 100644 --- a/common/src/leap/soledad/common/ddocs/syncs/views/changes_to_return/map.js +++ b/common/src/leap/soledad/common/ddocs/syncs/views/changes_to_return/map.js @@ -6,6 +6,7 @@ function(doc) { emit([source_replica_uid, 0], null); else if (changes.length == 0) emit([source_replica_uid, 0], []); + else for (var i = 0; i < changes['changes_to_return'].length; i++) emit( [source_replica_uid, i], diff --git a/server/src/leap/soledad/server/sync.py b/server/src/leap/soledad/server/sync.py index 3b8b69fb..16926f14 100644 --- a/server/src/leap/soledad/server/sync.py +++ b/server/src/leap/soledad/server/sync.py @@ -120,7 +120,7 @@ class ServerSyncState(object): resource = self._db._database.resource(*ddoc_path) response = resource.get_json(key=self._key(self._source_replica_uid)) data = response[2] - if len(data['rows']) > 0: + if data['rows']: entry = data['rows'].pop() return entry['value']['seen_ids'] return [] @@ -153,7 +153,7 @@ class ServerSyncState(object): Return information about the current sync state. :return: The generation and transaction id of the target database - which will be synced, and the number of documents do return, + which will be synced, and the number of documents to return, or a tuple of Nones if those have not already been sent to server. :rtype: tuple @@ -165,7 +165,7 @@ class ServerSyncState(object): gen = None trans_id = None number_of_changes = None - if len(data['rows']) > 0 and data['rows'][0]['value'] is not None: + if data['rows'] and data['rows'][0]['value'] is not None: value = data['rows'][0]['value'] gen = value['gen'] trans_id = value['trans_id'] @@ -186,7 +186,7 @@ class ServerSyncState(object): key=self._key( [self._source_replica_uid, received])) data = response[2] - if len(data['rows']) == 0: + if not data['rows']: return None, None, None value = data['rows'][0]['value'] gen = value['gen'] @@ -215,14 +215,6 @@ class SyncExchange(sync.SyncExchange): self._trace_hook = None # recover sync state self._sync_state = ServerSyncState(self._db, self.source_replica_uid) - # for tests - #self._incoming_trace = [] - #if hasattr(self._db, '_incoming_trace'): - # self._incoming_trace = self._db._incoming_trace - #self._db._last_exchange_log = { - # 'receive': {'docs': self._incoming_trace}, - # 'return': None - # } def find_changes_to_return(self, received): @@ -243,10 +235,6 @@ class SyncExchange(sync.SyncExchange): to the source syncing replica. :rtype: int """ - if hasattr(self._db, '_last_exchange_log'): - self._db._last_exchange_log['receive'].update({ # for tests - 'last_known_gen': self.source_last_known_generation - }) # check if changes to return have already been calculated new_gen, new_trans_id, number_of_changes = self._sync_state.sync_info() if number_of_changes is None: @@ -269,9 +257,7 @@ class SyncExchange(sync.SyncExchange): self.new_gen = new_gen self.new_trans_id = new_trans_id # and append one change - self.changes_to_return = [] - if next_change_to_return is not None: - self.changes_to_return.append(next_change_to_return) + self.change_to_return = next_change_to_return return self.new_gen, number_of_changes def return_one_doc(self, return_doc_cb): @@ -287,27 +273,10 @@ class SyncExchange(sync.SyncExchange): replica. :type return_doc_cb: callable(doc, gen, trans_id) """ - changes_to_return = self.changes_to_return - # return docs, including conflicts - changed_doc_ids = [doc_id for doc_id, _, _ in changes_to_return] - self._trace('before get_docs') - docs = self._db.get_docs( - changed_doc_ids, check_for_conflicts=False, include_deleted=True) - - docs_by_gen = izip( - docs, (gen for _, gen, _ in changes_to_return), - (trans_id for _, _, trans_id in changes_to_return)) - for doc, gen, trans_id in docs_by_gen: + if self.change_to_return is not None: + changed_doc_id, gen, trans_id = self.change_to_return + doc = self._db.get_doc(changed_doc_id, include_deleted=True) return_doc_cb(doc, gen, trans_id) - # for tests - if hasattr(self._db, '_outgoing_trace'): - self._db._outgoing_trace.append((doc.doc_id, doc.rev)) - # for tests - if hasattr(self._db, '_outgoing_trace'): - self._db._last_exchange_log['return'] = { - 'docs': self._db._outgoing_trace, - 'last_gen': self.new_gen - } def insert_doc_from_source(self, doc, source_gen, trans_id): """Try to insert synced document from source. @@ -342,14 +311,6 @@ class SyncExchange(sync.SyncExchange): else: # conflict that we will returne assert state == 'conflicted' - # for tests - if hasattr(self._db, '_incoming_trace') \ - and hasattr(self._db, '_last_exchange_log'): - self._db._incoming_trace.append((doc.doc_id, doc.rev)) - self._db._last_exchange_log['receive'].update({ - 'source_uid': self.source_replica_uid, - 'source_gen': source_gen - }) class SyncResource(http_app.SyncResource): @@ -373,7 +334,7 @@ class SyncResource(http_app.SyncResource): :param last_known_trans_id: The last server replica transaction_id the client knows about. :type last_known_trans_id: str - :param ensure: Wether the server replica should be created if it does + :param ensure: Whether the server replica should be created if it does not already exist. :type ensure: bool """ -- cgit v1.2.3