From 869d9836d14e3f4283185e0e62aaa54a6d4f545d Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Wed, 12 Sep 2018 17:37:41 +0100
Subject: [PATCH 1/6] WiP - this illustrates the general approach.

---
 securedrop_client/models.py  | 16 ++++---
 securedrop_client/storage.py | 85 +++++++++++++++++++++++++++++++++++
 tests/test_models.py         | 20 ++++++---
 tests/test_storage.py        | 87 ++++++++++++++++++++++++++++++++++++
 4 files changed, 196 insertions(+), 12 deletions(-)

diff --git a/securedrop_client/models.py b/securedrop_client/models.py
index c4ed73f17..d32e9cb86 100644
--- a/securedrop_client/models.py
+++ b/securedrop_client/models.py
@@ -24,12 +24,18 @@ class Source(Base):
     is_starred = Column(Boolean, server_default="false")
     last_updated = Column(DateTime)
 
-    def __init__(self, uuid, journalist_designation):
+    def __init__(self, uuid, journalist_designation, is_flagged, public_key,
+                 interaction_count, is_starred, last_updated):
         self.uuid = uuid
         self.journalist_designation = journalist_designation
+        self.is_flagged = is_flagged
+        self.public_key = public_key
+        self.interaction_count = interaction_count
+        self.is_starred = is_starred
+        self.last_updated = last_updated
 
     def __repr__(self):
-        return '<Source %r>' % (self.journalist_designation)
+        return f'<Source {self.journalist_designation}>'
 
 
 class Submission(Base):
@@ -57,7 +63,7 @@ def __init__(self, source, uuid, filename):
         self.filename = filename
 
     def __repr__(self):
-        return '<Submission %r>' % (self.filename)
+        return f'<Submission {self.filename}>'
 
 
 class Reply(Base):
@@ -83,7 +89,7 @@ def __init__(self, journalist, source, filename, size):
         self.size = size
 
     def __repr__(self):
-        return '<Reply %r>' % (self.filename)
+        return f'<Reply {self.filename}>'
 
 
 class User(Base):
@@ -96,4 +102,4 @@ def __init__(self, username):
         self.username = username
 
     def __repr__(self):
