media store monitoring: fixed keeping favourites on move

lib/model/favourite_repo.dart

@@ -24,33 +24,37 @@ class FavouriteRepo {
 
   Future<void> add(Iterable<AvesEntry> entries) async {
     final newRows = entries.map(_entryToRow);
+
     await metadataDb.addFavourites(newRows);
     _rows.addAll(newRows);
+
     changeNotifier.notifyListeners();
   }
 
   Future<void> remove(Iterable<AvesEntry> entries) async {
     final removedRows = entries.map(_entryToRow);
+
     await metadataDb.removeFavourites(removedRows);
     removedRows.forEach(_rows.remove);
+
     changeNotifier.notifyListeners();
   }
 
   Future<void> move(int oldContentId, AvesEntry entry) async {
     final oldRow = _rows.firstWhere((row) => row.contentId == oldContentId, orElse: () => null);
-    if (oldRow != null) {
-      _rows.remove(oldRow);
-
     final newRow = _entryToRow(entry);
+
     await metadataDb.updateFavouriteId(oldContentId, newRow);
+    _rows.remove(oldRow);
     _rows.add(newRow);
+
     changeNotifier.notifyListeners();
   }
-  }
 
   Future<void> clear() async {
     await metadataDb.clearFavourites();
     _rows.clear();
+
     changeNotifier.notifyListeners();
   }
 }

lib/model/source/collection_source.dart

@@ -91,12 +91,12 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
     invalidateFilterEntryCounts();
   }
 
