diff --git a/AppriseClient.py b/AppriseClient.py index f4d821f..8187a58 100755 --- a/AppriseClient.py +++ b/AppriseClient.py @@ -11,19 +11,16 @@ def apprise_notify(req_obj, host, port, aurls, title, body): class AppriseClient: def __init__(self): self.config = '' - try: - if os.environ["DOCKER"]: - self.host = os.environ["host"] - self.port = os.environ["port"] - self.aurls = os.environ["aurls"] - self.title = os.environ["title"] - self.body = os.environ["body"] - if os.environ["toml_path"]: - config_file_path=os.environ["toml_path"] - with open(config_file_path, 'rb') as c: - self.config = load(c) - except: - KeyError + if os.environ.get("DOCKER"): + self.host = os.environ["host"] + self.port = os.environ["port"] + self.aurls = os.environ["aurls"] + self.title = os.environ["title"] + self.body = os.environ["body"] + if os.environ.get("toml_path"): + config_file_path=os.environ["toml_path"] + with open(config_file_path, 'rb') as c: + self.config = load(c) if os.path.exists('./config.toml'): config_file_path = './config.toml' with open(config_file_path, 'rb') as c: @@ -37,4 +34,4 @@ class AppriseClient: self.apprise_response = apprise_notify(r,self.host,self.port,self.aurls,self.title,self.body) if __name__ == "__main__": - AppriseClient() \ No newline at end of file + AppriseClient() diff --git a/config.toml.example b/config.toml.example index 0a4c750..2471b71 100755 --- a/config.toml.example +++ b/config.toml.example @@ -50,6 +50,10 @@ aurls = 'mailto://user:pass@gmail.com' enable_dragnet = false dragnet_outfile = "./orphaned.csv" +[telemetry] +enable_telemetry = false +telemetry_outfile = "./telemetry.csv" + [ignored_categories] tech = "tech" books = "books" diff --git a/qbit-maid.py b/qbit-maid.py index 914f583..06c8641 100755 --- a/qbit-maid.py +++ b/qbit-maid.py @@ -89,7 +89,7 @@ class Qbt: self.tracker_whitelist = self.config["ignored_domains"] #ignored_tags - self.ignored_tags = self.config["ignored_domains"] + self.ignored_tags = self.config["ignored_tags"] #healthcheck self.use_healthcheck = self.config["healthcheck"]["use_healthcheck"] @@ -117,7 +117,7 @@ class Qbt: self.tl.info('Connected.') except qbittorrentapi.APIError as e: self.tl.exception(e) - self.po.send_message(e, title="qbit-maid API ERROR") + tor_notify_error(self, e) # Pulling all torrent data self.torrent_list = self.qbt_client.torrents_info() #Main process block diff --git a/qlist.py b/qlist.py index 5664175..387c128 100755 --- a/qlist.py +++ b/qlist.py @@ -22,24 +22,22 @@ def build_tor_list(self): self.ignored_counter += 1 continue if is_tag_blank(torrent['tags']): - self.tl.debug(f'Tagging new torrent: ["{torrent["name"][0:20]}..."] {torrent["tracker"]}hash: {torrent["hash"]}') - if is_protected_tracker(torrent['tracker'], self.tracker_whitelist.values()): - self.qbt_client.torrents_add_tags(self.tracker_protected_tag,torrent['hash']) - elif is_not_protected_tracker(torrent['tracker'], self.tracker_whitelist.values()): - self.qbt_client.torrents_add_tags(self.tracker_non_protected_tag,torrent['hash']) + tag = torrent_tag_for( + torrent['tracker'], + self.tracker_protected_tag, + self.tracker_non_protected_tag, + self.tracker_whitelist.values(), + ) + if tag: + self.tl.debug(f'Tagging new torrent: ["{torrent["name"][0:20]}..."] {torrent["tracker"]}hash: {torrent["hash"]}') + self.qbt_client.torrents_add_tags(tag, torrent['hash']) if is_preme(torrent['seeding_time'], self.min_age): self.preme_tor_counter += 1 self.tl.debug(f'Premature torrent: ["{torrent["name"][0:20]}..."] Seconds Seeded: [{torrent["seeding_time"]}] hash: {torrent["hash"]}') continue elif is_protected_tracker(torrent['tracker'], self.tracker_whitelist.values()): - if is_tag_blank(torrent['tags']): - self.qbt_client.torrents_add_tags(self.tracker_protected_tag,torrent['hash']) - self.tl.debug(f'Tagging Protected torrent: ["{torrent["name"][0:20]}..."] {torrent["tracker"]}hash: {torrent["hash"]}') self.tracker_list.append(torrent) elif is_not_protected_tracker(torrent['tracker'], self.tracker_whitelist.values()): - if is_tag_blank(torrent['tags']): - self.qbt_client.torrents_add_tags(self.tracker_non_protected_tag,torrent['hash']) - self.tl.debug(f'Tagging Non-protected torrent: ["{torrent["name"][0:20]}..."] {torrent["tracker"]}hash: {torrent["hash"]}') self.tracker_list.append(torrent) @@ -67,11 +65,26 @@ def is_not_protected_tracker(tracker, trackerlist): if tracker.split('/')[2] not in trackerlist: return True +def torrent_tag_for(tracker, protected_tag, non_protected_tag, trackerlist): + """Return the tag a torrent should carry, or None for an unknown tracker.""" + if is_protected_tracker(tracker, trackerlist): + return protected_tag + if is_not_protected_tracker(tracker, trackerlist): + return non_protected_tag + return None + def is_tag_blank(tag): if tag == '': return True def is_ignored_tag(igtags, tortags): - for igt in igtags: - if igt in tortags: - return True \ No newline at end of file + """True when the torrent carries one of the configured ignored tags. + + qBittorrent returns tags as a comma-separated string, so the comparison is + per tag: a configured ``ipt`` must not match a torrent tagged + ``iptorrents``. + """ + if not tortags: + return False + torrent_tags = {tag.strip() for tag in str(tortags).split(',') if tag.strip()} + return any(igt in torrent_tags for igt in igtags) \ No newline at end of file diff --git a/qlogging.py b/qlogging.py index e461876..ec0af1b 100755 --- a/qlogging.py +++ b/qlogging.py @@ -77,9 +77,30 @@ def tor_notify_summary(self): except Exception as e: # never let a notification failure abort a run logger.error(f"Failed to send pushover notification: {e}") +def tor_notify_error(self, error, title="qbit-maid API ERROR"): + """Report an error through pushover when it is configured. + + The vendored ``pushover`` module only exposes the ``Pushover`` class, so the + message has to go through the client built by :func:`tor_notify`. + """ + logger = _logger(self) + logger.error(f"{title}: {error}") + if not getattr(self, "use_pushover", False): + return + client = getattr(self, "poc", None) + if client is None: + return + try: + client.message(self.po_key, str(error), title=title) + except Exception as exc: # never let a notification failure abort a run + logger.error(f"Failed to send pushover error notification: {exc}") + def list_first_tor(self, index=0): """Only lists the first torrent""" self.tl.debug('First torrent in the list:') + if not self.torrent_list: + self.tl.debug('No torrents found.') + return torrent = self.torrent_list[index] for k,v in torrent.items(): self.tl.debug(f'{k}: {v}') @@ -337,6 +358,7 @@ def setup_logging(use_log: bool, log_path: str, log_level: str, # Make setup idempotent: clear any handlers from a previous call for handler in list(logger.handlers): logger.removeHandler(handler) + handler.close() if not use_log: logger.addHandler(logging.NullHandler()) diff --git a/test_apprise.py b/test_apprise.py index c404162..0af6321 100644 --- a/test_apprise.py +++ b/test_apprise.py @@ -143,8 +143,6 @@ class TestAppriseClientConfig(unittest.TestCase): return config_path def test_toml_path_is_read_when_docker_is_set(self): - # The DOCKER branch reads five env vars before toml_path is reached, so - # all of them must be present for the toml_path branch to run at all. os.environ.update({ "DOCKER": "1", "host": "env-host", @@ -158,15 +156,12 @@ class TestAppriseClientConfig(unittest.TestCase): self.assertEqual(self.captured["host"], "toml-host") self.assertEqual(self.captured["port"], 4321) - def test_toml_path_is_ignored_without_docker(self): - """KNOWN BUG: the bare ``except`` swallows the DOCKER KeyError and - skips the rest of the try block, so ``toml_path`` is never read unless - ``DOCKER`` is also set. With no ./config.toml either, the client then - crashes on the missing ``host`` attribute. - """ + def test_toml_path_is_read_without_docker(self): os.environ["toml_path"] = self._write_elsewhere_config() - with self.assertRaises(AttributeError): - AppriseClient() + AppriseClient() + self.assertEqual(self.captured["host"], "toml-host") + self.assertEqual(self.captured["port"], 4321) + self.assertEqual(self.captured["title"], "Toml Title") if __name__ == "__main__": diff --git a/test_qbitmaid.py b/test_qbitmaid.py index ce91b13..1c89f4d 100755 --- a/test_qbitmaid.py +++ b/test_qbitmaid.py @@ -113,13 +113,16 @@ class TestPredicates(unittest.TestCase): self.assertTrue(is_ignored_tag(["save"], "save,public,ipt")) self.assertFalse(is_ignored_tag(["save"], "public,ipt")) - def test_is_ignored_tag_matches_substrings_not_tags(self): - """KNOWN BUG: matching is substring-based, so a short tag matches any - longer tag that contains it. 'ipt' matches 'iptorrents' and 'save' - matches 'unsaved', which silently ignores unrelated torrents. + def test_is_ignored_tag_matches_whole_tags_only(self): + """A configured tag must match a whole tag, not a substring of one. + + 'ipt' must not match 'iptorrents' and 'save' must not match 'unsaved'. """ - self.assertTrue(is_ignored_tag(["ipt"], "iptorrents")) - self.assertTrue(is_ignored_tag(["save"], "unsaved")) + self.assertFalse(is_ignored_tag(["ipt"], "iptorrents")) + self.assertFalse(is_ignored_tag(["save"], "unsaved")) + self.assertTrue(is_ignored_tag(["ipt"], "save,ipt")) + self.assertTrue(is_ignored_tag(["save"], " save , public")) + self.assertFalse(is_ignored_tag(["save"], "")) class TestSendPing(unittest.TestCase): diff --git a/test_qbt.py b/test_qbt.py index c8c94e1..b5c7b05 100644 --- a/test_qbt.py +++ b/test_qbt.py @@ -13,7 +13,13 @@ import types import unittest from unittest import mock -from qbt_fakes import FakeAPIError, FakeQbtClient, StubPushover, torrent +from qbt_fakes import ( + FailingPushover, + FakeAPIError, + FakeQbtClient, + StubPushover, + torrent, +) REPO = os.path.dirname(os.path.abspath(__file__)) @@ -200,13 +206,21 @@ class TestQbtPipeline(QbtTestCase): self.assertIsNotNone(qbt.et) self.assertGreaterEqual(qbt.et, qbt.st) - def test_empty_torrent_list_crashes(self): - """KNOWN BUG: list_first_tor() indexes torrent_list[0] unconditionally, - so a qBittorrent instance with zero torrents raises IndexError and the - whole run aborts before any summary is sent. - """ - with self.assertRaises(IndexError): - self.build([]) + def test_empty_torrent_list_is_handled(self): + """A qBittorrent instance with zero torrents must not abort the run.""" + qbt, client = self.build([]) + self.assertEqual(qbt.total_torrents, 0) + self.assertEqual(qbt.torrent_hash_delete_list, []) + self.assertEqual(client.deleted, []) + + def test_empty_torrent_list_still_sends_a_summary(self): + stub = StubPushover() + with mock.patch.object(self.module.pushover, "Pushover", + return_value=stub): + self.build([], use_pushover="true") + self.assertEqual(len(stub.calls), 1) + self.assertEqual(stub.calls[0]["kwargs"]["title"], + "qbit-maid: nothing removed") class TestQbtConfigSources(QbtTestCase): @@ -281,23 +295,32 @@ class TestQbtLoginFailure(QbtTestCase): def test_login_error_notifies_and_continues(self): client = FakeQbtClient(torrents=[torrent(tags="ipt")], login_error=FakeAPIError("bad credentials")) - with mock.patch.object(self.module.pushover, "send_message", - create=True) as send_message: - qbt, _ = self.build([torrent(tags="ipt")], client=client) - send_message.assert_called_once() - self.assertEqual(send_message.call_args.kwargs["title"], + stub = StubPushover() + with mock.patch.object(self.module.pushover, "Pushover", + return_value=stub): + qbt, _ = self.build([torrent(tags="ipt")], client=client, + use_pushover="true") + # The error notification goes out first, then the run summary. + self.assertEqual(len(stub.calls), 2) + self.assertEqual(stub.calls[0]["kwargs"]["title"], "qbit-maid API ERROR") + self.assertIn("bad credentials", stub.calls[0]["body"]) self.assertEqual(qbt.total_torrents, 1) - def test_login_error_crashes_without_a_send_message_helper(self): - """KNOWN BUG: the vendored pushover module has no module-level - ``send_message``, so the error path raises AttributeError instead of - notifying anyone. - """ + def test_login_error_without_pushover_does_not_crash(self): client = FakeQbtClient(torrents=[torrent(tags="ipt")], login_error=FakeAPIError("bad credentials")) - with self.assertRaises(AttributeError): - self.build([torrent(tags="ipt")], client=client) + qbt, _ = self.build([torrent(tags="ipt")], client=client) + self.assertEqual(qbt.total_torrents, 1) + + def test_login_error_survives_a_failing_pushover(self): + client = FakeQbtClient(torrents=[torrent(tags="ipt")], + login_error=FakeAPIError("bad credentials")) + with mock.patch.object(self.module.pushover, "Pushover", + return_value=FailingPushover()): + qbt, _ = self.build([torrent(tags="ipt")], client=client, + use_pushover="true") + self.assertEqual(qbt.total_torrents, 1) class TestMainGuard(QbtTestCase): @@ -312,14 +335,15 @@ class TestMainGuard(QbtTestCase): class TestIgnoredTagsConfig(QbtTestCase): - def test_ignored_tags_reads_the_ignored_domains_section(self): - """KNOWN BUG: qbit-maid.py assigns ``ignored_tags`` from - ``config["ignored_domains"]``, so the ``[ignored_tags]`` section is - never read and tag-based ignoring silently does nothing. - """ + def test_ignored_tags_reads_the_ignored_tags_section(self): qbt, _ = self.build([torrent(tags="ipt")]) - self.assertEqual(qbt.ignored_tags, {"tracker-example": "tracker.example"}) - self.assertNotIn("save", qbt.ignored_tags) + self.assertEqual(qbt.ignored_tags, {"save": "save"}) + self.assertNotIn("tracker-example", qbt.ignored_tags) + + def test_ignored_tag_from_config_is_skipped(self): + qbt, _ = self.build([torrent(tags="save", infohash="ignored-tag")]) + self.assertEqual(qbt.ignored_counter, 1) + self.assertEqual(qbt.tracker_list, []) if __name__ == "__main__": diff --git a/test_qlist.py b/test_qlist.py index f2e4dfe..15fe7bd 100644 --- a/test_qlist.py +++ b/test_qlist.py @@ -2,7 +2,28 @@ import unittest from qbt_fakes import FakeQbt, torrent -from qlist import build_tor_list +from qlist import build_tor_list, torrent_tag_for + + +class TestTorrentTagFor(unittest.TestCase): + def test_protected_tracker_gets_the_protected_tag(self): + self.assertEqual( + torrent_tag_for("https://tracker.example/a", "ipt", "public", + ["tracker.example"]), + "ipt", + ) + + def test_unknown_tracker_gets_the_public_tag(self): + self.assertEqual( + torrent_tag_for("https://other.example/a", "ipt", "public", + ["tracker.example"]), + "public", + ) + + def test_blank_tracker_gets_no_tag(self): + self.assertIsNone( + torrent_tag_for("", "ipt", "public", ["tracker.example"]) + ) class TestBuildTorList(unittest.TestCase): @@ -36,6 +57,14 @@ class TestBuildTorList(unittest.TestCase): self.assertEqual(qbt.ignored_counter, 1) self.assertEqual(qbt.tracker_list, []) + def test_ignored_tag_does_not_match_a_longer_tag(self): + qbt = self.run_build( + [torrent(tags="iptorrents", tracker="https://other.example/a")], + ignored_tags={"ipt": "ipt"}, + ) + self.assertEqual(qbt.ignored_counter, 0) + self.assertEqual(len(qbt.tracker_list), 1) + def test_blank_tag_on_protected_tracker_gets_protected_tag(self): qbt = self.run_build([torrent(tags="", infohash="h1")]) self.assertIn(("ipt", "h1"), qbt.qbt_client.added_tags) @@ -48,16 +77,10 @@ class TestBuildTorList(unittest.TestCase): self.assertIn(("public", "h2"), qbt.qbt_client.added_tags) self.assertEqual(len(qbt.tracker_list), 1) - def test_blank_tag_is_applied_twice_for_non_premature_torrents(self): - """KNOWN REDUNDANCY: the tagging block runs twice for the same torrent. - - Lines 24-29 tag any blank-tag torrent; lines 34-43 tag it again inside - the tracker branch. qBittorrent treats the second call as a no-op, so - the outcome is correct but one API call per new torrent is wasted. - Pinned here so the duplication is visible rather than silent. - """ + def test_blank_tag_is_applied_once_for_non_premature_torrents(self): + """A blank-tag torrent is tagged once, not once per code path.""" qbt = self.run_build([torrent(tags="", infohash="h1")]) - self.assertEqual(qbt.qbt_client.added_tags, [("ipt", "h1"), ("ipt", "h1")]) + self.assertEqual(qbt.qbt_client.added_tags, [("ipt", "h1")]) def test_premature_blank_tag_torrent_is_tagged_once(self): """Premature torrents are tagged by the first block, then skipped.""" diff --git a/test_qlogging_extra.py b/test_qlogging_extra.py index 2024e5c..44760d2 100644 --- a/test_qlogging_extra.py +++ b/test_qlogging_extra.py @@ -5,10 +5,9 @@ import os import shutil import tempfile import unittest -import warnings from collections import Counter -from qbt_fakes import FakeQbt, StubPushover, torrent +from qbt_fakes import FailingPushover, FakeQbt, StubPushover, torrent from qlogging import ( LOGGER_NAME, debug_torrent_list, @@ -24,6 +23,7 @@ from qlogging import ( summary_runtime, summary_stats, tor_notify, + tor_notify_error, tor_notify_summary, tornotifytest, torrent_count, @@ -76,6 +76,35 @@ class TestNotifySetup(unittest.TestCase): self.assertEqual(qbt.poc.calls[0]["kwargs"]["title"], "qbit-maid") +class TestTorNotifyError(unittest.TestCase): + def test_sends_through_the_pushover_client(self): + qbt = FakeQbt(use_pushover=True) + tor_notify_error(qbt, RuntimeError("boom")) + self.assertEqual(len(qbt.poc.calls), 1) + self.assertEqual(qbt.poc.calls[0]["user"], "USERKEY") + self.assertIn("boom", qbt.poc.calls[0]["body"]) + self.assertEqual(qbt.poc.calls[0]["kwargs"]["title"], + "qbit-maid API ERROR") + + def test_skips_pushover_when_disabled(self): + qbt = FakeQbt(use_pushover=False) + tor_notify_error(qbt, RuntimeError("boom")) + self.assertEqual(qbt.poc.calls, []) + + def test_skips_pushover_when_no_client_was_built(self): + qbt = FakeQbt(use_pushover=True, poc=None) + tor_notify_error(qbt, RuntimeError("boom")) # must not raise + + def test_survives_a_failing_pushover(self): + qbt = FakeQbt(use_pushover=True, poc=FailingPushover()) + tor_notify_error(qbt, RuntimeError("boom")) # must not raise + + def test_accepts_a_custom_title(self): + qbt = FakeQbt(use_pushover=True) + tor_notify_error(qbt, RuntimeError("boom"), title="Custom") + self.assertEqual(qbt.poc.calls[0]["kwargs"]["title"], "Custom") + + class TestCounters(unittest.TestCase): def test_process_counts_buckets_by_tag(self): qbt = FakeQbt( @@ -117,6 +146,12 @@ class TestDebugHelpers(unittest.TestCase): list_first_tor(qbt, index=1) self.assertIn("One", "\n".join(captured.output)) + def test_list_first_tor_handles_an_empty_list(self): + qbt = FakeQbt(torrent_list=[]) + with self.assertLogs("qbit-maid-test", level="DEBUG") as captured: + list_first_tor(qbt) + self.assertIn("No torrents found", "\n".join(captured.output)) + def test_list_qbit_api_info_logs_versions(self): qbt = FakeQbt() with self.assertLogs("qbit-maid-test", level="DEBUG") as captured: @@ -233,14 +268,13 @@ class TestSetupLogging(unittest.TestCase): self.assertEqual(logger.level, logging.INFO) def test_setup_is_idempotent(self): - setup_logging(True, self.log_path, "INFO") - # setup_logging drops the previous handlers without closing them, so a - # second call leaks the old file descriptor. Known issue; silenced here - # so the suite output stays readable. - with warnings.catch_warnings(): - warnings.simplefilter("ignore", ResourceWarning) - logger = setup_logging(True, self.log_path, "INFO") + first = setup_logging(True, self.log_path, "INFO") + previous_file_handler = first.handlers[0] + self.assertIsNotNone(previous_file_handler.stream) + logger = setup_logging(True, self.log_path, "INFO") self.assertEqual(len(logger.handlers), 2) + # The replaced handler is closed, so its file descriptor is released. + self.assertIsNone(previous_file_handler.stream) def test_rotation_parameters_are_applied(self): logger = setup_logging(True, self.log_path, "INFO",