Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add seen tables #1164

Merged
merged 1 commit into from
Nov 5, 2020
Merged

Add seen tables #1164

merged 1 commit into from
Nov 5, 2020

Conversation

sssoleileraaa
Copy link
Contributor

Description

Fixes #1163
(In draft mode until #1162 is merged)

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
  • query all three seen tables and see that they exist
  1. repeat steps 1-2 above but for the downgrade test
  • seen tables no longer exist

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HEAD is at

✦ ❯ git log
commit aa8e485531f6e6976131a3131e164edf5cbfcd11 (HEAD -> add-seen-tables, origin/add-seen-tables)
Author: Allie Crevier <allie@freedom.press>
Date:   Wed Oct 21 18:30:31 2020 -0700

    add seen tables and test migration

The test command TESTS=tests/test_alembic.py::test_alembic_migration_upgrade_with_data make test passes,
but none of the seen tables is seen in the database.

✦ ❯ sqlite3 /tmp/sdc-vywvim7u/svs.sqlite 
SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> .schema
CREATE TABLE alembic_version (
	version_num VARCHAR(32) NOT NULL, 
	CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num)
);
CREATE TABLE sources (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	journalist_designation VARCHAR(255) NOT NULL, 
	document_count INTEGER DEFAULT 0 NOT NULL, 
	is_flagged BOOLEAN DEFAULT 0, 
	public_key TEXT, 
	fingerprint VARCHAR(64), 
	interaction_count INTEGER DEFAULT 0 NOT NULL, 
	is_starred BOOLEAN DEFAULT 0, 
	last_updated DATETIME, 
	CONSTRAINT pk_sources PRIMARY KEY (id), 
	CONSTRAINT uq_sources_uuid UNIQUE (uuid), 
	CONSTRAINT ck_sources_is_flagged CHECK (is_flagged IN (0, 1)), 
	CONSTRAINT ck_sources_is_starred CHECK (is_starred IN (0, 1))
);
CREATE TABLE users (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	username VARCHAR(255) NOT NULL, firstname VARCHAR(64), lastname VARCHAR(64), 
	CONSTRAINT pk_users PRIMARY KEY (id), 
	CONSTRAINT uq_users_uuid UNIQUE (uuid)
);
CREATE TABLE replysendstatuses (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_replysendstatuses PRIMARY KEY (id), 
	CONSTRAINT uq_replysendstatuses_name UNIQUE (name)
);
CREATE TABLE draftreplies (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	timestamp DATETIME NOT NULL, 
	source_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	file_counter INTEGER NOT NULL, 
	content TEXT, 
	send_status_id INTEGER, 
	CONSTRAINT pk_draftreplies PRIMARY KEY (id), 
	CONSTRAINT uq_draftreplies_uuid UNIQUE (uuid), 
	CONSTRAINT fk_draftreplies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id), 
	CONSTRAINT fk_draftreplies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_draftreplies_send_status_id_replysendstatuses FOREIGN KEY(send_status_id) REFERENCES replysendstatuses (id)
);
CREATE TABLE downloaderrors (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_downloaderrors PRIMARY KEY (id), 
	CONSTRAINT uq_downloaderrors_name UNIQUE (name)
);
CREATE TABLE files (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT files_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_files PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_files_uuid UNIQUE (uuid),
        CONSTRAINT ck_files_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_files_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_files_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_files_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_files_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
);
CREATE TABLE messages (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT messages_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        content TEXT CONSTRAINT ck_message_compare_download_vs_content CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_messages PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_messages_uuid UNIQUE (uuid),
        CONSTRAINT ck_messages_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_messages_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_messages_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_messages_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_messages_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
    );
CREATE TABLE replies (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        source_id INTEGER NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER,
        content TEXT,
        is_decrypted BOOLEAN,
        is_downloaded BOOLEAN,
        download_error_id INTEGER,
        journalist_id INTEGER,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_replies PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_replies_uuid UNIQUE (uuid),
        CONSTRAINT fk_replies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id),
        CONSTRAINT fk_replies_download_error_id_downloaderrors
            FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT fk_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id),
        CONSTRAINT replies_compare_download_vs_content
            CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        CONSTRAINT replies_compare_is_downloaded_vs_is_decrypted
            CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        CONSTRAINT ck_replies_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT ck_replies_is_downloaded CHECK (is_downloaded IN (0, 1))
    );

