Fix stuff from kali's review.

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
         """