diff --git a/README.md b/README.md index 43f19a1..b15caa6 100644 --- a/README.md +++ b/README.md @@ -455,7 +455,11 @@ This is the interesting section. It defines which job you want decluttarr to run - Steers whether slow downloads are removed from the queue - Blocklisted: Yes -- Note: Does not apply to usenet downloads (since there users pay for certain speed, slowness should not occur) +- Note: + - Does not apply to usenet downloads (since there users pay for certain speed, slowness should not occur) + - Applies only if qBittorrent is configured: The remove_slow check is automatically temporarily disabled if qBittorrent is already using more than 80% of your available download bandwidth. + For this to work, you must set a Global Download Rate Limit in qBittorrent. Otherwise, unlimited capacity is assumed, and the auto-disable feature will never trigger. + Make sure to configure the limit in the correct place — either the standard or the alternative limits, depending on which one is active in your setup. - Type: Boolean or Dict - Permissible Values: If bool: True, False diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 9846245..feb2387 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -29,7 +29,7 @@ class RemovalHandler: # Print out detailed removal messages (if any) if "removal_messages" in affected_download: for msg in affected_download["removal_messages"]: - logger.info(msg) + logger.verbose(msg) self.arr.tracker.deleted.append(download_id) diff --git a/src/jobs/remove_slow.py b/src/jobs/remove_slow.py index 0e3e3e3..9d82c9c 100644 --- a/src/jobs/remove_slow.py +++ b/src/jobs/remove_slow.py @@ -1,6 +1,7 @@ from src.jobs.removal_job import RemovalJob from src.utils.log_setup import logger +DISABLE_OVER_BANDWIDTH_USAGE = 0.8 class RemoveSlow(RemovalJob): queue_scope = "normal" @@ -10,8 +11,12 @@ class RemoveSlow(RemovalJob): affected_items = [] checked_ids = set() + # Refreshes bandwidth usage for each client + await self.add_download_client_to_queue_items() + await self.update_bandwidth_usage() + for item in self.queue: - if not self._is_valid_item(item): + if not self._check_required_keys(item): continue download_id = item["downloadId"] @@ -29,8 +34,11 @@ class RemoveSlow(RemovalJob): ) continue + if self._high_bandwidth_usage(download_client=item["download_client"], download_client_type=item["download_client_type"]): + continue + downloaded, previous, increment, speed = await self._get_progress_stats( - item, + item ) if self._is_slow(speed): affected_items.append(item) @@ -43,8 +51,8 @@ class RemoveSlow(RemovalJob): return affected_items @staticmethod - def _is_valid_item(item) -> bool: - required_keys = {"downloadId", "size", "sizeleft", "status", "protocol"} + def _check_required_keys(item) -> bool: + required_keys = {"downloadId", "size", "sizeleft", "status", "protocol", "download_client", "download_client_type"} return required_keys.issubset(item) @staticmethod @@ -68,7 +76,7 @@ class RemoveSlow(RemovalJob): async def _get_progress_stats(self, item): download_id = item["downloadId"] - download_progress = self._get_download_progress(item, download_id) + download_progress = await self._get_download_progress(item, download_id) previous_progress, increment, speed = self._compute_increment_and_speed( download_id, download_progress, ) @@ -76,36 +84,53 @@ class RemoveSlow(RemovalJob): self.arr.tracker.download_progress[download_id] = download_progress return download_progress, previous_progress, increment, speed - def _get_download_progress(self, item, download_id): - download_client_name = item.get("downloadClient") - if download_client_name: - download_client, download_client_type = self.settings.download_clients.get_download_client_by_name(download_client_name) - if download_client_type == "qbitorrent": - progress = self._try_get_qbit_progress(download_client, download_id) + + async def _get_download_progress(self, item, download_id): + # Grabs the progress from qbit if possible, else calculates it based on progress (imprecise) + if item["download_client_type"] == "qbittorrent": + try: + progress = await item["download_client"].fetch_download_progress(download_id) if progress is not None: return progress - return self._fallback_progress(item) - - @staticmethod - def _try_get_qbit_progress(qbit, download_id): - # noinspection PyBroadException - try: - return qbit.get_download_progress(download_id) - except Exception: # noqa: BLE001 - return None - - @staticmethod - def _fallback_progress(item): - logger.debug( - "get_progress_stats: Using imprecise method to determine download increments because either a different download client than qBitorrent is used, or the download client name in the config does not match with what is configured in your *arr download client settings", - ) + except Exception: # noqa: BLE001 + pass # fall back below return item["size"] - item["sizeleft"] def _compute_increment_and_speed(self, download_id, current_progress): + # Calculates the increment based on progress since last check previous_progress = self.arr.tracker.download_progress.get(download_id) if previous_progress is not None: increment = current_progress - previous_progress speed = round(increment / 1000 / (self.settings.general.timer * 60), 1) else: + # don't calculate a speed delta the first time a download comes up as it may not have done a full cycle increment = speed = None return previous_progress, increment, speed + + @staticmethod + def _high_bandwidth_usage(download_client, download_client_type): + if download_client_type == "qbittorrent": + if download_client.bandwidth_usage > DISABLE_OVER_BANDWIDTH_USAGE: + return True + return False + + async def add_download_client_to_queue_items(self): + # Adds the download client to the queue item + for item in self.queue: + download_client_name = item["downloadClient"] + download_client, download_client_type = self.settings.download_clients.get_download_client_by_name(download_client_name) + item["download_client"] = download_client + item["download_client_type"] = download_client_type + + + async def update_bandwidth_usage(self): + # Refreshes the current bandwidth usage for each client + processed_clients = set() + + for item in self.queue: + download_client = item["download_client"] + if item["download_client"] in processed_clients: + continue + if item["download_client_type"] == "qbittorrent": + await download_client.set_bandwidth_usage() + processed_clients.add(item["download_client"]) diff --git a/src/jobs/strikes_handler.py b/src/jobs/strikes_handler.py index c0998aa..9c3dda0 100644 --- a/src/jobs/strikes_handler.py +++ b/src/jobs/strikes_handler.py @@ -61,6 +61,6 @@ class StrikesHandler: self.job_name, strikes, self.max_strikes, title, ) logger.info( - '>>> [Tip!] Since this download should already have been removed in a previous iteration but keeps coming back, this indicates the blocking of the torrent does not work correctly. Consider turning on the option "Reject Blocklisted Torrent Hashes While Grabbing" on the indexer in the *arr app: %s', + '>>> 💡 Tip: Since this download should already have been removed in a previous iteration but keeps coming back, this indicates the blocking of the torrent does not work correctly. Consider turning on the option "Reject Blocklisted Torrent Hashes While Grabbing" on the indexer in the *arr app: %s', title, ) diff --git a/src/settings/_download_clients_qbit.py b/src/settings/_download_clients_qbit.py index 15564bc..004e4c9 100644 --- a/src/settings/_download_clients_qbit.py +++ b/src/settings/_download_clients_qbit.py @@ -1,7 +1,7 @@ from packaging import version from src.settings._constants import ApiEndpoints, MinVersions -from src.utils.common import make_request, wait_and_exit +from src.utils.common import make_request, wait_and_exit, extract_json_from_response from src.utils.log_setup import logger @@ -37,6 +37,7 @@ class QbitClient: cookie: dict[str, str] = None version: str = None + bandwidth_usage: int = 0 def __init__( self, @@ -172,7 +173,10 @@ class QbitClient: ) endpoint = f"{self.api_url}/app/preferences" response = await make_request( - "get", endpoint, self.settings, cookies=self.cookie, + "get", + endpoint, + self.settings, + cookies=self.cookie, ) qbit_settings = response.json() @@ -255,6 +259,7 @@ class QbitClient: # Continue with other setup tasks regardless of version check result await self.create_required_tags() await self.set_unwanted_folder() + await self.warn_no_bandwidth_limit_set() async def get_protected_and_private(self): """Fetch torrents from qBittorrent and checks for protected and private status.""" @@ -335,7 +340,7 @@ class QbitClient: cookies=self.cookie, ) - async def get_download_progress(self, download_id): + async def fetch_download_progress(self, download_id): items = await self.get_qbit_items(download_id) return items[0]["completed"] @@ -383,3 +388,33 @@ class QbitClient: data=data, cookies=self.cookie, ) + + async def set_bandwidth_usage(self): + # Gets the current overall bandwidth consumption + logger.debug("_download_clients_qBit/get_bandwidth_usage") + response = await make_request( + method="get", + endpoint=self.api_url + "/transfer/info", + settings=self.settings, + cookies=self.cookie, + ) + records = extract_json_from_response(response) + limit = records["dl_rate_limit"] + speed = records["dl_info_speed"] + if limit == 0: + self.bandwidth_usage = 0 + else: + self.bandwidth_usage = speed / limit + return limit, speed + + async def warn_no_bandwidth_limit_set(self): + logger.debug("_download_clients_qBit/warn_no_bandwidth_limit_set") + if self.settings.jobs.remove_slow.enabled: + limit, _ = await self.set_bandwidth_usage() + if limit == 0: + logger.info( + "💡 Tip: No global download speed limit is set in your qBittorrent instance. " + "If you configure one, the 'remove_slow' check will automatically disable itself " + "when your bandwidth is fully utilized. This prevents slow downloads from being mistakenly removed — " + "not because they lack seeds, but because your own download capacity is saturated." + ) diff --git a/tests/jobs/test_remove_slow.py b/tests/jobs/test_remove_slow.py index 82b6fe9..c6b82b6 100644 --- a/tests/jobs/test_remove_slow.py +++ b/tests/jobs/test_remove_slow.py @@ -1,10 +1,11 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, AsyncMock import pytest -from tests.jobs.utils import shared_fix_affected_items, shared_test_affected_items +from tests.jobs.utils import shared_fix_affected_items from src.jobs.remove_slow import RemoveSlow +# pylint: disable=W0212 @pytest.mark.asyncio @pytest.mark.parametrize( ("item", "expected_result"), @@ -17,142 +18,338 @@ from src.jobs.remove_slow import RemoveSlow "sizeleft": 500, "status": "downloading", "protocol": "torrent", + "download_client": AsyncMock(), + "download_client_type": "qBittorrent", }, True, ), ( - # Invalid: missing sizeleft - { - "downloadId": "abc", - "size": 1000, - "status": "downloading", - "protocol": "torrent", - }, - False, - ), - ( - # Invalid: missing size - { - "downloadId": "abc", - "sizeleft": 500, - "status": "downloading", - "protocol": "torrent", - }, - False, - ), - ( - # Invalid: missing status - {"downloadId": "abc", "size": 1000, "sizeleft": 500, "protocol": "torrent"}, - False, - ), - ( - # Invalid: missing protocol + # Invalid: missing one { "downloadId": "abc", "size": 1000, "sizeleft": 500, "status": "downloading", + "protocol": "torrent", + "download_client": AsyncMock(), + }, + False, + ), + ( + # Invalid: missing multiple + { + "size": 1000, + "sizeleft": 500, }, False, ), ], ) -async def test_is_valid_item(item, expected_result): - # Arrange +async def test_check_required_keys(item, expected_result): removal_job = shared_fix_affected_items(RemoveSlow) - - # Act - result = removal_job._is_valid_item(item) # pylint: disable=W0212 - - # Assert + result = removal_job._check_required_keys(item) assert result == expected_result -@pytest.fixture(name="queue_data") -def fixture_queue_data(): - return [ - { - "downloadId": "usenet", - "progress_previous": 800, # previous progress - "progress_now": 800, # current progress - "total_size": 1000, - "protocol": "usenet", # should be ignored - }, - { - "downloadId": "importing", - "progress_previous": 0, - "progress_now": 1000, - "total_size": 1000, - "protocol": "torrent", - }, - { - "downloadId": "stuck", - "progress_previous": 200, - "progress_now": 200, - "total_size": 1000, - "protocol": "torrent", - }, - { - "downloadId": "slow", - "progress_previous": 100, - "progress_now": 150, - "total_size": 1000, - "protocol": "torrent", - }, - { - "downloadId": "medium", - "progress_previous": 500, - "progress_now": 900, - "total_size": 1000, - "protocol": "torrent", - }, - { - "downloadId": "fast", - "progress_previous": 100, - "progress_now": 900, - "total_size": 1000, - "protocol": "torrent", - }, - ] +@pytest.mark.parametrize( + ("item", "expected_result"), + [ + ({"protocol": "usenet"}, True), + ({"protocol": "torrent"}, False), + ({}, False), + ], +) +def test_is_usenet(item, expected_result): + removal_job = shared_fix_affected_items(RemoveSlow) + result = removal_job._is_usenet(item) + assert result == expected_result + + +@pytest.mark.parametrize( + ("item", "expected_result"), + [ + ({"status": "downloading", "size": 1000, "sizeleft": 0}, True), + ({"status": "completed", "size": 1000, "sizeleft": 0}, False), + ({"status": "downloading", "size": 0, "sizeleft": 0}, False), + ], +) +def test_is_completed_but_stuck(item, expected_result): + removal_job = shared_fix_affected_items(RemoveSlow) + result = removal_job._is_completed_but_stuck(item) + assert result == expected_result + + +@pytest.mark.parametrize( + ("speed", "expected_result"), + [ + (None, False), # speed is None -> not slow + (0, True), # speed less than min_speed -> slow (assuming min_speed > 0) + (5, True), # speed less than min_speed + (10, False), # speed equal or above min_speed (assuming min_speed=10) + (15, False), # speed above min_speed + ], +) +def test_is_slow(speed, expected_result): + removal_job = shared_fix_affected_items(RemoveSlow) + removal_job.job.min_speed = 10 + result = removal_job._is_slow(speed) + assert result == expected_result @pytest.mark.asyncio @pytest.mark.parametrize( - ("min_speed", "expected_download_ids"), + "client_type, expected_progress", [ - (0, []), # No min download speed; all torrents pass - (500, ["stuck"]), # Only stuck and slow are included - (1000, ["stuck", "slow"]), # Same as above - (10000, ["stuck", "slow", "medium"]), # Only stuck and slow are below 5.0 - (1000000, ["stuck", "slow", "medium", "fast"]), # Fast torrent included (but not importing) + ("something_else", 100), + ( + "qbittorrent", + 800, + ), # qbit is more updated, has progressed from 100 to 800 since arr refreshed last ], ) -async def test_find_affected_items_with_varied_speeds( - queue_data, min_speed, expected_download_ids, -): - # Arrange - removal_job = shared_fix_affected_items(RemoveSlow, queue_data) +async def test_get_download_progress(client_type, expected_progress): + mock_client = AsyncMock() + mock_client.fetch_download_progress.return_value = 800 - # Job-specific arrangements - removal_job.job = MagicMock() - removal_job.job.min_speed = min_speed - removal_job.settings = MagicMock() - removal_job.settings.general.timer = 1 - - removal_job._is_valid_item = MagicMock(return_value=True) # Mock the _is_valid_item method to always return True # pylint: disable=W0212 - - # Inject size and sizeleft into each item in the queue - for item in queue_data: - item["size"] = item["total_size"] * 1000000 # Inject total size as 'size' - item["sizeleft"] = item["size"] - item["progress_now"] * 1000000 # Calculate sizeleft - item["status"] = "downloading" - item["title"] = item["downloadId"] - - # Mock the download progress in `arr.tracker.download_progress` - removal_job.arr.tracker.download_progress = { - item["downloadId"]: item["progress_previous"] * 1000000 - for item in queue_data + item = { + "download_client_type": client_type, + "download_client": mock_client, + "size": 1000, + "sizeleft": 900, } - # Act - await shared_test_affected_items(removal_job, expected_download_ids) + removal_job = shared_fix_affected_items(RemoveSlow) + result = await removal_job._get_download_progress(item, "some_id") + + assert result == expected_progress + + +@pytest.mark.parametrize( + "download_id, tracker_data, current_progress, expected", + [ + ( + "id1", + {"id1": 10_000_000}, # previous_progress = 10 MB + 16_000_000, # current_progress = 16 MB + (10_000_000, 6_000_000, 100), # 6 MB in 1 min => 100 MB/h + ), # increment case + ("id2", {}, 800, (None, None, None)), # no previous_progress + ], +) +def test_compute_increment_and_speed( + download_id, tracker_data, current_progress, expected +): + removal_job = shared_fix_affected_items(RemoveSlow) + removal_job.arr.tracker.download_progress = tracker_data + removal_job.settings.general.timer = 1 # 1 minute interval + + result = removal_job._compute_increment_and_speed(download_id, current_progress) + assert result == expected + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "item, previous_progress, mock_progress, expected_increment, expected_speed", + [ + ( + {"downloadId": "id1"}, + 1_000_000, + 1_600_000, + 600_000, + 10.0, + ), + ( + {"downloadId": "id2"}, + None, + 800_000, + None, + None, + ), + ], +) +async def test_get_progress_stats( + item, previous_progress, mock_progress, expected_increment, expected_speed +): + """ + Test `_get_progress_stats` to ensure it correctly returns the current progress, + previous progress, increment, and calculated speed. It also verifies that the + download progress is updated in the tracker after execution. + + - If a previous progress value exists in the tracker, the increment and speed + are calculated. + - If no previous value exists, increment and speed should be None. + """ + removal_job = shared_fix_affected_items(RemoveSlow) + + # Ensure tracker dict is initialized properly + removal_job.arr.tracker.download_progress = {} + + download_id = item["downloadId"] + if previous_progress is not None: + removal_job.arr.tracker.download_progress[download_id] = previous_progress + + # Mock _get_download_progress to return a high fixed value + removal_job._get_download_progress = AsyncMock(return_value=mock_progress) + removal_job.settings.general.timer = 1 # 1-minute interval + + result = await removal_job._get_progress_stats(item) + + expected = ( + mock_progress, + previous_progress, + expected_increment, + expected_speed, + ) + assert result == expected + assert removal_job.arr.tracker.download_progress[download_id] == mock_progress + + +@pytest.mark.parametrize( + "download_client_type, bandwidth_usage, expected", + [ + ("qbittorrent", 0.81, True), # above threshold 0.8 + ("qbittorrent", 0.8, False), # equal to threshold 0.8 + ("qbittorrent", 0.79, False), # below threshold 0.8 + ("other_client", 0.9, False), # different client type + ], +) +def test_high_bandwidth_usage(download_client_type, bandwidth_usage, expected): + """ + Test RemoveSlow._high_bandwidth_usage method. + + Checks if the method correctly identifies high bandwidth usage + only when the download client type is 'qbittorrent' and the + bandwidth usage exceeds the defined threshold (0.8). + For other client types or bandwidth usage below or equal to threshold, + it should return False. + """ + + class DummyClient: + def __init__(self, usage): + self.bandwidth_usage = usage + + download_client = DummyClient(bandwidth_usage) + result = RemoveSlow._high_bandwidth_usage(download_client, download_client_type) + assert result == expected + + +@pytest.mark.asyncio +async def test_add_download_client_to_queue_items_simple(): + """ + Test that 'add_download_client_to_queue_items' correctly adds + the download client object and its type to each queue item, + based on the client's name retrieved from settings. + """ + removal_job = shared_fix_affected_items(RemoveSlow) + client_name = "MyQbitInstance" + download_client_type = "qbittorrent" + removal_job.queue = [{"downloadClient": client_name}] + + dummy_client = MagicMock(name="QBClient") + removal_job.settings.download_clients = MagicMock() + removal_job.settings.download_clients.get_download_client_by_name = MagicMock( + return_value=(dummy_client, download_client_type) + ) + + await removal_job.add_download_client_to_queue_items() + + item = removal_job.queue[0] + assert item["download_client"] == dummy_client + assert item["download_client_type"] == download_client_type + + + +@pytest.mark.asyncio +async def test_update_bandwidth_usage_calls_once_per_client(): + """ + Test that 'update_bandwidth_usage' calls 'set_bandwidth_usage' exactly once + per unique download client of type 'qbittorrent' in the queue, + and does not call it for other client types. + """ + removal_job = shared_fix_affected_items(RemoveSlow) + + # Create two dummy clients + qb_client1 = MagicMock(name="QBClient1") + qb_client1.set_bandwidth_usage = AsyncMock() + qb_client2 = MagicMock(name="QBClient2") + qb_client2.set_bandwidth_usage = AsyncMock() + other_client = MagicMock(name="OtherClient") + other_client.set_bandwidth_usage = AsyncMock() + + removal_job.queue = [ + {"download_client": qb_client1, "download_client_type": "qbittorrent"}, + { + "download_client": qb_client1, + "download_client_type": "qbittorrent", + }, # duplicate client + {"download_client": qb_client2, "download_client_type": "qbittorrent"}, + {"download_client": other_client, "download_client_type": "other"}, + ] + + await removal_job.update_bandwidth_usage() + + # Verify set_bandwidth_usage called once per unique qbittorrent client + qb_client1.set_bandwidth_usage.assert_awaited_once() + qb_client2.set_bandwidth_usage.assert_awaited_once() + # Verify other client method was not called + other_client.set_bandwidth_usage.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "queue_item, should_be_affected", + [ + # Keys not present -> skip + ({"downloadClient": "client1"}, False), + + # Already checked downloadId -> skip (simulate by repeating downloadId) + ({"downloadId": "checked_before", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, False), + + # Is Usenet -> skip + ({"downloadId": "usenet", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, False), + + # Completed but stuck -> skip + ({"downloadId": "stuck", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, False), + + # High bandwidth usage -> skip + ({"downloadId": "highbw", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, False), + + # Not slow -> skip + ({"downloadId": "notslow", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, False), + + # None of above, should be affected + ({"downloadId": "good", "title": "Good Item", "download_client": MagicMock(), "download_client_type": "qbittorrent"}, True), + ], +) +async def test_find_affected_items_simple(queue_item, should_be_affected): + # Add minimum fields required + queue_item["title"] = queue_item.get("downloadId", "dummy") + removal_job = shared_fix_affected_items(RemoveSlow, queue_data=[queue_item]) + + # Setup queue differently based on test case + if queue_item.get("downloadId") == "dup": + # Add duplicate entries to test skipping by checked_ids + removal_job.queue = [queue_item, queue_item] + else: + removal_job.queue = [queue_item] + + # Mock methods + removal_job._check_required_keys = MagicMock(return_value="downloadId" in queue_item) + removal_job._is_usenet = MagicMock(return_value=queue_item.get("downloadId") == "usenet") + removal_job._is_completed_but_stuck = MagicMock(return_value=queue_item.get("downloadId") == "stuck") + removal_job._high_bandwidth_usage = MagicMock(return_value=queue_item.get("downloadId") == "highbw") + removal_job._get_progress_stats = AsyncMock(return_value=(1000, 900, 100, 10)) # arbitrary numbers + removal_job._is_slow = MagicMock(return_value=queue_item.get("downloadId") == "good") + + # Mock add_download_client_to_queue_items and update_bandwidth_usage as no-ops + removal_job.add_download_client_to_queue_items = AsyncMock() + removal_job.update_bandwidth_usage = AsyncMock() + + # Run the method under test + affected_items = await removal_job._find_affected_items() + + if should_be_affected: + assert affected_items, f"Item {queue_item.get('downloadId')} should be affected" + assert affected_items[0]["downloadId"] == queue_item["downloadId"] + else: + assert not affected_items, f"Item {queue_item.get('downloadId')} should NOT be affected"