mirror of
https://github.com/oskvr37/tiddl.git
synced 2026-06-13 04:05:08 +03:00
fix: handle null/missing fields in Video API responses (#295)
* fix: handle null/missing fields in Video API responses
Tidal's API returns some Video objects (lyric/visualiser videos on
artist pages) with fields that don't match the current strict models:
- `imageId` can be null instead of a string
- The nested `album` object can be present but missing `id`, `title`,
and `cover`
These validation failures cause the entire `ArtistVideosItems` page
to be rejected by Pydantic before any video can be parsed, resulting
in 0 downloads when targeting an artist with `--videos`.
A second independent bug causes an `AttributeError` on every video:
the default template `{album.artist}/{album.title}/{item.title}` is
shared with videos, but many videos have no album. When `album=None`
is passed to `format_template`, Python's `str.format()` evaluates
`None.artist` and raises `AttributeError: 'NoneType' object has no
attribute 'artist'`, which is caught and printed as an error for
every single video.
Fix:
- `resources.py`: make `Video.imageId` and `Video.Album.{id,title,
cover}` optional so incomplete API responses pass validation
- `format.py`: give `AlbumTemplate` field defaults so it can be
instantiated empty; use `AlbumTemplate()` as fallback instead of
`None` when no album is present, so `{album.*}` tokens render as
empty strings rather than raising AttributeError
- `download/__init__.py`: guard `video.album.id` accesses against
`None` (now possible after the model fix) in both video code paths
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add tests for Video model null fields and AlbumTemplate fallback
Covers the two bugs fixed in the previous commit:
- Video model accepts null/missing imageId and partial album objects
- format_template does not raise AttributeError when album is None
and the template references {album.*} tokens
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,86 @@
|
|||||||
|
import pytest
|
||||||
|
from pydantic import ValidationError
|
||||||
|
|
||||||
|
from tiddl.core.api.models.resources import Video
|
||||||
|
|
||||||
|
|
||||||
|
# Minimal valid payload shared across tests
|
||||||
|
BASE_VIDEO = {
|
||||||
|
"id": 123,
|
||||||
|
"title": "Test Video",
|
||||||
|
"volumeNumber": 1,
|
||||||
|
"trackNumber": 1,
|
||||||
|
"duration": 180,
|
||||||
|
"quality": "MP4_1080P",
|
||||||
|
"streamReady": True,
|
||||||
|
"adSupportedStreamReady": False,
|
||||||
|
"djReady": False,
|
||||||
|
"stemReady": False,
|
||||||
|
"allowStreaming": True,
|
||||||
|
"explicit": False,
|
||||||
|
"popularity": 50,
|
||||||
|
"type": "Music Video",
|
||||||
|
"adsPrePaywallOnly": False,
|
||||||
|
"artists": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_null_image_id():
|
||||||
|
"""imageId=null should be accepted (Tidal returns this for some videos)."""
|
||||||
|
video = Video.model_validate({**BASE_VIDEO, "imageId": None})
|
||||||
|
assert video.imageId is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_missing_image_id():
|
||||||
|
"""imageId absent entirely should default to None."""
|
||||||
|
video = Video.model_validate(BASE_VIDEO)
|
||||||
|
assert video.imageId is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_valid_image_id():
|
||||||
|
"""A normal imageId string should still be accepted."""
|
||||||
|
video = Video.model_validate({**BASE_VIDEO, "imageId": "abc123"})
|
||||||
|
assert video.imageId == "abc123"
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_album_missing_required_fields():
|
||||||
|
"""album object present but missing id/title/cover should be accepted."""
|
||||||
|
payload = {
|
||||||
|
**BASE_VIDEO,
|
||||||
|
"album": {"vibrantColor": None},
|
||||||
|
}
|
||||||
|
video = Video.model_validate(payload)
|
||||||
|
assert video.album is not None
|
||||||
|
assert video.album.id is None
|
||||||
|
assert video.album.title is None
|
||||||
|
assert video.album.cover is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_album_none():
|
||||||
|
"""album=null should still be accepted (existing behaviour)."""
|
||||||
|
video = Video.model_validate({**BASE_VIDEO, "album": None})
|
||||||
|
assert video.album is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_album_fully_populated():
|
||||||
|
"""A fully populated album object should still parse correctly."""
|
||||||
|
payload = {
|
||||||
|
**BASE_VIDEO,
|
||||||
|
"album": {
|
||||||
|
"id": 42,
|
||||||
|
"title": "Greatest Hits",
|
||||||
|
"cover": "cover-uuid",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
video = Video.model_validate(payload)
|
||||||
|
assert video.album is not None
|
||||||
|
assert video.album.id == 42
|
||||||
|
assert video.album.title == "Greatest Hits"
|
||||||
|
assert video.album.cover == "cover-uuid"
|
||||||
|
|
||||||
|
|
||||||
|
def test_video_still_requires_core_fields():
|
||||||
|
"""Removing a genuinely required field (title) should still raise."""
|
||||||
|
payload = {k: v for k, v in BASE_VIDEO.items() if k != "title"}
|
||||||
|
with pytest.raises(ValidationError):
|
||||||
|
Video.model_validate(payload)
|
||||||
@@ -0,0 +1,103 @@
|
|||||||
|
from datetime import datetime
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from tiddl.core.utils.format import AlbumTemplate, format_template, generate_template_data
|
||||||
|
from tiddl.core.api.models.resources import Video
|
||||||
|
|
||||||
|
|
||||||
|
# Minimal Video instance used across format tests
|
||||||
|
BASE_VIDEO = Video.model_validate(
|
||||||
|
{
|
||||||
|
"id": 1,
|
||||||
|
"title": "My Video",
|
||||||
|
"volumeNumber": 1,
|
||||||
|
"trackNumber": 1,
|
||||||
|
"duration": 200,
|
||||||
|
"quality": "MP4_1080P",
|
||||||
|
"streamReady": True,
|
||||||
|
"adSupportedStreamReady": False,
|
||||||
|
"djReady": False,
|
||||||
|
"stemReady": False,
|
||||||
|
"allowStreaming": True,
|
||||||
|
"explicit": False,
|
||||||
|
"popularity": 10,
|
||||||
|
"type": "Music Video",
|
||||||
|
"adsPrePaywallOnly": False,
|
||||||
|
"artists": [{"id": 1, "name": "Gorillaz", "type": "MAIN"}],
|
||||||
|
"artist": {"id": 1, "name": "Gorillaz", "type": "MAIN"},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestAlbumTemplateDefaults:
|
||||||
|
def test_can_be_instantiated_with_no_args(self):
|
||||||
|
t = AlbumTemplate()
|
||||||
|
assert t.id == 0
|
||||||
|
assert t.title == ""
|
||||||
|
assert t.artist == ""
|
||||||
|
assert t.artists == ""
|
||||||
|
assert t.release == ""
|
||||||
|
|
||||||
|
def test_date_defaults_to_datetime_min(self):
|
||||||
|
assert AlbumTemplate().date == datetime.min
|
||||||
|
|
||||||
|
def test_explicit_formats_to_empty_string(self):
|
||||||
|
assert f"{AlbumTemplate().explicit}" == ""
|
||||||
|
|
||||||
|
def test_master_formats_to_empty_string(self):
|
||||||
|
assert f"{AlbumTemplate().master:MASTER}" == ""
|
||||||
|
|
||||||
|
|
||||||
|
class TestFormatTemplateNoAlbum:
|
||||||
|
def test_album_artist_token_does_not_raise(self):
|
||||||
|
"""Default template must not raise AttributeError when album is None."""
|
||||||
|
result = format_template(
|
||||||
|
template="{album.artist}/{album.title}/{item.title}",
|
||||||
|
item=BASE_VIDEO,
|
||||||
|
album=None,
|
||||||
|
with_asterisk_ext=False,
|
||||||
|
)
|
||||||
|
# album tokens render as "_" (empty string → sanitised fallback)
|
||||||
|
assert result == "_/_/My Video"
|
||||||
|
|
||||||
|
def test_album_title_token_does_not_raise(self):
|
||||||
|
result = format_template(
|
||||||
|
template="{album.title}",
|
||||||
|
item=BASE_VIDEO,
|
||||||
|
album=None,
|
||||||
|
with_asterisk_ext=False,
|
||||||
|
)
|
||||||
|
assert result == "_"
|
||||||
|
|
||||||
|
def test_item_title_still_rendered(self):
|
||||||
|
result = format_template(
|
||||||
|
template="{item.title}",
|
||||||
|
item=BASE_VIDEO,
|
||||||
|
album=None,
|
||||||
|
with_asterisk_ext=False,
|
||||||
|
)
|
||||||
|
assert result == "My Video"
|
||||||
|
|
||||||
|
def test_item_artist_still_rendered(self):
|
||||||
|
result = format_template(
|
||||||
|
template="{item.artist}",
|
||||||
|
item=BASE_VIDEO,
|
||||||
|
album=None,
|
||||||
|
with_asterisk_ext=False,
|
||||||
|
)
|
||||||
|
assert result == "Gorillaz"
|
||||||
|
|
||||||
|
|
||||||
|
class TestGenerateTemplateDataAlbumFallback:
|
||||||
|
def test_album_template_is_never_none(self):
|
||||||
|
"""generate_template_data should always return an AlbumTemplate, never None."""
|
||||||
|
data = generate_template_data(item=BASE_VIDEO, album=None)
|
||||||
|
assert data["album"] is not None
|
||||||
|
assert isinstance(data["album"], AlbumTemplate)
|
||||||
|
|
||||||
|
def test_album_template_has_empty_fields_when_no_album(self):
|
||||||
|
data = generate_template_data(item=BASE_VIDEO, album=None)
|
||||||
|
album = data["album"]
|
||||||
|
assert album.title == ""
|
||||||
|
assert album.artist == ""
|
||||||
@@ -414,7 +414,7 @@ def download_callback(
|
|||||||
video = ctx.obj.api.get_video(resource.id)
|
video = ctx.obj.api.get_video(resource.id)
|
||||||
template = TEMPLATE or CONFIG.templates.video
|
template = TEMPLATE or CONFIG.templates.video
|
||||||
|
|
||||||
if "{album" in template and video.album:
|
if "{album" in template and video.album and video.album.id is not None:
|
||||||
album = ctx.obj.api.get_album(video.album.id)
|
album = ctx.obj.api.get_album(video.album.id)
|
||||||
else:
|
else:
|
||||||
album = None
|
album = None
|
||||||
|
|||||||
@@ -69,9 +69,9 @@ class Video(BaseModel):
|
|||||||
picture: Optional[str] = None
|
picture: Optional[str] = None
|
||||||
|
|
||||||
class Album(BaseModel):
|
class Album(BaseModel):
|
||||||
id: int
|
id: Optional[int] = None
|
||||||
title: str
|
title: Optional[str] = None
|
||||||
cover: str
|
cover: Optional[str] = None
|
||||||
vibrantColor: Optional[str] = None
|
vibrantColor: Optional[str] = None
|
||||||
videoCover: Optional[str] = None
|
videoCover: Optional[str] = None
|
||||||
|
|
||||||
@@ -81,7 +81,7 @@ class Video(BaseModel):
|
|||||||
trackNumber: int
|
trackNumber: int
|
||||||
streamStartDate: Optional[datetime] = None
|
streamStartDate: Optional[datetime] = None
|
||||||
imagePath: Optional[str] = None
|
imagePath: Optional[str] = None
|
||||||
imageId: str
|
imageId: Optional[str] = None
|
||||||
vibrantColor: Optional[str] = None
|
vibrantColor: Optional[str] = None
|
||||||
duration: int
|
duration: int
|
||||||
quality: Literal["MP4_1080P"] | str
|
quality: Literal["MP4_1080P"] | str
|
||||||
|
|||||||
+10
-10
@@ -1,5 +1,5 @@
|
|||||||
import re
|
import re
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass, field
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
|
||||||
from tiddl.core.api.models import Track, Video, Album, Playlist
|
from tiddl.core.api.models import Track, Video, Album, Playlist
|
||||||
@@ -67,14 +67,14 @@ class UserFormat:
|
|||||||
|
|
||||||
@dataclass(slots=True)
|
@dataclass(slots=True)
|
||||||
class AlbumTemplate:
|
class AlbumTemplate:
|
||||||
id: int
|
id: int = 0
|
||||||
title: str
|
title: str = ""
|
||||||
artist: str
|
artist: str = ""
|
||||||
artists: str
|
artists: str = ""
|
||||||
date: datetime
|
date: datetime = datetime.min
|
||||||
explicit: Explicit
|
explicit: Explicit = field(default_factory=lambda: Explicit(None))
|
||||||
master: UserFormat
|
master: UserFormat = field(default_factory=lambda: UserFormat(False))
|
||||||
release: str
|
release: str = ""
|
||||||
|
|
||||||
|
|
||||||
@dataclass(slots=True)
|
@dataclass(slots=True)
|
||||||
@@ -156,7 +156,7 @@ def generate_template_data(
|
|||||||
dolby=dolby,
|
dolby=dolby,
|
||||||
)
|
)
|
||||||
|
|
||||||
album_template = None
|
album_template = AlbumTemplate()
|
||||||
if album:
|
if album:
|
||||||
album_template = AlbumTemplate(
|
album_template = AlbumTemplate(
|
||||||
id=album.id,
|
id=album.id,
|
||||||
|
|||||||
Reference in New Issue
Block a user