-  // `dateModifiedSecs` changes when moving entries to another directory,
+  Future<void> _moveEntry(AvesEntry entry, Map newFields, bool isFavourite) async {
-  // but it does not change when renaming the containing directory
-  Future<void> moveEntry(AvesEntry entry, Map newFields) async {
     final oldContentId = entry.contentId;
     final newContentId = newFields['contentId'] as int;
     final newDateModifiedSecs = newFields['dateModifiedSecs'] as int;
+    // `dateModifiedSecs` changes when moving entries to another directory,
+    // but it does not change when renaming the containing directory
     if (newDateModifiedSecs != null) entry.dateModifiedSecs = newDateModifiedSecs;
     entry.path = newFields['path'] as String;
     entry.uri = newFields['uri'] as String;
@@ -107,14 +107,17 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
     await metadataDb.updateEntryId(oldContentId, entry);
     await metadataDb.updateMetadataId(oldContentId, entry.catalogMetadata);
     await metadataDb.updateAddressId(oldContentId, entry.addressDetails);
+    if (isFavourite) {
       await favourites.move(oldContentId, entry);
     }
+  }
 
   void updateAfterMove({
-    @required Set<AvesEntry> selection,
+    @required Set<AvesEntry> todoEntries,
+    @required Set<AvesEntry> favouriteEntries,
     @required bool copy,
     @required String destinationAlbum,
-    @required Iterable<MoveOpEvent> movedOps,
+    @required Set<MoveOpEvent> movedOps,
   }) async {
     if (movedOps.isEmpty) return;
 
@@ -124,7 +127,7 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
       movedOps.forEach((movedOp) {
         final sourceUri = movedOp.uri;
         final newFields = movedOp.newFields;
-        final sourceEntry = selection.firstWhere((entry) => entry.uri == sourceUri, orElse: () => null);
+        final sourceEntry = todoEntries.firstWhere((entry) => entry.uri == sourceUri, orElse: () => null);
         fromAlbums.add(sourceEntry.directory);
         movedEntries.add(sourceEntry?.copyWith(
           uri: newFields['uri'] as String,
@@ -141,11 +144,14 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
         final newFields = movedOp.newFields;
         if (newFields.isNotEmpty) {
           final sourceUri = movedOp.uri;
-          final entry = selection.firstWhere((entry) => entry.uri == sourceUri, orElse: () => null);
+          final entry = todoEntries.firstWhere((entry) => entry.uri == sourceUri, orElse: () => null);
           if (entry != null) {
             fromAlbums.add(entry.directory);
             movedEntries.add(entry);
-            await moveEntry(entry, newFields);
+            // do not rely on current favourite repo state to assess whether the moved entry is a favourite
+            // as source monitoring may already have removed the entry from the favourite repo
+            final isFavourite = favouriteEntries.contains(entry);
+            await _moveEntry(entry, newFields, isFavourite);
           }
         }
       });

lib/widgets/collection/entry_set_action_delegate.dart

@@ -78,24 +78,32 @@ class EntrySetActionDelegate with FeedbackMixin, PermissionAwareMixin, SizeAware
 
     if (!await checkFreeSpaceForMove(context, selection, destinationAlbum, moveType)) return;
 
+    // do not directly use selection when moving and post-processing items
+    // as source monitoring may remove obsolete items from the original selection
+    final todoEntries = selection.toSet();
+
     final copy = moveType == MoveType.copy;
-    final selectionCount = selection.length;
+    final todoCount = todoEntries.length;
+    // while the move is ongoing, source monitoring may remove entries from itself and the favourites repo
+    // so we save favourites beforehand, and will mark the moved entries as such after the move
+    final favouriteEntries = todoEntries.where((entry) => entry.isFavourite).toSet();
     showOpReport<MoveOpEvent>(
       context: context,
-      opStream: ImageFileService.move(selection, copy: copy, destinationAlbum: destinationAlbum),
+      opStream: ImageFileService.move(todoEntries, copy: copy, destinationAlbum: destinationAlbum),
-      itemCount: selectionCount,
+      itemCount: todoCount,
       onDone: (processed) async {
-        final movedOps = processed.where((e) => e.success);
+        final movedOps = processed.where((e) => e.success).toSet();
         final movedCount = movedOps.length;
-        if (movedCount < selectionCount) {
+        if (movedCount < todoCount) {
-          final count = selectionCount - movedCount;
+          final count = todoCount - movedCount;
           showFeedback(context, 'Failed to ${copy ? 'copy' : 'move'} ${Intl.plural(count, one: '$count item', other: '$count items')}');
         } else {
           final count = movedCount;
           showFeedback(context, '${copy ? 'Copied' : 'Moved'} ${Intl.plural(count, one: '$count item', other: '$count items')}');
         }
         await source.updateAfterMove(
-          selection: selection,
+          todoEntries: todoEntries,
+          favouriteEntries: favouriteEntries,
           copy: copy,
           destinationAlbum: destinationAlbum,
           movedOps: movedOps,

lib/widgets/filter_grids/common/chip_action_delegate.dart

@@ -108,28 +108,32 @@ class AlbumChipActionDelegate extends ChipActionDelegate with FeedbackMixin, Per
 
     if (!await checkStoragePermissionForAlbums(context, {album})) return;
 
-    final selection = source.rawEntries.where(filter.filter).toSet();
+    final todoEntries = source.rawEntries.where(filter.filter).toSet();
     final destinationAlbum = path.join(path.dirname(album), newName);
 
-    if (!await checkFreeSpaceForMove(context, selection, destinationAlbum, MoveType.move)) return;
+    if (!await checkFreeSpaceForMove(context, todoEntries, destinationAlbum, MoveType.move)) return;
 
-    final selectionCount = selection.length;
+    final todoCount = todoEntries.length;
+    // while the move is ongoing, source monitoring may remove entries from itself and the favourites repo
+    // so we save favourites beforehand, and will mark the moved entries as such after the move
+    final favouriteEntries = todoEntries.where((entry) => entry.isFavourite).toSet();
     showOpReport<MoveOpEvent>(
       context: context,
-      opStream: ImageFileService.move(selection, copy: false, destinationAlbum: destinationAlbum),
+      opStream: ImageFileService.move(todoEntries, copy: false, destinationAlbum: destinationAlbum),
-      itemCount: selectionCount,
+      itemCount: todoCount,
       onDone: (processed) async {
-        final movedOps = processed.where((e) => e.success);
+        final movedOps = processed.where((e) => e.success).toSet();
         final movedCount = movedOps.length;
-        if (movedCount < selectionCount) {
+        if (movedCount < todoCount) {
-          final count = selectionCount - movedCount;
+          final count = todoCount - movedCount;
           showFeedback(context, 'Failed to move ${Intl.plural(count, one: '$count item', other: '$count items')}');
         } else {
           showFeedback(context, 'Done!');
         }
         final pinned = settings.pinnedFilters.contains(filter);
         await source.updateAfterMove(
-          selection: selection,
+          todoEntries: todoEntries,
+          favouriteEntries: favouriteEntries,
           copy: false,
           destinationAlbum: destinationAlbum,
           movedOps: movedOps,