After digging more into the migrations and the test, I found the following migrations are being applied
into the database during the test.

tests/test_alembic.py::test_alembic_migration_upgrade_with_data[a4bf1f58ce69]
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 2f363b3d680e, init
INFO  [alembic.runtime.migration] Running upgrade 2f363b3d680e -> fecf1191b6f0, remove decryption_vs_content contraint
INFO  [alembic.runtime.migration] Running upgrade fecf1191b6f0 -> bafdcae12f97, Add files.original_filename
INFO  [alembic.runtime.migration] Running upgrade bafdcae12f97 -> 36a79ffcfbfb, add first_name, last_name, fullname, initials
INFO  [alembic.runtime.migration] Running upgrade 36a79ffcfbfb -> 86b01b6290da, add reply draft
INFO  [alembic.runtime.migration] Running upgrade 86b01b6290da -> fb657f2ee8a7, drop File.original_filename
INFO  [alembic.runtime.migration] Running upgrade fb657f2ee8a7 -> 7f682532afa2, add download error
main: /tmp/sdc-jnwhx3r2/svs.sqlite
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
PASSED

This is because the seen tables are in bd57477f19a2 revision, but DATA_MIGRATIONS variable is pointing to a4bf1f58ce69, which is
the previous revision. If I manually update the migration revision in the test using the following patch, I get a new set of errors, but the seen tables are now present.

diff --git a/tests/test_alembic.py b/tests/test_alembic.py
index 2a406dc..e142692 100644
--- a/tests/test_alembic.py
+++ b/tests/test_alembic.py
@@ -137,7 +137,7 @@ def test_alembic_migration_upgrade(alembic_config, config, migration):
         upgrade(alembic_config, mig)
 
 
-@pytest.mark.parametrize("migration", DATA_MIGRATIONS)
+@pytest.mark.parametrize("migration", ["bd57477f19a2",])
 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
