fix: deep-linked playlist loads no songs and crashes from null id (#729)#789
fix: deep-linked playlist loads no songs and crashes from null id (#729)#789herrerad85 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)❌ Error creating Unit Test PR.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ddyizm#729) Opening a playlist by id — e.g. a tempo://asset/playlist/<id> deep link from Tasker — showed the playlist metadata but never its songs (perpetual spinner), then crashed after a minute or two. Root cause: PlaylistWithSongs, the type the Subsonic getPlaylist response deserializes into, annotated its id with @SerializedName("_id"), but the API sends "id". The "_id" was a workaround for Gson's "declares multiple JSON fields named 'id'" error (the property shadowed Playlist.id), but it left the id null. The page then re-fetched the songs with a null id; Retrofit dropped the null @query("id"), the server replied "missing parameter: 'id'", and the song list spun forever behind a misleading "Playlist not found" dialog. The null id is also what produced the reporter's String.equals NPE. Fix: - PlaylistWithSongs no longer redeclares id; it inherits Playlist.id, which maps "id" correctly. The value still survives Parcelize (the base class is @parcelize too) and a secondary constructor keeps the local "all songs" use. - Caching a deep-linked playlist's songs then hit a FOREIGN KEY constraint, because the single-playlist endpoint never inserts the playlist row (only the full-list path does, via cacheAllPlaylists). cachePlaylistSongs now inserts the parent first, through a new insertIfAbsent (IGNORE) so it can't delete-replace a row that already has child songs. Verified on the android-35 emulator against the Navidrome demo: the tempo://asset/playlist/<id> intent now loads metadata, cover art and the full song list with no crash, no FK violation and no false "not found" dialog; back-to-back deep links to two different playlists both succeed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
db60e8e to
a957d67
Compare
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
2 similar comments
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
2 similar comments
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
Fixes #729.
Problem
Opening a playlist by id — e.g. a
tempo://asset/playlist/<id>deep link fromTasker — shows the playlist's metadata but never its songs (endless spinner),
then crashes after a minute or two.
Root cause
PlaylistWithSongs(the type the SubsonicgetPlaylistresponse deserializesinto) annotated its
idwith@SerializedName("_id"), but the API sendsid.The
_idwas a workaround for Gson's "declares multiple JSON fields named 'id'"error (the property shadows
Playlist.id), but it left the id null. The pagethen re-fetched the songs with a null id; Retrofit drops the null
@Query("id"),the server replies
error 10: missing parameter: 'id', and the song list spinsforever behind a misleading "Playlist not found" dialog. The null id is also what
produces the reported
String.equalsNPE. Opening a playlist from the in-applist works because the list endpoint deserializes the base
Playlist, which mapsidcorrectly.Fix
PlaylistWithSongsno longer redeclaresid; it inheritsPlaylist.id, whichmaps
idcorrectly. The value still survives@Parcelize(the base class is@Parcelizetoo), and a secondary constructor keeps the local "all songs" use.FOREIGN KEYconstraint,because the single-playlist endpoint never inserts the playlist row (only the
full-list path does, via
cacheAllPlaylists).cachePlaylistSongsnow insertsthe parent first, via a new
insertIfAbsent(IGNORE) so it can'tdelete-replace a row that already has child songs.
Testing
Verified on an android-35 emulator against the Navidrome public demo:
dialog, then crash.
no FK violation, no false "not found"; back-to-back deep links to two different
playlists both succeed.
Builds successfully on the development toolchain (Gradle 9.4.1 / AGP 9.2.1).
🤖 Generated with Claude Code