diff --git a/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java b/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java index 628d122..17022b7 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java @@ -8,6 +8,7 @@ import at.lockstep.jukebox.api.dto.ArtistDto; import at.lockstep.jukebox.api.dto.FullPlaylistDto; import at.lockstep.jukebox.api.dto.ImageDto; import at.lockstep.jukebox.api.dto.PlaylistTrackItemDto; +import at.lockstep.jukebox.api.dto.SimplifiedPlaylistDto; import at.lockstep.jukebox.api.dto.TrackDto; import at.lockstep.jukebox.db.PlaylistEntity; import at.lockstep.jukebox.db.PlaylistImageEntity; @@ -62,6 +63,34 @@ public final class PlaylistMappers { ); } + /** + * Maps a playlist list entry to a {@link PlaylistEntity} without track items (shell row only). + */ + @NonNull + public static PlaylistEntity toPlaylistEntity(@NonNull SimplifiedPlaylistDto dto) { + if (dto.id == null || dto.id.isEmpty()) { + throw new IllegalArgumentException("Simplified playlist missing id"); + } + String tracksHref = dto.tracks != null ? dto.tracks.href : null; + Integer tracksTotal = dto.tracks != null ? dto.tracks.total : null; + String rawName = dto.name; + boolean unnamed = rawName == null || rawName.trim().isEmpty(); + String displayName = unnamed ? "Untitled playlist" : rawName; + if (unnamed) { + Log.i(TAG, "Playlist has empty name, id=" + dto.id); + } + String snapshotId = dto.snapshotId != null ? dto.snapshotId : ""; + return new PlaylistEntity( + dto.id, + dto.description, + displayName, + dto.primaryColor, + snapshotId, + tracksHref, + tracksTotal + ); + } + /** * Maps the playlist metadata from a full playlist response to a {@link PlaylistEntity} row. * Empty or whitespace-only {@code name} becomes {@code "Untitled playlist"} and is logged for debugging. @@ -81,7 +110,7 @@ public final class PlaylistMappers { dto.description, displayName, dto.primaryColor, - dto.snapshotId, + dto.snapshotId != null ? dto.snapshotId : "", tracksHref, tracksTotal ); diff --git a/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java b/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java index 64bd3ab..5f019c4 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java @@ -5,84 +5,63 @@ import androidx.annotation.NonNull; import at.lockstep.jukebox.api.LockstepApiException; import at.lockstep.jukebox.api.LockstepPlaylistClient; import at.lockstep.jukebox.api.dto.FullPlaylistDto; +import at.lockstep.jukebox.api.dto.ImageDto; import at.lockstep.jukebox.api.dto.SimplifiedPlaylistDto; import at.lockstep.jukebox.db.PlaylistDao; +import at.lockstep.jukebox.db.PlaylistEntity; +import at.lockstep.jukebox.db.PlaylistImageEntity; import at.lockstep.jukebox.db.PlaylistSnapshotRow; import at.lockstep.jukebox.map.PlaylistMappers; -import at.lockstep.jukebox.map.PlaylistMappers.PlaylistStorageRows; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; /** * Orchestrates loading playlist metadata from {@link LockstepPlaylistClient} and writing it into - * {@link PlaylistDao}: full initial import (fetch all details, then clear + replace) and incremental - * sync using {@code snapshot_id} from the playlist list endpoint. Individual playlists are always replaced - * wholesale (images, memberships, and tracks for that playlist) inside a single DAO transaction. + * {@link PlaylistDao}. *

- * Network detail fetches run in parallel up to the pool size passed to - * {@link #SyncCoordinator(PlaylistDao, LockstepPlaylistClient, int)}; methods block until finished and - * must not be called on the Android main thread. + * Normal refreshes use {@link #syncDelta(boolean)}: only the playlist list endpoint (names, images, + * snapshot ids). When a playlist's snapshot changes, its cached track rows are cleared until the user opens + * it again. Full track lists load lazily via {@link #syncPlaylistDetail(String)}. + *

+ * {@link #syncInitial()} performs a full reset ({@link PlaylistDao#clearAllTables()}) then persists list + * metadata only—use rarely; prefer {@link #syncDelta(boolean)} so lazily loaded tracks survive app restarts. + *

+ * Methods block until finished and must not be called on the Android main thread. */ public final class SyncCoordinator { private final PlaylistDao dao; private final LockstepPlaylistClient remote; - private final int detailParallelism; - /** - * @param detailParallelism maximum concurrent calls to {@link LockstepPlaylistClient#fetchPlaylistDetail(String)} - */ - public SyncCoordinator( - @NonNull PlaylistDao dao, - @NonNull LockstepPlaylistClient remote, - int detailParallelism - ) { + public SyncCoordinator(@NonNull PlaylistDao dao, @NonNull LockstepPlaylistClient remote) { this.dao = dao; this.remote = remote; - this.detailParallelism = detailParallelism; } /** - * Same as {@link #SyncCoordinator(PlaylistDao, LockstepPlaylistClient, int)} with parallel detail - * concurrency of {@code 3} to reduce burst traffic against Spotify via the Lockstep API. - */ - public SyncCoordinator(@NonNull PlaylistDao dao, @NonNull LockstepPlaylistClient remote) { - this(dao, remote, 3); - } - - /** - * Fetches every playlist id from the list endpoint, loads each full playlist, then clears local - * cache tables and persists the new snapshot. Network I/O happens before {@link PlaylistDao#clearAllTables()} - * so a failed import (e.g. HTTP 429 mid-way) does not leave the UI with an empty cache. + * Fetches playlist summaries, clears local cache, and stores list metadata and cover images only + * (no per-playlist track fetch). Network I/O is list-only so opening the app does not fan out + * {@code GET /playlists/{id}} for every playlist. */ public void syncInitial() throws IOException, LockstepApiException { List summaries = remote.fetchPlaylistSummaries(); - List ids = new ArrayList<>(summaries.size()); - for (SimplifiedPlaylistDto s : summaries) { - ids.add(s.id); - } - List details = fetchDetailsParallel(ids); dao.clearAllTables(); - for (FullPlaylistDto detail : details) { - persistFullPlaylist(detail); + for (SimplifiedPlaylistDto s : summaries) { + persistPlaylistShellFromSummary(s); } } /** - * Fetches the current playlist list, compares each {@code snapshot_id} to the local DB, and refetches - * full detail only for new or changed playlists. Playlists missing from the remote list are deleted - * locally (including cascading images and track links) only when {@code retainRemovedPlaylists} is - * {@code false}, in which case orphan {@link at.lockstep.jukebox.db.TrackEntity} rows are pruned. + * Fetches the playlist list, updates local shells when a playlist is new or its {@code snapshot_id} + * changed, and optionally drops removed playlists. Does not call the detail endpoint; when a snapshot + * changes, cached track rows for that playlist are cleared until the user opens it again. */ public void syncDelta(boolean retainRemovedPlaylists) throws IOException, LockstepApiException { List remoteSummaries = remote.fetchPlaylistSummaries(); @@ -90,21 +69,22 @@ public final class SyncCoordinator { for (PlaylistSnapshotRow row : dao.getPlaylistSnapshots()) { localSnapshots.put(row.id, row.snapshotId); } - List idsToRefresh = new ArrayList<>(); for (SimplifiedPlaylistDto summary : remoteSummaries) { - String local = localSnapshots.get(summary.id); - if (local == null || !local.equals(summary.snapshotId)) { - idsToRefresh.add(summary.id); + if (summary.id == null || summary.id.isEmpty()) { + continue; + } + String remoteSnap = summary.snapshotId != null ? summary.snapshotId : ""; + String local = localSnapshots.get(summary.id); + if (local == null || !local.equals(remoteSnap)) { + persistPlaylistShellFromSummary(summary); } - } - List details = fetchDetailsParallel(idsToRefresh); - for (FullPlaylistDto detail : details) { - persistFullPlaylist(detail); } if (!retainRemovedPlaylists) { Set remoteIds = new HashSet<>(); for (SimplifiedPlaylistDto s : remoteSummaries) { - remoteIds.add(s.id); + if (s.id != null && !s.id.isEmpty()) { + remoteIds.add(s.id); + } } List removedLocally = new ArrayList<>(); for (String localId : localSnapshots.keySet()) { @@ -114,64 +94,40 @@ public final class SyncCoordinator { } if (!removedLocally.isEmpty()) { dao.deletePlaylistsByIds(removedLocally); - dao.deleteOrphanTracks(); } } + dao.deleteOrphanTracks(); } - /** - * Loads full playlist JSON for each id using a bounded thread pool; preserves input order in the result. - */ - @NonNull - private List fetchDetailsParallel(@NonNull List ids) - throws IOException, LockstepApiException { - if (ids.isEmpty()) { - return new ArrayList<>(); + private void persistPlaylistShellFromSummary(@NonNull SimplifiedPlaylistDto s) { + if (s.id == null || s.id.isEmpty()) { + return; } - ExecutorService pool = Executors.newFixedThreadPool(detailParallelism); - try { - List> futures = new ArrayList<>(ids.size()); - for (String id : ids) { - futures.add(pool.submit(() -> remote.fetchPlaylistDetail(id))); - } - List out = new ArrayList<>(ids.size()); - try { - for (Future f : futures) { - out.add(f.get()); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException(e); - } catch (ExecutionException e) { - Throwable c = e.getCause(); - if (c instanceof IOException) { - throw (IOException) c; - } - if (c instanceof LockstepApiException) { - throw (LockstepApiException) c; - } - throw new IOException(c != null ? c : e); - } - return out; - } finally { - pool.shutdown(); + PlaylistEntity playlist = PlaylistMappers.toPlaylistEntity(s); + List images = new ArrayList<>(); + List imageDtos = s.imagesOrEmpty(); + for (int i = 0; i < imageDtos.size(); i++) { + images.add(PlaylistMappers.toImageEntity(imageDtos.get(i), s.id, i)); } + dao.replacePlaylistContent( + playlist, + images, + Collections.emptyList(), + Collections.emptyList() + ); } /** * Fetches {@code GET /playlists/{playlistId}} and replaces cached images, tracks, and memberships for - * that playlist only. Use when the playlist header exists locally but track rows are missing. + * that playlist only. */ public void syncPlaylistDetail(@NonNull String playlistId) throws IOException, LockstepApiException { FullPlaylistDto detail = remote.fetchPlaylistDetail(playlistId); persistFullPlaylist(detail); } - /** - * Maps {@code detail} to entities and runs {@link PlaylistDao#replacePlaylistContent} for that playlist. - */ private void persistFullPlaylist(@NonNull FullPlaylistDto detail) { - PlaylistStorageRows rows = PlaylistMappers.toPlaylistStorageRows(detail); + PlaylistMappers.PlaylistStorageRows rows = PlaylistMappers.toPlaylistStorageRows(detail); dao.replacePlaylistContent( PlaylistMappers.toPlaylistEntity(detail), rows.images, diff --git a/jukebox/src/test/java/at/lockstep/jukebox/sync/SyncCoordinatorTest.java b/jukebox/src/test/java/at/lockstep/jukebox/sync/SyncCoordinatorTest.java index 98d3ab6..8e6ba6f 100644 --- a/jukebox/src/test/java/at/lockstep/jukebox/sync/SyncCoordinatorTest.java +++ b/jukebox/src/test/java/at/lockstep/jukebox/sync/SyncCoordinatorTest.java @@ -53,34 +53,45 @@ public class SyncCoordinatorTest { } @Test - public void syncDelta_sameSnapshot_skipsDetailFetch() throws Exception { + public void syncInitial_listOnly_noDetailCalls() throws Exception { FakeRemote remote = new FakeRemote(); remote.listItems.add(simplified("p1", "snap1")); remote.details.put("p1", detailWithTrack("p1", "snap1", "t1", "Song")); SyncCoordinator coordinator = new SyncCoordinator(db.playlistDao(), remote); coordinator.syncInitial(); + Assert.assertTrue(remote.detailCallIds.isEmpty()); + Assert.assertTrue(db.playlistDao().getTracksForPlaylist("p1").isEmpty()); + } + + @Test + public void syncDelta_sameSnapshot_skipsShellRewrite() throws Exception { + FakeRemote remote = new FakeRemote(); + remote.listItems.add(simplified("p1", "snap1")); + SyncCoordinator coordinator = new SyncCoordinator(db.playlistDao(), remote); + coordinator.syncInitial(); remote.detailCallIds.clear(); coordinator.syncDelta(true); Assert.assertTrue(remote.detailCallIds.isEmpty()); } @Test - public void syncDelta_changedSnapshot_refetchesDetail() throws Exception { + public void syncDelta_changedSnapshot_clearsTracksWithoutDetailFetch() throws Exception { FakeRemote remote = new FakeRemote(); remote.listItems.add(simplified("p1", "s1")); remote.details.put("p1", detailWithTrack("p1", "s1", "t1", "Old")); SyncCoordinator coordinator = new SyncCoordinator(db.playlistDao(), remote); coordinator.syncInitial(); + coordinator.syncPlaylistDetail("p1"); + Assert.assertEquals(1, db.playlistDao().getTracksForPlaylist("p1").size()); + remote.listItems.clear(); remote.listItems.add(simplified("p1", "s2")); remote.details.put("p1", detailWithTrack("p1", "s2", "t2", "New")); remote.detailCallIds.clear(); coordinator.syncDelta(true); - Assert.assertEquals(Collections.singletonList("p1"), remote.detailCallIds); + Assert.assertTrue(remote.detailCallIds.isEmpty()); List tracks = db.playlistDao().getTracksForPlaylist("p1"); - Assert.assertEquals(1, tracks.size()); - Assert.assertEquals("t2", tracks.get(0).trackId); - Assert.assertEquals("New", tracks.get(0).trackName); + Assert.assertEquals(0, tracks.size()); } @Test @@ -88,8 +99,6 @@ public class SyncCoordinatorTest { FakeRemote remote = new FakeRemote(); remote.listItems.add(simplified("p1", "s1")); remote.listItems.add(simplified("gone", "sg")); - remote.details.put("p1", minimalDetail("p1", "s1")); - remote.details.put("gone", minimalDetail("gone", "sg")); SyncCoordinator coordinator = new SyncCoordinator(db.playlistDao(), remote); coordinator.syncInitial(); remote.listItems.clear(); @@ -103,7 +112,6 @@ public class SyncCoordinatorTest { public void syncDelta_retainsRemovedPlaylist() throws Exception { FakeRemote remote = new FakeRemote(); remote.listItems.add(simplified("p1", "s1")); - remote.details.put("p1", minimalDetail("p1", "s1")); SyncCoordinator coordinator = new SyncCoordinator(db.playlistDao(), remote); coordinator.syncInitial(); FullPlaylistDto orphan = minimalDetail("orphan", "so");