-        return "<Journalist: {}".format(self.username)
+        return "<Journalist: {}>".format(self.username)
diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py
index e69de29bb..9b4b2b7e6 100644
--- a/securedrop_client/storage.py
+++ b/securedrop_client/storage.py
@@ -0,0 +1,85 @@
+"""
+Functions needed to synchronise sources and submissions with the SecureDrop
+server (via the securedrop_sdk). Each function requires but two arguments: an
+authenticated instance of the securedrop_sdk API class, and a SQLAlchemy
+session to the local database.
+
+Copyright (C) 2018  The Freedom of the Press Foundation.
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published
+by the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""
+import logging
+from dateutil.parser import parse
+from securedrop_client.models import Source, Submission
+
+
+logger = logging.getLogger(__name__)
+
+
+def sync_sources(api, session):
+    """
+    Gets all sources from the remote server's API and ensure the local database
+    session updates as follows:
+
+    * Existing sources are updated in the local database.
+    * New sources have an entry in the local database.
+    * Local sources not returned by the remote server are deleted from the
+      local database.
+    """
+    try:
+        remote_sources = api.get_sources()
+    except Exception as ex:
+        # Log any errors but allow the caller to handle the exception.
+        logger.error(ex)
+        raise(ex)
+    logger.info('Fetched {} remote sources.'.format(len(remote_sources)))
+    local_sources = session.query(Source)
+    local_uuids = {source.uuid for source in local_sources}
+    for source in remote_sources:
+        if source.uuid in local_uuids:
+            # Update an existing record.
+            existing_source = [s for s in local_sources if s.uuid==source.uuid][0]
+            existing_source.journalist_designation = source.journalist_designation
+            existing_source.is_flagged = source.is_flagged
+            existing_source.public_key = source.key
+            existing_source.interaction_count = source.interaction_count
+            existing_source.is_starred = source.is_starred
+            existing_source.last_updated = parse(source.last_updated)
+            # Removing the UUID from local_uuids ensures this record won't be
+            # deleted at the end of this function.
+            local_uuids.remove(source.uuid)
+            logger.info(f'Updated source {source.uuid}')
+        else:
+            # A new source to be added to the database.
+            new_source = Source(uuid=source.uuid,
+                                journalist_designation=source.journalist_designation,
+                                is_flagged=source.is_flagged,
+                                public_key=source.key,
+                                interaction_count=source.interaction_count,
+                                is_starred=source.is_starred,
+                                last_updated=parse(source.last_updated))
+            session.add(new_source)
+            logger.info(f'Added new source {source.uuid}')
+    # The uuids remaining in local_uuids do not exist on the remote server, so
+    # delete the related records.
+    for deleted_source in [s for s in local_sources if s.uuid in local_uuids]:
+        session.delete(deleted_source)
+        logger.info('Deleted source {deleted_source.uuid}')
+    session.commit()
+
+
+def sync_submissions(api, session):
+    """
+    """
+    pass
diff --git a/tests/test_models.py b/tests/test_models.py
index 567a3f0de..45d8e9def 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -3,23 +3,29 @@
 
 def test_string_representation_of_user():
     user = User('hehe')
-    user.__repr__()
+    assert user.__repr__() == f'<Journalist: {user.username}>'
 
 
 def test_string_representation_of_source():
-    source = Source(journalist_designation="testy test", uuid="test")
-    source.__repr__()
+    source = Source(journalist_designation="testy test", uuid="test",
+                    is_flagged=False, public_key='test', interaction_count=1,
+                    is_starred=False, last_updated='test')
+    assert source.__repr__() == f'<Source {source.journalist_designation}>'
 
 
 def test_string_representation_of_submission():
-    source = Source(journalist_designation="testy test", uuid="test")
+    source = Source(journalist_designation="testy test", uuid="test",
+                    is_flagged=False, public_key='test', interaction_count=1,
+                    is_starred=False, last_updated='test')
     submission = Submission(source=source, uuid="test", filename="test.docx")
-    submission.__repr__()
+    assert submission.__repr__() == f'<Submission {submission.filename}>'
 
 
 def test_string_representation_of_reply():
     user = User('hehe')
-    source = Source(journalist_designation="testy test", uuid="test")
+    source = Source(journalist_designation="testy test", uuid="test",
+                    is_flagged=False, public_key='test', interaction_count=1,
+                    is_starred=False, last_updated='test')
     reply = Reply(source=source, journalist=user, filename="reply.gpg",
                   size=1234)
-    reply.__repr__()
+    assert reply.__repr__() == f'<Reply {reply.filename}>'
diff --git a/tests/test_storage.py b/tests/test_storage.py
index e69de29bb..48a94592b 100644
--- a/tests/test_storage.py
+++ b/tests/test_storage.py
@@ -0,0 +1,87 @@
+"""
+Tests for storage sync logic.
+"""
+import pytest
+import uuid
+from dateutil.parser import parse
+from unittest import mock
+from securedrop_client.storage import sync_sources, sync_submissions
+from sdclientapi import Source
+
+
+def make_remote_source():
+    """
+    Utility function for generating sdclientapi Source instances to act upon
+    in the following unit tests.
+    """
+    return Source(add_star_url='foo', interaction_count=1, is_flagged=False,
+                  is_starred=True, journalist_designation='foo', key='bar',
+                  last_updated='2018-09-11T11:42:31.366649Z',
+                  number_of_documents=1, number_of_messages=1,
+                  remove_star_url='baz', replies_url='qux',
+                  submissions_url='wibble', url='url',
+                  uuid=str(uuid.uuid4()))
+
+
+def test_sync_sources_handles_api_error():
+    """
+    Ensure any error encountered when accessing the API is logged but the
+    caller handles the exception.
+    """
+    mock_api = mock.MagicMock()
+    mock_api.get_sources.side_effect = Exception('BANG!')
+    mock_session = mock.MagicMock()
+    with pytest.raises(Exception):
+        sync_sources(mock_api, mock_session)
+
+
+def test_sync_remote_sources():
+    """
+    Check that:
+
+    * Existing sources are updated in the local database.
+    * New sources have an entry in the local database.
+    * Local sources not returned by the remote server are deleted from the
+      local database.
+    """
+    # Some source objects from the API, one of which will exist in the local
+    # database, the other will NOT exist in the local source database (this
+    # will be added to the database)
+    mock_api = mock.MagicMock()
+    source_to_update = make_remote_source()
+    source_to_create = make_remote_source()
+    mock_api.get_sources.return_value = [source_to_update, source_to_create]
+    # Some local source objects. One already exists in the API results (this
+    # will be updated), one does NOT exist in the API results (this will be
+    # deleted from the local database).
+    mock_session = mock.MagicMock()
+    mock_local_source1 = mock.MagicMock()
+    mock_local_source1.uuid = source_to_update.uuid
+    mock_local_source2 = mock.MagicMock()
+    mock_local_source2.uuid = str(uuid.uuid4())
+    mock_session.query.return_value = [mock_local_source1, mock_local_source2]
+    sync_sources(mock_api, mock_session)
+    # Check the expected local source object has been updated with values from
+    # the API.
+    assert mock_local_source1.journalist_designation == source_to_update.journalist_designation
+    assert mock_local_source1.is_flagged == source_to_update.is_flagged
+    assert mock_local_source1.public_key == source_to_update.key
+    assert mock_local_source1.interaction_count == source_to_update.interaction_count
+    assert mock_local_source1.is_starred == source_to_update.is_starred
+    assert mock_local_source1.last_updated == parse(source_to_update.last_updated)
+    # Check the expected local source object has been created with values from
+    # the API.
+    assert mock_session.add.call_count == 1
+    new_local_session = mock_session.add.call_args_list[0][0][0]
+    assert new_local_session.uuid == source_to_create.uuid
+    assert new_local_session.journalist_designation == source_to_create.journalist_designation
+    assert new_local_session.is_flagged == source_to_create.is_flagged
+    assert new_local_session.public_key == source_to_create.key
+    assert new_local_session.interaction_count == source_to_create.interaction_count
+    assert new_local_session.is_starred == source_to_create.is_starred
+    assert new_local_session.last_updated == parse(source_to_create.last_updated)
+    # Ensure the record for the local source that is missing from the results
+    # of the API is deleted.
+    mock_session.delete.assert_called_once_with(mock_local_source2)
+    # Session is committed to database.
+    assert mock_session.commit.call_count == 1

From 7f0e566b2d4cd69d4df150c97fa86775554fc173 Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Thu, 13 Sep 2018 18:50:14 +0100
Subject: [PATCH 2/6] Source and Submission sync. Initial spike/solution with
 unit tests.

---
 securedrop_client/models.py  |   6 +-
 securedrop_client/storage.py | 104 ++++++++++++++++-------
 tests/test_models.py         |   8 +-
 tests/test_storage.py        | 154 ++++++++++++++++++++++++++++-------
 4 files changed, 205 insertions(+), 67 deletions(-)

diff --git a/securedrop_client/models.py b/securedrop_client/models.py
index b1b58153b..545bd1312 100644
--- a/securedrop_client/models.py
+++ b/securedrop_client/models.py
@@ -35,7 +35,7 @@ def __init__(self, uuid, journalist_designation, is_flagged, public_key,
         self.last_updated = last_updated
 
     def __repr__(self):
-        return f'<Source {self.journalist_designation}>'
+        return '<Source {}>'.format(self.journalist_designation)
 
 
 class Submission(Base):
@@ -62,7 +62,7 @@ def __init__(self, source, uuid, filename):
         self.filename = filename
 
     def __repr__(self):
-        return f'<Submission {self.filename}>'
+        return '<Submission {}>'.format(self.filename)
 
 
 class Reply(Base):
@@ -87,7 +87,7 @@ def __init__(self, journalist, source, filename, size):
         self.size = size
 
     def __repr__(self):
-        return f'<Reply {self.filename}>'
+        return '<Reply {}>'.format(self.filename)
 
 
 class User(Base):
diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py
index 9b4b2b7e6..fef696210 100644
--- a/securedrop_client/storage.py
+++ b/securedrop_client/storage.py
@@ -27,59 +27,105 @@
 logger = logging.getLogger(__name__)
 
 
-def sync_sources(api, session):
+def sync_with_api(api, session):
     """
