fix(downloads): tighten SourceAdapter resolution + neutral nfo default (review)
- resolve_adapter: identify from the yt-dlp extractor_key only, not the full webpage_url (a sibling source's name in a URL path no longer misresolves); add youtube-nocookie.com to the YouTube host match so nocookie embeds keep the POT/player-client extraction strategy. - _download: re-resolve the adapter with the post-extraction info dict so the .nfo uniqueid type and uploader_url agree on one source (dedups the inline resolve too). - MediaMeta.uniqueid_type defaults to source-neutral "generic" instead of "youtube", so an un-updated caller can never falsely label a non-YouTube download as youtube. - Tests: lock extractor-key-only resolution + nocookie host; default nfo type.
This commit is contained in:
@@ -122,12 +122,17 @@ def resolve_adapter(
|
|||||||
"""Pick the adapter for a job. Matches on `source_kind` first, then the yt-dlp `extractor_key`
|
"""Pick the adapter for a job. Matches on `source_kind` first, then the yt-dlp `extractor_key`
|
||||||
from `info` (post-extraction), then the URL host in `source_ref` (pre-extraction). Falls back to
|
from `info` (post-extraction), then the URL host in `source_ref` (pre-extraction). Falls back to
|
||||||
the generic adapter — so behaviour is unchanged for any source without a dedicated one."""
|
the generic adapter — so behaviour is unchanged for any source without a dedicated one."""
|
||||||
tag = ""
|
# Identify from the yt-dlp extractor key alone — NOT the webpage_url, whose path could contain a
|
||||||
if info:
|
# sibling source's name (e.g. a reddit post slug '.../youtube_is_dead') and cause a misresolve.
|
||||||
extractor = info.get("extractor_key") or info.get("extractor") or ""
|
tag = (info.get("extractor_key") or info.get("extractor") or "").lower() if info else ""
|
||||||
tag = f"{extractor} {info.get('webpage_url') or ''}".lower()
|
|
||||||
host = urlparse(source_ref).netloc.lower() if source_ref else ""
|
host = urlparse(source_ref).netloc.lower() if source_ref else ""
|
||||||
if source_kind == "youtube" or "youtube" in tag or "youtu.be" in host or "youtube.com" in host:
|
if (
|
||||||
|
source_kind == "youtube"
|
||||||
|
or "youtube" in tag
|
||||||
|
or "youtu.be" in host
|
||||||
|
or "youtube.com" in host
|
||||||
|
or "youtube-nocookie.com" in host
|
||||||
|
):
|
||||||
return _YOUTUBE
|
return _YOUTUBE
|
||||||
if "facebook" in tag or "facebook.com" in host or "fb.watch" in host:
|
if "facebook" in tag or "facebook.com" in host or "fb.watch" in host:
|
||||||
return _FACEBOOK
|
return _FACEBOOK
|
||||||
|
|||||||
@@ -60,8 +60,9 @@ class MediaMeta:
|
|||||||
uploader: str
|
uploader: str
|
||||||
upload_date: date | None
|
upload_date: date | None
|
||||||
# The .nfo <uniqueid type=...> — the source's scraper type (youtube / facebook / …), set from
|
# The .nfo <uniqueid type=...> — the source's scraper type (youtube / facebook / …), set from
|
||||||
# the SourceAdapter. Defaults to "youtube" only as a safety net for un-updated callers.
|
# the SourceAdapter. Defaults to the source-neutral "generic" so an un-updated caller is at worst
|
||||||
uniqueid_type: str = "youtube"
|
# uninformative, never actively mislabelling a non-YouTube download as "youtube".
|
||||||
|
uniqueid_type: str = "generic"
|
||||||
duration_s: int | None = None
|
duration_s: int | None = None
|
||||||
description: str | None = None
|
description: str | None = None
|
||||||
thumbnail_url: str | None = None
|
thumbnail_url: str | None = None
|
||||||
|
|||||||
@@ -791,6 +791,9 @@ def _download(job_id: int, asset_id: int, source_kind: str, source_ref: str, spe
|
|||||||
adapter = adapters.resolve_adapter(source_kind, source_ref)
|
adapter = adapters.resolve_adapter(source_kind, source_ref)
|
||||||
url = service.source_url(source_kind, source_ref)
|
url = service.source_url(source_kind, source_ref)
|
||||||
info = _media_info(_extract_with_fallback(job_id, asset_id, url, spec, staging, adapter))
|
info = _media_info(_extract_with_fallback(job_id, asset_id, url, spec, staging, adapter))
|
||||||
|
# Re-resolve now that yt-dlp's info reveals the true source (a bare/redirected url only
|
||||||
|
# identifies post-extraction) so the .nfo uniqueid type + uploader_url agree on one source.
|
||||||
|
adapter = adapters.resolve_adapter(source_kind, source_ref, info)
|
||||||
|
|
||||||
produced = _produced_file(info, staging)
|
produced = _produced_file(info, staging)
|
||||||
produced = _ensure_browser_playable(job_id, produced, spec, info)
|
produced = _ensure_browser_playable(job_id, produced, spec, info)
|
||||||
@@ -834,7 +837,7 @@ def _download(job_id: int, asset_id: int, source_kind: str, source_ref: str, spe
|
|||||||
asset.acodec = info.get("acodec") if info.get("acodec") not in (None, "none") else None
|
asset.acodec = info.get("acodec") if info.get("acodec") not in (None, "none") else None
|
||||||
asset.title = meta.title
|
asset.title = meta.title
|
||||||
asset.uploader = meta.uploader
|
asset.uploader = meta.uploader
|
||||||
asset.uploader_url = adapters.resolve_adapter(source_kind, source_ref, info).uploader_url(info)
|
asset.uploader_url = adapter.uploader_url(info)
|
||||||
asset.upload_date = meta.upload_date
|
asset.upload_date = meta.upload_date
|
||||||
asset.thumbnail_url = meta.thumbnail_url
|
asset.thumbnail_url = meta.thumbnail_url
|
||||||
# Record a self-hosted poster for every download: it's the reliable link-preview
|
# Record a self-hosted poster for every download: it's the reliable link-preview
|
||||||
|
|||||||
@@ -27,6 +27,19 @@ def test_host_match_is_netloc_not_a_loose_substring():
|
|||||||
assert resolve_adapter("url", "https://example.com/facebook.com-mirror").key == "generic"
|
assert resolve_adapter("url", "https://example.com/facebook.com-mirror").key == "generic"
|
||||||
|
|
||||||
|
|
||||||
|
def test_info_match_is_extractor_key_not_the_webpage_url():
|
||||||
|
# The webpage_url path may carry a SIBLING source's name (a reddit slug about youtube); only the
|
||||||
|
# yt-dlp extractor_key identifies the source, so this must resolve reddit, not youtube.
|
||||||
|
info = {"extractor_key": "Reddit", "webpage_url": "https://reddit.com/r/x/comments/y/youtube_is_dead"}
|
||||||
|
assert resolve_adapter("url", None, info).key == "reddit"
|
||||||
|
|
||||||
|
|
||||||
|
def test_youtube_nocookie_host_still_gets_the_youtube_adapter():
|
||||||
|
# An embed on youtube-nocookie.com is YouTube content and needs the POT/player-client strategy,
|
||||||
|
# even though its host does not contain the substring "youtube.com".
|
||||||
|
assert resolve_adapter("url", "https://www.youtube-nocookie.com/embed/abc123").key == "youtube"
|
||||||
|
|
||||||
|
|
||||||
def test_uploader_url_youtube_uses_channel_url():
|
def test_uploader_url_youtube_uses_channel_url():
|
||||||
a = YouTubeAdapter()
|
a = YouTubeAdapter()
|
||||||
assert a.uploader_url({"channel_url": "https://youtube.com/@chan"}) == "https://youtube.com/@chan"
|
assert a.uploader_url({"channel_url": "https://youtube.com/@chan"}) == "https://youtube.com/@chan"
|
||||||
|
|||||||
@@ -145,8 +145,10 @@ def test_nfo_uniqueid_type_reflects_the_source():
|
|||||||
from app.downloads.storage import _nfo_xml
|
from app.downloads.storage import _nfo_xml
|
||||||
|
|
||||||
base = dict(video_id="v1", title="t", uploader="c", upload_date=None)
|
base = dict(video_id="v1", title="t", uploader="c", upload_date=None)
|
||||||
assert '<uniqueid type="youtube"' in _nfo_xml(MediaMeta(**base)) # default preserved
|
# An un-set caller gets the source-neutral "generic" default, never a false "youtube".
|
||||||
|
assert '<uniqueid type="generic"' in _nfo_xml(MediaMeta(**base))
|
||||||
assert '<uniqueid type="facebook"' in _nfo_xml(MediaMeta(**base, uniqueid_type="facebook"))
|
assert '<uniqueid type="facebook"' in _nfo_xml(MediaMeta(**base, uniqueid_type="facebook"))
|
||||||
|
assert '<uniqueid type="youtube"' in _nfo_xml(MediaMeta(**base, uniqueid_type="youtube"))
|
||||||
|
|
||||||
|
|
||||||
def test_sidecars_keep_the_full_basename_when_the_title_has_a_dot(tmp_path: Path):
|
def test_sidecars_keep_the_full_basename_when_the_title_has_a_dot(tmp_path: Path):
|
||||||
|
|||||||
Reference in New Issue
Block a user