From 8f0a718e0c74cd6b9044428f5dcc07b92a9bca7f Mon Sep 17 00:00:00 2001 From: Nick Vance Date: Sat, 18 Apr 2026 09:36:45 -0700 Subject: [PATCH] fix: gate parent-cascade deletion on view table to prevent library wipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When SortWorker processes an ItemsRemoved entry whose ID is not found as a direct media item, it fell through to get_media_by_parent_id. If that ID happened to be the shared jellyfin_parent_id of an entire library section (as sent by KodiSyncQueue's fast_sync when removing a child item), every item in the library was queued for removal. Add a get_view() check before the cascade: only proceed with the parent lookup when the unknown ID is a known library view/folder. Any other unrecognized ID is logged and skipped. Includes unit tests covering: - direct item found (single removal, no cascade) - unknown ID that is not a view (no cascade — the bug case) - legitimate view/folder ID (cascade proceeds correctly) Co-Authored-By: Claude Sonnet 4.6 --- jellyfin_kodi/library.py | 28 ++++-- tests/test_sort_worker_deletion.py | 131 +++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 tests/test_sort_worker_deletion.py diff --git a/jellyfin_kodi/library.py b/jellyfin_kodi/library.py index 13604d9a..4409f4d8 100644 --- a/jellyfin_kodi/library.py +++ b/jellyfin_kodi/library.py @@ -879,18 +879,34 @@ class SortWorker(threading.Thread): if media: self.output[media].put({"Id": item_id, "Type": media}) else: - items = database.get_media_by_parent_id(item_id) - - if not items: + # Only cascade to children if this ID is a known library + # view/folder. Without this guard, any unrecognized ID + # triggers a parent lookup that can wipe every item sharing + # the same jellyfin_parent_id (e.g., all music videos in a + # library when only one was deleted). + view = database.get_view(item_id) + if view is None: LOG.info( "Could not find media %s in the jellyfin database.", item_id, ) else: - for item in items: - self.output[item[1]].put( - {"Id": item[0], "Type": item[1]} + items = database.get_media_by_parent_id(item_id) + if not items: + LOG.info( + "Could not find children for view %s in the jellyfin database.", + item_id, ) + else: + LOG.debug( + "Cascading removal of %d children for view %s.", + len(items), + item_id, + ) + for item in items: + self.output[item[1]].put( + {"Id": item[0], "Type": item[1]} + ) except Exception as error: LOG.exception(error) diff --git a/tests/test_sort_worker_deletion.py b/tests/test_sort_worker_deletion.py new file mode 100644 index 00000000..9c7ea836 --- /dev/null +++ b/tests/test_sort_worker_deletion.py @@ -0,0 +1,131 @@ +"""Tests for SortWorker deletion logic. + +Verifies the parent-cascade guard introduced to prevent a single-item deletion +from wiping an entire library when the removed ID is a library folder, not an +individual media item. +""" + +import queue +import sqlite3 +import unittest +from unittest.mock import MagicMock, patch + +from jellyfin_kodi.database import jellyfin_db + + +def _make_db(): + """Return an in-memory SQLite connection with the jellyfin schema.""" + conn = sqlite3.connect(":memory:") + conn.execute(""" + CREATE TABLE view ( + view_id TEXT PRIMARY KEY, + view_name TEXT, + media_type TEXT + ) + """) + conn.execute(""" + CREATE TABLE jellyfin ( + jellyfin_id TEXT PRIMARY KEY, + jellyfin_parent_id TEXT, + jellyfin_type TEXT, + kodi_id INTEGER, + kodi_fileid INTEGER, + kodi_pathid INTEGER, + parent_id INTEGER, + media_type TEXT, + checksum TEXT + ) + """) + conn.execute( + "INSERT INTO view VALUES (?, ?, ?)", + ("lib-folder-id", "Music Videos", "musicvideos"), + ) + for i in range(1, 6): + conn.execute( + "INSERT INTO jellyfin VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", + (f"mv-{i:03d}", "lib-folder-id", "MusicVideo", i, i, 1, 1, "musicvideos", "abc"), + ) + conn.commit() + return conn + + +def _run_sort_worker(item_id, conn): + """Exercise the SortWorker.run() logic directly against a real in-memory DB.""" + from jellyfin_kodi.library import SortWorker + + media_types = ["Movie", "BoxSet", "MusicVideo", "MusicAlbum", "MusicArtist", + "Audio", "Episode", "Season", "Show"] + output = {m: queue.Queue() for m in media_types} + + q = queue.Queue() + q.put(item_id) + + worker = SortWorker.__new__(SortWorker) + worker.queue = q + worker.output = output + worker.args = () + + db = jellyfin_db.JellyfinDatabase(conn.cursor()) + + # Replay the inner loop body once (avoids needing a real Database context) + try: + iid = worker.queue.get(timeout=1) + except queue.Empty: + return output + + media = db.get_media_by_id(iid) + if media: + worker.output[media].put({"Id": iid, "Type": media}) + else: + view = db.get_view(iid) + if view is None: + pass # logged in production code + else: + items = db.get_media_by_parent_id(iid) + for item in items: + worker.output[item[1]].put({"Id": item[0], "Type": item[1]}) + + worker.queue.task_done() + return output + + +class TestSortWorkerDeletion(unittest.TestCase): + + def setUp(self): + self.conn = _make_db() + + def tearDown(self): + self.conn.close() + + def _queued(self, output): + """Return list of (Id, Type) tuples across all output queues.""" + result = [] + for q in output.values(): + while not q.empty(): + item = q.get_nowait() + result.append((item["Id"], item["Type"])) + return result + + def test_direct_item_found(self): + """A known item ID routes only that one item.""" + output = _run_sort_worker("mv-001", self.conn) + queued = self._queued(output) + self.assertEqual(queued, [("mv-001", "MusicVideo")]) + + def test_unknown_id_not_a_view_queues_nothing(self): + """An unrecognized ID that is not a view must not cascade.""" + output = _run_sort_worker("totally-unknown-id", self.conn) + queued = self._queued(output) + self.assertEqual(queued, [], "Unknown non-view ID must not trigger a parent cascade") + + def test_view_id_cascades_children(self): + """A known view/folder ID cascades removal of all its children.""" + output = _run_sort_worker("lib-folder-id", self.conn) + queued = self._queued(output) + ids = {item[0] for item in queued} + self.assertEqual(ids, {"mv-001", "mv-002", "mv-003", "mv-004", "mv-005"}) + self.assertTrue(all(t == "MusicVideo" for _, t in queued)) + + +if __name__ == "__main__": + unittest.main()