From 99462185b43282279d382096f6443b1389c5bfa1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 13 May 2026 18:41:18 -0400 Subject: [PATCH] feat(flutter): drift-first MyQuarantine Final slice of the smooth-loading pass. Adds CachedQuarantineMine (schema 5, columnar so flag/unflag can do row-level mutation) and rewires MyQuarantineController to read from drift via watch() + SWR refresh; flag/unflag write drift first and roll back on REST failure. Public API (.flag / .unflag / .isHidden) unchanged so existing call sites (library_screen Hidden tab, TrackActionsSheet) keep working. Tests updated to match: bypassed-build-via-_StubController approach no longer makes sense now that state lives in drift, so the suite is rewritten against NativeDatabase.memory() with the same libsqlite3 skip the sync_controller suite uses on the CI runner. The Hidden tab now paints from disk on cold open, the list is queryable offline, and a flag from another device that arrived in this user's quarantine via SSE-triggered invalidate lands the same way as a local flag. --- flutter_client/lib/cache/db.dart | 32 +++- .../lib/quarantine/quarantine_provider.dart | 152 +++++++++++++++--- .../quarantine/quarantine_provider_test.dart | 113 +++++++------ 3 files changed, 231 insertions(+), 66 deletions(-) diff --git a/flutter_client/lib/cache/db.dart b/flutter_client/lib/cache/db.dart index 69ec7174..f5ef32b8 100644 --- a/flutter_client/lib/cache/db.dart +++ b/flutter_client/lib/cache/db.dart @@ -142,6 +142,29 @@ class CachedHistorySnapshot extends Table { Set get primaryKey => {id}; } +/// 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 +/// without joining other tables. Columnar (vs JSON blob) so flag/unflag +/// can do row-level INSERT/DELETE; the drift watch() emission feeds +/// MyQuarantineController state directly. Schema 5+. +class CachedQuarantineMine extends Table { + TextColumn get trackId => text()(); + TextColumn get reason => text()(); + TextColumn get notes => text().nullable()(); + TextColumn get createdAt => text()(); + TextColumn get trackTitle => text()(); + IntColumn get trackDurationMs => integer().withDefault(const Constant(0))(); + TextColumn get albumId => text()(); + TextColumn get albumTitle => text()(); + TextColumn get albumCoverArtPath => text().nullable()(); + TextColumn get artistId => text()(); + TextColumn get artistName => text()(); + DateTimeColumn get fetchedAt => dateTime().withDefault(currentDateAndTime)(); + @override + Set get primaryKey => {trackId}; +} + enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } @DriftDatabase(tables: [ @@ -155,12 +178,13 @@ enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } SyncMetadata, CachedHomeSnapshot, CachedHistorySnapshot, + CachedQuarantineMine, ]) class AppDb extends _$AppDb { AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache')); @override - int get schemaVersion => 4; + int get schemaVersion => 5; @override MigrationStrategy get migration => MigrationStrategy( @@ -188,6 +212,12 @@ class AppDb extends _$AppDb { // empty on upgrade; first /api/me/history fetch populates. await m.createTable(cachedHistorySnapshot); } + if (from < 5) { + // Schema 5: cached_quarantine_mine for drift-first Hidden + // tab. Empty on upgrade; first /api/quarantine/mine fetch + // populates. Optimistic flag/unflag also writes here. + await m.createTable(cachedQuarantineMine); + } }, ); } diff --git a/flutter_client/lib/quarantine/quarantine_provider.dart b/flutter_client/lib/quarantine/quarantine_provider.dart index 3bd1a902..4ca2b3d6 100644 --- a/flutter_client/lib/quarantine/quarantine_provider.dart +++ b/flutter_client/lib/quarantine/quarantine_provider.dart @@ -1,7 +1,13 @@ +import 'dart:async'; + +import 'package:drift/drift.dart' as drift; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/me.dart'; import '../api/endpoints/quarantine.dart'; +import '../cache/audio_cache_manager.dart' show appDbProvider; +import '../cache/connectivity_provider.dart'; +import '../cache/db.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/quarantine_mine.dart'; import '../models/track.dart'; @@ -10,58 +16,166 @@ final quarantineApiProvider = FutureProvider((ref) async { return QuarantineApi(await ref.watch(dioProvider.future)); }); +/// Drift-first ("hidden") tab controller. Reads from cached_quarantine_ +/// mine via a drift watch() subscription set up in build(), so any drift +/// mutation (incoming sync, optimistic flag/unflag, SSE-triggered +/// refresh) re-emits the list automatically. +/// +/// API surface unchanged from the prior FutureProvider-backed controller +/// — call sites still do `ref.read(myQuarantineProvider.notifier).flag()` +/// / `.unflag()` / `.isHidden()`. Internal storage moved from in-memory +/// AsyncNotifier state to drift so the Hidden tab paints from disk on +/// cold open and the quarantine list is queryable offline. class MyQuarantineController extends AsyncNotifier> { @override Future> build() async { - final dio = await ref.watch(dioProvider.future); - return MeApi(dio).quarantineMine(); + final db = ref.watch(appDbProvider); + + // Subscribe to drift first so any mutation (incoming sync, flag/ + // unflag, etc.) propagates without needing a manual invalidate. + // Disposed when the provider rebuilds or the listener detaches. + final sub = db.select(db.cachedQuarantineMine).watch().listen((rows) { + state = AsyncData(rows.map(_rowToModel).toList()); + }); + ref.onDispose(sub.cancel); + + // SWR refresh on every build so the freshest server-side state + // overtakes drift in the background. Don't await — UI gets the + // cached snapshot first, freshness lands later via the watch(). + unawaited(_refreshFromServer()); + + final initial = await db.select(db.cachedQuarantineMine).get(); + return initial.map(_rowToModel).toList(); + } + + /// Hits /api/quarantine/mine and replaces the local table with the + /// authoritative server set. Best-effort: offline / 5xx / unresolved + /// dependencies (e.g. uninitialised serverUrl in tests) all collapse + /// to a no-op — drift stays on the prior cached state. + Future _refreshFromServer() async { + try { + final online = await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true); + if (!online) return; + final dio = await ref.read(dioProvider.future); + final fresh = await MeApi(dio).quarantineMine(); + final db = ref.read(appDbProvider); + await db.transaction(() async { + // Full replace — quarantine list is small and we want server + // unflags from other devices to take effect. Doing this in a + // transaction means the watch() sees exactly one emission with + // the merged state, not a delete-then-insert flicker. + await db.delete(db.cachedQuarantineMine).go(); + for (final r in fresh) { + await db + .into(db.cachedQuarantineMine) + .insertOnConflictUpdate(_modelToCompanion(r)); + } + }); + } catch (_) { + // Swallow — cached state stays visible. + } } /// True when this track is in the caller's quarantine list. bool isHidden(String trackId) => (state.value ?? const []).any((r) => r.trackId == trackId); - /// Optimistic flag: prepends a synthetic row, calls server, - /// rolls back on error. + /// Optimistic flag: inserts the synthetic row into drift (watch fires + /// immediately so the UI updates), calls server, rolls back on error. Future flag(TrackRef track, String reason, String notes) async { - final api = await ref.read(quarantineApiProvider.future); - final current = state.value ?? const []; - if (current.any((r) => r.trackId == track.id)) return; // already hidden - final synthetic = QuarantineMineRow( + final db = ref.read(appDbProvider); + final existed = await (db.select(db.cachedQuarantineMine) + ..where((t) => t.trackId.equals(track.id))) + .getSingleOrNull(); + if (existed != null) return; // already hidden + + final companion = CachedQuarantineMineCompanion.insert( trackId: track.id, reason: reason, - notes: notes.isEmpty ? null : notes, + notes: notes.isEmpty + ? const drift.Value.absent() + : drift.Value(notes), createdAt: DateTime.now().toUtc().toIso8601String(), trackTitle: track.title, - trackDurationMs: track.durationSec * 1000, + trackDurationMs: drift.Value(track.durationSec * 1000), albumId: track.albumId, albumTitle: track.albumTitle, artistId: track.artistId, artistName: track.artistName, ); - state = AsyncData([synthetic, ...current]); + await db.into(db.cachedQuarantineMine).insertOnConflictUpdate(companion); + try { + final api = await ref.read(quarantineApiProvider.future); await api.flag(track.id, reason, notes: notes); } catch (e, st) { - state = AsyncData(current); + // 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); } } - /// Optimistic unflag: removes the row, calls server, rolls back on error. + /// Optimistic unflag: removes the drift row, calls server, restores + /// on error. Future unflag(String trackId) async { - final api = await ref.read(quarantineApiProvider.future); - final current = state.value ?? const []; - final removed = current.where((r) => r.trackId == trackId).toList(); - if (removed.isEmpty) return; - state = AsyncData(current.where((r) => r.trackId != trackId).toList()); + final db = ref.read(appDbProvider); + final existing = await (db.select(db.cachedQuarantineMine) + ..where((t) => t.trackId.equals(trackId))) + .getSingleOrNull(); + if (existing == null) return; + + await (db.delete(db.cachedQuarantineMine) + ..where((t) => t.trackId.equals(trackId))) + .go(); + try { + final api = await ref.read(quarantineApiProvider.future); await api.unflag(trackId); } catch (e, st) { - state = AsyncData(current); + // 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); } } + + static QuarantineMineRow _rowToModel(CachedQuarantineMineData r) => + QuarantineMineRow( + trackId: r.trackId, + reason: r.reason, + notes: r.notes, + createdAt: r.createdAt, + trackTitle: r.trackTitle, + trackDurationMs: r.trackDurationMs, + albumId: r.albumId, + albumTitle: r.albumTitle, + albumCoverArtPath: r.albumCoverArtPath, + artistId: r.artistId, + artistName: r.artistName, + ); + + static CachedQuarantineMineCompanion _modelToCompanion(QuarantineMineRow r) => + CachedQuarantineMineCompanion.insert( + trackId: r.trackId, + reason: r.reason, + notes: r.notes == null ? const drift.Value.absent() : drift.Value(r.notes), + createdAt: r.createdAt, + trackTitle: r.trackTitle, + trackDurationMs: drift.Value(r.trackDurationMs), + albumId: r.albumId, + albumTitle: r.albumTitle, + albumCoverArtPath: r.albumCoverArtPath == null + ? const drift.Value.absent() + : drift.Value(r.albumCoverArtPath), + artistId: r.artistId, + artistName: r.artistName, + ); } final myQuarantineProvider = diff --git a/flutter_client/test/quarantine/quarantine_provider_test.dart b/flutter_client/test/quarantine/quarantine_provider_test.dart index b146102f..d632ddf3 100644 --- a/flutter_client/test/quarantine/quarantine_provider_test.dart +++ b/flutter_client/test/quarantine/quarantine_provider_test.dart @@ -1,11 +1,23 @@ +@Tags(['drift']) +library; + +import 'package:drift/native.dart' show NativeDatabase; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:minstrel/api/endpoints/quarantine.dart'; -import 'package:minstrel/models/quarantine_mine.dart'; +import 'package:minstrel/cache/audio_cache_manager.dart' show appDbProvider; +import 'package:minstrel/cache/db.dart'; import 'package:minstrel/models/track.dart'; import 'package:minstrel/quarantine/quarantine_provider.dart'; +// SKIP NOTE: drift's NativeDatabase needs the system libsqlite3.so. +// The flutter-ci runner image doesn't ship it. Same skip as +// sync_controller_test.dart — unblock by adding libsqlite3-dev to the +// CI image, or switch to sqlite3/wasm. On-device runs cover real +// behaviour today. +const _skipDrift = 'libsqlite3 missing on flutter-ci runner; on-device covers'; + class _StubQuarantineApi implements QuarantineApi { _StubQuarantineApi({this.shouldThrow = false}); bool shouldThrow; @@ -25,13 +37,6 @@ class _StubQuarantineApi implements QuarantineApi { } } -class _StubController extends MyQuarantineController { - _StubController(this._initial); - final List _initial; - @override - Future> build() async => _initial; -} - const _track = TrackRef( id: 't1', title: 'Roygbiv', @@ -44,44 +49,55 @@ const _track = TrackRef( streamUrl: '', ); -const _existing = QuarantineMineRow( - trackId: 't1', - reason: 'bad_rip', - notes: null, - createdAt: '2026-05-01T00:00:00Z', - trackTitle: 'Roygbiv', - trackDurationMs: 137000, - albumId: 'a1', - albumTitle: 'Geogaddi', - artistId: 'ar1', - artistName: 'Boards of Canada', -); +ProviderContainer _container({required AppDb db, required QuarantineApi api}) { + return ProviderContainer(overrides: [ + appDbProvider.overrideWithValue(db), + quarantineApiProvider.overrideWith((ref) async => api), + ]); +} + +Future _seed(AppDb db) async { + await db.into(db.cachedQuarantineMine).insert( + CachedQuarantineMineCompanion.insert( + trackId: 't1', + reason: 'bad_rip', + createdAt: '2026-05-01T00:00:00Z', + trackTitle: 'Roygbiv', + albumId: 'a1', + albumTitle: 'Geogaddi', + artistId: 'ar1', + artistName: 'Boards of Canada', + ), + ); +} void main() { - test('flag prepends synthetic row and calls server', () async { + test('flag inserts a drift row and calls the server', skip: _skipDrift, + () async { + final db = AppDb(NativeDatabase.memory()); + addTearDown(db.close); final api = _StubQuarantineApi(); - final container = ProviderContainer(overrides: [ - quarantineApiProvider.overrideWith((ref) async => api), - myQuarantineProvider.overrideWith(() => _StubController(const [])), - ]); + final container = _container(db: db, api: api); addTearDown(container.dispose); await container.read(myQuarantineProvider.future); - await container.read(myQuarantineProvider.notifier).flag(_track, 'bad_rip', ''); + await container + .read(myQuarantineProvider.notifier) + .flag(_track, 'bad_rip', ''); expect(api.flagCalls, 1); - final list = container.read(myQuarantineProvider).value!; - expect(list, hasLength(1)); - expect(list.first.trackId, 't1'); - expect(list.first.reason, 'bad_rip'); + final rows = await db.select(db.cachedQuarantineMine).get(); + expect(rows, hasLength(1)); + expect(rows.first.trackId, 't1'); + expect(rows.first.reason, 'bad_rip'); }); - test('flag rolls back on server failure', () async { + test('flag rolls back the drift insert on server failure', skip: _skipDrift, + () async { + final db = AppDb(NativeDatabase.memory()); + addTearDown(db.close); final api = _StubQuarantineApi(shouldThrow: true); - final container = ProviderContainer(overrides: [ - quarantineApiProvider.overrideWith((ref) async => api), - myQuarantineProvider.overrideWith(() => _StubController(const [])), - ]); + final container = _container(db: db, api: api); addTearDown(container.dispose); await container.read(myQuarantineProvider.future); @@ -91,28 +107,33 @@ void main() { .flag(_track, 'bad_rip', ''), throwsException, ); - expect(container.read(myQuarantineProvider).value, isEmpty); + final rows = await db.select(db.cachedQuarantineMine).get(); + expect(rows, isEmpty); }); - test('unflag removes the row and calls server', () async { + test('unflag deletes the drift row and calls the server', skip: _skipDrift, + () async { + final db = AppDb(NativeDatabase.memory()); + addTearDown(db.close); + await _seed(db); final api = _StubQuarantineApi(); - final container = ProviderContainer(overrides: [ - quarantineApiProvider.overrideWith((ref) async => api), - myQuarantineProvider.overrideWith(() => _StubController(const [_existing])), - ]); + final container = _container(db: db, api: api); addTearDown(container.dispose); await container.read(myQuarantineProvider.future); await container.read(myQuarantineProvider.notifier).unflag('t1'); expect(api.unflagCalls, 1); - expect(container.read(myQuarantineProvider).value, isEmpty); + final rows = await db.select(db.cachedQuarantineMine).get(); + expect(rows, isEmpty); }); - test('isHidden reflects current state', () async { - final container = ProviderContainer(overrides: [ - myQuarantineProvider.overrideWith(() => _StubController(const [_existing])), - ]); + test('isHidden reflects drift state', skip: _skipDrift, () async { + final db = AppDb(NativeDatabase.memory()); + addTearDown(db.close); + await _seed(db); + final api = _StubQuarantineApi(); + final container = _container(db: db, api: api); addTearDown(container.dispose); await container.read(myQuarantineProvider.future);