@@ -146,6 +146,7 @@ def test_alembic_migration_upgrade_with_data(alembic_config, config, migration,
     migrations = list_migrations(alembic_config, migration)
     if len(migrations) == 1:
         return
+    print("reached here")
     upgrade(alembic_config, migrations[-2])
     mod_name = "tests.migrations.test_{}".format(migration)
     mod = __import__(mod_name, fromlist=["UpgradeTester"])

The test command and the error:

 python -m pytest -vv -s tests/test_alembic.py::test_alembic_migration_upgrade_with_data 
============================================================== test session starts ===============================================================
platform linux -- Python 3.7.3, pytest-5.2.1, py-1.7.0, pluggy-0.13.0 -- /home/kdas/code/securedrop-client/.venv/bin/python
cachedir: .pytest_cache
PyQt5 5.11.3 -- Qt runtime 5.11.2 -- Qt compiled 5.11.2
Test order randomisation NOT enabled. Enable with --random-order or --random-order-bucket=<bucket_type>
rootdir: /home/kdas/code/securedrop-client, inifile: pytest.ini
plugins: qt-3.3.0, xdist-1.30.0, mock-1.10.0, forked-1.1.3, flaky-3.6.1, cov-2.8.1, vcr-1.0.2, random-order-1.0.4
collected 1 item                                                                                                                                 

tests/test_alembic.py::test_alembic_migration_upgrade_with_data[bd57477f19a2] reached here
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 2f363b3d680e, init
INFO  [alembic.runtime.migration] Running upgrade 2f363b3d680e -> fecf1191b6f0, remove decryption_vs_content contraint
INFO  [alembic.runtime.migration] Running upgrade fecf1191b6f0 -> bafdcae12f97, Add files.original_filename
INFO  [alembic.runtime.migration] Running upgrade bafdcae12f97 -> 36a79ffcfbfb, add first_name, last_name, fullname, initials
INFO  [alembic.runtime.migration] Running upgrade 36a79ffcfbfb -> 86b01b6290da, add reply draft
INFO  [alembic.runtime.migration] Running upgrade 86b01b6290da -> fb657f2ee8a7, drop File.original_filename
INFO  [alembic.runtime.migration] Running upgrade fb657f2ee8a7 -> 7f682532afa2, add download error
INFO  [alembic.runtime.migration] Running upgrade 7f682532afa2 -> a4bf1f58ce69, fix journalist association in replies table
main: /tmp/sdc-ng2c5tmw/svs.sqlite
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade a4bf1f58ce69 -> bd57477f19a2, add seen tables
FAILED

==================================================================== FAILURES ====================================================================
_____________________________________________ test_alembic_migration_upgrade_with_data[bd57477f19a2] _____________________________________________

alembic_config = '/tmp/sdc-ng2c5tmw/alembic.ini', config = '/tmp/sdc-ng2c5tmw/config.json', migration = 'bd57477f19a2'
homedir = '/tmp/sdc-ng2c5tmw'

    @pytest.mark.parametrize("migration", ["bd57477f19a2",])
    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
        print("reached here")
        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()
        upgrade(alembic_config, migration)
>       upgrade_tester.check_upgrade()

tests/test_alembic.py:156: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/migrations/test_bd57477f19a2.py:85: in check_upgrade
    self.session.query(User).filter_by(id=reply.journalist_id).one()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

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

    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

The database schema:

SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> .schema
CREATE TABLE alembic_version (
	version_num VARCHAR(32) NOT NULL, 
	CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num)
);
CREATE TABLE sources (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	journalist_designation VARCHAR(255) NOT NULL, 
	document_count INTEGER DEFAULT 0 NOT NULL, 
	is_flagged BOOLEAN DEFAULT 0, 
	public_key TEXT, 
	fingerprint VARCHAR(64), 
	interaction_count INTEGER DEFAULT 0 NOT NULL, 
	is_starred BOOLEAN DEFAULT 0, 
	last_updated DATETIME, 
	CONSTRAINT pk_sources PRIMARY KEY (id), 
	CONSTRAINT uq_sources_uuid UNIQUE (uuid), 
	CONSTRAINT ck_sources_is_flagged CHECK (is_flagged IN (0, 1)), 
	CONSTRAINT ck_sources_is_starred CHECK (is_starred IN (0, 1))
);
CREATE TABLE users (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	username VARCHAR(255) NOT NULL, firstname VARCHAR(64), lastname VARCHAR(64), 
	CONSTRAINT pk_users PRIMARY KEY (id), 
	CONSTRAINT uq_users_uuid UNIQUE (uuid)
);
CREATE TABLE replysendstatuses (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_replysendstatuses PRIMARY KEY (id), 
	CONSTRAINT uq_replysendstatuses_name UNIQUE (name)
);
CREATE TABLE draftreplies (
	id INTEGER NOT NULL, 
	uuid VARCHAR(36) NOT NULL, 
	timestamp DATETIME NOT NULL, 
	source_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	file_counter INTEGER NOT NULL, 
	content TEXT, 
	send_status_id INTEGER, 
	CONSTRAINT pk_draftreplies PRIMARY KEY (id), 
	CONSTRAINT uq_draftreplies_uuid UNIQUE (uuid), 
	CONSTRAINT fk_draftreplies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id), 
	CONSTRAINT fk_draftreplies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_draftreplies_send_status_id_replysendstatuses FOREIGN KEY(send_status_id) REFERENCES replysendstatuses (id)
);
CREATE TABLE downloaderrors (
	id INTEGER NOT NULL, 
	name VARCHAR(36) NOT NULL, 
	CONSTRAINT pk_downloaderrors PRIMARY KEY (id), 
	CONSTRAINT uq_downloaderrors_name UNIQUE (name)
);
CREATE TABLE files (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT files_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_files PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_files_uuid UNIQUE (uuid),
        CONSTRAINT ck_files_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_files_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_files_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_files_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_files_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
);
CREATE TABLE messages (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER NOT NULL,
        download_url VARCHAR(255) NOT NULL,
        is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
        is_decrypted BOOLEAN CONSTRAINT messages_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        download_error_id INTEGER,
        is_read BOOLEAN DEFAULT 0 NOT NULL,
        content TEXT CONSTRAINT ck_message_compare_download_vs_content CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        source_id INTEGER NOT NULL,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_messages PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_messages_uuid UNIQUE (uuid),
        CONSTRAINT ck_messages_is_downloaded CHECK (is_downloaded IN (0, 1)),
        CONSTRAINT ck_messages_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT fk_messages_download_error_id_downloaderrors FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT ck_messages_is_read CHECK (is_read IN (0, 1)),
        CONSTRAINT fk_messages_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id)
    );
