From 66c5f5a86a5bf87de41cacacd0969b8cb79ef838 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Tue, 6 Oct 2026 22:00:47 +0700 Subject: [PATCH] fix(recents): preserve early edits during history loading --- lib/providers/recent_access_provider.dart | 161 ++++++++---- test/recent_access_persistence_test.dart | 283 ++++++++++++++++++++++ 2 files changed, 398 insertions(+), 46 deletions(-) create mode 100644 test/recent_access_persistence_test.dart diff --git a/lib/providers/recent_access_provider.dart b/lib/providers/recent_access_provider.dart index 7bb60054..7fa61871 100644 --- a/lib/providers/recent_access_provider.dart +++ b/lib/providers/recent_access_provider.dart @@ -2,8 +2,10 @@ import 'dart:async'; import 'dart:convert'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:spotiflac_android/services/app_state_database.dart'; +import 'package:spotiflac_android/utils/logger.dart'; const _maxRecentItems = 20; +final _log = AppLogger('RecentAccess'); bool isRecentDownloadAfterClear( DateTime downloadedAt, @@ -100,23 +102,55 @@ class RecentAccessState { } class RecentAccessNotifier extends Notifier { - final AppStateDatabase _appStateDb = AppStateDatabase.instance; + RecentAccessNotifier({AppStateDatabase? database}) + : _appStateDb = database ?? AppStateDatabase.instance; + + final AppStateDatabase _appStateDb; + Future _writeChain = Future.value(); + final _loadingEdits = []; + final _clearingEdits = + >[]; @override RecentAccessState build() { - _loadHistory(); + _writeChain = _loadHistory(); return const RecentAccessState(); } + void _editState(RecentAccessState Function(RecentAccessState) edit) { + if (!state.isLoaded) _loadingEdits.add(edit); + for (final edits in _clearingEdits) { + edits.add(edit); + } + state = edit(state); + } + + Future _enqueueWrite(Future Function() write) { + final pending = _writeChain.then((_) => write()); + // The owner logs failures and keeps later edits writable. Awaitable + // operations still receive their original persistence error. + _writeChain = pending.then( + (_) {}, + onError: (Object error, StackTrace stack) { + _log.e('Failed to save recent access history', error, stack); + }, + ); + return pending; + } + Future _loadHistory() async { try { await _appStateDb.migrateRecentAccessFromSharedPreferences(); + if (!ref.mounted) return; final rows = await _appStateDb.getRecentAccessRows( limit: _maxRecentItems, ); + if (!ref.mounted) return; final hiddenIds = await _appStateDb.getHiddenRecentDownloadIds(); + if (!ref.mounted) return; final downloadsClearedAt = await _appStateDb .getRecentDownloadsClearedAt(); + if (!ref.mounted) return; final items = []; for (final row in rows) { @@ -133,14 +167,21 @@ class RecentAccessNotifier extends Notifier { } } - state = state.copyWith( + var loaded = RecentAccessState( items: items, hiddenDownloadIds: hiddenIds, downloadsClearedAt: downloadsClearedAt, isLoaded: true, ); - } catch (_) { - state = state.copyWith(isLoaded: true); + for (final edit in _loadingEdits) { + loaded = edit(loaded); + } + if (ref.mounted) state = loaded; + } catch (error, stack) { + _log.e('Failed to load recent access history', error, stack); + if (ref.mounted) state = state.copyWith(isLoaded: true); + } finally { + _loadingEdits.clear(); } } @@ -203,56 +244,84 @@ class RecentAccessNotifier extends Notifier { } void _recordAccess(RecentAccessItem item) { - final updatedItems = state.items - .where((e) => e.uniqueKey != item.uniqueKey) - .toList(); - - updatedItems.insert(0, item); - - RecentAccessItem? removedTail; - if (updatedItems.length > _maxRecentItems) { - removedTail = updatedItems.removeLast(); - } - - state = state.copyWith(items: updatedItems); - unawaited( - _appStateDb.upsertRecentAccessRow( - uniqueKey: item.uniqueKey, - itemJson: jsonEncode(item.toJson()), - accessedAt: item.accessedAt.toIso8601String(), + final previousTail = state.items.length == _maxRecentItems + ? state.items.last + : null; + _editState( + (current) => current.copyWith( + items: [ + item, + ...current.items.where((e) => e.uniqueKey != item.uniqueKey), + ].take(_maxRecentItems).toList(), ), ); - if (removedTail != null) { - unawaited(_appStateDb.deleteRecentAccessRow(removedTail.uniqueKey)); - } + final removedTail = + previousTail != null && !state.items.contains(previousTail) + ? previousTail + : null; + unawaited( + _enqueueWrite(() async { + await _appStateDb.upsertRecentAccessRow( + uniqueKey: item.uniqueKey, + itemJson: jsonEncode(item.toJson()), + accessedAt: item.accessedAt.toIso8601String(), + ); + if (removedTail != null) { + await _appStateDb.deleteRecentAccessRow(removedTail.uniqueKey); + } + }), + ); } void removeItem(RecentAccessItem item) { - final updatedItems = state.items - .where((e) => e.uniqueKey != item.uniqueKey) - .toList(); - state = state.copyWith(items: updatedItems); - unawaited(_appStateDb.deleteRecentAccessRow(item.uniqueKey)); - } - - void hideDownloadFromRecents(String downloadId) { - final updatedHidden = {...state.hiddenDownloadIds, downloadId}; - state = state.copyWith(hiddenDownloadIds: updatedHidden); - unawaited(_appStateDb.addHiddenRecentDownloadId(downloadId)); - } - - Future clearHistory() async { - final clearedAt = await _appStateDb.clearAllRecentAccess(); - state = state.copyWith( - items: [], - hiddenDownloadIds: {}, - downloadsClearedAt: clearedAt, + _editState( + (current) => current.copyWith( + items: current.items + .where((e) => e.uniqueKey != item.uniqueKey) + .toList(), + ), + ); + unawaited( + _enqueueWrite(() => _appStateDb.deleteRecentAccessRow(item.uniqueKey)), ); } + void hideDownloadFromRecents(String downloadId) { + _editState( + (current) => current.copyWith( + hiddenDownloadIds: {...current.hiddenDownloadIds, downloadId}, + ), + ); + unawaited( + _enqueueWrite(() => _appStateDb.addHiddenRecentDownloadId(downloadId)), + ); + } + + Future clearHistory() async { + final laterEdits = []; + _clearingEdits.add(laterEdits); + try { + await _enqueueWrite(() async { + final clearedAt = await _appStateDb.clearAllRecentAccess(); + if (!ref.mounted) return; + var cleared = state.copyWith( + items: [], + hiddenDownloadIds: {}, + downloadsClearedAt: clearedAt, + ); + for (final edit in laterEdits) { + cleared = edit(cleared); + } + state = cleared; + }); + } finally { + _clearingEdits.remove(laterEdits); + } + } + void clearHiddenDownloads() { - state = state.copyWith(hiddenDownloadIds: {}); - unawaited(_appStateDb.clearHiddenRecentDownloadIds()); + _editState((current) => current.copyWith(hiddenDownloadIds: {})); + unawaited(_enqueueWrite(_appStateDb.clearHiddenRecentDownloadIds)); } } diff --git a/test/recent_access_persistence_test.dart b/test/recent_access_persistence_test.dart new file mode 100644 index 00000000..6ff5976c --- /dev/null +++ b/test/recent_access_persistence_test.dart @@ -0,0 +1,283 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/providers/recent_access_provider.dart'; +import 'package:spotiflac_android/services/app_state_database.dart'; +import 'package:spotiflac_android/utils/logger.dart'; + +RecentAccessItem _artist(String id, {String? name}) => RecentAccessItem( + id: id, + name: name ?? id, + type: RecentAccessType.artist, + providerId: 'example', + accessedAt: DateTime.utc(2026, 1, 1), +); + +class _Database implements AppStateDatabase { + _Database({Iterable items = const []}) + : items = {for (final item in items) item.uniqueKey: item}; + + final Map items; + final hiddenIds = {'saved-download'}; + final clearedAt = DateTime.utc(2025, 12, 1); + final readStarted = Completer(); + final releaseRead = Completer(); + final clearStarted = Completer(); + final releaseClear = Completer(); + final operations = []; + final failures = {}; + final _completed = >{}; + bool holdClear = false; + bool failRead = false; + + Future whenCompleted(String operation) => + (_completed[operation] ??= Completer()).future; + + void _complete(String operation) { + operations.add(operation); + if (failures.remove(operation)) throw StateError(operation); + (_completed[operation] ??= Completer()).complete(); + } + + @override + Future migrateRecentAccessFromSharedPreferences() async => true; + + @override + Future>> getRecentAccessRows({int? limit}) async { + final snapshot = items.values + .take(limit ?? items.length) + .map((item) => {'item_json': jsonEncode(item.toJson())}) + .toList(); + readStarted.complete(); + await releaseRead.future; + if (failRead) throw StateError('recent_load_failed'); + return snapshot; + } + + @override + Future> getHiddenRecentDownloadIds() async => {...hiddenIds}; + + @override + Future getRecentDownloadsClearedAt() async => clearedAt; + + @override + Future upsertRecentAccessRow({ + required String uniqueKey, + required String itemJson, + required String accessedAt, + }) async { + final operation = 'record:$uniqueKey'; + if (failures.contains(operation)) { + _complete(operation); + return; + } + items[uniqueKey] = RecentAccessItem.fromJson( + jsonDecode(itemJson) as Map, + ); + _complete(operation); + } + + @override + Future deleteRecentAccessRow(String uniqueKey) async { + items.remove(uniqueKey); + _complete('remove:$uniqueKey'); + } + + @override + Future addHiddenRecentDownloadId(String downloadId) async { + hiddenIds.add(downloadId); + _complete('hide:$downloadId'); + } + + @override + Future clearHiddenRecentDownloadIds() async { + hiddenIds.clear(); + _complete('clear-hidden'); + } + + @override + Future clearAllRecentAccess() async { + clearStarted.complete(); + if (holdClear) await releaseClear.future; + items.clear(); + hiddenIds.clear(); + _complete('clear'); + return DateTime.utc(2026, 10, 1); + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +ProviderContainer _container(_Database database) => ProviderContainer( + overrides: [ + recentAccessProvider.overrideWith( + () => RecentAccessNotifier(database: database), + ), + ], +); + +Future _waitUntilLoaded(ProviderContainer container) async { + if (container.read(recentAccessProvider).isLoaded) return; + final loaded = Completer(); + final subscription = container.listen(recentAccessProvider, (_, next) { + if (next.isLoaded && !loaded.isCompleted) loaded.complete(); + }); + await loaded.future; + subscription.close(); +} + +void main() { + test( + 'delayed startup replays edits without discarding saved recents', + () async { + final removed = _artist('removed'); + final database = _Database( + items: [ + _artist('edited', name: 'Old name'), + _artist('kept'), + removed, + ], + ); + final container = _container(database); + addTearDown(container.dispose); + final notifier = container.read(recentAccessProvider.notifier); + await database.readStarted.future; + + notifier.recordArtistAccess( + id: 'edited', + name: 'New name', + providerId: 'example', + ); + notifier.removeItem(removed); + notifier.hideDownloadFromRecents('new-download'); + expect( + container.read(recentAccessProvider).items.single.name, + 'New name', + ); + expect(database.operations, isEmpty); + + database.releaseRead.complete(); + await _waitUntilLoaded(container); + final state = container.read(recentAccessProvider); + expect(state.items.map((item) => item.id), ['edited', 'kept']); + expect(state.items.first.name, 'New name'); + expect(state.hiddenDownloadIds, {'saved-download', 'new-download'}); + expect(state.downloadsClearedAt, database.clearedAt); + await database.whenCompleted('hide:new-download'); + expect(database.items.keys, [ + _artist('edited').uniqueKey, + _artist('kept').uniqueKey, + ]); + expect(database.items[_artist('edited').uniqueKey]!.name, 'New name'); + }, + ); + + test('early hidden clear is replayed before a later hide', () async { + final database = _Database(); + final container = _container(database); + addTearDown(container.dispose); + final notifier = container.read(recentAccessProvider.notifier); + await database.readStarted.future; + notifier.clearHiddenDownloads(); + notifier.hideDownloadFromRecents('new-download'); + database.releaseRead.complete(); + await _waitUntilLoaded(container); + await database.whenCompleted('hide:new-download'); + expect(container.read(recentAccessProvider).hiddenDownloadIds, { + 'new-download', + }); + expect(database.hiddenIds, {'new-download'}); + expect(database.operations, ['clear-hidden', 'hide:new-download']); + }); + + test( + 'startup clear preserves accesses made after the clear request', + () async { + final database = _Database(items: [_artist('old')])..holdClear = true; + final container = _container(database); + addTearDown(container.dispose); + final notifier = container.read(recentAccessProvider.notifier); + await database.readStarted.future; + final clearing = notifier.clearHistory(); + notifier.recordArtistAccess( + id: 'later', + name: 'Later', + providerId: 'example', + ); + database.releaseRead.complete(); + await database.clearStarted.future; + database.releaseClear.complete(); + await clearing; + await database.whenCompleted('record:${_artist('later').uniqueKey}'); + expect(container.read(recentAccessProvider).items.single.id, 'later'); + expect(database.items.values.single.id, 'later'); + expect( + container.read(recentAccessProvider).downloadsClearedAt, + DateTime.utc(2026, 10, 1), + ); + expect(database.operations, [ + 'clear', + 'record:${_artist('later').uniqueKey}', + ]); + }, + ); + + test('owned write failure is logged and permits later writes', () async { + final database = _Database(); + final container = _container(database); + addTearDown(container.dispose); + final notifier = container.read(recentAccessProvider.notifier); + await database.readStarted.future; + database.releaseRead.complete(); + await _waitUntilLoaded(container); + final failedOperation = 'record:${_artist('failed').uniqueKey}'; + database.failures.add(failedOperation); + notifier.recordArtistAccess( + id: 'failed', + name: 'Failed', + providerId: 'example', + ); + notifier.hideDownloadFromRecents('after-failure'); + await database.whenCompleted('hide:after-failure'); + expect(container.read(recentAccessProvider).items.single.id, 'failed'); + expect(database.items, isEmpty); + expect(database.hiddenIds, contains('after-failure')); + expect( + LogBuffer().entries.any( + (entry) => + entry.tag == 'RecentAccess' && + entry.error?.contains(failedOperation) == true, + ), + isTrue, + ); + }); + + test('failed load preserves early state and still owns its writes', () async { + final database = _Database()..failRead = true; + final container = _container(database); + addTearDown(container.dispose); + final notifier = container.read(recentAccessProvider.notifier); + await database.readStarted.future; + notifier.recordArtistAccess( + id: 'early', + name: 'Early', + providerId: 'example', + ); + database.releaseRead.complete(); + await _waitUntilLoaded(container); + await database.whenCompleted('record:${_artist('early').uniqueKey}'); + expect(container.read(recentAccessProvider).items.single.id, 'early'); + expect(database.items.values.single.id, 'early'); + expect( + LogBuffer().entries.any( + (entry) => + entry.tag == 'RecentAccess' && + entry.error?.contains('recent_load_failed') == true, + ), + isTrue, + ); + }); +}