Make sources.filesystem_id non-nullable

We already set this field all the time and in all places expect it to be
a string, not checking if its None first.

The database migration will delete any sources that have a NULL filesystem_id,
plus anything that references those sources.

Discovered by the mypy work I'm doing in #6346.

Refs #6226.

securedrop/alembic/versions/b7f98cfd6a70_make_filesystem_id_non_nullable.py

@@ -0,0 +1,58 @@
+"""make_filesystem_id_non_nullable
+
+Revision ID: b7f98cfd6a70
+Revises: d9d36b6f4d1e
+Create Date: 2022-03-18 18:10:27.842201
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'b7f98cfd6a70'
+down_revision = 'd9d36b6f4d1e'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # Not having a filesystem_id makes the source useless, so if any of those do exist, we'll
+    # delete them first, as part of this migration.
+    # Because we can't rely on SQLAlchemy's cascade deletion, we have to do it manually.
+    # First we delete out of replies/seen_files/seen_messages (things that refer to things that refer
+    # to sources)
+    op.execute("DELETE FROM seen_replies WHERE reply_id IN ("
+               "SELECT replies.id FROM replies "
+               "JOIN sources ON sources.id=replies.source_id "
+               "WHERE filesystem_id IS NULL)")
+    op.execute("DELETE FROM seen_files WHERE file_id IN ("
+               "SELECT submissions.id FROM submissions "
+               "JOIN sources ON sources.id=submissions.source_id "
+               "WHERE filesystem_id IS NULL)")
+    op.execute("DELETE FROM seen_messages WHERE message_id IN ("
+               "SELECT submissions.id FROM submissions "
+               "JOIN sources ON sources.id=submissions.source_id "
+               "WHERE filesystem_id IS NULL)")
+    # Now things that directly refer to sources
+    for table in ("source_stars", "submissions", "replies"):
+        op.execute(
+            f"DELETE FROM {table} WHERE source_id IN "
+            f"(SELECT id FROM sources WHERE filesystem_id IS NULL)")
+    # And now the sources
+    op.execute("DELETE FROM sources WHERE filesystem_id IS NULL")
+    with op.batch_alter_table('sources', schema=None) as batch_op:
+        batch_op.alter_column(
+            'filesystem_id',
+            existing_type=sa.VARCHAR(length=96),
+            nullable=False
+        )
+
+
+def downgrade() -> None:
+    with op.batch_alter_table('sources', schema=None) as batch_op:
+        batch_op.alter_column(
+            'filesystem_id',
+            existing_type=sa.VARCHAR(length=96),
+            nullable=True
+        )

securedrop/models.py

@@ -71,7 +71,7 @@ class Source(db.Model):
     __tablename__ = 'sources'
     id = Column(Integer, primary_key=True)
     uuid = Column(String(36), unique=True, nullable=False)
-    filesystem_id = Column(String(96), unique=True)
+    filesystem_id = Column(String(96), unique=True, nullable=False)
     journalist_designation = Column(String(255), nullable=False)
     last_updated = Column(DateTime)
     star = relationship(

securedrop/tests/migrations/migration_b7f98cfd6a70.py

@@ -0,0 +1,97 @@
+import uuid
+
+from sqlalchemy import text
+
+from db import db
+from journalist_app import create_app
+
+
+# Random, chosen by fair dice roll
+FILESYSTEM_ID = 'FPLIUY3FWFROQ52YHDMXFYKOTIK2FT4GAFN6HTCPPG3TSNAHYOPDDT5C3TR' \
+                'JWDV2IL3JDOS4NFAJNEI73KRQ7HQEVNAF35UCCW5M7VI='
+
+
+class UpgradeTester:
+    """Insert a source, verify the filesystem_id makes it through untouched
+    """
+
+    def __init__(self, config):
+        """This function MUST accept an argument named `config`.
+           You will likely want to save a reference to the config in your
+           class, so you can access the database later.
+        """
+        self.config = config
+        self.app = create_app(config)
+
+    def load_data(self):
+        """This function loads data into the database and filesystem. It is
+           executed before the upgrade.
+        """
+        with self.app.app_context():
+            sources = [
+                {
+                    'uuid': str(uuid.uuid4()),
+                    'filesystem_id': FILESYSTEM_ID,
+                    'journalist_designation': 'sunburned arraignment',
+                    'interaction_count': 0,
+                },
+                {
+                    'uuid': str(uuid.uuid4()),
+                    'filesystem_id': None,
+                    'journalist_designation': 'needy transponder',
+                    'interaction_count': 0,
+                }
+            ]
+            sql = """\
+                INSERT INTO sources (uuid, filesystem_id, journalist_designation, interaction_count)
+                VALUES (:uuid, :filesystem_id, :journalist_designation, :interaction_count)"""
+            for params in sources:
+                db.engine.execute(text(sql), **params)
+            # Insert a source_stars row associated each source
+            for source_id in (1, 2):
+                db.engine.execute(
+                    text("INSERT INTO source_stars (source_id, starred) "
+                         "VALUES (:source_id, :starred)"),
+                    {"source_id": source_id, "starred": True}
+                )
+
+    def check_upgrade(self):
+        """This function is run after the upgrade and verifies the state
+           of the database or filesystem. It MUST raise an exception if the
+           check fails.
+        """
+        with self.app.app_context():
+            # Verify remaining single source is the one with a non-NULL filesystem_id
+            sources = db.engine.execute(
+                'SELECT filesystem_id FROM sources'
+            ).fetchall()
+            assert len(sources) == 1
+            assert sources[0][0] == FILESYSTEM_ID
+            # Verify that the source_stars #2 row got deleted
+            stars = db.engine.execute('SELECT source_id FROM source_stars').fetchall()
+            assert stars == [(1,)]
+
+
+class DowngradeTester:
+    """Downgrading only makes fields nullable again, which is a
+    non-destructive and safe operation"""
+
+    def __init__(self, config):
+        """This function MUST accept an argument named `config`.
+           You will likely want to save a reference to the config in your
+           class, so you can access the database later.
+        """
+        self.config = config
+
+    def load_data(self):
+        """This function loads data into the database and filesystem. It is
+           executed before the downgrade.
+        """
+        pass
+
+    def check_downgrade(self):
+        """This function is run after the downgrade and verifies the state
+           of the database or filesystem. It MUST raise an exception if the
+           check fails.
+        """
+        pass