refactor(downloads): introduce SourceAdapter — a thin per-source Strategy over yt-dlp
Consolidates source-specific policy (youtube / facebook / reddit / generic) that was scattered across worker.py, backfill.py, service.py, formats.py and storage.py with no shared source identity. app/downloads/adapters.py holds the base (= generic yt-dlp behaviour) + per-source overrides; resolve_adapter(source_kind, source_ref, info) picks one by source_kind, the yt-dlp extractor_key, or the URL host, falling back to generic. yt-dlp stays the engine — the adapter is policy only. Moved into adapters (behaviour-equivalent): - uploader_url derivation (YouTube channel_url / Facebook uploader_id / reddit None) — removes the worker/backfill duplication (build_uploader_url + _should_refetch gone). - the player-client fallback sets + retriable markers + YouTube extractor_args/POT (build_ydl_opts now takes source-agnostic opts plus an `extra` dict from the adapter). - source_url reconstruction (service.source_url delegates to the adapter). Two intentional (approved) behaviour changes: - the player-client fallback is now YouTube-only; non-YouTube sources make a single extraction attempt instead of a pointless double attempt with youtube-only client args. - the .nfo <uniqueid type=...> follows the source (adapter.nfo_uniqueid_type) instead of the hardcoded "youtube" that mislabelled every url-source download. Deliberately left generic (documented non-moves): title cleaning (normalize_title is harmless across sources), the classify marker table, thumbnail_to_jpg, the format selectors, and og.py's YouTube-hqdefault og:image preference (a self-contained thumbnail heuristic on the share path). Verified end-to-end on localdev across all four adapters: downloads succeed and uploader_url + the .nfo uniqueid type are correct per source.
This commit is contained in:
@@ -0,0 +1,83 @@
|
||||
"""Per-source adapter selection + the source-specific policy each one carries. These lock the
|
||||
behaviour that used to live inline in worker.build_uploader_url / backfill._should_refetch."""
|
||||
from app.downloads.adapters import (
|
||||
FacebookAdapter,
|
||||
RedditAdapter,
|
||||
SourceAdapter,
|
||||
YouTubeAdapter,
|
||||
resolve_adapter,
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_by_source_kind_and_host_and_info():
|
||||
assert resolve_adapter("youtube", "abc123").key == "youtube"
|
||||
assert resolve_adapter("url", "https://www.youtube.com/watch?v=x").key == "youtube"
|
||||
assert resolve_adapter("url", "https://youtu.be/x").key == "youtube"
|
||||
assert resolve_adapter("url", "https://www.facebook.com/reel/123/").key == "facebook"
|
||||
assert resolve_adapter("url", "https://fb.watch/abc/").key == "facebook"
|
||||
assert resolve_adapter("url", "https://www.reddit.com/r/x/comments/y/z").key == "reddit"
|
||||
assert resolve_adapter("url", "https://example.com/video.mp4").key == "generic"
|
||||
# After extraction the info dict's extractor decides even for a bare/redirected url.
|
||||
assert resolve_adapter("url", None, {"extractor_key": "facebook"}).key == "facebook"
|
||||
assert resolve_adapter("url", None, {"extractor_key": "Reddit"}).key == "reddit"
|
||||
|
||||
|
||||
def test_host_match_is_netloc_not_a_loose_substring():
|
||||
# A path that merely CONTAINS "facebook.com" must not select the Facebook adapter.
|
||||
assert resolve_adapter("url", "https://example.com/facebook.com-mirror").key == "generic"
|
||||
|
||||
|
||||
def test_uploader_url_youtube_uses_channel_url():
|
||||
a = YouTubeAdapter()
|
||||
assert a.uploader_url({"channel_url": "https://youtube.com/@chan"}) == "https://youtube.com/@chan"
|
||||
assert a.uploader_url({"uploader_url": "https://youtube.com/user/u"}) == "https://youtube.com/user/u"
|
||||
assert a.uploader_url({}) is None
|
||||
|
||||
|
||||
def test_uploader_url_facebook_falls_back_to_uploader_id():
|
||||
a = FacebookAdapter()
|
||||
assert a.uploader_url({"uploader_id": "12345"}) == "https://www.facebook.com/12345"
|
||||
# A direct channel_url still wins over the id fallback.
|
||||
assert a.uploader_url({"channel_url": "https://facebook.com/pageX", "uploader_id": "9"}) == (
|
||||
"https://facebook.com/pageX"
|
||||
)
|
||||
assert a.uploader_url({}) is None
|
||||
|
||||
|
||||
def test_uploader_url_reddit_and_generic_have_no_channel():
|
||||
assert RedditAdapter().uploader_url({"uploader_id": "99"}) is None # not facebook → no fallback
|
||||
assert SourceAdapter().uploader_url({}) is None
|
||||
|
||||
|
||||
def test_generic_source_url_is_passthrough_youtube_reconstructs():
|
||||
assert SourceAdapter().source_url("https://x/y") == "https://x/y"
|
||||
assert YouTubeAdapter().source_url("VIDID12345_") == "https://www.youtube.com/watch?v=VIDID12345_"
|
||||
|
||||
|
||||
def test_client_sets_and_retriable_are_youtube_only():
|
||||
# Generic sources make a single attempt and never retry on the YouTube per-client markers.
|
||||
assert SourceAdapter().client_sets() == [None]
|
||||
assert SourceAdapter().retriable("Sign in to confirm you're not a bot") is False
|
||||
assert YouTubeAdapter().retriable("Sign in to confirm you're not a bot") is True
|
||||
assert YouTubeAdapter().retriable("some unrelated network blip") is False
|
||||
|
||||
|
||||
def test_youtube_ydl_opts_extra_carries_player_client_and_pot():
|
||||
ea = YouTubeAdapter().ydl_opts_extra(["android_vr"])["extractor_args"]
|
||||
assert ea["youtube"]["player_client"] == ["android_vr"]
|
||||
assert "youtubepot-bgutilhttp" in ea # the bgutil PO-token base_url is wired
|
||||
# A client-less attempt still wires POT but no player_client key.
|
||||
assert "youtube" not in YouTubeAdapter().ydl_opts_extra(None)["extractor_args"]
|
||||
assert SourceAdapter().ydl_opts_extra(["android_vr"]) == {} # generic contributes nothing
|
||||
|
||||
|
||||
def test_wants_metadata_backfill_matches_old_should_refetch():
|
||||
assert YouTubeAdapter().wants_metadata_backfill() is True
|
||||
assert FacebookAdapter().wants_metadata_backfill() is True
|
||||
assert RedditAdapter().wants_metadata_backfill() is False
|
||||
assert SourceAdapter().wants_metadata_backfill() is False
|
||||
|
||||
|
||||
def test_nfo_uniqueid_type_is_the_adapter_key():
|
||||
assert YouTubeAdapter().nfo_uniqueid_type() == "youtube"
|
||||
assert SourceAdapter().nfo_uniqueid_type() == "generic"
|
||||
@@ -9,6 +9,15 @@ def _opts(spec):
|
||||
return formats.build_ydl_opts(spec, "/tmp/%(id)s.%(ext)s", lambda d: None)
|
||||
|
||||
|
||||
def test_adapter_extra_opts_are_merged_in():
|
||||
# The SourceAdapter contributes source-specific opts (e.g. YouTube's extractor_args) via `extra`.
|
||||
extra = {"extractor_args": {"youtube": {"player_client": ["web"]}}}
|
||||
opts = formats.build_ydl_opts({}, "/tmp/%(id)s.%(ext)s", lambda d: None, extra=extra)
|
||||
assert opts["extractor_args"]["youtube"]["player_client"] == ["web"]
|
||||
# No extra → no extractor_args (a generic source contributes nothing).
|
||||
assert "extractor_args" not in formats.build_ydl_opts({}, "/tmp/x", lambda d: None)
|
||||
|
||||
|
||||
def test_mp4_default_selects_avc1_with_progressive_fallback():
|
||||
fmt = _opts({"mode": "av", "container": "mp4"})["format"]
|
||||
# avc1 video-only first, then the avc1 progressive, then an unrestricted fallback.
|
||||
|
||||
@@ -137,3 +137,13 @@ class TestSafeAbsPath:
|
||||
|
||||
def test_returns_none_for_a_nonexistent_file_inside_the_root(self, tmp_path: Path):
|
||||
assert safe_abs_path(str(tmp_path), "does/not/exist.mp4") is None
|
||||
|
||||
|
||||
def test_nfo_uniqueid_type_reflects_the_source():
|
||||
# The .nfo <uniqueid type=...> now follows the source (via SourceAdapter.nfo_uniqueid_type),
|
||||
# instead of the old hardcoded "youtube" that mislabelled every url-source download.
|
||||
from app.downloads.storage import _nfo_xml
|
||||
|
||||
base = dict(video_id="v1", title="t", uploader="c", upload_date=None)
|
||||
assert '<uniqueid type="youtube"' in _nfo_xml(MediaMeta(**base)) # default preserved
|
||||
assert '<uniqueid type="facebook"' in _nfo_xml(MediaMeta(**base, uniqueid_type="facebook"))
|
||||
|
||||
Reference in New Issue
Block a user