fix(recents): preserve early edits during history loading

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<RecentAccessState> {
-  final AppStateDatabase _appStateDb = AppStateDatabase.instance;
+  RecentAccessNotifier({AppStateDatabase? database})
+    : _appStateDb = database ?? AppStateDatabase.instance;
+
+  final AppStateDatabase _appStateDb;
+  Future<void> _writeChain = Future<void>.value();
+  final _loadingEdits = <RecentAccessState Function(RecentAccessState)>[];
+  final _clearingEdits =
+      <List<RecentAccessState Function(RecentAccessState)>>[];
 
   @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<void> _enqueueWrite(Future<void> 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<void>(
+      (_) {},
+      onError: (Object error, StackTrace stack) {
+        _log.e('Failed to save recent access history', error, stack);
+      },
+    );
+    return pending;
+  }
+
   Future<void> _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 = <RecentAccessItem>[];
       for (final row in rows) {
@@ -133,14 +167,21 @@ class RecentAccessNotifier extends Notifier<RecentAccessState> {
         }
       }
 
-      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<RecentAccessState> {
   }
 
   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<void> 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<void> clearHistory() async {
+    final laterEdits = <RecentAccessState Function(RecentAccessState)>[];
+    _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));
   }
 }
 

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<RecentAccessItem> items = const []})
+    : items = {for (final item in items) item.uniqueKey: item};
+
+  final Map<String, RecentAccessItem> items;
+  final hiddenIds = <String>{'saved-download'};
+  final clearedAt = DateTime.utc(2025, 12, 1);
+  final readStarted = Completer<void>();
+  final releaseRead = Completer<void>();
+  final clearStarted = Completer<void>();
+  final releaseClear = Completer<void>();
+  final operations = <String>[];
+  final failures = <String>{};
+  final _completed = <String, Completer<void>>{};
+  bool holdClear = false;
+  bool failRead = false;
+
+  Future<void> whenCompleted(String operation) =>
+      (_completed[operation] ??= Completer<void>()).future;
+
+  void _complete(String operation) {
+    operations.add(operation);
+    if (failures.remove(operation)) throw StateError(operation);
+    (_completed[operation] ??= Completer<void>()).complete();
+  }
+
+  @override
+  Future<bool> migrateRecentAccessFromSharedPreferences() async => true;
+
+  @override
+  Future<List<Map<String, dynamic>>> 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<Set<String>> getHiddenRecentDownloadIds() async => {...hiddenIds};
+
+  @override
+  Future<DateTime?> getRecentDownloadsClearedAt() async => clearedAt;
+
+  @override
+  Future<void> 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<String, dynamic>,
+    );
+    _complete(operation);
+  }
+
+  @override
+  Future<void> deleteRecentAccessRow(String uniqueKey) async {
+    items.remove(uniqueKey);
+    _complete('remove:$uniqueKey');
+  }
+
+  @override
+  Future<void> addHiddenRecentDownloadId(String downloadId) async {
+    hiddenIds.add(downloadId);
+    _complete('hide:$downloadId');
+  }
+
+  @override
+  Future<void> clearHiddenRecentDownloadIds() async {
+    hiddenIds.clear();
+    _complete('clear-hidden');
+  }
+
+  @override
+  Future<DateTime> 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<void> _waitUntilLoaded(ProviderContainer container) async {
+  if (container.read(recentAccessProvider).isLoaded) return;
+  final loaded = Completer<void>();
+  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,
+    );
+  });
+}