diff --git a/jukebox/src/main/java/at/lockstep/jukebox/DefaultPlaylistRepository.java b/jukebox/src/main/java/at/lockstep/jukebox/DefaultPlaylistRepository.java index 7999a09..82c54dc 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/DefaultPlaylistRepository.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/DefaultPlaylistRepository.java @@ -9,6 +9,7 @@ import androidx.lifecycle.Transformations; import at.lockstep.jukebox.api.LockstepApiException; import at.lockstep.jukebox.api.PlaylistRemoteClient; import at.lockstep.jukebox.api.PlaylistRetrofitApi; +import at.lockstep.jukebox.api.RetryOnRateLimitInterceptor; import at.lockstep.jukebox.db.JukeboxDatabase; import at.lockstep.jukebox.db.PlaylistDao; import at.lockstep.jukebox.db.PlaylistImageEntity; @@ -65,6 +66,11 @@ public final class DefaultPlaylistRepository implements PlaylistRepository { syncCoordinator.syncDelta(retainRemovedPlaylists); } + @Override + public void syncPlaylistDetail(@NonNull String playlistId) throws IOException, LockstepApiException { + syncCoordinator.syncPlaylistDetail(playlistId); + } + @Override @NonNull public LiveData> observePlaylists() { @@ -161,6 +167,7 @@ public final class DefaultPlaylistRepository implements PlaylistRepository { .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) .create(); okhttp3.OkHttpClient okHttp = new okhttp3.OkHttpClient.Builder() + .addInterceptor(new RetryOnRateLimitInterceptor()) .addInterceptor(authInterceptor) .build(); Retrofit retrofit = new Retrofit.Builder() diff --git a/jukebox/src/main/java/at/lockstep/jukebox/PlaylistRepository.java b/jukebox/src/main/java/at/lockstep/jukebox/PlaylistRepository.java index 5069cec..85ac29c 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/PlaylistRepository.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/PlaylistRepository.java @@ -16,6 +16,11 @@ public interface PlaylistRepository { void syncDelta(boolean retainRemovedPlaylists) throws IOException, LockstepApiException; + /** + * Fetches full playlist JSON for one id and replaces cached rows for that playlist only. + */ + void syncPlaylistDetail(@NonNull String playlistId) throws IOException, LockstepApiException; + @NonNull LiveData> observePlaylists(); diff --git a/jukebox/src/main/java/at/lockstep/jukebox/api/RetryOnRateLimitInterceptor.java b/jukebox/src/main/java/at/lockstep/jukebox/api/RetryOnRateLimitInterceptor.java new file mode 100644 index 0000000..f790b14 --- /dev/null +++ b/jukebox/src/main/java/at/lockstep/jukebox/api/RetryOnRateLimitInterceptor.java @@ -0,0 +1,99 @@ +package at.lockstep.jukebox.api; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import java.io.IOException; +import java.util.concurrent.ThreadLocalRandom; + +import okhttp3.Interceptor; +import okhttp3.Request; +import okhttp3.Response; + +/** + * Retries idempotent GET requests on HTTP 429 and 503 with exponential backoff, optional jitter, + * and {@code Retry-After} (seconds) when the server sends it. Helps when Lockstep proxies Spotify + * and inherits rate limits. + */ +public final class RetryOnRateLimitInterceptor implements Interceptor { + + private final int maxRetries; + private final long initialBackoffMs; + private final double backoffMultiplier; + private final long maxBackoffMs; + + public RetryOnRateLimitInterceptor() { + // Spotify / Lockstep can stay hot after bulk sync; extra attempts + longer cap help pairing-on-demand. + this(8, 1000L, 2.0, 120_000L); + } + + public RetryOnRateLimitInterceptor( + int maxRetries, + long initialBackoffMs, + double backoffMultiplier, + long maxBackoffMs + ) { + this.maxRetries = maxRetries; + this.initialBackoffMs = initialBackoffMs; + this.backoffMultiplier = backoffMultiplier; + this.maxBackoffMs = maxBackoffMs; + } + + @NonNull + @Override + public Response intercept(@NonNull Chain chain) throws IOException { + Request request = chain.request(); + if (!"GET".equalsIgnoreCase(request.method())) { + return chain.proceed(request); + } + + long backoffMs = initialBackoffMs; + Response response = chain.proceed(request); + for (int retry = 0; retry <= maxRetries; retry++) { + if (response.code() != 429 && response.code() != 503) { + return response; + } + if (retry == maxRetries) { + return response; + } + long retryAfterMs = parseRetryAfterMs(response.header("Retry-After")); + long waitMs = Math.max(backoffMs, retryAfterMs); + response.close(); + sleepWithJitter(waitMs); + backoffMs = Math.min((long) (backoffMs * backoffMultiplier), maxBackoffMs); + response = chain.proceed(request); + } + return response; + } + + private void sleepWithJitter(long baseMs) throws IOException { + if (baseMs <= 0) { + return; + } + long jitter = baseMs > 4 ? ThreadLocalRandom.current().nextLong(0, baseMs / 4) : 0L; + try { + Thread.sleep(baseMs + jitter); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("interrupted during rate-limit backoff", e); + } + } + + /** + * Parses {@code Retry-After} as a delay in seconds (Spotify / many proxies use this form). + * HTTP-date form is not parsed; returns 0 so exponential backoff applies alone. + */ + private static long parseRetryAfterMs(@Nullable String header) { + if (header == null || header.isEmpty()) { + return 0L; + } + try { + long seconds = Long.parseLong(header.trim()); + if (seconds > 0) { + return Math.min(seconds * 1000L, 300_000L); + } + } catch (NumberFormatException ignored) { + } + return 0L; + } +} 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 0c6343d..628d122 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/map/PlaylistMappers.java @@ -1,5 +1,7 @@ package at.lockstep.jukebox.map; +import android.util.Log; + import androidx.annotation.NonNull; import at.lockstep.jukebox.api.dto.ArtistDto; @@ -24,6 +26,8 @@ import java.util.Map; */ public final class PlaylistMappers { + private static final String TAG = "JukeboxPlaylist"; + private PlaylistMappers() { } @@ -60,15 +64,22 @@ public final class PlaylistMappers { /** * 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. */ @NonNull public static PlaylistEntity toPlaylistEntity(@NonNull FullPlaylistDto dto) { 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); + } return new PlaylistEntity( dto.id, dto.description, - dto.name, + displayName, dto.primaryColor, dto.snapshotId, tracksHref, 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 e18d31b..64bd3ab 100644 --- a/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java +++ b/jukebox/src/main/java/at/lockstep/jukebox/sync/SyncCoordinator.java @@ -25,8 +25,8 @@ import java.util.concurrent.Future; /** * Orchestrates loading playlist metadata from {@link LockstepPlaylistClient} and writing it into - * {@link PlaylistDao}: full initial import (clear + fetch all details) and incremental sync using - * {@code snapshot_id} from the playlist list endpoint. Individual playlists are always replaced + * {@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. *

* Network detail fetches run in parallel up to the pool size passed to @@ -54,24 +54,25 @@ public final class SyncCoordinator { /** * Same as {@link #SyncCoordinator(PlaylistDao, LockstepPlaylistClient, int)} with parallel detail - * concurrency of {@code 6}. + * 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, 6); + this(dao, remote, 3); } /** - * Clears all local cache tables, fetches every playlist id from the list endpoint, loads each full - * playlist, and persists them. Use for first-time or “reset” sync. + * 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. */ public void syncInitial() throws IOException, LockstepApiException { - dao.clearAllTables(); 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); } @@ -157,6 +158,15 @@ public final class SyncCoordinator { } } + /** + * 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. + */ + 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. */