From 561f042b7635239863d60394444e1937a5262351 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Fri, 5 Feb 2021 12:19:37 +0900 Subject: [PATCH] media store monitoring: fixed keeping favourites on move --- lib/model/favourite_repo.dart | 18 ++++++++------ lib/model/source/collection_source.dart | 24 ++++++++++++------- .../collection/entry_set_action_delegate.dart | 22 +++++++++++------ .../common/chip_action_delegate.dart | 22 ++++++++++------- 4 files changed, 54 insertions(+), 32 deletions(-) diff --git a/lib/model/favourite_repo.dart b/lib/model/favourite_repo.dart index 78daf8883..e80eb6fd6 100644 --- a/lib/model/favourite_repo.dart +++ b/lib/model/favourite_repo.dart @@ -24,33 +24,37 @@ class FavouriteRepo { Future add(Iterable entries) async { final newRows = entries.map(_entryToRow); + await metadataDb.addFavourites(newRows); _rows.addAll(newRows); + changeNotifier.notifyListeners(); } Future remove(Iterable entries) async { final removedRows = entries.map(_entryToRow); + await metadataDb.removeFavourites(removedRows); removedRows.forEach(_rows.remove); + changeNotifier.notifyListeners(); } Future 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); - final newRow = _entryToRow(entry); - await metadataDb.updateFavouriteId(oldContentId, newRow); - _rows.add(newRow); - changeNotifier.notifyListeners(); - } + await metadataDb.updateFavouriteId(oldContentId, newRow); + _rows.remove(oldRow); + _rows.add(newRow); + + changeNotifier.notifyListeners(); } Future clear() async { await metadataDb.clearFavourites(); _rows.clear(); + changeNotifier.notifyListeners(); } } diff --git a/lib/model/source/collection_source.dart b/lib/model/source/collection_source.dart index 3a9509455..ac57ff7e6 100644 --- a/lib/model/source/collection_source.dart +++ b/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, - // but it does not change when renaming the containing directory - Future moveEntry(AvesEntry entry, Map newFields) async { + Future _moveEntry(AvesEntry entry, Map newFields, bool isFavourite) 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); - await favourites.move(oldContentId, entry); + if (isFavourite) { + await favourites.move(oldContentId, entry); + } } void updateAfterMove({ - @required Set selection, + @required Set todoEntries, + @required Set favouriteEntries, @required bool copy, @required String destinationAlbum, - @required Iterable movedOps, + @required Set 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); } } }); diff --git a/lib/widgets/collection/entry_set_action_delegate.dart b/lib/widgets/collection/entry_set_action_delegate.dart index b51111f77..fc4441c79 100644 --- a/lib/widgets/collection/entry_set_action_delegate.dart +++ b/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( context: context, - opStream: ImageFileService.move(selection, copy: copy, destinationAlbum: destinationAlbum), - itemCount: selectionCount, + opStream: ImageFileService.move(todoEntries, copy: copy, destinationAlbum: destinationAlbum), + 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) { - final count = selectionCount - movedCount; + if (movedCount < todoCount) { + 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, diff --git a/lib/widgets/filter_grids/common/chip_action_delegate.dart b/lib/widgets/filter_grids/common/chip_action_delegate.dart index 7760fac44..d8eb689df 100644 --- a/lib/widgets/filter_grids/common/chip_action_delegate.dart +++ b/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( context: context, - opStream: ImageFileService.move(selection, copy: false, destinationAlbum: destinationAlbum), - itemCount: selectionCount, + opStream: ImageFileService.move(todoEntries, copy: false, destinationAlbum: destinationAlbum), + 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) { - final count = selectionCount - movedCount; + if (movedCount < todoCount) { + 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,