#293 fixed entry duplication when media store triggered uri refresh during initial loading between entry fetch and addition

This commit is contained in:
Thibault Deckers 2022-08-29 20:14:13 +02:00
parent 1a92768c5c
commit b012fc9ff5
20 changed files with 83 additions and 34 deletions

View file

@ -40,6 +40,12 @@ mixin SourceBase {
ValueNotifier<SourceState> stateNotifier = ValueNotifier(SourceState.ready);
set state(SourceState value) => stateNotifier.value = value;
SourceState get state => stateNotifier.value;
bool get isReady => state == SourceState.ready;
ValueNotifier<ProgressEvent> progressNotifier = ValueNotifier(const ProgressEvent(done: 0, total: 0));
void setProgress({required int done, required int total}) => progressNotifier.value = ProgressEvent(done: done, total: total);
@ -430,7 +436,7 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
updateDerivedFilters(todoEntries);
}
}
stateNotifier.value = SourceState.ready;
state = SourceState.ready;
}
// monitoring

View file

@ -51,7 +51,7 @@ mixin LocationMixin on SourceBase {
final todo = (force ? candidateEntries.where((entry) => entry.hasGps) : candidateEntries.where(locateCountriesTest)).toSet();
if (todo.isEmpty) return;
stateNotifier.value = SourceState.locatingCountries;
state = SourceState.locatingCountries;
var progressDone = 0;
final progressTotal = todo.length;
setProgress(done: progressDone, total: progressTotal);
@ -106,7 +106,7 @@ mixin LocationMixin on SourceBase {
knownLocations.putIfAbsent(approximateLatLng(entry), () => entry.addressDetails);
});
stateNotifier.value = SourceState.locatingPlaces;
state = SourceState.locatingPlaces;
var progressDone = 0;
final progressTotal = todo.length;
setProgress(done: progressDone, total: progressTotal);

View file