-    Gets all sources from the remote server's API and ensure the local database
-    session updates as follows:
-
-    * Existing sources are updated in the local database.
-    * New sources have an entry in the local database.
-    * Local sources not returned by the remote server are deleted from the
-      local database.
+    Synchronises sources and submissions from the remote server's API.
     """
+    remote_submissions = []
     try:
         remote_sources = api.get_sources()
+        for source in remote_sources:
+            remote_submissions.extend(api.get_submissions(source))
     except Exception as ex:
         # Log any errors but allow the caller to handle the exception.
         logger.error(ex)
         raise(ex)
-    logger.info('Fetched {} remote sources.'.format(len(remote_sources)))
+    logger.info('Fetched {} remote sources and {} remote submissions.'.format(
+                len(remote_sources), len(remote_submissions)))
     local_sources = session.query(Source)
+    local_submissions = session.query(Submission)
+    update_sources(remote_sources, local_sources, session)
+    update_submissions(remote_submissions, local_submissions, session)
+
+
+def update_sources(remote_sources, local_sources, session):
+    """
+    Given collections of remote sources, the current local sources and a
+    session to the local database, ensure the state of the local database
+    matches that of the remote sources:
+
+    * Existing items are updated in the local database.
+    * New items are created in the local database.
+    * Local items not returned in the remote sources are deleted from the
+      local database.
+    """
     local_uuids = {source.uuid for source in local_sources}
     for source in remote_sources:
         if source.uuid in local_uuids:
             # Update an existing record.
-            existing_source = [s for s in local_sources if s.uuid==source.uuid][0]
-            existing_source.journalist_designation = source.journalist_designation
-            existing_source.is_flagged = source.is_flagged
-            existing_source.public_key = source.key
-            existing_source.interaction_count = source.interaction_count
-            existing_source.is_starred = source.is_starred
-            existing_source.last_updated = parse(source.last_updated)
+            local_source = [s for s in local_sources
+                            if s.uuid == source.uuid][0]
+            local_source.journalist_designation = source.journalist_designation
+            local_source.is_flagged = source.is_flagged
+            local_source.public_key = source.key
+            local_source.interaction_count = source.interaction_count
+            local_source.is_starred = source.is_starred
+            local_source.last_updated = parse(source.last_updated)
             # Removing the UUID from local_uuids ensures this record won't be
             # deleted at the end of this function.
             local_uuids.remove(source.uuid)
-            logger.info(f'Updated source {source.uuid}')
+            logger.info('Updated source {}'.format(source.uuid))
         else:
             # A new source to be added to the database.
-            new_source = Source(uuid=source.uuid,
-                                journalist_designation=source.journalist_designation,
-                                is_flagged=source.is_flagged,
-                                public_key=source.key,
-                                interaction_count=source.interaction_count,
-                                is_starred=source.is_starred,
-                                last_updated=parse(source.last_updated))
-            session.add(new_source)
-            logger.info(f'Added new source {source.uuid}')
+            ns = Source(uuid=source.uuid,
+                        journalist_designation=source.journalist_designation,
+                        is_flagged=source.is_flagged,
+                        public_key=source.key,
+                        interaction_count=source.interaction_count,
+                        is_starred=source.is_starred,
+                        last_updated=parse(source.last_updated))
+            session.add(ns)
+            logger.info('Added new source {}'.format(source.uuid))
     # The uuids remaining in local_uuids do not exist on the remote server, so
     # delete the related records.
     for deleted_source in [s for s in local_sources if s.uuid in local_uuids]:
         session.delete(deleted_source)
-        logger.info('Deleted source {deleted_source.uuid}')
+        logger.info('Deleted source {}'.format(deleted_source.uuid))
     session.commit()
 
 
-def sync_submissions(api, session):
+def update_submissions(remote_submissions, local_submissions, session):
     """