CREATE TABLE replies (
        id INTEGER NOT NULL,
        uuid VARCHAR(36) NOT NULL,
        source_id INTEGER NOT NULL,
        filename VARCHAR(255) NOT NULL,
        file_counter INTEGER NOT NULL,
        size INTEGER,
        content TEXT,
        is_decrypted BOOLEAN,
        is_downloaded BOOLEAN,
        download_error_id INTEGER,
        journalist_id INTEGER,
        last_updated DATETIME NOT NULL,
        CONSTRAINT pk_replies PRIMARY KEY (id),
        CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
        CONSTRAINT uq_replies_uuid UNIQUE (uuid),
        CONSTRAINT fk_replies_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id),
        CONSTRAINT fk_replies_download_error_id_downloaderrors
            FOREIGN KEY(download_error_id) REFERENCES downloaderrors (id),
        CONSTRAINT fk_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id),
        CONSTRAINT replies_compare_download_vs_content
            CHECK (CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END),
        CONSTRAINT replies_compare_is_downloaded_vs_is_decrypted
            CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
        CONSTRAINT ck_replies_is_decrypted CHECK (is_decrypted IN (0, 1)),
        CONSTRAINT ck_replies_is_downloaded CHECK (is_downloaded IN (0, 1))
    );
CREATE TABLE seen_files (
	id INTEGER NOT NULL, 
	file_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_files PRIMARY KEY (id), 
	CONSTRAINT fk_seen_files_file_id_files FOREIGN KEY(file_id) REFERENCES files (id), 
	CONSTRAINT fk_seen_files_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT uq_seen_files_file_id UNIQUE (file_id, journalist_id)
);
CREATE TABLE seen_messages (
	id INTEGER NOT NULL, 
	message_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_messages PRIMARY KEY (id), 
	CONSTRAINT fk_seen_messages_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_seen_messages_message_id_messages FOREIGN KEY(message_id) REFERENCES messages (id), 
	CONSTRAINT uq_seen_messages_message_id UNIQUE (message_id, journalist_id)
);
CREATE TABLE seen_replies (
	id INTEGER NOT NULL, 
	reply_id INTEGER NOT NULL, 
	journalist_id INTEGER, 
	CONSTRAINT pk_seen_replies PRIMARY KEY (id), 
	CONSTRAINT fk_seen_replies_journalist_id_users FOREIGN KEY(journalist_id) REFERENCES users (id), 
	CONSTRAINT fk_seen_replies_reply_id_replies FOREIGN KEY(reply_id) REFERENCES replies (id), 
	CONSTRAINT uq_seen_replies_reply_id UNIQUE (reply_id, journalist_id)
);

`

@sssoleileraaa
Copy link
Contributor Author

I see the same error when I run test_alembic_migration_upgrade_with_data[bd57477f19a2] which we weren't seeing fail in CI earlier because it wasn't included. Now that it's included, you can see the same issue you discovered in CI.

This is the first time we'll be using the new migration test code with more than one migration, so time for a little debugging since the issue is likely within that code (not with the seen-tables migration itself since it's quite simple and works if you run through the test plan).

@sssoleileraaa sssoleileraaa force-pushed the add-seen-tables branch 2 times, most recently from d2bdf0b to 67c729c Compare November 4, 2020 18:26
@sssoleileraaa
Copy link
Contributor Author

Addressed Kushal's comment above. Pushing it back to ready for review.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test plan looks good to me, and @kushaldas ' findings seem to be addressed in the latest revision. Tested as follows:

  • upgrade revision: query all three seen tables and see that they exist
  • downgrade revision: seen tables no longer exist
  • starting the client locally and inspecting the database.
  • a deb package on this branch upgrade the client in sd-app with a populated database and inspecting the local database.

Functionally, the changes here look good to me, I have a few questions inline.

tests/migrations/test_bd57477f19a2.py Outdated Show resolved Hide resolved
tests/migrations/test_bd57477f19a2.py Show resolved Hide resolved
tests/migrations/test_bd57477f19a2.py Show resolved Hide resolved
tests/migrations/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks !

@emkll emkll dismissed kushaldas’s stale review November 5, 2020 18:29

Comments have been addressed, test plan is now passing

@emkll emkll merged commit 97cc9f9 into main Nov 5, 2020
@emkll emkll deleted the add-seen-tables branch November 5, 2020 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add seen tables for read/unread
3 participants