diff --git a/app/src/main/java/com/cappielloantonio/tempo/database/dao/PlaylistDao.java b/app/src/main/java/com/cappielloantonio/tempo/database/dao/PlaylistDao.java index f6aea8d2a..b98892bf5 100644 --- a/app/src/main/java/com/cappielloantonio/tempo/database/dao/PlaylistDao.java +++ b/app/src/main/java/com/cappielloantonio/tempo/database/dao/PlaylistDao.java @@ -23,6 +23,13 @@ public interface PlaylistDao { @Insert(onConflict = OnConflictStrategy.REPLACE) void insert(Playlist playlist); + // Insert only if the row is absent. REPLACE would delete-then-reinsert an existing + // playlist, which fails the playlist_song foreign key when its songs already exist + // (and can race when several callers cache the same playlist at once). Used to make + // sure a deep-linked playlist's row exists before caching its songs. See issue #729. + @Insert(onConflict = OnConflictStrategy.IGNORE) + void insertIfAbsent(Playlist playlist); + @Insert(onConflict = OnConflictStrategy.REPLACE) void insertAll(List playlists); diff --git a/app/src/main/java/com/cappielloantonio/tempo/repository/PlaylistRepository.java b/app/src/main/java/com/cappielloantonio/tempo/repository/PlaylistRepository.java index 5d822dae9..4199a246e 100644 --- a/app/src/main/java/com/cappielloantonio/tempo/repository/PlaylistRepository.java +++ b/app/src/main/java/com/cappielloantonio/tempo/repository/PlaylistRepository.java @@ -177,7 +177,7 @@ public void onResponse(@NonNull Call call, @NonNull Response(); } listLivePlaylistSongs.setValue(songs); - cachePlaylistSongs(id, songs); + cachePlaylistSongs(sr.getPlaylist(), songs); } else if (sr.getError() != null && sr.getError().getCode() != null && sr.getError().getCode() == 70) { // Subsonic Standard Error Code 70: The requested data was not found. handleMissingPlaylist(id, null); @@ -199,8 +199,16 @@ public void onFailure(@NonNull Call call, @NonNull Throwable t) { return listLivePlaylistSongs; } - private void cachePlaylistSongs(String playlistId, List songs) { + private void cachePlaylistSongs(Playlist playlist, List songs) { new Thread(() -> { + String playlistId = playlist.getId(); + // playlist_song has a foreign key to playlist.id, so the playlist row must + // exist before its songs are inserted. The full-list path caches playlists via + // cacheAllPlaylists, but the single-playlist endpoint (used by deep links) does + // not — without this the songs insert crashes with a FOREIGN KEY constraint. + // Insert before the early-return dedup checks so the row is ensured even when + // the songs are already cached. See issue #729. + playlistDao.insertIfAbsent(playlist); List cached = playlistSongDao.getSongsForPlaylistSync(playlistId); if (songs == null || songs.isEmpty()) { if (cached != null && !cached.isEmpty()) { diff --git a/app/src/main/java/com/cappielloantonio/tempo/subsonic/models/PlaylistWithSongs.kt b/app/src/main/java/com/cappielloantonio/tempo/subsonic/models/PlaylistWithSongs.kt index 350dcbd4a..eae7c94b3 100644 --- a/app/src/main/java/com/cappielloantonio/tempo/subsonic/models/PlaylistWithSongs.kt +++ b/app/src/main/java/com/cappielloantonio/tempo/subsonic/models/PlaylistWithSongs.kt @@ -8,8 +8,24 @@ import kotlinx.parcelize.Parcelize @Keep @Parcelize class PlaylistWithSongs( - @SerializedName("_id") - override var id: String, @SerializedName("entry") var entries: List? = null, -) : Playlist(id), Parcelable \ No newline at end of file +) : Playlist(""), Parcelable { + + // Do NOT redeclare `id` here. Overriding the base property creates a second `id` + // backing field, and Gson then sees two JSON fields named "id" (this class's and + // Playlist's) and throws "declares multiple JSON fields named 'id'" at startup. + // The previous workaround mapped this field to "_id", which dodged the clash but + // left the id null for the Subsonic getPlaylist response (it sends "id"): opening a + // playlist by id — e.g. a tempo://asset/playlist/ deep link — then re-fetched + // its songs with a null id, the server replied "missing parameter: 'id'", the page + // showed a perpetual spinner plus a misleading "Playlist not found" dialog, and the + // null id later crashed with a String.equals NPE. Inheriting Playlist.id (which maps + // "id" correctly) fixes all of that; the value still survives Parcelize because the + // base class is @Parcelize too. See issue #729. + + // The synthetic "all songs" search result is built locally with a fixed id. + constructor(id: String, entries: List?) : this(entries) { + this.id = id + } +}