diff --git a/src/job_manager.py b/src/job_manager.py index 223f4aa..564f9ba 100644 --- a/src/job_manager.py +++ b/src/job_manager.py @@ -136,12 +136,48 @@ async def _queue_has_items(self): "job_runner/full_queue at start: %s", queue_manager.format_queue(full_queue), ) + self._forget_deleted_downloads_still_in_queue(full_queue) return True self.arr.tracker.reset() logger.verbose("Removal Jobs: None triggered (Queue is empty)") return False + def _forget_deleted_downloads_still_in_queue(self, full_queue): + """ + Drop downloads from the deleted tracker that are still in the queue. + + The deleted tracker stops the same download from being removed twice + within one run, but is otherwise only cleared once the queue is empty. + A download that is still in the queue was evidently not removed, so + keeping it in the tracker would silently suppress every future removal + attempt for as long as decluttarr runs. + + Downloads handled via obsolete_tag are exempt: they are meant to stay in + the queue, so their presence is not evidence of a failed removal. + """ + queue_download_ids = {item.get("downloadId") for item in full_queue} - {None} + still_in_queue = { + download_id + for download_id in self.arr.tracker.deleted + if download_id in queue_download_ids + and download_id not in self.arr.tracker.obsolete_tagged + } + if not still_in_queue: + return + + logger.debug( + "job_manager.py/_forget_deleted_downloads_still_in_queue: " + "Marked as deleted but still in queue (removal did not take effect), " + "thus eligible for removal again: %s", + ", ".join(sorted(still_in_queue)), + ) + self.arr.tracker.deleted = [ + download_id + for download_id in self.arr.tracker.deleted + if download_id not in still_in_queue + ] + async def _download_clients_connected(self): for clients in [ self.settings.download_clients.qbittorrent, diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 122c193..d64077a 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -23,6 +23,7 @@ async def remove_downloads(self, affected_downloads, blocklist): await self._remove_download(affected_download, download_id, blocklist) elif handling_method == "obsolete_tag": await self._tag_as_obsolete(affected_download, download_id) + self.arr.tracker.obsolete_tagged.append(download_id) # Print out detailed removal messages (if any) if "removal_messages" in affected_download: diff --git a/src/settings/_instances.py b/src/settings/_instances.py index 99bd772..56046d2 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -29,6 +29,7 @@ def __init__(self): self.defective = {} self.download_progress = {} self.deleted = [] + self.obsolete_tagged = [] self.extension_checked = [] def reset(self) -> None: @@ -38,6 +39,7 @@ def reset(self) -> None: self.defective, self.download_progress, self.deleted, + self.obsolete_tagged, self.extension_checked, ): attr.clear() diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py index 7a1d774..f5101a0 100644 --- a/tests/jobs/test_job_manager.py +++ b/tests/jobs/test_job_manager.py @@ -4,6 +4,9 @@ import requests from src.job_manager import JobManager +from src.jobs.removal_handler import RemovalHandler +from src.settings._instances import Tracker +from src.utils.queue_manager import QueueManager @pytest.mark.asyncio @@ -75,3 +78,136 @@ async def test_run_download_client_jobs_handles_connection_check_request_errors( result = await manager.run_download_client_jobs() assert result is None + + +def _job_manager_with_deleted(deleted): + manager = JobManager(MagicMock()) + manager.arr = MagicMock() + manager.arr.tracker = Tracker() + manager.arr.tracker.deleted.extend(deleted) + return manager + + +def _queue_item(download_id, queue_id=1): + return { + "id": queue_id, + "downloadId": download_id, + "title": "Some.Release", + "protocol": "torrent", + "downloadClient": "qBittorrent", + } + + +@pytest.mark.asyncio +async def test_queue_has_items_forgets_deleted_downloads_that_are_back_in_queue(): + """A removal that did not stick must not block all future removals. + + The deleted tracker is otherwise only cleared once the queue is empty, so a + download that keeps coming back would be silently skipped by RemovalHandler + forever (strikes keep counting up, but no removal is ever triggered). + """ + # Arrange + manager = _job_manager_with_deleted(["still-in-queue", "actually-removed"]) + + get_queue_items = AsyncMock(return_value=[_queue_item("still-in-queue")]) + + # Act + with patch.object(QueueManager, "get_queue_items", get_queue_items): + has_items = await manager._queue_has_items() # pylint: disable=W0212 + + # Assert + assert has_items is True + assert manager.arr.tracker.deleted == ["actually-removed"] + # The orphans job that triggered this bug only shows up in the full queue + get_queue_items.assert_awaited_once_with("full") + + +@pytest.mark.asyncio +async def test_forgotten_download_is_removed_again_on_the_next_run(): + """After being forgotten, the download is removed instead of silently skipped.""" + # Arrange + manager = _job_manager_with_deleted(["comes-back"]) + manager.settings.general.public_tracker_handling = "remove" + manager.settings.download_clients.qbittorrent = [] + manager.settings.download_clients.get_download_client_by_name.return_value = ( + None, + None, + ) + manager.arr.remove_queue_item = AsyncMock() + queue_item = _queue_item("comes-back", queue_id=42) + + # Act + with patch.object( + QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item]) + ): + await manager._queue_has_items() # pylint: disable=W0212 + await RemovalHandler( + arr=manager.arr, + settings=manager.settings, + job_name="remove_metadata_missing", + ).remove_downloads( + {"comes-back": {**queue_item, "queue_ids": [queue_item["id"]]}}, + blocklist=True, + ) + + # Assert + manager.arr.remove_queue_item.assert_awaited_once_with(queue_id=42, blocklist=True) + + +@pytest.mark.asyncio +async def test_queue_has_items_keeps_deleted_downloads_that_are_gone(): + """Downloads that really were removed stay in the tracker (no double removal).""" + # Arrange + manager = _job_manager_with_deleted(["actually-removed"]) + + # Act + with patch.object( + QueueManager, + "get_queue_items", + AsyncMock(return_value=[_queue_item("something-else")]), + ): + has_items = await manager._queue_has_items() # pylint: disable=W0212 + + # Assert + assert has_items is True + assert manager.arr.tracker.deleted == ["actually-removed"] + + +@pytest.mark.asyncio +async def test_obsolete_tagged_download_is_not_tagged_again_on_the_next_run(): + """Obsolete-tagged downloads stay in the queue by design and must not be re-tagged. + + With private_tracker_handling/public_tracker_handling set to "obsolete_tag", + the download is tagged instead of removed and deliberately remains in the + queue, so its presence is not evidence of a failed removal. + """ + # Arrange + manager = _job_manager_with_deleted([]) + manager.settings.general.public_tracker_handling = "obsolete_tag" + manager.settings.general.obsolete_tag = "Obsolete" + qbit = MagicMock(ready=True, set_tag=AsyncMock()) + manager.settings.download_clients.qbittorrent = [qbit] + manager.settings.download_clients.get_download_client_by_name.return_value = ( + qbit, + "qbittorrent", + ) + queue_item = _queue_item("tagged", queue_id=7) + + # Act + for _ in range(2): + with patch.object( + QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item]) + ): + await manager._queue_has_items() # pylint: disable=W0212 + await RemovalHandler( + arr=manager.arr, + settings=manager.settings, + job_name="remove_stalled", + ).remove_downloads( + {"tagged": {**queue_item, "queue_ids": [queue_item["id"]]}}, + blocklist=False, + ) + + # Assert + qbit.set_tag.assert_awaited_once_with(tags=["Obsolete"], hashes=["tagged"]) + manager.arr.remove_queue_item.assert_not_called()