add data migration testing and fix journalist association in replies table

[sssoleileraaa]

Description

Fixes #1149
Fixes #1068

Test Plan

1. rm -r /tmp/sdc-*
2. TESTS=tests/test_alembic.py::test_alembic_migration_upgrade_with_data make test
3. sqlite3 /tmp/sdc-<random-string>/svs.sqlite

• no journalist uuids exist in replies table
• only journalist ids exist in replies table

4. repeat steps above but for the downgrade test

[sssoleileraaa]

To demonstrate the use of the data migration test code in this PR, I added a migration test for the fix to resolve #1149.

[kushaldas]

Even after installing the latest sdk from git, I am seeing 7 errors like this one below:

homedir = '/tmp/sdc-u7_vtwa7', mocker = <pytest_mock.MockFixture object at 0x7f9e479cd9e8>

    def test_update_files(homedir, mocker):
        """
        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.
        """
        data_dir = os.path.join(homedir, "data")
        mock_session = mocker.MagicMock()
        # Source object related to the submissions.
        source = mocker.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)
>       remote_sub_update = make_remote_submission(source.uuid)

tests/test_storage.py:598: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_storage.py:82: in make_remote_submission
    uuid=str(uuid.uuid4()),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <sdclientapi.sdlocalobjects.Submission object at 0x7f9e4782afd0>, kwargs = {'download_url': 'test', 'filename': '1-submission.filename', 'is_read': False, 'size': 123, ...}
key = 'seen_by'

    def __init__(self, **kwargs) -> None:  # type: ignore
        self.download_url = ""  # type: str
        self.filename = ""  # type: str
        self.is_read = False  # type: bool
        self.size = 0  # type: int
        self.source_url = ""  # type: str
        self.source_uuid = ""  # type: str
        self.submission_url = ""  # type: str
        self.uuid = ""  # type: str
    
        if ["uuid"] == list(kwargs.keys()):
            # Means we are creating an object only for fetching from server.
            self.uuid = kwargs["uuid"]
            return
    
        for key in [
            "download_url",
            "filename",
            "is_read",
            "size",
            "source_url",
            "submission_url",
            "uuid",
            "seen_by",
        ]:
            if key not in kwargs:
                AttributeError("Missing key {}".format(key))
>           setattr(self, key, kwargs[key])
E           KeyError: 'seen_by'

.venv/lib/python3.7/site-packages/sdclientapi/sdlocalobjects.py:133: KeyError

But, that is not there on the CI and CI also has a different error.

____________ test_alembic_migration_upgrade_with_data[a4bf1f58ce69] ____________

alembic_config = '/tmp/sdc-9ptlpy4q/alembic.ini'
config = '/tmp/sdc-9ptlpy4q/config.json', migration = 'a4bf1f58ce69'
homedir = '/tmp/sdc-9ptlpy4q'

    @pytest.mark.parametrize("migration", DATA_MIGRATIONS)
    def test_alembic_migration_upgrade_with_data(alembic_config, config, migration, homedir):
        """
        Upgrade to one migration before the target migration, load data, then upgrade in order to test
        that the upgrade is successful when there is data.
        """
        migrations = list_migrations(alembic_config, migration)
        if len(migrations) == 1:
            return
        upgrade(alembic_config, migrations[-2])
        mod_name = "tests.migrations.test_{}".format(migration)
        mod = __import__(mod_name, fromlist=["UpgradeTester"])
        upgrade_tester = mod.UpgradeTester(homedir)
>       upgrade_tester.load_data()

tests/test_alembic.py:153: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/migrations/test_a4bf1f58ce69.py:42: in load_data
    journalist = self.session.query(User).filter_by(id=journalist_id).one()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <sqlalchemy.orm.query.Query object at 0x7f2888bc0a90>

    def one(self):
        """Return exactly one result or raise an exception.
    
        Raises ``sqlalchemy.orm.exc.NoResultFound`` if the query selects
        no rows.  Raises ``sqlalchemy.orm.exc.MultipleResultsFound``
        if multiple object identities are returned, or if multiple
        rows are returned for a query that returns only scalar values
        as opposed to full identity-mapped entities.
    
        Calling :meth:`.one` results in an execution of the underlying query.
    
        .. seealso::
    
            :meth:`.Query.first`
    
            :meth:`.Query.one_or_none`
    
        """
        try:
            ret = self.one_or_none()
        except orm_exc.MultipleResultsFound:
            raise orm_exc.MultipleResultsFound(
                "Multiple rows were found for one()"
            )
        else:
            if ret is None:
>               raise orm_exc.NoResultFound("No row was found for one()")
E               sqlalchemy.orm.exc.NoResultFound: No row was found for one()

.venv/lib/python3.7/site-packages/sqlalchemy/orm/query.py:3282: NoResultFound

Spent a few hours and still scratching my head on why these errors?.

[sssoleileraaa]

The first error indicates that you are using a version of the sdk that expects there to be seen_by fields for the /submissions and /replies endpoints, so I think when you installed the sdk, it didn't undo previous changes you made when you were testing that version of the sdk with the client in a previous pr:

Here's what you can do to make sure to use the version on the sdk that is specified in the requirements file:

1. pip uninstall securedrop-sdk
2. pip install --require-hashes -r requirements/dev-requirements.txt

The second error I think indicates a flaky test: I just repro'd it locally and will fix.

[sssoleileraaa]

issue was that i assumed the id of the journalist would start at 0, not 1. test has been updated accordingly.

[rmol]

The journalist UUID/ID correction migration works well. I checked out main, ran the client against the dev server, shut it down, and set all replies' journalist IDs to their journalists' UUIDs. Applying this migration corrected the replies' journalist IDs:

sqlite> select id, journalist_id from replies;
1|2
2|2
3|3
4|3
5|3
6|3
sqlite> update replies set journalist_id = (select uuid from users where id = journalist_id);
sqlite> select id, journalist_id from replies;
1|deleted
2|deleted
3|ac9a5f0c-e784-460b-9501-1f0efad53640
4|ac9a5f0c-e784-460b-9501-1f0efad53640
5|ac9a5f0c-e784-460b-9501-1f0efad53640
6|ac9a5f0c-e784-460b-9501-1f0efad53640
sqlite> 
(.venv) user@sd-dev [1149_data_migration *$] ~/src/fpf/securedrop-client $ alembic upgrade head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 7f682532afa2 -> a4bf1f58ce69, fix journalist association in replies table
(.venv) user@sd-dev [1149_data_migration *$] ~/src/fpf/securedrop-client $ sqlite3 $SDC_HOME/svs.sqlite 
SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> select id, journalist_id from replies;
1|2
2|2
3|3
4|3
5|3
6|3
sqlite> 

Did have a couple of questions about the tests. Pretty sure one is dead code, but the other is definitely at your option.

[rmol]

This looks good now, but I'll give it a final check once rebased.