From cf0d1cd362c5a2e56c8139790d8cb04a3e23c566 Mon Sep 17 00:00:00 2001 From: Filip Voska Date: Thu, 26 Feb 2026 14:35:33 +0100 Subject: [PATCH] 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 * 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 --------- Co-authored-by: Claude Sonnet 4.6 --- tests/core/api/test_api_models_resources.py | 86 ++++++++++++++++ tests/core/utils/test_utils_format.py | 103 ++++++++++++++++++++ tiddl/cli/commands/download/__init__.py | 2 +- tiddl/core/api/models/resources.py | 8 +- tiddl/core/utils/format.py | 20 ++-- 5 files changed, 204 insertions(+), 15 deletions(-) create mode 100644 tests/core/api/test_api_models_resources.py create mode 100644 tests/core/utils/test_utils_format.py diff --git a/tests/core/api/test_api_models_resources.py b/tests/core/api/test_api_models_resources.py new file mode 100644 index 0000000..d951e9f --- /dev/null +++ b/tests/core/api/test_api_models_resources.py @@ -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) diff --git a/tests/core/utils/test_utils_format.py b/tests/core/utils/test_utils_format.py new file mode 100644 index 0000000..24fa1c3 --- /dev/null +++ b/tests/core/utils/test_utils_format.py @@ -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 == "" diff --git a/tiddl/cli/commands/download/__init__.py b/tiddl/cli/commands/download/__init__.py index 92b5ce9..55582e8 100644 --- a/tiddl/cli/commands/download/__init__.py +++ b/tiddl/cli/commands/download/__init__.py @@ -414,7 +414,7 @@ def download_callback( video = ctx.obj.api.get_video(resource.id) 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) else: album = None diff --git a/tiddl/core/api/models/resources.py b/tiddl/core/api/models/resources.py index b8f6609..df65746 100644 --- a/tiddl/core/api/models/resources.py +++ b/tiddl/core/api/models/resources.py @@ -69,9 +69,9 @@ class Video(BaseModel): picture: Optional[str] = None class Album(BaseModel): - id: int - title: str - cover: str + id: Optional[int] = None + title: Optional[str] = None + cover: Optional[str] = None vibrantColor: Optional[str] = None videoCover: Optional[str] = None @@ -81,7 +81,7 @@ class Video(BaseModel): trackNumber: int streamStartDate: Optional[datetime] = None imagePath: Optional[str] = None - imageId: str + imageId: Optional[str] = None vibrantColor: Optional[str] = None duration: int quality: Literal["MP4_1080P"] | str diff --git a/tiddl/core/utils/format.py b/tiddl/core/utils/format.py index 420549a..22e99c1 100644 --- a/tiddl/core/utils/format.py +++ b/tiddl/core/utils/format.py @@ -1,5 +1,5 @@ import re -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import datetime from tiddl.core.api.models import Track, Video, Album, Playlist @@ -67,14 +67,14 @@ class UserFormat: @dataclass(slots=True) class AlbumTemplate: - id: int - title: str - artist: str - artists: str - date: datetime - explicit: Explicit - master: UserFormat - release: str + id: int = 0 + title: str = "" + artist: str = "" + artists: str = "" + date: datetime = datetime.min + explicit: Explicit = field(default_factory=lambda: Explicit(None)) + master: UserFormat = field(default_factory=lambda: UserFormat(False)) + release: str = "" @dataclass(slots=True) @@ -156,7 +156,7 @@ def generate_template_data( dolby=dolby, ) - album_template = None + album_template = AlbumTemplate() if album: album_template = AlbumTemplate( id=album.id,