+    * Existing submissions are updated in the local database.
+    * New submissions have an entry created in the local database.
+    * Local submissions not returned in the remote submissions are deleted
+      from the local database.
     """
-    pass
+    local_uuids = {submission.uuid for submission in local_submissions}
+    for submission in remote_submissions:
+        if submission.uuid in local_uuids:
+            # Update an existing record.
+            local_submission = [s for s in local_submissions
+                                if s.uuid == submission.uuid][0]
+            local_submission.filename = submission.filename
+            local_submission.size = submission.size
+            local_submission.is_read = submission.is_read
+            # Removing the UUID from local_uuids ensures this record won't be
+            # deleted at the end of this function.
+            local_uuids.remove(submission.uuid)
+            logger.info('Updated submission {}'.format(submission.uuid))
+        else:
+            # A new submission to be added to the database.
+            source_uuid = submission.source_url.rsplit('/', 1)[1]
+            source = session.query(Source).filter_by(uuid=source_uuid)[0]
+            ns = Submission(source=source, uuid=submission.uuid,
+                            filename=submission.filename)
+            session.add(ns)
+            logger.info('Added new submission {}'.format(submission.uuid))
+    # The uuids remaining in local_uuids do not exist on the remote server, so
+    # delete the related records.
+    for deleted_submission in [s for s in local_submissions
+                               if s.uuid in local_uuids]:
+        session.delete(deleted_submission)
+        logger.info('Deleted submission {}'.format(deleted_submission.uuid))
+    session.commit()
diff --git a/tests/test_models.py b/tests/test_models.py
index 45d8e9def..03717ce34 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -3,14 +3,14 @@
 
 def test_string_representation_of_user():
     user = User('hehe')
-    assert user.__repr__() == f'<Journalist: {user.username}>'
+    user.__repr__()
 
 
 def test_string_representation_of_source():
     source = Source(journalist_designation="testy test", uuid="test",
                     is_flagged=False, public_key='test', interaction_count=1,
                     is_starred=False, last_updated='test')
-    assert source.__repr__() == f'<Source {source.journalist_designation}>'
+    source.__repr__()
 
 
 def test_string_representation_of_submission():
@@ -18,7 +18,7 @@ def test_string_representation_of_submission():
                     is_flagged=False, public_key='test', interaction_count=1,
                     is_starred=False, last_updated='test')
     submission = Submission(source=source, uuid="test", filename="test.docx")
-    assert submission.__repr__() == f'<Submission {submission.filename}>'
+    submission.__repr__()
 
 
 def test_string_representation_of_reply():
@@ -28,4 +28,4 @@ def test_string_representation_of_reply():
                     is_starred=False, last_updated='test')
     reply = Reply(source=source, journalist=user, filename="reply.gpg",
                   size=1234)
-    assert reply.__repr__() == f'<Reply {reply.filename}>'
+    reply.__repr__()
diff --git a/tests/test_storage.py b/tests/test_storage.py
index 48a94592b..24daf3ef2 100644
--- a/tests/test_storage.py
+++ b/tests/test_storage.py
@@ -5,8 +5,9 @@
 import uuid
 from dateutil.parser import parse
 from unittest import mock
-from securedrop_client.storage import sync_sources, sync_submissions
-from sdclientapi import Source
+from securedrop_client.storage import (sync_with_api, update_sources,
+                                       update_submissions)
+from sdclientapi import Source, Submission
 
 
 def make_remote_source():
@@ -23,7 +24,19 @@ def make_remote_source():
                   uuid=str(uuid.uuid4()))
 
 
-def test_sync_sources_handles_api_error():
+def make_remote_submission(source_uuid):
+    """
+    Utility function for generating sdclientapi Submission instances to act
+    upon in the following unit tests. The passed in source_uuid is used to
+    generate a valid URL.
+    """
+    source_url = '/api/v1/sources/{}'.format(source_uuid)
+    return Submission(download_url='test', filename='test', is_read=False,
+                      size=123, source_url=source_url, submission_url='test',
+                      uuid=str(uuid.uuid4()))
+
+
+def test_sync_with_api_handles_api_error():
     """
     Ensure any error encountered when accessing the API is logged but the
     caller handles the exception.
@@ -32,10 +45,36 @@ def test_sync_sources_handles_api_error():
     mock_api.get_sources.side_effect = Exception('BANG!')
     mock_session = mock.MagicMock()
     with pytest.raises(Exception):
-        sync_sources(mock_api, mock_session)
+        sync_with_api(mock_api, mock_session)
+
+
+def test_sync_with_api():
+    """
+    Assuming no errors getting data, check the expected functions to update
+    the state of the local database are called with the necessary data.
+    """
+    # Some source and submission objects from the API.
+    mock_api = mock.MagicMock()
+    source = make_remote_source()
+    mock_api.get_sources.return_value = [source, ]
+    submission = mock.MagicMock()
+    mock_api.get_submissions.return_value = [submission, ]
+    # Some local source and submission objects from the local database.
+    mock_session = mock.MagicMock()
+    local_source = mock.MagicMock()
+    local_submission = mock.MagicMock()
+    mock_session.query.side_effect = [[local_source, ], [local_submission, ]]
+    with mock.patch('securedrop_client.storage.update_sources') as src_fn, \
+            mock.patch('securedrop_client.storage.update_submissions') \
+            as sub_fn:
+        sync_with_api(mock_api, mock_session)
+        src_fn.assert_called_once_with([source, ], [local_source, ],
+                                       mock_session)
+        sub_fn.assert_called_once_with([submission, ], [local_submission, ],
+                                       mock_session)
 
 
-def test_sync_remote_sources():
+def test_update_sources():
     """
     Check that:
 
@@ -44,44 +83,97 @@ def test_sync_remote_sources():
     * Local sources not returned by the remote server are deleted from the
       local database.
     """
+    mock_session = mock.MagicMock()
     # Some source objects from the API, one of which will exist in the local
     # database, the other will NOT exist in the local source database (this
     # will be added to the database)
-    mock_api = mock.MagicMock()
-    source_to_update = make_remote_source()
-    source_to_create = make_remote_source()
-    mock_api.get_sources.return_value = [source_to_update, source_to_create]
+    source_update = make_remote_source()
+    source_create = make_remote_source()
+    remote_sources = [source_update, source_create]
     # Some local source objects. One already exists in the API results (this
     # will be updated), one does NOT exist in the API results (this will be
     # deleted from the local database).
-    mock_session = mock.MagicMock()
-    mock_local_source1 = mock.MagicMock()
-    mock_local_source1.uuid = source_to_update.uuid
-    mock_local_source2 = mock.MagicMock()
-    mock_local_source2.uuid = str(uuid.uuid4())
-    mock_session.query.return_value = [mock_local_source1, mock_local_source2]
-    sync_sources(mock_api, mock_session)
+    local_source1 = mock.MagicMock()
+    local_source1.uuid = source_update.uuid
+    local_source2 = mock.MagicMock()
+    local_source2.uuid = str(uuid.uuid4())
+    local_sources = [local_source1, local_source2]
+    update_sources(remote_sources, local_sources, mock_session)
     # Check the expected local source object has been updated with values from
     # the API.
