Enable foreign key support

[sssoleileraaa]

Description

Right now we do not have foreign key support enabled for db.sqlite so we're not getting foreign key protections to ensure referential integrity, see https://docs.sqlalchemy.org/en/13/dialects/sqlite.html#foreign-key-support.

Steps to Reproduce

sqlite> select * from sources;
1|db9d0ad6-b45c-4293-819f-6f181b87be91|5LL6MCTO4HJRVPRTHOEUHEV7PLLCKYET2OIL3PEB2NI27EADJZENCZCECXGDKDCZRS3IZLYR44EI5KHVCOWI2WYAV65OGPG4776O4XY=|neither carelessness|0|2020-09-16 07:11:19.777508|0|4|
2|bcc07e32-a366-4335-8db2-eda19601cc3d|HAMMETVY2ZO23ACL3YBNZS47ZFYVHNSI7EFQ63LGL52WKSMXMOGYLEYIHQZ7XE6B6XU2EVRC6JDILXKPJPCPOJMONDRMDMSMEMNIZPI=|featured down|0|2020-09-16 07:11:20.087783|0|4|
sqlite> insert into source_stars (source_id, starred) values ('a', 1);
sqlite> insert into source_stars (source_id, starred) values (99999999, 1);
sqlite> select * from source_stars;
1|a|1
2|99999999|1

To enable foreign keys and see them working:

sqlite> PRAGMA foreign_keys = ON;
sqlite> insert into source_stars (source_id, starred) values ('b', 1);
Error: FOREIGN KEY constraint failed
sqlite> insert into source_stars (source_id, starred) values (66666666, 1);
Error: FOREIGN KEY constraint failed

Expected Behavior

For foreign keys to be enforced.

Actual Behavior

Foreign keys are not enforced.

[eloquence]

As part of the 9/17-10/1 sprint, we agreed during sprint planning to have a preliminary discussion and investigation focused on enabling foreign key support, e.g.:

• What do we gain by enforcing foreign key constraints?
• What changes do we need to make to existing code to do so?
• What migrations would have to be applied?
• What referential integrity violations could exist in long-running instances?
• How would we guard against unexpected side effects of making this change?

We'll set up an initial tech chat about this (probably a small group), and go from there, aiming to limit our commitment for this sprint to about about 4-8 hours total.

[eloquence]

We discussed this a bit more in the context of #5467 in the last sprint, and will likely continue that discussion at the next tech meeting. Keeping the issue on the sprint, but only to capture emerging consensus.

[eloquence]

We'll likely revisit this after we've completed the complex template consolidation effort (in the SecureDrop Workstation project, see freedomofpress/securedrop-workstation#471), and after #5467 has landed, which is one important prerequisite.

[cfm]

Just to close the loop on my own (pedantic) point of confusion during yesterday's discussion of #5467 (comment): A SQLite FOREIGNKEY constraint, enforced by PRAGMA foreign_keys = ON, will indeed reject invalid foreign-key values but accept null foreign-key values without an additional NOT NULL constraint:

sqlite> PRAGMA foreign_keys = ON;  -- as above
sqlite> insert into source_stars (source_id, starred) values ('b', 1);  -- as above
Error: FOREIGN KEY constraint failed
sqlite> insert into source_stars (source_id, starred) values (null, 1);
sqlite> SELECT * FROM source_stars;
1||1

So the reasons to avoid null source_ids and (especially, e.g. in #6192) journalist_ids are API- rather than database-level. (UUIDs, required to be both non-null and unique, are another matter. :-)

[sssoleileraaa]

@cfm - it's a good point that I missed when I created this issue. Null is a special case in sqlite and will not cause a foreign key contraint error, e.g.

sqlite> select id from journalists;
1
sqlite> update replies set journalist_id=123 where journalist_id=1;   # journalist 123 doesn't exist
Error: FOREIGN KEY constraint failed
sqlite> update replies set journalist_id=null where journalist_id=1;   # journalist <null> doesn't exist but this will work
sqlite> 

One might assume both update statements would fail due to the table constraint:FOREIGN KEY(journalist_id) REFERENCES journalists (id), but, yeah, nulls are special. I'll update my comment to avoid confusion. But we still want to enable foreign key constraints, so this is a db-level issue which I will keep open and push for us to do.

Update: I elaborate on the reason the current NULL journalist ID in the db paired with the "deleted" journalist UUID in the API is problematic here: #6192 (comment)

[zenmonkeykstop]

Moving this out of the 2.2.0 milestone (and moving #6192 in) - at this stage it's too ambitious to get it into the next release.