Fix foreign keys being disabled in threads other than the creating one.

CHANGES.rst

@@ -20,11 +20,40 @@ Unreleased
 
 * Add :meth:`~Reader.copy_entry`. (:issue:`290`)
 
+* Fix bug causing :class:`Reader` operations
+  from a thread other than the one that created the instance
+  to happen with foreign key constraint enforcement disabled
+  (e.g. deleting a feed from another thread would not delete its entries).
+
+  This bug exists since using :class:`Reader` instances from other threads
+  became allowed in `2.15 <Version 2.15_>`_.
+
+  Serving the web application with ``python -m reader serve``
+  is known to be affected.
+  Serving it with uWSGI without threads (the default)
+  should not be affected.
+
+  .. attention::
+
+    **Your database may be in an inconsistent state because of this bug.**
+
+    It is recommended you run `PRAGMA foreign_key_check`_ on your database.
+
+    If you are upgrading from a version prior to 3.16
+    (i.e. were not using a pre-release version of *reader*),
+    the migration will do so for you.
+    If there are inconsistencies, you will get this error::
+
+      StorageError: integrity error: after migrating to version 43:
+        integrity error: FOREIGN KEY constraint failed
+
 * Fix :meth:`~Reader.enable_search` / :meth:`~Reader.update_search`
   not working when the search database is missing but change tracking is enabled
   (e.g. when restoring the main database from backup).
   (:issue:`362`)
 
+.. _PRAGMA foreign_key_check: https://www.sqlite.org/pragma.html#pragma_foreign_key_check
+
 
 Version 3.15
 ------------

src/reader/_storage/_base.py

@@ -57,16 +57,12 @@ def __init__(self, path: str, timeout: float | None = None):
         if timeout is not None:
             kwargs['timeout'] = timeout
 
-        self.factory = _sqlite_utils.LocalConnectionFactory(path, **kwargs)
-        db = self.factory()
-        try:
-            self.setup_db(db)
-        except BaseException:
-            db.close()
-            raise
-
-        self.path = path
-        self.timeout = timeout
+        # at least the "PRAGMA foreign_keys = ON" part of setup_db
+        # has to run for every connection (in every thread),
+        # since it's not persisted across connections
+        self.factory = _sqlite_utils.LocalConnectionFactory(
+            path, self.setup_db, **kwargs
+        )
 
     def get_db(self) -> sqlite3.Connection:
         return self.factory()

src/reader/_storage/_sqlite_utils.py

@@ -426,8 +426,11 @@ class LocalConnectionFactory:
 
     INLINE_OPTIMIZE_TIMEOUT = 0.1
 
-    def __init__(self, path: str, **kwargs: Any):
+    def __init__(
+        self, path: str, setup_db: _DBFunction = lambda _: None, **kwargs: Any
+    ):
         self.path = path
+        self.setup_db = setup_db
         self.kwargs = kwargs
         if kwargs.get('uri'):  # pragma: no cover
             raise NotImplementedError("is_private() does not work for uri=True")
@@ -463,6 +466,12 @@ def __call__(self) -> sqlite3.Connection:
         assert db is not None, "for mypy"
         self._local.call_count = 0
 
+        try:
+            self.setup_db(db)
+        except BaseException:
+            db.close()
+            raise
+
         # http://threebean.org/blog/atexit-for-threads/
         # works on cpython (finalizer runs in thread),
         # but not on pypy (finalizer runs in main thread);

tests/test_reader.py

@@ -2911,3 +2911,42 @@ def get_tags(entry):
     reader.copy_entry(src_id, dst_id)
     dst = reader.get_entry(dst_id)
     assert dst.source.title == 'user' if not src.source else src.source.title
+
+
+@pytest.mark.parametrize('same_thread', [True, False])
+def test_delete_feed(make_reader, db_path, same_thread):
+    reader = make_reader(db_path)
+    reader._parser = parser = Parser()
+    feed = parser.feed(1)
+    entry = parser.entry(1, 'feed')
+
+    reader.add_feed(feed)
+    reader.update_feeds()
+    reader.add_entry(dict(feed_url='1', id='user'))
+    reader.copy_entry(entry, ('1', 'copy'))
+    reader.set_tag(feed, 'tag', 'feed')
+    reader.set_tag(entry, 'tag', 'entry')
+
+    assert {e.resource_id for e in reader.get_entries()} == {
+        ('1', 'feed'),
+        ('1', 'user'),
+        ('1', 'copy'),
+    }
+    assert dict(reader.get_tags(feed)) == {'tag': 'feed'}
+    assert dict(reader.get_tags(entry)) == {'tag': 'entry'}
+
+    if same_thread:
+        reader.delete_feed(feed)
+    else:
+        # would cause the test to fail if foreign key enforcement
+        # was not enabled in other threads (like prior to 3.16)
+        thread = threading.Thread(target=reader.delete_feed, args=(feed,))
+        thread.start()
+        thread.join()
+
+    reader.add_feed(feed)
+    assert {e.resource_id for e in reader.get_entries()} == set()
+    assert dict(reader.get_tags(feed)) == {}
+
+    reader.update_feeds()
+    assert dict(reader.get_tags(entry)) == {}

tests/test_storage.py

@@ -111,11 +111,6 @@ def test_database_error_permissions(db_path):
         Storage(db_path)
 
 
-def test_path(db_path):
-    storage = Storage(db_path)
-    assert storage.path == db_path
-
-
 def test_timeout(db_path, monkeypatch):
     """Storage.__init__ must pass timeout= to connect."""
 
@@ -170,7 +165,7 @@ def execute(*args):
 
 
 def init(storage, _, __):
-    Storage(storage.path, timeout=0)
+    Storage(storage.factory.path, timeout=0)
 
 
 def add_feed(storage, feed, __):