Skip to content

Commit ff0ef77

Browse files
authored
fix: take every parent podcast out of the library, not every second one (#305)
`--resolve-podcasts` replaces every parent podcast with its episodes and then takes the parents out of the library. It removed them from the very list it was walking: [library.data.remove(i) for i in library if i.is_parent_podcast()] `Library.__iter__` yields from `self._data`, and `data` is that same list, so each removal moved the rest along underneath the walk and the following item was stepped over. Where parents sit next to each other, every second one stayed behind; a library that alternates parents and books never showed it. A parent left in the library becomes a download job of its own, which cannot succeed: a parent podcast has no audio to fetch. The removal now belongs to `resolve_podcasts()`, which takes a `remove_parents` argument and builds the list once rather than editing it while reading it. `Catalog` gets the same argument from the same body, and the deprecated `resolve_podcats()` awaits what it forwards to instead of handing back a coroutine that nobody awaited. Closes #305
1 parent 0502c91 commit ff0ef77

4 files changed

Lines changed: 287 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1212
- `audible manage auth-file remove` with a wrong password now says the file does not open, instead of a traceback about PKCS7 padding (#313, #315)
1313
- `audible manage auth-file add` asks whether the auth file should have a password. The option was there but was never offered, so anyone going through the questions got an unencrypted file, while `quickstart` asked. A script that passed every other option now has to pass `--password ''` as well, to say that it wants none (#313, #315)
1414
- `--version` no longer exits 1 with the error glued to the version when the update check cannot reach GitHub (#309)
15+
- The deprecated `resolve_podcats()` handed back a coroutine instead of awaiting it, so it warned and then did nothing (#305)
16+
- `--resolve-podcasts` now takes every parent podcast out of the library, not every second one where several sit next to each other (#305)
1517
- A podcast episode that is already downloaded no longer costs a voucher on every run (#299)
1618
- A cover size a title does not offer is reported as a warning, not an error (#300)
1719
- A request to Audible's CDE host is tried up to three times when it gets no answer (#298)
@@ -30,10 +32,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
3032

3133
### Added
3234

35+
<<<<<<< HEAD
36+
- `resolve_podcasts()` on `Library` and `Catalog` takes a `remove_parents` argument, for callers that want the episodes without the shows they came from (#305)
37+
=======
3338
- `audible manage auth-file encrypt` puts a password on an auth file that has none. Whatever is not given is asked for, so a password need not be typed where the shell keeps it, and a script that passes both options is asked nothing. It says so and changes nothing when the file is not an auth file, when it is encrypted already, or when it has a second name in the file system. The rewritten file keeps the permissions it had, so a decrypted one is as readable to others as it was before it was encrypted (#313, #315)
3439
- `audible manage auth-file decrypt` takes the password off again, asking and refusing the same way (#313, #315)
3540
- `--log-file PATH` writes the log to a file as well, with timestamp, module and line (#306)
3641
- `NO_COLOR` and `FORCE_COLOR` decide whether diagnostics are coloured (#306)
42+
>>>>>>> master
3743
- Standalone builds for arm64 on Linux and Windows; the macOS build stays Apple silicon only (#296)
3844
- Release download names now carry the architecture, for example `audible_win_arm64.zip`; the previous names stay until 15 November 2026 (#296)
3945
- A `pycryptodome` extra for platforms `cryptography` publishes no wheel for (#296)

‎src/audible_cli/cmds/cmd_download.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,8 +1012,11 @@ async def cli(session, api_client, **params):
10121012
)
10131013

10141014
if resolve_podcasts:
1015-
await library.resolve_podcasts(start_date=start_date, end_date=end_date)
1016-
[library.data.remove(i) for i in library if i.is_parent_podcast()]
1015+
# The parents go: they are what the episodes came from, and a
1016+
# parent has no audio of its own to download.
1017+
await library.resolve_podcasts(
1018+
start_date=start_date, end_date=end_date, remove_parents=True
1019+
)
10171020

10181021
# collect jobs
10191022
jobs = []

‎src/audible_cli/models.py‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,31 @@ def __init__(
511511
def __len__(self):
512512
return len(self._data)
513513

514+
async def _resolve_podcasts(
515+
self,
516+
remove_parents: bool = False,
517+
**request_params
518+
):
519+
"""Add the episodes of every parent podcast to this list.
520+
521+
Shared by the lists that hold library items. The parents stay
522+
unless `remove_parents` is asked for: they are ordinary entries to
523+
list and to export, and only a download has no use for them,
524+
because a parent carries no audio of its own.
525+
"""
526+
podcast_items = await asyncio.gather(
527+
*[i.get_child_items(**request_params)
528+
for i in self if i.is_parent_podcast()]
529+
)
530+
for i in podcast_items:
531+
self.data.extend(i.data)
532+
533+
if remove_parents:
534+
# Replaced in place rather than removed one at a time: taking
535+
# items out of the list being walked moves the rest along under
536+
# the walk, and every second parent in a row was stepped over.
537+
self.data[:] = [i for i in self if not i.is_parent_podcast()]
538+
514539
def __iter__(self):
515540
return iter(self._data)
516541

@@ -686,19 +711,20 @@ async def resolve_podcats(
686711
DeprecationWarning,
687712
stacklevel=2
688713
)
689-
return self.resolve_podcasts(start_date, end_date)
714+
return await self.resolve_podcasts(start_date, end_date)
690715

691716
async def resolve_podcasts(
692717
self,
693718
start_date: datetime | None = None,
694-
end_date: datetime | None = None
719+
end_date: datetime | None = None,
720+
remove_parents: bool = False
695721
):
696-
podcast_items = await asyncio.gather(
697-
*[i.get_child_items(start_date=start_date, end_date=end_date)
698-
for i in self if i.is_parent_podcast()]
722+
"""Add the episodes of every parent podcast to the library."""
723+
await self._resolve_podcasts(
724+
remove_parents=remove_parents,
725+
start_date=start_date,
726+
end_date=end_date
699727
)
700-
for i in podcast_items:
701-
self.data.extend(i.data)
702728

703729

704730
class Catalog(BaseList):
@@ -757,12 +783,9 @@ async def fetch_catalog(params):
757783

758784
return cls(resp, api_client=api_client)
759785

760-
async def resolve_podcasts(self):
761-
podcast_items = await asyncio.gather(
762-
*[i.get_child_items() for i in self if i.is_parent_podcast()]
763-
)
764-
for i in podcast_items:
765-
self.data.extend(i.data)
786+
async def resolve_podcasts(self, remove_parents: bool = False):
787+
"""Add the episodes of every parent podcast to the catalog."""
788+
await self._resolve_podcasts(remove_parents=remove_parents)
766789

767790

768791
class Wishlist(BaseList):

‎tests/test_resolve_podcasts.py‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
"""Resolving podcasts, and who takes the parents out.
2+
3+
The episodes are added to the library and the parents stay: they are
4+
ordinary entries to list and to export. Only a download asks for them to
5+
go, because a parent carries no audio of its own.
6+
"""
7+
8+
import asyncio
9+
10+
import pytest
11+
from click.testing import CliRunner
12+
13+
from audible_cli.cmds import cmd_download
14+
from audible_cli.config import Session
15+
from audible_cli.models import Catalog, Library
16+
17+
18+
def an_item(asin, title, parent=False):
19+
item = {
20+
"asin": asin,
21+
"title": title,
22+
"content_delivery_type": "PodcastParent" if parent else "SinglePartBook",
23+
"has_children": parent,
24+
"purchase_date": "2020-01-01T00:00:00.000Z",
25+
}
26+
if parent:
27+
item["content_type"] = "Podcast"
28+
return item
29+
30+
31+
def a_library(*items, api_client=None):
32+
return Library({"items": list(items)}, api_client=api_client)
33+
34+
35+
def a_catalog(*items, api_client=None):
36+
# `response_groups` is not optional here: `_prepare_data` splits it
37+
# without looking first.
38+
return Catalog(
39+
{"products": list(items), "response_groups": "product_desc"},
40+
api_client=api_client,
41+
)
42+
43+
44+
@pytest.fixture
45+
def neighbouring_parents(monkeypatch):
46+
"""Three shows in a row, which is what removing one at a time skipped.
47+
48+
Each hands back one episode, the way the real call does.
49+
"""
50+
51+
async def fake_children(self, **request_params):
52+
# `_children` as the real one leaves it, or a show that survived a
53+
# resolve would trip over None before it could show the symptom
54+
# this file is about.
55+
self._children = a_library(an_item(f"{self.asin}EP", f"{self.title} Episode"))
56+
return self._children
57+
58+
monkeypatch.setattr("audible_cli.models.LibraryItem.get_child_items", fake_children)
59+
return a_library(
60+
an_item("BOOK0001", "A Book"),
61+
an_item("CAST0001", "First Show", parent=True),
62+
an_item("CAST0002", "Second Show", parent=True),
63+
an_item("CAST0003", "Third Show", parent=True),
64+
an_item("BOOK0002", "Another Book"),
65+
)
66+
67+
68+
# --- the model ------------------------------------------------------------
69+
70+
71+
def test_the_shows_stay_unless_they_are_asked_to_go(neighbouring_parents):
72+
asyncio.run(neighbouring_parents.resolve_podcasts())
73+
74+
assert sorted(i.asin for i in neighbouring_parents) == [
75+
"BOOK0001",
76+
"BOOK0002",
77+
"CAST0001",
78+
"CAST0001EP",
79+
"CAST0002",
80+
"CAST0002EP",
81+
"CAST0003",
82+
"CAST0003EP",
83+
]
84+
85+
86+
def test_asking_takes_every_show_out_including_neighbours(neighbouring_parents):
87+
asyncio.run(neighbouring_parents.resolve_podcasts(remove_parents=True))
88+
89+
assert sorted(i.asin for i in neighbouring_parents) == [
90+
"BOOK0001",
91+
"BOOK0002",
92+
"CAST0001EP",
93+
"CAST0002EP",
94+
"CAST0003EP",
95+
], "a show was left behind, or something else went with them"
96+
97+
98+
def test_a_catalog_resolves_the_same_way(monkeypatch):
99+
# The two lists share the body, so the switch has to work on both.
100+
async def fake_children(self, **request_params):
101+
self._children = a_library(an_item(f"{self.asin}EP", f"{self.title} Episode"))
102+
return self._children
103+
104+
monkeypatch.setattr("audible_cli.models.LibraryItem.get_child_items", fake_children)
105+
shows = [
106+
an_item("CAST0001", "First Show", parent=True),
107+
an_item("CAST0002", "Second Show", parent=True),
108+
an_item("CAST0003", "Third Show", parent=True),
109+
]
110+
111+
kept = a_catalog(*shows)
112+
asyncio.run(kept.resolve_podcasts())
113+
assert sorted(i.asin for i in kept) == [
114+
"CAST0001",
115+
"CAST0001EP",
116+
"CAST0002",
117+
"CAST0002EP",
118+
"CAST0003",
119+
"CAST0003EP",
120+
], "the shows should still be there beside the episodes"
121+
122+
stripped = a_catalog(*shows)
123+
asyncio.run(stripped.resolve_podcasts(remove_parents=True))
124+
assert sorted(i.asin for i in stripped) == [
125+
"CAST0001EP",
126+
"CAST0002EP",
127+
"CAST0003EP",
128+
]
129+
130+
131+
# --- and the command that asks ---------------------------------------------
132+
133+
134+
class FakeHTTPSession:
135+
async def __aenter__(self):
136+
return self
137+
138+
async def __aexit__(self, *exc_info):
139+
return False
140+
141+
142+
class FakeApiClient:
143+
def __init__(self):
144+
self.session = FakeHTTPSession()
145+
146+
147+
@pytest.fixture
148+
def session(monkeypatch):
149+
monkeypatch.setattr(Session, "get_client", lambda self, **kw: FakeApiClient())
150+
return Session()
151+
152+
153+
def test_the_download_command_asks_for_the_shows_to_go(
154+
monkeypatch, session, neighbouring_parents, tmp_path
155+
):
156+
asked = {}
157+
queued = []
158+
159+
async def fake_from_api_full_sync(cls, api_client, **request_params):
160+
return neighbouring_parents
161+
162+
real_resolve = Library.resolve_podcasts
163+
164+
async def note(self, **kwargs):
165+
asked.update(kwargs)
166+
await real_resolve(self, **kwargs)
167+
168+
async def record(*args, **kwargs):
169+
queued.append(kwargs["item"].asin if "item" in kwargs else args[3].asin)
170+
return True
171+
172+
monkeypatch.setattr(
173+
Library, "from_api_full_sync", classmethod(fake_from_api_full_sync)
174+
)
175+
monkeypatch.setattr(Library, "resolve_podcasts", note)
176+
monkeypatch.setattr(cmd_download, "download_cover", record)
177+
178+
result = CliRunner().invoke(
179+
cmd_download.cli,
180+
[
181+
"--all",
182+
"--resolve-podcasts",
183+
"--cover",
184+
"--output-dir",
185+
str(tmp_path),
186+
"--filename-mode",
187+
"ascii",
188+
"--chapter-type",
189+
"flat",
190+
],
191+
obj=session,
192+
)
193+
194+
assert result.exception is None, result.output
195+
assert asked.get("remove_parents") is True, asked
196+
# The books and every episode, and not one of the shows they came from
197+
assert sorted(queued) == [
198+
"BOOK0001",
199+
"BOOK0002",
200+
"CAST0001EP",
201+
"CAST0002EP",
202+
"CAST0003EP",
203+
], queued
204+
# A show left behind is picked up as a job of its own and makes a
205+
# directory for episodes that are queued already
206+
left = sorted(p.name for p in tmp_path.iterdir() if p.is_dir())
207+
assert left == [], f"a show was still in the library: {left}"
208+
209+
210+
def test_the_dates_reach_the_episodes(monkeypatch):
211+
# A library resolves within a date range, and the range is the whole
212+
# reason the caller passed one: it decides which episodes come back.
213+
asked = []
214+
215+
async def fake_children(self, **request_params):
216+
asked.append(request_params)
217+
return a_library()
218+
219+
monkeypatch.setattr("audible_cli.models.LibraryItem.get_child_items", fake_children)
220+
library = a_library(an_item("CAST0001", "A Show", parent=True))
221+
222+
asyncio.run(library.resolve_podcasts(start_date="from", end_date="until"))
223+
224+
assert asked == [{"start_date": "from", "end_date": "until"}], asked
225+
226+
227+
def test_the_deprecated_spelling_actually_resolves(monkeypatch, recwarn):
228+
# `resolve_podcats` handed back the coroutine of the method it forwards
229+
# to instead of awaiting it, so it warned and then did nothing.
230+
async def fake_children(self, **request_params):
231+
self._children = a_library(an_item(f"{self.asin}EP", "An Episode"))
232+
return self._children
233+
234+
monkeypatch.setattr("audible_cli.models.LibraryItem.get_child_items", fake_children)
235+
library = a_library(an_item("CAST0001", "A Show", parent=True))
236+
237+
asyncio.run(library.resolve_podcats())
238+
239+
assert sorted(i.asin for i in library) == ["CAST0001", "CAST0001EP"]
240+
assert any(issubclass(w.category, DeprecationWarning) for w in recwarn)

0 commit comments

Comments
 (0)