fix(api): correct TransmissionSession edge cases from code review

- add() now diffs before/after list() for fallback ID detection
- remove unreachable return in info_file
- simplify wait_for_all break logic
- always call on_complete even when info_file returns empty string

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Justin Bollinger
2026-04-25 00:31:56 -04:00
co-authored by Claude Sonnet 4.6
parent 9b6de8da71
commit 6064b79fbd
2 changed files with 32 additions and 11 deletions
+6 -9
View File
@@ -342,6 +342,7 @@ class TransmissionSession:
import re
import subprocess
before_ids = {e["id"] for e in self.list()}
result = subprocess.run(
[
"transmission-remote",
@@ -360,9 +361,10 @@ class TransmissionSession:
m = re.search(r"torrent added\s*\(id\s+(\d+)\)", out, re.IGNORECASE)
if m:
return int(m.group(1))
entries = self.list()
if entries:
return max(entry["id"] for entry in entries)
after_entries = self.list()
new_ids = [e["id"] for e in after_entries if e["id"] not in before_ids]
if new_ids:
return new_ids[0]
raise RuntimeError(f"Failed to add torrent: {torrent_path}")
def list(self) -> list:
@@ -461,7 +463,6 @@ class TransmissionSession:
return rest
except Exception:
return ""
return ""
def remove(self, torrent_id: int):
import subprocess
@@ -483,9 +484,6 @@ class TransmissionSession:
time.sleep(self.poll_interval)
entries = self.list()
if not entries:
if completed_ids:
break
# Nothing left to do
break
for entry in entries:
if (
@@ -494,8 +492,7 @@ class TransmissionSession:
):
completed_ids.add(entry["id"])
file_name = self.info_file(entry["id"])
if file_name:
on_complete(entry["id"], file_name)
on_complete(entry["id"], file_name)
self.remove(entry["id"])
+26 -2
View File
@@ -164,11 +164,16 @@ class TestTransmissionSession:
ts = TransmissionSession(str(tmp_path))
ts._rpc = "127.0.0.1:9999"
result = MagicMock(returncode=0, stdout="garbage output\n", stderr="")
# Before: IDs 3 and 5 exist. After: ID 7 appears as the newly added torrent.
list_calls = iter([
[{"id": 3}, {"id": 5}], # before snapshot
[{"id": 3}, {"id": 5}, {"id": 7}], # after snapshot
])
with patch("subprocess.run", return_value=result), patch.object(
ts, "list", return_value=[{"id": 3}, {"id": 5}]
ts, "list", side_effect=list_calls
):
tid = ts.add("/tmp/foo.torrent")
assert tid == 5
assert tid == 7
def test_add_raises_when_list_empty(self, tmp_path):
ts = TransmissionSession(str(tmp_path))
@@ -261,6 +266,25 @@ class TestTransmissionSession:
assert remove_calls == [1]
assert info_calls == [1]
def test_wait_for_all_calls_on_complete_when_info_file_empty(self, tmp_path):
"""on_complete must be called even when info_file returns "" so the caller
can account for the torrent (e.g. increment a failure counter)."""
ts = TransmissionSession(str(tmp_path), poll_interval=0.0)
ts._rpc = "127.0.0.1:9999"
list_results = [
[{"id": 2, "percent_done": 100.0, "status": "Idle", "name": "x"}],
[],
]
callbacks = []
with patch.object(ts, "list", side_effect=lambda: list_results.pop(0)), \
patch.object(ts, "info_file", return_value=""), \
patch.object(ts, "remove"), \
patch("time.sleep"):
ts.wait_for_all(on_complete=lambda tid, name: callbacks.append((tid, name)))
assert callbacks == [(2, "")], "on_complete must fire even when info_file returns empty"
def test_wait_for_all_keyboard_interrupt_propagates(self, tmp_path):
ts = TransmissionSession(str(tmp_path), poll_interval=0.0)
ts._rpc = "127.0.0.1:9999"