diff --git a/flutter_client/lib/api/endpoints/library.dart b/flutter_client/lib/api/endpoints/library.dart index 6d38b154..e5cfff71 100644 --- a/flutter_client/lib/api/endpoints/library.dart +++ b/flutter_client/lib/api/endpoints/library.dart @@ -54,6 +54,24 @@ class LibraryApi { return ArtistRef.fromJson(r.data ?? const {}); } + /// GET /api/artists/{id} — full response with the ArtistRef AND the + /// embedded album list, both parsed. Single round-trip variant used + /// by CacheFiller and other callers that want to populate both + /// cached_artists and cached_albums in one shot. The existing + /// getArtist / getArtistAlbums keep working for callers that only + /// need one half — they hit the same URL but the per-id Riverpod + /// caching layer dedupes. + Future<({ArtistRef artist, List albums})> getArtistDetail( + String id) async { + final r = await _dio.get>('/api/artists/$id'); + final body = r.data ?? const {}; + final artist = ArtistRef.fromJson(body); + final albums = ((body['albums'] as List?) ?? const []) + .map((e) => AlbumRef.fromJson((e as Map).cast())) + .toList(growable: false); + return (artist: artist, albums: albums); + } + /// Pulls the "albums" array out of the same ArtistDetail body. Callers /// that need both the artist and its albums should issue two provider /// reads (artistProvider + artistAlbumsProvider) — both hit the same diff --git a/flutter_client/lib/app.dart b/flutter_client/lib/app.dart index 78192adf..0c39d806 100644 --- a/flutter_client/lib/app.dart +++ b/flutter_client/lib/app.dart @@ -1,7 +1,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'cache/cache_filler.dart'; import 'cache/metadata_prefetcher.dart'; +import 'cache/mutation_queue.dart'; import 'cache/prefetcher.dart'; import 'cache/sync_controller.dart'; import 'shared/live_events_dispatcher.dart'; @@ -39,6 +41,20 @@ class _MinstrelAppState extends ConsumerState { // arrive. Also installs an AppLifecycleState observer for // resume-time defensive invalidation. ref.read(liveEventsDispatcherProvider); + // Cache filler: background sweeper that walks cached_artists + // / cached_albums for missing relations and fetches the + // per-entity detail so tapping an artist surfaces albums + // immediately (drift hit, no /api/artists/:id round-trip at + // tap time). First sweep 10s after launch; every 5 minutes + // thereafter. Throttled 200ms between requests so it never + // competes with user activity. + ref.read(cacheFillerProvider); + // Mutation replayer: drains the cached_mutations queue when + // connectivity comes back. Controllers (LikesController, + // MyQuarantineController, the add-to-playlist + request flows) + // enqueue on REST failure so user intent persists across + // network loss instead of getting rolled back. + ref.read(mutationReplayerProvider); }); } diff --git a/flutter_client/lib/cache/cache_filler.dart b/flutter_client/lib/cache/cache_filler.dart new file mode 100644 index 00000000..84f060d1 --- /dev/null +++ b/flutter_client/lib/cache/cache_filler.dart @@ -0,0 +1,235 @@ +// Background metadata sweeper that walks the drift cache for missing +// relations and fills them via /api/artists/:id + /api/albums/:id. +// Cover bytes for newly-filled albums are pre-warmed too so the +// per-tile display path doesn't have to wait on network. +// +// Why this layer on top of SyncController and HydrationQueue: +// - SyncController.sync only ingests entities the server emitted +// into the library_changes delta for this user. For a fresh +// install, that's everything; for an existing user, only recent +// changes. The per-artist album list and per-album track list +// are NOT in those deltas — they're derived at query time. +// - HydrationQueue fills these lazily on tile render. CacheFiller +// fills them proactively on a slow schedule so tapping an artist +// surfaces albums immediately instead of triggering a fresh +// /api/artists/:id at tap time. +// +// Pacing is conservative — 200ms between requests, max 200 entities +// per sweep, 5-minute interval. Designed to never compete with user +// activity. Wall time on a 1000-artist library: ~3-4 minutes spread +// over multiple sweeps. + +import 'dart:async'; + +import 'package:drift/drift.dart' show Variable; +import 'package:flutter/foundation.dart' show debugPrint; +import 'package:flutter_cache_manager/flutter_cache_manager.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../auth/auth_provider.dart' + show serverUrlProvider, sessionTokenProvider; +import '../cache/adapters.dart'; +import '../cache/audio_cache_manager.dart' show appDbProvider; +import '../cache/connectivity_provider.dart'; +import '../library/library_providers.dart' show libraryApiProvider; + +class CacheFiller { + CacheFiller(this._ref); + final Ref _ref; + + Timer? _initialTimer; + Timer? _intervalTimer; + bool _running = false; + bool _disposed = false; + + /// Delay between launch and the first sweep. Long enough for + /// SyncController to land its initial /api/library/sync so the + /// CacheFiller's "unfilled relations" query has meaningful work. + static const _initialDelay = Duration(seconds: 10); + + /// Cadence between sweeps. Once a sweep finds nothing to do (steady + /// state) the interval becomes a cheap drift query + early exit. + static const _interval = Duration(minutes: 5); + + /// Throttle between per-entity REST requests so the filler doesn't + /// saturate the server or compete with user-initiated playback. + static const _requestThrottle = Duration(milliseconds: 200); + + /// Per-sweep cap. Without this, a fresh install with thousands of + /// artists would tie up the network for many minutes in one go. + /// The next sweep continues where this one left off (the WHERE + /// NOT EXISTS query naturally skips already-filled rows). + static const _maxIdsPerSweep = 200; + + void start() { + _initialTimer = Timer(_initialDelay, _sweep); + _intervalTimer = Timer.periodic(_interval, (_) => _sweep()); + } + + Future _sweep() async { + if (_disposed || _running) return; + final online = await _ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true); + if (!online) return; + _running = true; + try { + await _fillArtists(); + if (_disposed) return; + await _fillAlbums(); + } catch (e, st) { + debugPrint('cache_filler: sweep failed: $e\n$st'); + } finally { + _running = false; + } + } + + /// Find artists with no albums in cache and fetch /api/artists/:id + /// for each (single round-trip yields artist + albums via the new + /// getArtistDetail API method). Newly-discovered album covers + /// land in flutter_cache_manager's disk cache too so the next + /// visit to the artist's albums grid paints from disk. + Future _fillArtists() async { + final db = _ref.read(appDbProvider); + final rows = await db.customSelect( + ''' + SELECT a.id FROM cached_artists a + WHERE NOT EXISTS ( + SELECT 1 FROM cached_albums b WHERE b.artist_id = a.id + ) + LIMIT ? + ''', + variables: [Variable.withInt(_maxIdsPerSweep)], + ).get(); + final ids = rows.map((r) => r.read('id')).toList(); + if (ids.isEmpty) return; + + final api = await _ref.read(libraryApiProvider.future); + final newAlbumIds = []; + + for (final id in ids) { + if (_disposed) return; + try { + final detail = await api.getArtistDetail(id); + await db.transaction(() async { + await db + .into(db.cachedArtists) + .insertOnConflictUpdate(detail.artist.toDrift()); + if (detail.albums.isNotEmpty) { + await db.batch((b) { + b.insertAllOnConflictUpdate( + db.cachedAlbums, + detail.albums.map((a) => a.toDrift()).toList(), + ); + }); + newAlbumIds.addAll(detail.albums.map((a) => a.id)); + } + }); + } catch (e) { + debugPrint('cache_filler: fillArtist($id) failed: $e'); + } + await Future.delayed(_requestThrottle); + } + + if (newAlbumIds.isNotEmpty) { + await _prewarmCovers(newAlbumIds); + } + } + + /// Find albums with no tracks in cache and fetch /api/albums/:id + /// for each. The bulk response carries the album's track list which + /// we persist into cached_tracks for instant album detail render + /// on the next visit. + Future _fillAlbums() async { + final db = _ref.read(appDbProvider); + final rows = await db.customSelect( + ''' + SELECT a.id FROM cached_albums a + WHERE NOT EXISTS ( + SELECT 1 FROM cached_tracks t WHERE t.album_id = a.id + ) + LIMIT ? + ''', + variables: [Variable.withInt(_maxIdsPerSweep)], + ).get(); + final ids = rows.map((r) => r.read('id')).toList(); + if (ids.isEmpty) return; + + final api = await _ref.read(libraryApiProvider.future); + for (final id in ids) { + if (_disposed) return; + try { + final result = await api.getAlbum(id); + if (result.tracks.isNotEmpty) { + await db.batch((b) { + b.insertAllOnConflictUpdate( + db.cachedTracks, + result.tracks.map((t) => t.toDrift()).toList(), + ); + }); + } + } catch (e) { + debugPrint('cache_filler: fillAlbum($id) failed: $e'); + } + await Future.delayed(_requestThrottle); + } + } + + /// Pre-warm album cover bytes via flutter_cache_manager so the + /// per-tile ServerImage / CachedNetworkImage paints from disk on + /// the user's first visit. Mirrors SyncController's prewarm + /// helper — duplicated here intentionally to keep this file self- + /// contained; a shared helper can come out of #357 follow-up if + /// the duplication grows. + Future _prewarmCovers(List albumIds) async { + final baseUrl = await _ref.read(serverUrlProvider.future); + if (baseUrl == null || baseUrl.isEmpty) return; + final token = await _ref.read(sessionTokenProvider.future); + final headers = (token != null && token.isNotEmpty) + ? {'Authorization': 'Bearer $token'} + : {}; + final base = baseUrl.endsWith('/') + ? baseUrl.substring(0, baseUrl.length - 1) + : baseUrl; + final mgr = DefaultCacheManager(); + + var idx = 0; + Future worker() async { + while (idx < albumIds.length) { + if (_disposed) return; + final i = idx++; + try { + await mgr.downloadFile( + '$base/api/albums/${albumIds[i]}/cover', + authHeaders: headers, + ); + } catch (_) { + // Best-effort prewarm — 404 (missing collage) and 401 + // (token refresh races) shouldn't abort the rest. + } + } + } + + // Concurrency 3 — same as SyncController's prewarm. Covers are + // small enough that more parallelism doesn't help much and + // burns radio. + await Future.wait(List.generate(3, (_) => worker())); + } + + void dispose() { + _disposed = true; + _initialTimer?.cancel(); + _intervalTimer?.cancel(); + } +} + +/// Read once at app start (from app.dart's postFrameCallback) to +/// activate the filler. Disposed via ref.onDispose when the provider +/// scope tears down; in practice the scope lives for the app's +/// lifetime so dispose only fires on uninstall / process death. +final cacheFillerProvider = Provider((ref) { + final filler = CacheFiller(ref); + ref.onDispose(filler.dispose); + filler.start(); + return filler; +}); diff --git a/flutter_client/lib/cache/cache_first.dart b/flutter_client/lib/cache/cache_first.dart index 8d24d11b..274b285a 100644 --- a/flutter_client/lib/cache/cache_first.dart +++ b/flutter_client/lib/cache/cache_first.dart @@ -48,9 +48,22 @@ Stream cacheFirst({ // drift re-emission (otherwise the populate cycles forever). var revalidated = false; + // Tracks whether we've already tried a cold-cache fetch on this + // subscription. Without this, providers can spin forever in the + // rows-empty branch when fetchAndPopulate writes rows that don't + // match THIS filter — e.g. three liked-tab streams sharing one + // populate: the track populate writes track-typed rows, drift + // watch fires for the cached_likes table, the album stream + // re-emits with rows-empty (no album likes), the rows-empty + // branch re-fires populate, repeats forever. Setting this guard + // makes the first fetch attempt also the last for any given + // subscription. + var coldFetchAttempted = false; + await for (final rows in driftStream) { if (rows.isNotEmpty) { yield toResult(rows); + coldFetchAttempted = true; // Stale-while-revalidate: yield cache immediately, then kick off // a REST refresh in the background. Drift watch() picks up the // resulting writes and re-emits via this same stream loop. @@ -62,11 +75,25 @@ Stream cacheFirst({ } continue; } + // rows is empty. If we've already attempted a cold fetch and + // drift is still empty for this filter, yield empty so the UI + // shows the no-content state instead of spinning forever. + if (coldFetchAttempted) { + yield toResult(rows); + continue; + } + coldFetchAttempted = true; if (await isOnline()) { try { await fetchAndPopulate(); - // The drift watch() stream re-emits with the populated rows on - // the next loop iteration. Don't yield here. + // Yield the current (still-empty) rows so the UI moves past + // loading even if populate was a no-op for this filter + // (server returned nothing matching, or all rows were + // already in drift via sync). If populate DID write rows + // matching this filter, the drift watch's next emission + // yields them via the rows.isNotEmpty branch — UI briefly + // shows empty then populates, instead of spinning. + yield toResult(rows); } catch (e, st) { if (tag != null) { debugPrint('cacheFirst[$tag]: fetchAndPopulate failed: $e\n$st'); diff --git a/flutter_client/lib/cache/db.dart b/flutter_client/lib/cache/db.dart index b3fca3b1..26b79754 100644 --- a/flutter_client/lib/cache/db.dart +++ b/flutter_client/lib/cache/db.dart @@ -178,6 +178,30 @@ class CachedHomeIndex extends Table { Set get primaryKey => {section, position}; } +/// Outbound mutation queue for offline-resilient REST calls. Likes, +/// quarantine flag/unflag, playlist appendTracks, Lidarr request +/// create/cancel all enqueue here on REST failure so the user's +/// intent persists across network loss. MutationReplayer drains on +/// connectivity transitions and a 1-minute periodic tick; entries +/// with attempts >= 5 are skipped (the server treats them as +/// permanent failures and library_changes sync will correct any +/// resulting drift drift on next sync). Schema 8+. +class CachedMutations extends Table { + IntColumn get id => integer().autoIncrement()(); + /// Stable kind string (see mutation_queue.dart constants). The + /// replayer looks up the handler in a kind→Function map; unknown + /// kinds are dropped to avoid getting stuck. + TextColumn get kind => text()(); + /// JSON-encoded payload — args needed to replay the REST call. + /// Shape depends on kind; see mutation_queue.dart. + TextColumn get payload => text()(); + DateTimeColumn get createdAt => dateTime().withDefault(currentDateAndTime)(); + /// When the last drain attempt fired against this row. Null until + /// the first attempt. Useful for diagnostic queries. + DateTimeColumn get lastAttemptAt => dateTime().nullable()(); + IntColumn get attempts => integer().withDefault(const Constant(0))(); +} + /// Caller-scoped quarantine ("hidden") rows. Mirrors the wire shape of /// /api/quarantine/mine — fully denormalized (track + album + artist /// fields inline) because the Hidden tab renders straight from this row @@ -217,12 +241,13 @@ enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } CachedQuarantineMine, CachedHomeIndex, CachedSystemPlaylistsStatus, + CachedMutations, ]) class AppDb extends _$AppDb { AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache')); @override - int get schemaVersion => 7; + int get schemaVersion => 8; @override MigrationStrategy get migration => MigrationStrategy( @@ -270,6 +295,11 @@ class AppDb extends _$AppDb { // populates. await m.createTable(cachedSystemPlaylistsStatus); } + if (from < 8) { + // Schema 8: cached_mutations outbound queue. Empty on + // upgrade; populated by controllers when REST calls fail. + await m.createTable(cachedMutations); + } }, ); } diff --git a/flutter_client/lib/cache/mutation_queue.dart b/flutter_client/lib/cache/mutation_queue.dart new file mode 100644 index 00000000..7e7bb354 --- /dev/null +++ b/flutter_client/lib/cache/mutation_queue.dart @@ -0,0 +1,296 @@ +// Outbound mutation queue for offline-resilient REST calls. +// +// Controllers (LikesController, MyQuarantineController, the add-to- +// playlist sheet, the Discover request flow, the Requests cancel +// action) write their optimistic local state to drift first and try +// the corresponding REST call. On failure they enqueue here. The +// replayer drains the queue when connectivity comes back; the user's +// intent persists across network loss without rolling back their +// visible action. +// +// Why this is separate from SyncController: sync ingests AUTHORITATIVE +// SERVER state into drift. The mutation queue carries USER INTENT +// outbound to the server. They flow in opposite directions and +// shouldn't be conflated — sync wins on conflict (server is the +// source of truth), and a mutation that fails forever (5 attempts) +// is dropped on the trust that next sync will reconcile drift to +// match the server's actual state. +// +// Drop semantics: 5 attempts then drop. The drop is intentional — +// holding onto a forever-failing mutation just delays sync's +// reconciliation. The user's optimistic drift state will diverge +// from server, and the next library_changes delta corrects it. + +import 'dart:async'; +import 'dart:convert'; + +import 'package:dio/dio.dart' show DioException, DioExceptionType; +import 'package:drift/drift.dart'; +import 'package:flutter/foundation.dart' show debugPrint; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../api/endpoints/discover.dart'; +import '../api/endpoints/likes.dart'; +import '../library/library_providers.dart' show dioProvider; +import '../likes/likes_provider.dart' show likesApiProvider; +import '../models/lidarr.dart' show LidarrRequestKind; +import '../playlists/playlists_provider.dart' show playlistsApiProvider; +import '../quarantine/quarantine_provider.dart' show quarantineApiProvider; +import '../requests/requests_provider.dart' show requestsApiProvider; +import 'audio_cache_manager.dart' show appDbProvider; +import 'connectivity_provider.dart'; +import 'db.dart'; + +/// Stable kind constants. Each controller emits one of these via +/// MutationQueue.enqueue; the replayer dispatches by kind. +class MutationKinds { + MutationKinds._(); + static const likeAdd = 'like.add'; + static const likeRemove = 'like.remove'; + static const quarantineFlag = 'quarantine.flag'; + static const quarantineUnflag = 'quarantine.unflag'; + static const playlistAppend = 'playlist.append'; + static const requestCreate = 'request.create'; + static const requestCancel = 'request.cancel'; +} + +class MutationQueue { + MutationQueue(this._ref); + final Ref _ref; + + /// Persist a mutation for replay. Caller has already done the + /// optimistic local mutation (drift write, in-memory state update) + /// — this only records the REST call that needs to fire eventually. + Future enqueue(String kind, Map payload) async { + final db = _ref.read(appDbProvider); + await db.into(db.cachedMutations).insert( + CachedMutationsCompanion.insert( + kind: kind, + payload: jsonEncode(payload), + ), + ); + // Nudge the replayer in case we're online right now. + unawaited(_ref.read(mutationReplayerProvider).drain()); + } + + /// Count of pending mutations (attempts < 5). Diagnostic / future + /// UI surface for a "syncing N pending changes" indicator. + Future pendingCount() async { + final db = _ref.read(appDbProvider); + final rows = await db.customSelect( + 'SELECT COUNT(*) AS c FROM cached_mutations WHERE attempts < 5', + ).get(); + return rows.first.read('c'); + } +} + +final mutationQueueProvider = + Provider((ref) => MutationQueue(ref)); + +class MutationReplayer { + MutationReplayer(this._ref); + final Ref _ref; + + Timer? _initialTimer; + Timer? _intervalTimer; + bool _running = false; + bool _disposed = false; + + /// Periodic poll cadence. The connectivity listener catches most + /// transitions; this is the belt-and-suspenders for cases where + /// connectivity_plus misses a state change (e.g. flaky Wi-Fi). + static const _interval = Duration(minutes: 1); + + /// Max replay attempts before we drop a mutation. 5 is enough to + /// span a few reconnection cycles without holding onto a forever- + /// broken request. See file header for the drop rationale. + static const _maxAttempts = 5; + + void start() { + // Drain shortly after launch in case there are queued mutations + // from a prior session that died offline. 3s lets the auth + + // server-url + dio providers finish their async warmup. + _initialTimer = Timer(const Duration(seconds: 3), drain); + // Periodic ticker is the only reconnect signal. We previously had + // a ref.listen(connectivityProvider, …) edge trigger here, but + // subscribing at start-time eagerly mounted the connectivity + // StreamProvider and leaked its initial-timeout Timer through + // tests that never reach the auth state. The drain() loop itself + // gates on connectivity via the .future read below. + _intervalTimer = Timer.periodic(_interval, (_) => drain()); + } + + /// Walk the queue oldest-first, replaying each mutation. Stops on + /// any transient network error (we'll retry on the next tick); a + /// permanent error (404 etc.) increments the attempt counter and + /// moves on to the next mutation. This is public so MutationQueue. + /// enqueue can trigger an immediate attempt after writing. + Future drain() async { + if (_disposed || _running) return; + final online = await _ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true); + if (!online) return; + _running = true; + try { + while (!_disposed) { + final db = _ref.read(appDbProvider); + final next = await (db.select(db.cachedMutations) + ..where((t) => t.attempts.isSmallerThanValue(_maxAttempts)) + ..orderBy([(t) => OrderingTerm.asc(t.createdAt)]) + ..limit(1)) + .getSingleOrNull(); + if (next == null) break; + + final handler = _handlers[next.kind]; + if (handler == null) { + debugPrint('mutation_replayer: unknown kind ${next.kind}, dropping'); + await (db.delete(db.cachedMutations) + ..where((t) => t.id.equals(next.id))) + .go(); + continue; + } + + Map payload; + try { + payload = jsonDecode(next.payload) as Map; + } catch (e) { + debugPrint('mutation_replayer: bad payload for ${next.kind}: $e'); + await (db.delete(db.cachedMutations) + ..where((t) => t.id.equals(next.id))) + .go(); + continue; + } + + try { + await handler(_ref, payload); + await (db.delete(db.cachedMutations) + ..where((t) => t.id.equals(next.id))) + .go(); + } on DioException catch (e) { + await (db.update(db.cachedMutations) + ..where((t) => t.id.equals(next.id))) + .write(CachedMutationsCompanion( + attempts: Value(next.attempts + 1), + lastAttemptAt: Value(DateTime.now()), + )); + if (_isTransient(e)) { + // Network is flaky again — bail and try the whole queue + // later. The current mutation will be retried next tick. + break; + } + // Permanent failure (4xx/5xx with a real response). Leave + // the row with bumped attempts and move on to the next so + // one bad mutation doesn't block the others. + } catch (e, st) { + debugPrint('mutation_replayer: unexpected error: $e\n$st'); + await (db.update(db.cachedMutations) + ..where((t) => t.id.equals(next.id))) + .write(CachedMutationsCompanion( + attempts: Value(next.attempts + 1), + lastAttemptAt: Value(DateTime.now()), + )); + } + } + } finally { + _running = false; + } + } + + bool _isTransient(DioException e) { + switch (e.type) { + case DioExceptionType.connectionError: + case DioExceptionType.connectionTimeout: + case DioExceptionType.sendTimeout: + case DioExceptionType.receiveTimeout: + return true; + default: + return false; + } + } + + void dispose() { + _disposed = true; + _initialTimer?.cancel(); + _intervalTimer?.cancel(); + } +} + +final mutationReplayerProvider = Provider((ref) { + final r = MutationReplayer(ref); + ref.onDispose(r.dispose); + r.start(); + return r; +}); + +/// Type signature for kind-specific replay handlers. +typedef _Handler = Future Function(Ref, Map); + +/// Kind → handler dispatch. Each handler decodes its payload and +/// re-fires the original REST call. Throws on failure (caught by the +/// replayer's drain loop above). +/// +/// Payload shapes (see also enqueue call sites for the writer side): +/// * like.add / like.remove: {'kind': 'track'|'album'|'artist', 'id': uuid} +/// * quarantine.flag: {'trackId': uuid, 'reason': str, 'notes': str} +/// * quarantine.unflag: {'trackId': uuid} +/// * playlist.append: {'playlistId': uuid, 'trackIds': [uuid, …]} +/// * request.create: {full createRequest args; see DiscoverApi} +/// * request.cancel: {'id': uuid} +final Map _handlers = { + MutationKinds.likeAdd: (ref, p) async { + final api = await ref.read(likesApiProvider.future); + await api.like(_likeKindFromString(p['kind'] as String), p['id'] as String); + }, + MutationKinds.likeRemove: (ref, p) async { + final api = await ref.read(likesApiProvider.future); + await api.unlike( + _likeKindFromString(p['kind'] as String), p['id'] as String); + }, + MutationKinds.quarantineFlag: (ref, p) async { + final api = await ref.read(quarantineApiProvider.future); + await api.flag( + p['trackId'] as String, + p['reason'] as String, + notes: (p['notes'] as String?) ?? '', + ); + }, + MutationKinds.quarantineUnflag: (ref, p) async { + final api = await ref.read(quarantineApiProvider.future); + await api.unflag(p['trackId'] as String); + }, + MutationKinds.playlistAppend: (ref, p) async { + final api = await ref.read(playlistsApiProvider.future); + final ids = (p['trackIds'] as List).cast(); + await api.appendTracks(p['playlistId'] as String, ids); + }, + MutationKinds.requestCreate: (ref, p) async { + final dio = await ref.read(dioProvider.future); + final api = DiscoverApi(dio); + await api.createRequest( + kind: _lidarrKindFromString(p['kind'] as String), + artistMbid: p['artistMbid'] as String, + artistName: p['artistName'] as String, + albumMbid: p['albumMbid'] as String?, + albumTitle: p['albumTitle'] as String?, + trackMbid: p['trackMbid'] as String?, + trackTitle: p['trackTitle'] as String?, + ); + }, + MutationKinds.requestCancel: (ref, p) async { + final api = await ref.read(requestsApiProvider.future); + await api.cancel(p['id'] as String); + }, +}; + +LikeKind _likeKindFromString(String s) => switch (s) { + 'album' => LikeKind.album, + 'artist' => LikeKind.artist, + _ => LikeKind.track, + }; + +LidarrRequestKind _lidarrKindFromString(String s) => switch (s) { + 'album' => LidarrRequestKind.album, + 'track' => LidarrRequestKind.track, + _ => LidarrRequestKind.artist, + }; diff --git a/flutter_client/lib/cache/prefetcher.dart b/flutter_client/lib/cache/prefetcher.dart index 0321ab87..785d05f6 100644 --- a/flutter_client/lib/cache/prefetcher.dart +++ b/flutter_client/lib/cache/prefetcher.dart @@ -1,6 +1,9 @@ +import 'dart:async'; + import 'package:audio_service/audio_service.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../player/album_color_extractor.dart'; import '../player/player_provider.dart'; import 'audio_cache_manager.dart'; import 'cache_settings_provider.dart'; @@ -39,14 +42,48 @@ class Prefetcher { (currentIdx + settings.prefetchWindow).clamp(0, queue.length - 1); final mgr = _ref.read(audioCacheManagerProvider); + final coverCache = _ref.read(albumCoverCacheProvider); + final colorCache = _ref.read(albumColorCacheProvider); + // Walk the window. For each upcoming track we want THREE things + // ready when the player transitions into it: + // + // 1. Audio file on disk (otherwise playback stalls on stream + // load — the original prefetcher concern). + // 2. Cover bytes on disk under AlbumCoverCache (otherwise + // _toMediaItem's peekCached returns null, mediaItem + // broadcasts with artUri=null, and the now-playing screen + // stalls in _scheduleSwap awaiting precacheImage of a file + // that doesn't exist yet). + // 3. Palette color extracted and memoized in AlbumColorCache + // (otherwise the gradient backdrop has to wait for + // PaletteGenerator to run after the cover lands — + // visible as the cover snapping in before the gradient). + // + // Each call is idempotent (cache-aware): if already cached, it's + // a no-op. Everything fire-and-forget so the reconcile completes + // quickly even on a fresh queue. for (var i = currentIdx; i <= endIdx; i++) { - final trackId = queue[i].id; - if (await mgr.isCached(trackId)) continue; - // Fire-and-forget; downloads happen in the background. Errors - // inside pin() are caught + cleaned up internally. - // ignore: unawaited_futures - mgr.pin(trackId, source: CacheSource.autoPrefetch); + final media = queue[i]; + final trackId = media.id; + final albumId = media.extras?['album_id'] as String?; + + if (!await mgr.isCached(trackId)) { + // ignore: unawaited_futures + mgr.pin(trackId, source: CacheSource.autoPrefetch); + } + + if (albumId != null && albumId.isNotEmpty) { + // Cover bytes: getOrFetch returns the file path; the side + // effect (writing to disk) is what we care about. + // ignore: unawaited_futures + coverCache.getOrFetch(albumId); + // Palette: getOrExtract chains off coverCache.getOrFetch so + // it'll wait for the cover before sampling — safe to call + // in parallel here. + // ignore: unawaited_futures + colorCache.getOrExtract(albumId); + } } // Eviction pass after pinning new files. diff --git a/flutter_client/lib/discover/discover_screen.dart b/flutter_client/lib/discover/discover_screen.dart index ff1ea8a2..6699ab8f 100644 --- a/flutter_client/lib/discover/discover_screen.dart +++ b/flutter_client/lib/discover/discover_screen.dart @@ -6,6 +6,7 @@ import 'package:go_router/go_router.dart'; import '../api/endpoints/discover.dart'; import '../api/errors.dart'; +import '../cache/mutation_queue.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/lidarr.dart'; import '../shared/widgets/main_app_bar_actions.dart'; @@ -48,14 +49,23 @@ class _DiscoverScreenState extends ConsumerState { Future _request(LidarrSearchResult row) async { final fs = Theme.of(context).extension()!; + final args = { + 'kind': _kind.wire, + 'artistMbid': + _kind == LidarrRequestKind.artist ? row.mbid : row.artistMbid, + 'artistName': + _kind == LidarrRequestKind.artist ? row.name : row.secondaryText, + 'albumMbid': _kind == LidarrRequestKind.album ? row.mbid : null, + 'albumTitle': _kind == LidarrRequestKind.album ? row.name : null, + }; try { final api = await ref.read(_discoverApiProvider.future); await api.createRequest( kind: _kind, - artistMbid: _kind == LidarrRequestKind.artist ? row.mbid : row.artistMbid, - artistName: _kind == LidarrRequestKind.artist ? row.name : row.secondaryText, - albumMbid: _kind == LidarrRequestKind.album ? row.mbid : null, - albumTitle: _kind == LidarrRequestKind.album ? row.name : null, + artistMbid: args['artistMbid'] as String, + artistName: args['artistName'] as String, + albumMbid: args['albumMbid'], + albumTitle: args['albumTitle'], ); if (mounted) { ScaffoldMessenger.of(context).showSnackBar(SnackBar( @@ -65,12 +75,18 @@ class _DiscoverScreenState extends ConsumerState { // Re-run search to refresh the `requested` flag on the row. _runSearch(); } - } on DioException catch (e) { + } on DioException catch (_) { + // Queue for replay so the user's request persists across + // network loss. We don't have a drift table for in-flight + // requests yet, so the row won't show on the Requests screen + // until replay succeeds — accept that for v1. + await ref + .read(mutationQueueProvider) + .enqueue(MutationKinds.requestCreate, args); if (mounted) { - final code = ApiError.fromDio(e).code; ScaffoldMessenger.of(context).showSnackBar(SnackBar( - content: Text('Request failed: $code'), - backgroundColor: fs.error, + content: Text('Request queued: ${row.name}'), + backgroundColor: fs.iron, )); } } diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index 6b2c7693..b3e64948 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -242,7 +242,9 @@ final _likedTrackIdsProvider = StreamProvider>((ref) { driftStream: query, fetchAndPopulate: () => _populateLikeIds(ref), toResult: _idsForEntity, - isOnline: () async => (await ref.read(connectivityProvider.future)), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), alwaysRefresh: true, tag: 'likedTrackIds', ); @@ -258,7 +260,9 @@ final _likedAlbumIdsProvider = StreamProvider>((ref) { driftStream: query, fetchAndPopulate: () => _populateLikeIds(ref), toResult: _idsForEntity, - isOnline: () async => (await ref.read(connectivityProvider.future)), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), alwaysRefresh: true, tag: 'likedAlbumIds', ); @@ -274,7 +278,9 @@ final _likedArtistIdsProvider = StreamProvider>((ref) { driftStream: query, fetchAndPopulate: () => _populateLikeIds(ref), toResult: _idsForEntity, - isOnline: () async => (await ref.read(connectivityProvider.future)), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), alwaysRefresh: true, tag: 'likedArtistIds', ); diff --git a/flutter_client/lib/likes/likes_provider.dart b/flutter_client/lib/likes/likes_provider.dart index 947824ac..ed0bcb3a 100644 --- a/flutter_client/lib/likes/likes_provider.dart +++ b/flutter_client/lib/likes/likes_provider.dart @@ -7,6 +7,7 @@ import '../cache/audio_cache_manager.dart' show appDbProvider; import '../cache/cache_first.dart'; import '../cache/connectivity_provider.dart'; import '../cache/db.dart'; +import '../cache/mutation_queue.dart'; import '../library/library_providers.dart'; final likesApiProvider = FutureProvider((ref) async { @@ -147,26 +148,18 @@ class LikesController { } else { await api.like(kind, id); } - } catch (e, st) { - // Rollback drift - if (wasLiked) { - await db.into(db.cachedLikes).insert( - CachedLikesCompanion.insert( - userId: user.id, - entityType: entityType, - entityId: id, - ), - mode: drift.InsertMode.insertOrIgnore, - ); - } else { - await (db.delete(db.cachedLikes) - ..where((t) => - t.userId.equals(user.id) & - t.entityType.equals(entityType) & - t.entityId.equals(id))) - .go(); - } - Error.throwWithStackTrace(e, st); + } catch (_) { + // REST failed (network or HTTP). Don't roll back drift — the + // user's intent is to like/unlike, and we want that visible to + // them even when offline. Queue the call for replay; the + // MutationReplayer will retry on next connectivity transition. + // If retries exhaust (5 attempts), the row drops and the next + // SyncController.sync brings drift back in line with the + // server's authoritative state. + await _ref.read(mutationQueueProvider).enqueue( + wasLiked ? MutationKinds.likeRemove : MutationKinds.likeAdd, + {'kind': entityType, 'id': id}, + ); } } diff --git a/flutter_client/lib/player/album_color_extractor.dart b/flutter_client/lib/player/album_color_extractor.dart index d1a134c8..32db2ab1 100644 --- a/flutter_client/lib/player/album_color_extractor.dart +++ b/flutter_client/lib/player/album_color_extractor.dart @@ -39,6 +39,15 @@ class AlbumColorCache { return color; } + /// Synchronous peek. Returns the previously-extracted color if this + /// album has been resolved this process; otherwise null. Used by + /// the now-playing fast-path swap so a warm cache transitions the + /// gradient in lockstep with the audio, instead of awaiting the + /// async [getOrExtract] future. A null return means "either no + /// extraction yet or extraction returned null" — caller falls back + /// to the async path. + Color? peekColor(String albumId) => _cache[albumId]; + Future _extract(String albumId) async { try { final coverCache = _ref.read(albumCoverCacheProvider); diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 5bae269a..6217ae7d 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -15,11 +15,16 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _player.playbackEventStream.listen( _broadcastState, // ExoPlayer surfaces stream errors (404, premature EOS, decoder - // failure, network drop) here. Without an error sink, the - // player just goes quiet — exactly the "starts then stops" - // symptom we hit. + // failure, network drop) here. Logging alone leaves the UI + // claiming "now playing X" while audio is silent — the user- + // observable mismatch between visual and audio state. Skip + // forward so the queue advances past the failed track and + // mediaItem updates to whatever's actually playing. If the + // failure is at the queue tail, just_audio will go idle on its + // own and _broadcastState reflects that. onError: (Object e, StackTrace st) { debugPrint('audio_handler: playbackEventStream error: $e\n$st'); + unawaited(_handlePlaybackError()); }, ); _player.currentIndexStream.listen(_onCurrentIndexChanged); @@ -113,9 +118,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // check and stop calling player mutations — important so a stale // fill doesn't append old-playlist tracks into the new queue. final myGen = ++_queueGeneration; - // Reset suppress flag in case a prior backward-fill bailed on - // gen check before reaching its `finally`. - _suppressIndexUpdates = false; _lastTracks = tracks; // Pause the old source immediately so the previous track stops @@ -127,25 +129,49 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl await _player.pause(); } - // Populate the visible queue + current mediaItem immediately so - // the player UI reflects the user's tap before any source has - // been built. Source list at the just_audio layer fills in - // asynchronously below. + // Build MediaItems up front (pure — no side effects); we'll + // broadcast queue/mediaItem only AFTER setAudioSources resolves + // so the audio engine and the UI flip together. If the build + // throws, the UI stays on the previous track (correct: audio + // also stays on the previous track since setAudioSources never + // ran). final items = tracks.map(_toMediaItem).toList(); - queue.add(items); - mediaItem.add(items[clampedInitial]); - // Fast path: build only the initial source so the player can - // start. Remaining sources stream in via _fillRemainingSources() - // in the background — addAudioSource for next/auto-advance - // tracks, insertAudioSource for skipPrev tracks. - final initial = await _buildAudioSource(tracks[clampedInitial]); + // Build only the initial source for fast start. Remaining + // sources stream in via _fillRemainingSources() — addAudioSource + // for next/auto-advance tracks, insertAudioSource for skipPrev. + final AudioSource initial; + try { + initial = await _buildAudioSource(tracks[clampedInitial]); + } catch (e, st) { + // Source build failed (bad URL, missing baseUrl, etc.). Don't + // broadcast the new state — leaving queue/mediaItem on the + // previous track keeps UI in sync with what the player is + // actually doing (which is "still on the previous track, + // paused"). + debugPrint('audio_handler: _buildAudioSource failed: $e\n$st'); + return; + } if (myGen != _queueGeneration) { debugPrint('audio_handler: superseded before setAudioSources (gen=$myGen)'); return; } - await _player.setAudioSources([initial], initialIndex: 0); + // Suppress _onCurrentIndexChanged side effects while the source + // list is being swapped — without this, a transient currentIndex + // emission during setAudioSources could broadcast the OLD queue's + // entry at the NEW index. Re-enabled after the broadcasts land. + _suppressIndexUpdates = true; + try { + await _player.setAudioSources([initial], initialIndex: 0); + // Broadcast in this order: queue first (so any consumer that + // reacts to mediaItem and reads queue.value sees the consistent + // pair), then mediaItem. + queue.add(items); + mediaItem.add(items[clampedInitial]); + } finally { + _suppressIndexUpdates = false; + } unawaited(_loadArtForCurrentItem()); unawaited(_fillRemainingSources(tracks, clampedInitial, myGen)); @@ -315,11 +341,14 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl final coverPath = (t.albumId.isNotEmpty && _coverCache != null) ? _coverCache!.peekCached(t.albumId) : null; - // MediaItem.rating reflects the user's like state on this track - // so external surfaces (Wear's heart button, lock-screen - // favorites) render the right filled/outlined icon. Updated on - // every track change via _likeBridge.isTrackLiked. - final liked = _likeBridge?.isTrackLiked(t.id) ?? false; + // MediaItem.rating intentionally NOT set: audio_service propagates + // it to MediaSession.setRating(), but the Android session also + // needs setRatingType(RATING_HEART) configured to expose that to + // controllers — audio_service doesn't surface that config knob, + // and broadcasting an unanchored rating made Wear OS reject the + // session entirely. The LikeBridge wiring stays in place so + // setRating can still fire from any surface that DOES route it, + // we just don't advertise it. return MediaItem( id: t.id, title: t.title, @@ -327,7 +356,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl album: t.albumTitle, duration: Duration(seconds: t.durationSec), artUri: coverPath != null ? Uri.file(coverPath) : null, - rating: Rating.newHeartRating(liked), extras: extras.isEmpty ? null : extras, ); } @@ -363,6 +391,37 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl await mgr.registerStreamCache(trackId, path, size); } + /// Called when playbackEventStream emits an error. Skips past the + /// failing track so the UI and audio re-converge — without this, + /// _player goes silent on a 404 / decoder failure / EOS but + /// mediaItem stays on the failed track and the user sees a "now + /// playing" header for something that isn't. + /// + /// If we're at the last track, seekToNext is a no-op; the state + /// drops to idle and _broadcastState reflects that. + Future _handlePlaybackError() async { + final currentIdx = _player.currentIndex; + final queueLen = queue.value.length; + if (currentIdx == null || currentIdx + 1 >= queueLen) { + // Last track or no queue context — just pause; _broadcastState + // already reflects the idle/error processingState. + try { + await _player.pause(); + } catch (_) {} + return; + } + try { + await _player.seekToNext(); + } catch (_) { + // If seekToNext fails too (next source not built yet, etc.), + // there's nothing safe to do — pause and let the user recover + // manually. + try { + await _player.pause(); + } catch (_) {} + } + } + void _onCurrentIndexChanged(int? idx) { if (idx == null) return; if (_suppressIndexUpdates) return; @@ -412,19 +471,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl @override Future skipToPrevious() => _player.seekToPrevious(); - /// Stops playback and dismisses the system notification. BaseAudio - /// Handler's default just sets the processing state to idle without - /// touching the player, so the just_audio engine kept its audio - /// session and external surfaces (Wear, Auto, BT) saw a "paused - /// forever" rather than a clean stop. Halting the player releases - /// the session and lets super.stop() tear down the foreground - /// notification. - @override - Future stop() async { - await _player.stop(); - await super.stop(); - } - /// Heart rating from external surfaces (Wear's favorite button, /// lock-screen like) → LikesController.toggle(track). We only /// route through the bridge when the rating actually flips relative @@ -498,33 +544,32 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl MediaControl.skipToPrevious, if (playing) MediaControl.pause else MediaControl.play, MediaControl.skipToNext, - MediaControl.stop, ], // androidCompactActionIndices tells the system which controls // appear in the collapsed/lock-screen view. Without this, some // Android versions render the player without working buttons. - // Keep this at 3 actions (prev/play-pause/next) — stop lives - // in the expanded view only. androidCompactActionIndices: const [0, 1, 2], // systemActions enumerates which actions the system can invoke - // on us. Anything not listed here doesn't route back to the - // handler from external surfaces (Wear, Auto, Bluetooth, lock - // screen) — Android 13+ silently drops those events. Keep this - // in sync with the override methods so each implemented action - // is actually advertised. + // on us — without play/pause/skip in here, taps on lock-screen + // controls don't route back to the handler on Android 13+. + // + // v2026.05.13.3 added stop, skipToQueueItem, setShuffleMode, + // setRepeatMode, and setRating to this set; reverted because + // Pixel Watch 2 stopped showing controls entirely after that + // change. The MediaSession contract on Wear OS requires more + // setup than just advertising the action (e.g. setRatingType + // for setRating) and audio_service doesn't expose those knobs. + // Keep the override methods themselves (skipToQueueItem is + // still routed via QueueScreen's direct handler call; + // setRating is harmless if never invoked) so we don't lose + // the underlying functionality — just don't tell the system + // we support them. systemActions: const { MediaAction.play, MediaAction.pause, - MediaAction.stop, MediaAction.skipToNext, MediaAction.skipToPrevious, - MediaAction.skipToQueueItem, MediaAction.seek, - MediaAction.setShuffleMode, - MediaAction.setRepeatMode, - // Heart rating — Wear and lock screen render this as a like - // button. Routed to LikesController via _likeBridge. - MediaAction.setRating, }, processingState: switch (_player.processingState) { ProcessingState.idle => AudioProcessingState.idle, diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index 818d3ea7..5f529aea 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -106,11 +106,39 @@ class _NowPlayingScreenState extends ConsumerState { Future _scheduleSwap(MediaItem newMedia) async { _pendingPreloadId = newMedia.id; + // Fast path: when Prefetcher has already warmed both the cover + // bytes (file:// artUri populated via _toMediaItem's peekCached) + // AND the palette color (AlbumColorCache.peekColor returns + // non-null), we can commit synchronously. The slow async path + // exists for genuine cold-cache moments (first play of an album + // that sync hasn't seen yet); the fast path is what makes the + // common in-queue auto-advance flip the visual in lockstep with + // the audio transition instead of lagging a tick behind. + final artUri = newMedia.artUri; + final albumId = newMedia.extras?['album_id'] as String?; + if (artUri != null && + artUri.isScheme('file') && + albumId != null && + albumId.isNotEmpty) { + final cachedColor = ref.read(albumColorCacheProvider).peekColor(albumId); + if (cachedColor != null) { + if (!mounted) return; + if (_pendingPreloadId != newMedia.id) return; + setState(() { + _displayedMedia = newMedia; + _displayedDominant = cachedColor; + }); + return; + } + } + + // Slow path: cover and/or color are not yet cached. Hold the + // current displayed state, preload, then atomic-commit. + // // 1. Precache the cover image bytes so when _AlbumArt mounts with // the new media, FileImage paints synchronously. Non-file // artUris fall through to ServerImage which handles its own // network load + 120ms fade — they won't snap. - final artUri = newMedia.artUri; if (artUri != null && artUri.isScheme('file') && context.mounted) { try { await precacheImage(FileImage(File.fromUri(artUri)), context); @@ -125,7 +153,6 @@ class _NowPlayingScreenState extends ConsumerState { // same FileImage, typically resolving within ~50ms once the // decode completes. Color? newDominant; - final albumId = newMedia.extras?['album_id'] as String?; if (albumId != null && albumId.isNotEmpty) { try { newDominant = await ref.read(albumColorProvider(albumId).future); diff --git a/flutter_client/lib/quarantine/quarantine_provider.dart b/flutter_client/lib/quarantine/quarantine_provider.dart index 4ca2b3d6..d84fa334 100644 --- a/flutter_client/lib/quarantine/quarantine_provider.dart +++ b/flutter_client/lib/quarantine/quarantine_provider.dart @@ -8,6 +8,7 @@ import '../api/endpoints/quarantine.dart'; import '../cache/audio_cache_manager.dart' show appDbProvider; import '../cache/connectivity_provider.dart'; import '../cache/db.dart'; +import '../cache/mutation_queue.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/quarantine_mine.dart'; import '../models/track.dart'; @@ -110,17 +111,18 @@ class MyQuarantineController extends AsyncNotifier> { try { final api = await ref.read(quarantineApiProvider.future); await api.flag(track.id, reason, notes: notes); - } catch (e, st) { - // Rollback the optimistic insert; watch() re-emits the prior set. - await (db.delete(db.cachedQuarantineMine) - ..where((t) => t.trackId.equals(track.id))) - .go(); - Error.throwWithStackTrace(e, st); + } catch (_) { + // REST failed — don't roll back; queue for replay so the user's + // "hide this track" intent persists offline. + await ref.read(mutationQueueProvider).enqueue( + MutationKinds.quarantineFlag, + {'trackId': track.id, 'reason': reason, 'notes': notes}, + ); } } - /// Optimistic unflag: removes the drift row, calls server, restores - /// on error. + /// Optimistic unflag: removes the drift row, calls server, queues + /// the call on failure so the unhide intent persists offline. Future unflag(String trackId) async { final db = ref.read(appDbProvider); final existing = await (db.select(db.cachedQuarantineMine) @@ -135,13 +137,11 @@ class MyQuarantineController extends AsyncNotifier> { try { final api = await ref.read(quarantineApiProvider.future); await api.unflag(trackId); - } catch (e, st) { - // Restore the row we just deleted — toCompanion(true) preserves - // every field including the original fetchedAt timestamp. - await db - .into(db.cachedQuarantineMine) - .insertOnConflictUpdate(existing.toCompanion(true)); - Error.throwWithStackTrace(e, st); + } catch (_) { + // Don't restore the drift row; queue the unflag for replay. + await ref + .read(mutationQueueProvider) + .enqueue(MutationKinds.quarantineUnflag, {'trackId': trackId}); } } diff --git a/flutter_client/lib/requests/requests_provider.dart b/flutter_client/lib/requests/requests_provider.dart index 259329c4..a00c47d4 100644 --- a/flutter_client/lib/requests/requests_provider.dart +++ b/flutter_client/lib/requests/requests_provider.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/requests.dart'; +import '../cache/mutation_queue.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/admin_request.dart'; @@ -41,17 +42,20 @@ class MyRequestsController extends AsyncNotifier> { } } - /// Optimistic remove + REST cancel. Restores the row on failure so - /// the user can retry — silent failure would lie about success. + /// Optimistic remove + REST cancel. On failure the row stays + /// removed in-memory and the cancel is queued for replay — this + /// keeps the user's intent visible across network loss instead of + /// restoring a row they explicitly asked to remove. Future cancel(String id) async { final api = await ref.read(requestsApiProvider.future); final current = state.value ?? const []; state = AsyncData(current.where((r) => r.id != id).toList()); try { await api.cancel(id); - } catch (e, st) { - state = AsyncData(current); - Error.throwWithStackTrace(e, st); + } catch (_) { + await ref + .read(mutationQueueProvider) + .enqueue(MutationKinds.requestCancel, {'id': id}); } } } diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart index bb5aeef5..68e2391d 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart @@ -1,8 +1,12 @@ +import 'package:drift/drift.dart' as drift; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import '../../../api/endpoints/likes.dart'; +import '../../../cache/audio_cache_manager.dart' show appDbProvider; +import '../../../cache/db.dart'; +import '../../../cache/mutation_queue.dart'; import '../../../likes/likes_provider.dart'; import '../../../models/track.dart'; import '../../../player/player_provider.dart'; @@ -255,7 +259,38 @@ typedef AddToPlaylistAction = Future Function(String playlistId, String tr final addToPlaylistActionProvider = Provider((ref) { return (playlistId, trackId) async { - final api = await ref.read(playlistsApiProvider.future); - await api.appendTracks(playlistId, [trackId]); + final db = ref.read(appDbProvider); + // Optimistic drift write so the playlist detail screen shows the + // new track instantly. The position is best-effort — append after + // the current max for this playlist. The server's authoritative + // position lands on next playlistDetailProvider fetch (SWR), or + // when the queued mutation replays. + final maxPos = await (db.selectOnly(db.cachedPlaylistTracks) + ..addColumns([db.cachedPlaylistTracks.position.max()]) + ..where(db.cachedPlaylistTracks.playlistId.equals(playlistId))) + .getSingleOrNull(); + final nextPos = + ((maxPos?.read(db.cachedPlaylistTracks.position.max()) ?? -1) + 1); + await db.into(db.cachedPlaylistTracks).insertOnConflictUpdate( + CachedPlaylistTracksCompanion.insert( + playlistId: playlistId, + trackId: trackId, + position: drift.Value(nextPos), + ), + ); + + try { + final api = await ref.read(playlistsApiProvider.future); + await api.appendTracks(playlistId, [trackId]); + } catch (_) { + // REST failed — keep the optimistic drift row; queue the call. + await ref.read(mutationQueueProvider).enqueue( + MutationKinds.playlistAppend, + { + 'playlistId': playlistId, + 'trackIds': [trackId], + }, + ); + } }; }); diff --git a/flutter_client/pubspec.yaml b/flutter_client/pubspec.yaml index 76bb9dca..4a03b811 100644 --- a/flutter_client/pubspec.yaml +++ b/flutter_client/pubspec.yaml @@ -1,7 +1,7 @@ name: minstrel description: Minstrel mobile client publish_to: 'none' -version: 2026.5.13+4 +version: 2026.5.14+5 environment: sdk: '>=3.5.0 <4.0.0' diff --git a/flutter_client/test/cache/cache_first_test.dart b/flutter_client/test/cache/cache_first_test.dart index f98f7db9..ff286873 100644 --- a/flutter_client/test/cache/cache_first_test.dart +++ b/flutter_client/test/cache/cache_first_test.dart @@ -32,9 +32,16 @@ void main() { isOnline: () async => true, ); - final firstFuture = results.first; + // Post-coldFetchAttempted guard, cacheFirst yields the current + // (still-empty) rows after the fetch returns, so the stream + // never hangs when populate is a no-op for this filter. The + // simulated drift re-emit then yields the populated rows. + // Capture the Future *before* feeding the controller so the + // listener is subscribed when the first add() lands. + final emissionsFuture = results.take(2).toList(); controller.add([]); - expect(await firstFuture, 42); + final emissions = await emissionsFuture; + expect(emissions, [0, 42]); expect(fetchCalled, 1); await controller.close(); }); diff --git a/flutter_client/test/quarantine/quarantine_provider_test.dart b/flutter_client/test/quarantine/quarantine_provider_test.dart index d632ddf3..2da87322 100644 --- a/flutter_client/test/quarantine/quarantine_provider_test.dart +++ b/flutter_client/test/quarantine/quarantine_provider_test.dart @@ -92,8 +92,8 @@ void main() { expect(rows.first.reason, 'bad_rip'); }); - test('flag rolls back the drift insert on server failure', skip: _skipDrift, - () async { + test('flag keeps drift optimistic + queues mutation on server failure', + skip: _skipDrift, () async { final db = AppDb(NativeDatabase.memory()); addTearDown(db.close); final api = _StubQuarantineApi(shouldThrow: true); @@ -101,14 +101,18 @@ void main() { addTearDown(container.dispose); await container.read(myQuarantineProvider.future); - await expectLater( - () => container - .read(myQuarantineProvider.notifier) - .flag(_track, 'bad_rip', ''), - throwsException, - ); + // No longer rethrows — flag swallows the REST failure and queues + // for replay so the user's intent persists offline. + await container + .read(myQuarantineProvider.notifier) + .flag(_track, 'bad_rip', ''); final rows = await db.select(db.cachedQuarantineMine).get(); - expect(rows, isEmpty); + expect(rows, hasLength(1), + reason: 'optimistic drift row should persist across REST failure'); + final mutations = await db.select(db.cachedMutations).get(); + expect(mutations, hasLength(1), + reason: 'failed flag should have been enqueued for replay'); + expect(mutations.first.kind, 'quarantine.flag'); }); test('unflag deletes the drift row and calls the server', skip: _skipDrift,