-    assert mock_local_source1.journalist_designation == source_to_update.journalist_designation
-    assert mock_local_source1.is_flagged == source_to_update.is_flagged
-    assert mock_local_source1.public_key == source_to_update.key
-    assert mock_local_source1.interaction_count == source_to_update.interaction_count
-    assert mock_local_source1.is_starred == source_to_update.is_starred
-    assert mock_local_source1.last_updated == parse(source_to_update.last_updated)
+    assert local_source1.journalist_designation == \
+        source_update.journalist_designation
+    assert local_source1.is_flagged == source_update.is_flagged
+    assert local_source1.public_key == source_update.key
+    assert local_source1.interaction_count == source_update.interaction_count
+    assert local_source1.is_starred == source_update.is_starred
+    assert local_source1.last_updated == parse(source_update.last_updated)
+    # Check the expected local source object has been created with values from
+    # the API.
+    assert mock_session.add.call_count == 1
+    new_source = mock_session.add.call_args_list[0][0][0]
+    assert new_source.uuid == source_create.uuid
+    assert new_source.journalist_designation == \
+        source_create.journalist_designation
+    assert new_source.is_flagged == source_create.is_flagged
+    assert new_source.public_key == source_create.key
+    assert new_source.interaction_count == source_create.interaction_count
+    assert new_source.is_starred == source_create.is_starred
+    assert new_source.last_updated == parse(source_create.last_updated)
+    # Ensure the record for the local source that is missing from the results
+    # of the API is deleted.
+    mock_session.delete.assert_called_once_with(local_source2)
+    # Session is committed to database.
+    assert mock_session.commit.call_count == 1
+
+
+def test_update_submissions():
+    """
+    Check that:
+
+    * Existing submissions are updated in the local database.
+    * New submissions have an entry in the local database.
+    * Local submission not returned by the remote server are deleted from the
+      local database.
+    """
+    mock_session = mock.MagicMock()
+    # Source object related to the submissions.
+    source = mock.MagicMock()
+    source.uuid = str(uuid.uuid4())
+    # Some submission objects from the API, one of which will exist in the
+    # local database, the other will NOT exist in the local source database
+    # (this will be added to the database)
+    submission_update = make_remote_submission(source.uuid)
+    submission_create = make_remote_submission(source.uuid)
+    remote_submissions = [submission_update, submission_create]
+    # Some local submission objects. One already exists in the API results
+    # (this will be updated), one does NOT exist in the API results (this will
+    # be deleted from the local database).
+    local_sub1 = mock.MagicMock()
+    local_sub1.uuid = submission_update.uuid
+    local_sub2 = mock.MagicMock()
+    local_sub2.uuid = str(uuid.uuid4())
+    local_submissions = [local_sub1, local_sub2]
+    # There needs to be a corresponding local_source.
+    local_source = mock.MagicMock()
+    local_source.uuid = source.uuid
+    local_source.id = 666  # ;-)
+    mock_session.query().filter_by.return_value = [local_source, ]
+    update_submissions(remote_submissions, local_submissions, mock_session)
+    # Check the expected local submission object has been updated with values
+    # from the API.
+    assert local_sub1.filename == submission_update.filename
+    assert local_sub1.size == submission_update.size
+    assert local_sub1.is_read == submission_update.is_read
     # Check the expected local source object has been created with values from
     # the API.
     assert mock_session.add.call_count == 1
-    new_local_session = mock_session.add.call_args_list[0][0][0]
-    assert new_local_session.uuid == source_to_create.uuid
-    assert new_local_session.journalist_designation == source_to_create.journalist_designation
-    assert new_local_session.is_flagged == source_to_create.is_flagged
-    assert new_local_session.public_key == source_to_create.key
-    assert new_local_session.interaction_count == source_to_create.interaction_count
-    assert new_local_session.is_starred == source_to_create.is_starred
-    assert new_local_session.last_updated == parse(source_to_create.last_updated)
+    new_sub = mock_session.add.call_args_list[0][0][0]
+    assert new_sub.uuid == submission_create.uuid
+    assert new_sub.source_id == local_source.id
+    assert new_sub.filename == submission_create.filename
     # Ensure the record for the local source that is missing from the results
     # of the API is deleted.
-    mock_session.delete.assert_called_once_with(mock_local_source2)
+    mock_session.delete.assert_called_once_with(local_sub2)
     # Session is committed to database.
     assert mock_session.commit.call_count == 1

From 1f91a695373d69707197eb30252fc94c347b500d Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Fri, 14 Sep 2018 09:57:54 +0100
Subject: [PATCH 3/6] Fix securedrop-sdk dependency in pipenv.

---
 Pipfile      |  1 +
 Pipfile.lock | 36 +++++++++++-------------------------
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/Pipfile b/Pipfile
index dbf6f11cf..366843908 100644
--- a/Pipfile
+++ b/Pipfile
@@ -9,6 +9,7 @@ python_version = "3.5"
 [packages]
 SQLALchemy = "*"
 alembic = "*"
+securedrop-sdk = {git = "https://github.com/freedomofpress/securedrop-sdk.git"}
 
 [dev-packages]
 pytest = "*"
diff --git a/Pipfile.lock b/Pipfile.lock
index f717a7797..e116dcf1c 100644
--- a/Pipfile.lock
+++ b/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "ead5509da4fc616eb389e2caf50c35b71ee307ca5fbd9337bdab51e83515ab11"
+            "sha256": "3456dd3e059dd86c2440e4ec4a55d8a5faf1b664987fed15d8cff8b4838a8619"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -36,22 +36,6 @@
             ],
             "version": "==1.0"
         },
-        "pycodestyle": {
-            "hashes": [
-                "sha256:cbc619d09254895b0d12c2c691e237b2e91e9b2ecf5e84c26b35400f93dcfb83",
-                "sha256:cbfca99bd594a10f674d0cd97a3d802a1fdef635d4361e1a2658de47ed261e3a"
-            ],
-            "index": "pypi",
-            "version": "==2.4.0"
-        },
-        "pyflakes": {
-            "hashes": [
-                "sha256:9a7662ec724d0120012f6e29d6248ae3727d821bba522a0e6b356eff19126a49",
-                "sha256:f661252913bc1dbe7fcfcbf0af0db3f42ab65aabd1a6ca68fe5d466bace94dae"
-            ],
-            "index": "pypi",
-            "version": "==2.0.0"
-        },
         "python-dateutil": {
             "hashes": [
                 "sha256:1adb80e7a782c12e52ef9a8182bebeb73f1d7e24e374397af06fb4956c8dc5c0",
@@ -65,6 +49,10 @@
             ],
             "version": "==1.0.3"
         },
