1865166 - Restrict deleting Places associated with synced bookmarks and tags

Status: NEW

(Application Services :: Places, defect, P3)

Description

[Mark Hammond [:markh] [:mhammond]]

From github: https://github.com/mozilla/application-services/issues/2790.

Both of these bump the foreign count (and, after #2695, we shouldn't ever delete directly from moz_places without checking it), but the synced bookmarks table has ON DELETE SET NULL, and tags has ON DELETE CASCADE. Both of these should change to ON DELETE RESTRICT, to make sure we don't accidentally do anything silly.

Writing a schema migration for this is going to be tricky, because SQLite's ALTER TABLE doesn't support changing foreign key constraints. We can either leave existing databases as they are...or we can abuse PRAGMA writable_schema to tighten those constraints ourselves :smiling_imp: Here's how that would work:

-- To get the raw schema for tags:
SELECT sql FROM sqlite_master
WHERE type = 'table' AND
      name = 'moz_tags_relation';
-- Then, in that SQL string, replace
-- "REFERENCES moz_places(id) ON DELETE CASCADE"
-- with "REFERENCES moz_places(id) ON DELETE RESTRICT".

-- For synced bookmarks:
SELECT sql FROM sqlite_master
WHERE type = 'table' AND
      name = 'moz_bookmarks_synced';
-- And replace
-- "REFERENCES moz_places(id) ON DELETE SET NULL"
-- with "REFERENCES moz_places(id) ON DELETE RESTRICT".

-- ⚠️ Danger! ⚠️
PRAGMA writable_schema = ON;

UPDATE sqlite_master SET
  sql = <our new SQL string>
WHERE type = 'table' AND name = <the table>;

PRAGMA writable_schema = OFF;

-- Make sure we didn't do anything silly.
PRAGMA foreign_key_check;

...What could possibly go wrong? :grin: Since it's mostly for our own benefit, I think leaving them how they are (with a comment) is wiser than messing with the master table.

┆Issue is synchronized with this Jira Task
┆Epic: Important backlog

Change performed by the Move to Bugzilla add-on.