@ -42,7 +42,7 @@ class MediaStoreSource extends CollectionSource {
Future<void> _loadEssentials() async {
final stopwatch = Stopwatch()..start();
stateNotifier.value = SourceState.loading;
state = SourceState.loading;
await metadataDb.init();
await favourites.init();
await covers.init();
@ -69,7 +69,7 @@ class MediaStoreSource extends CollectionSource {
}) async {
debugPrint('$runtimeType refresh start');
final stopwatch = Stopwatch()..start();
stateNotifier.value = SourceState.loading;
state = SourceState.loading;
clearEntries();
final Set<AvesEntry> topEntries = {};
@ -195,7 +195,7 @@ class MediaStoreSource extends CollectionSource {
if (canAnalyze) {
await analyze(analysisController, entries: analysisEntries);
} else {
stateNotifier.value = SourceState.ready;
state = SourceState.ready;
}
// the home page may not reflect the current derived filters
@ -216,7 +216,7 @@ class MediaStoreSource extends CollectionSource {
// sometimes yields an entry with its temporary path: `/data/sec/camera/!@#$%^..._temp.jpg`
@override
Future<Set<String>> refreshUris(Set<String> changedUris, {AnalysisController? analysisController}) async {
if (_initState == SourceInitializationState.none || !isMonitoring) return changedUris;
if (_initState == SourceInitializationState.none || !isMonitoring || !isReady) return changedUris;
debugPrint('$runtimeType refreshUris ${changedUris.length} uris');
final uriByContentId = Map.fromEntries(changedUris.map((uri) {

View file

@ -31,7 +31,7 @@ mixin TagMixin on SourceBase {
final todo = force ? candidateEntries : candidateEntries.where(catalogEntriesTest).toSet();
if (todo.isEmpty) return;
stateNotifier.value = SourceState.cataloguing;
state = SourceState.cataloguing;
var progressDone = 0;
final progressTotal = todo.length;
setProgress(done: progressDone, total: progressTotal);

View file

@ -85,7 +85,7 @@ class Analyzer {
bool get isRunning => serviceState == AnalyzerState.running;
SourceState get sourceState => _source.stateNotifier.value;
SourceState get sourceState => _source.state;
static const notificationUpdateInterval = Duration(seconds: 1);
@ -151,7 +151,7 @@ class Analyzer {
}
void _onSourceStateChanged() {
if (sourceState == SourceState.ready) {
if (_source.isReady) {
_refreshApp();
_serviceStateNotifier.value = AnalyzerState.stopping;
}

View file

@ -4,7 +4,6 @@ import 'package:aves/app_flavor.dart';
import 'package:aves/model/entry.dart';
import 'package:aves/model/settings/settings.dart';
import 'package:aves/model/source/collection_lens.dart';
import 'package:aves/model/source/enums.dart';
import 'package:aves/model/source/media_store_source.dart';
import 'package:aves/services/common/services.dart';
import 'package:aves/widgets/home_widget.dart';
@ -64,7 +63,7 @@ Future<AvesEntry?> _getWidgetEntry(int widgetId, bool reuseEntry) async {
final source = MediaStoreSource();
final readyCompleter = Completer();
source.stateNotifier.addListener(() {
if (source.stateNotifier.value == SourceState.ready) {
if (source.isReady) {
readyCompleter.complete();
}
});

View file

@ -60,7 +60,7 @@ class SourceStateSubtitle extends StatelessWidget {
@override
Widget build(BuildContext context) {
final sourceState = source.stateNotifier.value;
final sourceState = source.state;
final subtitle = sourceState.getName(context.l10n);
if (subtitle == null) return const SizedBox();

View file

@ -266,7 +266,7 @@ class _HomePageState extends State<HomePage> {
// wait for collection to pass the `loading` state
final completer = Completer();
void _onSourceStateChanged() {
if (source.stateNotifier.value != SourceState.loading) {
if (source.state != SourceState.loading) {
source.stateNotifier.removeListener(_onSourceStateChanged);
completer.complete();
}

View file

@ -4,7 +4,6 @@ import 'package:aves/model/settings/enums/slideshow_interval.dart';
import 'package:aves/model/settings/settings.dart';
import 'package:aves/model/source/collection_lens.dart';
import 'package:aves/model/source/collection_source.dart';
import 'package:aves/model/source/enums.dart';
import 'package:aves/theme/icons.dart';
import 'package:aves/widgets/common/extensions/build_context.dart';
import 'package:aves/widgets/common/identity/empty.dart';
@ -112,7 +111,7 @@ class _ScreenSaverPageState extends State<ScreenSaverPage> with WidgetsBindingOb
}
void _initSlideshowCollection() {
if (source.stateNotifier.value != SourceState.ready || _slideshowCollection != null) return;
if (!source.isReady || _slideshowCollection != null) return;
final originalCollection = CollectionLens(
source: source,

View file

@ -1,7 +1,7 @@
import 'package:aves/services/android_app_service.dart';
import 'package:aves/utils/android_file_utils.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeAndroidAppService extends Fake implements AndroidAppService {
@override

View file

@ -1,6 +1,6 @@
import 'package:aves/model/availability.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeAvesAvailability extends Fake implements AvesAvailability {
@override

View file

@ -1,6 +1,6 @@
import 'package:aves/services/device_service.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeDeviceService extends Fake implements DeviceService {
@override

View file

@ -0,0 +1,15 @@
import 'package:aves/model/entry.dart';
import 'package:aves/services/media/media_fetch_service.dart';
import 'package:collection/collection.dart';
import 'package:test/fake.dart';
class FakeMediaFetchService extends Fake implements MediaFetchService {
Duration latency = Duration.zero;
Set<AvesEntry> entries = {};
@override
Future<AvesEntry?> getEntry(String uri, String? mimeType) async {
await Future.delayed(latency);
return entries.firstWhereOrNull((v) => v.uri == uri);
}
}

View file

@ -2,17 +2,23 @@ import 'package:aves/model/entry.dart';
import 'package:aves/ref/mime_types.dart';
import 'package:aves/services/common/image_op_events.dart';
import 'package:aves/services/media/media_store_service.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeMediaStoreService extends Fake implements MediaStoreService {
Duration latency = Duration.zero;
Set<AvesEntry> entries = {};
@override
Future<List<int>> checkObsoleteContentIds(List<int?> knownContentIds) => SynchronousFuture([]);
Future<List<int>> checkObsoleteContentIds(List<int?> knownContentIds) async {
await Future.delayed(latency);
return [];
}
@override
Future<List<int>> checkObsoletePaths(Map<int?, String?> knownPathById) => SynchronousFuture([]);
Future<List<int>> checkObsoletePaths(Map<int?, String?> knownPathById) async {
await Future.delayed(latency);
return [];
}
@override
Stream<AvesEntry> getEntries(Map<int?, int?> knownEntries, {String? directory}) => Stream.fromIterable(entries);
@ -23,14 +29,15 @@ class FakeMediaStoreService extends Fake implements MediaStoreService {
static int get dateSecs => DateTime.now().millisecondsSinceEpoch ~/ 1000;
static AvesEntry newImage(String album, String filenameWithoutExtension) {
final id = nextId;
static AvesEntry newImage(String album, String filenameWithoutExtension, {int? id, int? contentId}) {
id ??= nextId;
contentId ??= id;
final date = dateSecs;
return AvesEntry(
id: id,
uri: 'content://media/external/images/media/$id',
uri: 'content://media/external/images/media/$contentId',
path: '$album/$filenameWithoutExtension.jpg',
contentId: id,
contentId: contentId,
pageId: null,
sourceMimeType: MimeTypes.jpeg,
width: 360,

View file

@ -7,7 +7,7 @@ import 'package:aves/model/metadata/address.dart';
import 'package:aves/model/metadata/catalog.dart';
import 'package:aves/model/metadata/trash.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeMetadataDb extends Fake implements MetadataDb {
static int _lastId = 0;

View file

@ -2,7 +2,7 @@ import 'package:aves/model/entry.dart';
import 'package:aves/model/metadata/catalog.dart';
import 'package:aves/services/metadata/metadata_fetch_service.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeMetadataFetchService extends Fake implements MetadataFetchService {
final Map<AvesEntry, CatalogMetadata> _metaMap = {};

View file

@ -1,6 +1,5 @@
import 'package:aves_report/aves_report.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
class FakeReportService extends ReportService {
@override

View file

@ -1,7 +1,7 @@
import 'package:aves/services/storage_service.dart';
import 'package:aves/utils/android_file_utils.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeStorageService extends Fake implements StorageService {
static const primaryRootAlbum = '/storage/emulated/0';

View file

@ -1,7 +1,7 @@
import 'package:aves/services/window_service.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:test/fake.dart';
class FakeWindowService extends Fake implements WindowService {
@override

View file

@ -10,11 +10,12 @@ import 'package:aves/model/filters/tag.dart';
import 'package:aves/model/metadata/address.dart';
import 'package:aves/model/metadata/catalog.dart';
import 'package:aves/model/settings/settings.dart';
import 'package:aves/model/source/enums.dart';
import 'package:aves/model/source/collection_source.dart';
import 'package:aves/model/source/media_store_source.dart';
import 'package:aves/services/android_app_service.dart';
import 'package:aves/services/common/services.dart';
import 'package:aves/services/device_service.dart';
import 'package:aves/services/media/media_fetch_service.dart';
import 'package:aves/services/media/media_store_service.dart';
import 'package:aves/services/metadata/metadata_fetch_service.dart';
import 'package:aves/services/storage_service.dart';
@ -29,6 +30,7 @@ import 'package:path/path.dart' as p;
import '../fake/android_app_service.dart';
import '../fake/availability.dart';
import '../fake/device_service.dart';
import '../fake/media_fetch_service.dart';
import '../fake/media_store_service.dart';
import '../fake/metadata_db.dart';
import '../fake/metadata_fetch_service.dart';
@ -57,6 +59,7 @@ void main() {
getIt.registerLazySingleton<AndroidAppService>(FakeAndroidAppService.new);
getIt.registerLazySingleton<DeviceService>(FakeDeviceService.new);
getIt.registerLazySingleton<MediaFetchService>(FakeMediaFetchService.new);
getIt.registerLazySingleton<MediaStoreService>(FakeMediaStoreService.new);
getIt.registerLazySingleton<MetadataFetchService>(FakeMetadataFetchService.new);
getIt.registerLazySingleton<ReportService>(FakeReportService.new);
@ -65,6 +68,7 @@ void main() {
await settings.init(monitorPlatformSettings: false);
settings.canUseAnalysisService = false;
await androidFileUtils.init();
});
tearDown(() async {
@ -75,7 +79,7 @@ void main() {
final source = MediaStoreSource();
final readyCompleter = Completer();
source.stateNotifier.addListener(() {
if (source.stateNotifier.value == SourceState.ready) {
if (source.isReady) {
readyCompleter.complete();
}
});
@ -84,6 +88,26 @@ void main() {
return source;
}
test('initial load v. refresh race condition', () async {
const latency = Duration(milliseconds: 100);
final loadEntry = FakeMediaStoreService.newImage(testAlbum, 'image1', id: -1, contentId: 1);
final refreshEntry = FakeMediaStoreService.newImage(testAlbum, 'image1', id: -1, contentId: 1);
(mediaStoreService as FakeMediaStoreService)
..entries = {loadEntry}
..latency = latency;
(mediaFetchService as FakeMediaFetchService).entries = {refreshEntry};
final source = MediaStoreSource();
unawaited(source.init());
await Future.delayed(const Duration(milliseconds: 10));
expect(source.initState, SourceInitializationState.full);
await source.refreshUris({refreshEntry.uri});
await Future.delayed(const Duration(seconds: 1));
expect(source.allEntries.length, 1);
});
test('album/country/tag hidden on launch when their items are hidden by entry prop', () async {
settings.hiddenFilters = {const AlbumFilter(testAlbum, 'whatever')};