+        "securedrop-sdk": {
+            "git": "https://github.com/freedomofpress/securedrop-sdk.git",
+            "ref": "f18264d96bca9f7c00ba566a123586a50842e437"
+        },
         "six": {
             "hashes": [
                 "sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9",
@@ -196,19 +184,17 @@
         },
         "pycodestyle": {
             "hashes": [
-                "sha256:cbc619d09254895b0d12c2c691e237b2e91e9b2ecf5e84c26b35400f93dcfb83",
-                "sha256:cbfca99bd594a10f674d0cd97a3d802a1fdef635d4361e1a2658de47ed261e3a"
+                "sha256:682256a5b318149ca0d2a9185d365d8864a768a28db66a84a2ea946bcc426766",
+                "sha256:6c4245ade1edfad79c3446fadfc96b0de2759662dc29d07d80a6f27ad1ca6ba9"
             ],
-            "index": "pypi",
-            "version": "==2.4.0"
+            "version": "==2.3.1"
         },
         "pyflakes": {
             "hashes": [
-                "sha256:9a7662ec724d0120012f6e29d6248ae3727d821bba522a0e6b356eff19126a49",
-                "sha256:f661252913bc1dbe7fcfcbf0af0db3f42ab65aabd1a6ca68fe5d466bace94dae"
+                "sha256:08bd6a50edf8cffa9fa09a463063c425ecaaf10d1eb0335a7e8b1401aef89e6f",
+                "sha256:8d616a382f243dbf19b54743f280b80198be0bca3a5396f1d2e1fca6223e8805"
             ],
-            "index": "pypi",
-            "version": "==2.0.0"
+            "version": "==1.6.0"
         },
         "pytest": {
             "hashes": [

From eb33fbf7d25ada1ac1bafe6ef0af1d1e3c2d4ff1 Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Fri, 14 Sep 2018 10:02:43 +0100
Subject: [PATCH 4/6] Add pathlib2 via pipenv.

---
 Pipfile      |  1 +
 Pipfile.lock | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Pipfile b/Pipfile
index 366843908..bac6a11e4 100644
--- a/Pipfile
+++ b/Pipfile
@@ -10,6 +10,7 @@ python_version = "3.5"
 SQLALchemy = "*"
 alembic = "*"
 securedrop-sdk = {git = "https://github.com/freedomofpress/securedrop-sdk.git"}
+"pathlib2" = "*"
 
 [dev-packages]
 pytest = "*"
diff --git a/Pipfile.lock b/Pipfile.lock
index e116dcf1c..fb0028778 100644
--- a/Pipfile.lock
+++ b/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "3456dd3e059dd86c2440e4ec4a55d8a5faf1b664987fed15d8cff8b4838a8619"
+            "sha256": "26724d9042ef23868c442167f3a77bd1b05755753cbcd5781f8794e561231eae"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -36,6 +36,14 @@
             ],
             "version": "==1.0"
         },
+        "pathlib2": {
+            "hashes": [
+                "sha256:8eb170f8d0d61825e09a95b38be068299ddeda82f35e96c3301a8a5e7604cb83",
+                "sha256:d1aa2a11ba7b8f7b21ab852b1fb5afb277e1bb99d5dfc663380b5015c0d80c5a"
+            ],
+            "index": "pypi",
+            "version": "==2.3.2"
+        },
         "python-dateutil": {
             "hashes": [
                 "sha256:1adb80e7a782c12e52ef9a8182bebeb73f1d7e24e374397af06fb4956c8dc5c0",

From 916c86bf2ee9c6df71acd6b079bbce8f6357eb15 Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Mon, 17 Sep 2018 15:23:11 +0100
Subject: [PATCH 5/6] Added sync of replies and handling of users.

---
 securedrop_client/models.py  |   5 +-
 securedrop_client/storage.py |  72 ++++++++++++++++++++---
 tests/test_models.py         |   2 +-
 tests/test_storage.py        | 111 ++++++++++++++++++++++++++++++++++-
 4 files changed, 176 insertions(+), 14 deletions(-)

diff --git a/securedrop_client/models.py b/securedrop_client/models.py
index 545bd1312..d83b4051d 100644
--- a/securedrop_client/models.py
+++ b/securedrop_client/models.py
@@ -68,6 +68,7 @@ def __repr__(self):
 class Reply(Base):
     __tablename__ = 'replies'
     id = Column(Integer, primary_key=True)
+    uuid = Column(String(36), unique=True, nullable=False)
     source_id = Column(Integer, ForeignKey('sources.id'))
     source = relationship("Source",
                           backref=backref("replies", order_by=id,
@@ -80,7 +81,8 @@ class Reply(Base):
     filename = Column(String(255), nullable=False)
     size = Column(Integer, nullable=False)
 
-    def __init__(self, journalist, source, filename, size):
+    def __init__(self, uuid, journalist, source, filename, size):
+        self.uuid = uuid
         self.journalist_id = journalist.id
         self.source_id = source.id
         self.filename = filename
@@ -93,7 +95,6 @@ def __repr__(self):
 class User(Base):
     __tablename__ = 'users'
     id = Column(Integer, primary_key=True)
-    uuid = Column(Integer, nullable=False, unique=True)
     username = Column(String(255), nullable=False, unique=True)
 
     def __init__(self, username):
diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py
index fef696210..3439990bc 100644
--- a/securedrop_client/storage.py
+++ b/securedrop_client/storage.py
@@ -1,8 +1,8 @@
 """
-Functions needed to synchronise sources and submissions with the SecureDrop
-server (via the securedrop_sdk). Each function requires but two arguments: an
-authenticated instance of the securedrop_sdk API class, and a SQLAlchemy
-session to the local database.
+Functions needed to synchronise data with the SecureDrop server (via the
+securedrop_sdk). Each function requires but two arguments: an authenticated
+instance of the securedrop_sdk API class, and a SQLAlchemy session to the local
+database.
 
 Copyright (C) 2018  The Freedom of the Press Foundation.
 
@@ -21,7 +21,7 @@
 """
 import logging
 from dateutil.parser import parse
-from securedrop_client.models import Source, Submission
+from securedrop_client.models import Source, Submission, Reply, User
 
 
 logger = logging.getLogger(__name__)
@@ -36,16 +36,21 @@ def sync_with_api(api, session):
         remote_sources = api.get_sources()
         for source in remote_sources:
             remote_submissions.extend(api.get_submissions(source))
+        remote_replies = api.get_all_replies()
     except Exception as ex:
         # Log any errors but allow the caller to handle the exception.
         logger.error(ex)
         raise(ex)
-    logger.info('Fetched {} remote sources and {} remote submissions.'.format(
-                len(remote_sources), len(remote_submissions)))
+    logger.info('Fetched {} remote sources.'.format(len(remote_sources)))
+    logger.info('Fetched {} remote submissions.'.format(
+        len(remote_submissions)))
+    logger.info('Fetched {} remote replies.'.format(len(remote_replies)))
     local_sources = session.query(Source)
     local_submissions = session.query(Submission)
+    local_replies = session.query(Reply)
     update_sources(remote_sources, local_sources, session)
     update_submissions(remote_submissions, local_submissions, session)
+    update_replies(remote_replies, local_replies, session)
 
 
 def update_sources(remote_sources, local_sources, session):
@@ -116,7 +121,7 @@ def update_submissions(remote_submissions, local_submissions, session):
             logger.info('Updated submission {}'.format(submission.uuid))
         else:
             # A new submission to be added to the database.
-            source_uuid = submission.source_url.rsplit('/', 1)[1]
+            _, source_uuid = submission.source_url.rsplit('/', 1)
             source = session.query(Source).filter_by(uuid=source_uuid)[0]
             ns = Submission(source=source, uuid=submission.uuid,
                             filename=submission.filename)
@@ -129,3 +134,54 @@ def update_submissions(remote_submissions, local_submissions, session):
         session.delete(deleted_submission)
         logger.info('Deleted submission {}'.format(deleted_submission.uuid))
     session.commit()
+
+
+def update_replies(remote_replies, local_replies, session):
+    """
+    * Existing replies are updated in the local database.
+    * New replies have an entry created in the local database.
+    * Local replies not returned in the remote replies are deleted from the
+      local database.
+
+    If a reply references a new journalist username, add them to the database
+    as a new user.
+    """
+    local_uuids = {reply.uuid for reply in local_replies}
+    for reply in remote_replies:
+        if reply.uuid in local_uuids:
+            # Update an existing record.
+            local_reply = [r for r in local_replies if r.uuid == reply.uuid][0]
+            user = find_or_create_user(reply.journalist_username, session)
+            local_reply.journalist_id = user.id
+            local_reply.filename = reply.filename
+            local_reply.size = reply.size
+            local_uuids.remove(reply.uuid)
+            logger.info('Updated reply {}'.format(reply.uuid))
+        else:
+            # A new reply to be added to the database.
+            source_uuid = reply.source_uuid
+            source = session.query(Source).filter_by(uuid=source_uuid)[0]
+            user = find_or_create_user(reply.journalist_username, session)
+            nr = Reply(reply.uuid, user, source, reply.filename, reply.size)
+            session.add(nr)
+            logger.info('Added new reply {}'.format(reply.uuid))
+    # The uuids remaining in local_uuids do not exist on the remote server, so
+    # delete the related records.
+    for deleted_reply in [r for r in local_replies if r.uuid in local_uuids]:
+        session.delete(deleted_reply)
+        logger.info('Deleted reply {}'.format(deleted_reply.uuid))
+    session.commit()
+
+
+def find_or_create_user(username, session):
+    """
+    Returns a user object representing the referenced username. If the username
+    does not already exist in the data, a new instance is created.
+    """
+    user = session.query(User).filter_by(username=username)
+    if user:
+        return user[0]
+    new_user = User(username)
+    session.add(new_user)
+    session.commit()
+    return new_user
diff --git a/tests/test_models.py b/tests/test_models.py
index 03717ce34..0142835c3 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -27,5 +27,5 @@ def test_string_representation_of_reply():
                     is_flagged=False, public_key='test', interaction_count=1,
                     is_starred=False, last_updated='test')
     reply = Reply(source=source, journalist=user, filename="reply.gpg",
-                  size=1234)
+                  size=1234, uuid='test')
     reply.__repr__()
diff --git a/tests/test_storage.py b/tests/test_storage.py
index 24daf3ef2..173521bcf 100644
--- a/tests/test_storage.py
+++ b/tests/test_storage.py
@@ -6,8 +6,9 @@
 from dateutil.parser import parse
 from unittest import mock
 from securedrop_client.storage import (sync_with_api, update_sources,
-                                       update_submissions)
-from sdclientapi import Source, Submission
+                                       update_submissions, update_replies,
+                                       find_or_create_user)
+from sdclientapi import Source, Submission, Reply
 
 
 def make_remote_source():
@@ -36,6 +37,18 @@ def make_remote_submission(source_uuid):
                       uuid=str(uuid.uuid4()))
 
 
+def make_remote_reply(source_uuid, username='testymctestface'):
+    """
+    Utility function for generating sdclientapi Reply instances to act
+    upon in the following unit tests. The passed in source_uuid is used to
+    generate a valid URL.
+    """
+    source_url = '/api/v1/sources/{}'.format(source_uuid)
+    return Reply(filename='test.filename', journalist_username=username,
+                 is_deleted_by_source=False, reply_url='test', size=1234,
+                 source_url=source_url, uuid=str(uuid.uuid4()))
+
+
 def test_sync_with_api_handles_api_error():
     """
     Ensure any error encountered when accessing the API is logged but the
@@ -59,17 +72,24 @@ def test_sync_with_api():
     mock_api.get_sources.return_value = [source, ]
     submission = mock.MagicMock()
     mock_api.get_submissions.return_value = [submission, ]
+    reply = mock.MagicMock()
+    mock_api.get_all_replies.return_value = [reply, ]
     # Some local source and submission objects from the local database.
     mock_session = mock.MagicMock()
     local_source = mock.MagicMock()
     local_submission = mock.MagicMock()
-    mock_session.query.side_effect = [[local_source, ], [local_submission, ]]
+    local_replies = mock.MagicMock()
+    mock_session.query.side_effect = [[local_source, ], [local_submission, ],
+                                      [local_replies, ], ]
     with mock.patch('securedrop_client.storage.update_sources') as src_fn, \
+            mock.patch('securedrop_client.storage.update_replies') as rpl_fn, \
             mock.patch('securedrop_client.storage.update_submissions') \
             as sub_fn:
         sync_with_api(mock_api, mock_session)
         src_fn.assert_called_once_with([source, ], [local_source, ],
                                        mock_session)
+        rpl_fn.assert_called_once_with([reply, ], [local_replies, ],
+                                       mock_session)
         sub_fn.assert_called_once_with([submission, ], [local_submission, ],
                                        mock_session)
 
@@ -177,3 +197,88 @@ def test_update_submissions():
     mock_session.delete.assert_called_once_with(local_sub2)
     # Session is committed to database.
     assert mock_session.commit.call_count == 1
+
+
+def test_update_replies():
+    """
+    Check that:
+
+    * Existing replies are updated in the local database.
+    * New replies have an entry in the local database.
+    * Local replies not returned by the remote server are deleted from the
+      local database.
+    * References to journalist's usernames are correctly handled.
+    """
+    mock_session = mock.MagicMock()
+    # Source object related to the submissions.
+    source = mock.MagicMock()
+    source.uuid = str(uuid.uuid4())
+    # Some remote reply objects from the API, one of which will exist in the
+    # local database, the other will NOT exist in the local database
+    # (this will be added to the database)
+    reply_update = make_remote_reply(source.uuid)
+    reply_create = make_remote_reply(source.uuid, 'unknownuser')
+    remote_replies = [reply_update, reply_create]
+    # Some local reply objects. One already exists in the API results
+    # (this will be updated), one does NOT exist in the API results (this will
+    # be deleted from the local database).
+    local_reply1 = mock.MagicMock()
+    local_reply1.uuid = reply_update.uuid
+    local_reply2 = mock.MagicMock()
+    local_reply2.uuid = str(uuid.uuid4())
+    local_replies = [local_reply1, local_reply2]
+    # There needs to be a corresponding local_source and local_user
+    local_source = mock.MagicMock()
+    local_source.uuid = source.uuid
+    local_source.id = 666  # ;-)
+    local_user = mock.MagicMock()
+    local_user.username = reply_create.journalist_username
+    local_user.id = 42
+    mock_session.query().filter_by.side_effect = [[local_source, ],
+                                                  [local_user, ],
+                                                  [local_user, ], ]
+    mock_focu = mock.MagicMock(return_value=local_user)
+    with mock.patch('securedrop_client.storage.find_or_create_user',
+                    mock_focu):
+        update_replies(remote_replies, local_replies, mock_session)
+    # Check the expected local reply object has been updated with values
+    # from the API.
+    assert local_reply1.journalist_id == local_user.id
+    assert local_reply1.filename == reply_update.filename
+    assert local_reply1.size == reply_update.size
+    # Check the expected local source object has been created with values from
+    # the API.
+    assert mock_session.add.call_count == 1
+    new_reply = mock_session.add.call_args_list[0][0][0]
+    assert new_reply.uuid == reply_create.uuid
+    assert new_reply.source_id == local_source.id
+    assert new_reply.journalist_id == local_user.id
+    assert new_reply.size == reply_create.size
+    assert new_reply.filename == reply_create.filename
+    # Ensure the record for the local source that is missing from the results
+    # of the API is deleted.
+    mock_session.delete.assert_called_once_with(local_reply2)
+    # Session is committed to database.
+    assert mock_session.commit.call_count == 1
+
+
+def test_find_or_create_user_existing():
+    """
+    Return an existing user object.
+    """
+    mock_session = mock.MagicMock()
+    mock_user = mock.MagicMock()
+    mock_session.query().filter_by.return_value = [mock_user, ]
+    assert find_or_create_user('testymctestface', mock_session) == mock_user
+
+
+def test_find_or_create_user_new():
+    """
+    Create and return a user object for an unknown username.
+    """
+    mock_session = mock.MagicMock()
+    mock_session.query().filter_by.return_value = []
+    new_user = find_or_create_user('unknown', mock_session)
+    assert new_user.username == 'unknown'
+    mock_session.add.assert_called_once_with(new_user)
+    mock_session.commit.assert_called_once_with()

From 45d51067a43f178b55bf88ad9c7944b84777b9fb Mon Sep 17 00:00:00 2001
From: redshiftzero <jen@freedom.press>
Date: Fri, 21 Sep 2018 14:34:54 -0700
Subject: [PATCH 6/6] Add migration for uuid column

not adding a separate migration because we're going to squash
all of these prior to initial release
---
 .../versions/fe7656c21eaa_add_remainder_of_database_models.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/alembic/versions/fe7656c21eaa_add_remainder_of_database_models.py b/alembic/versions/fe7656c21eaa_add_remainder_of_database_models.py
index 0e8df8052..37914f9fc 100644
--- a/alembic/versions/fe7656c21eaa_add_remainder_of_database_models.py
+++ b/alembic/versions/fe7656c21eaa_add_remainder_of_database_models.py
@@ -38,13 +38,15 @@ def upgrade():
     op.create_table(
         'replies',
         sa.Column('id', sa.Integer(), nullable=False),
+        sa.Column('uuid', sa.String(length=36), nullable=False),
         sa.Column('source_id', sa.Integer(), nullable=True),
         sa.Column('journalist_id', sa.Integer(), nullable=True),
         sa.Column('filename', sa.String(length=255), nullable=False),
         sa.Column('size', sa.Integer(), nullable=False),
         sa.ForeignKeyConstraint(['journalist_id'], ['users.id'], ),
         sa.ForeignKeyConstraint(['source_id'], ['sources.id'], ),
-        sa.PrimaryKeyConstraint('id')
+        sa.PrimaryKeyConstraint('id'),
+        sa.UniqueConstraint('uuid')
     )
     op.create_table(
         'submissions',