diff --git a/integration_test/album_replaygain_completion_test.dart b/integration_test/album_replaygain_completion_test.dart index ae4d303a..e384207a 100644 --- a/integration_test/album_replaygain_completion_test.dart +++ b/integration_test/album_replaygain_completion_test.dart @@ -266,6 +266,8 @@ registerExtension({ ); final files = {}; final states = >{}; + var candidateChecks = 0; + var candidatesMatch = true; final tracks = [ for (var i = 0; i < 2; i++) Track( @@ -308,6 +310,21 @@ registerExtension({ var retried = false; String? failedId; final subscription = container.listen(downloadQueueProvider, (_, next) { + candidateChecks++; + candidatesMatch = + candidatesMatch && + identical( + next.lookup.firstDownloading, + next.items + .where((item) => item.status == DownloadStatus.downloading) + .firstOrNull, + ) && + identical( + next.lookup.firstFinalizing, + next.items + .where((item) => item.status == DownloadStatus.finalizing) + .firstOrNull, + ); for (final item in next.items) { final transitions = states.putIfAbsent(item.track.id, () => []); if (transitions.lastOrNull != item.status.name) { @@ -362,6 +379,13 @@ registerExtension({ await done.future.timeout(const Duration(seconds: 60)); } await queue.flushQueuePersistence(); + expect(candidateChecks, greaterThan(0)); + expect( + candidatesMatch, + isTrue, + reason: + 'Cached notification candidates must match each real queue snapshot', + ); } finally { subscription.close(); } diff --git a/integration_test/queue_progress_selection_test.dart b/integration_test/queue_progress_selection_test.dart new file mode 100644 index 00000000..7dd5d3be --- /dev/null +++ b/integration_test/queue_progress_selection_test.dart @@ -0,0 +1,158 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:integration_test/integration_test.dart'; +import 'package:spotiflac_android/models/download_item.dart'; +import 'package:spotiflac_android/providers/download_queue_state.dart'; +import 'package:spotiflac_android/utils/chunked_list.dart'; + +import '../test/support/queue_removal_benchmark.dart'; + +({DownloadItem? downloading, DownloadItem? finalizing}) _scan( + DownloadQueueState state, +) { + DownloadItem? downloading; + DownloadItem? finalizing; + final downloadingCount = state.lookup.activeDownloadsCount; + final hasFinalizing = state.lookup.finalizingCount > 0; + if (downloadingCount > 0 || hasFinalizing) { + for (final item in state.items) { + if (downloading == null && item.status == DownloadStatus.downloading) { + downloading = item; + } + if (finalizing == null && item.status == DownloadStatus.finalizing) { + finalizing = item; + } + if ((downloadingCount == 0 || downloading != null) && + (!hasFinalizing || finalizing != null)) { + break; + } + } + } + return (downloading: downloading, finalizing: finalizing); +} + +int _measure( + DownloadQueueState initial, + int activeIndex, + bool finalizing, { + required bool indexed, + required int iterations, +}) { + var state = initial; + final expectedDownload = initial.items[activeIndex].id; + final expectedFinalizing = finalizing + ? initial.items[activeIndex + 1].id + : null; + final watch = Stopwatch()..start(); + for (var tick = 0; tick < iterations; tick++) { + final previous = state.items; + final next = ChunkedList.from(previous).updated({ + activeIndex: previous[activeIndex].copyWith( + progress: (tick + 1) / (iterations + 1), + ), + }); + state = state.copyWith( + items: next, + lookup: state.lookup.updatedForIndices( + previousItems: previous, + nextItems: next, + changedIndices: [activeIndex], + ), + ); + final selected = indexed + ? ( + downloading: state.lookup.firstDownloading, + finalizing: state.lookup.firstFinalizing, + ) + : _scan(state); + if (selected.downloading?.id != expectedDownload || + selected.finalizing?.id != expectedFinalizing || + !identical(selected.downloading, next[activeIndex])) { + fail('Notification selected a different track or stale progress'); + } + } + watch.stop(); + expect(initial.items[activeIndex].progress, 0); + return watch.elapsedMicroseconds; +} + +void main() { + final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + final cases = >[]; + // Sub-millisecond front samples were noisy during plugin/driver startup. + // Let startup settle, and use longer front samples instead of filtering them. + setUpAll(() => Future.delayed(const Duration(seconds: 2))); + tearDownAll(() { + binding.reportData ??= {}; + binding.reportData!['queue_progress_selection'] = { + 'platform': Platform.operatingSystem, + 'completed_cases': cases.length, + 'note': + 'Five alternating pairs after warming both methods: 10000 sparse progress snapshots for front candidates, 500 for tail candidates. Both use the current index update; legacy selection scans items, current selection reads cached first positions. Setup excluded. CPU only, not rendered frames or download speed. iOS debug correctness only.', + 'cases': cases, + }; + }); + for (final count in [1000, 10000]) { + for (final atEnd in [false, true]) { + for (final finalizing in [false, true]) { + testWidgets( + 'select active items in $count, tail=$atEnd, finalizing=$finalizing', + (tester) async { + final fixture = await queueRemovalFixture(count); + final activeIndex = atEnd ? count - 3 : 0; + final iterations = atEnd ? 500 : 10000; + final initial = fixture.copyWith( + items: [ + for (var i = 0; i < count; i++) + fixture.items[i].copyWith( + status: i == activeIndex + ? DownloadStatus.downloading + : finalizing && i == activeIndex + 1 + ? DownloadStatus.finalizing + : DownloadStatus.completed, + ), + ], + ); + _measure( + initial, + activeIndex, + finalizing, + indexed: false, + iterations: iterations, + ); + _measure( + initial, + activeIndex, + finalizing, + indexed: true, + iterations: iterations, + ); + final samples = >{'scan': [], 'indexed': []}; + for (var pair = 0; pair < 5; pair++) { + for (final indexed + in pair.isEven ? [false, true] : [true, false]) { + samples[indexed ? 'indexed' : 'scan']!.add( + _measure( + initial, + activeIndex, + finalizing, + indexed: indexed, + iterations: iterations, + ), + ); + } + } + cases.add({ + 'queue_size': count, + 'active_at_tail': atEnd, + 'with_finalizing': finalizing, + 'updates_per_sample': iterations, + 'samples_us': samples, + }); + }, + ); + } + } + } +} diff --git a/lib/providers/download_queue_provider_progress.dart b/lib/providers/download_queue_provider_progress.dart index e0fae7f6..1ef0ed75 100644 --- a/lib/providers/download_queue_provider_progress.dart +++ b/lib/providers/download_queue_provider_progress.dart @@ -17,28 +17,11 @@ extension _DownloadQueueProgress on DownloadQueueNotifier { _progressTracker.pruneLogs(lookup); final queuedCount = lookup.queuedCount; final downloadingCount = lookup.activeDownloadsCount; - DownloadItem? firstDownloading; + final firstDownloading = lookup.firstDownloading; + final firstFinalizing = lookup.firstFinalizing; bool hasFinalizingItem = lookup.finalizingCount > 0; - String? finalizingTrackName; - String? finalizingArtistName; - if (downloadingCount > 0 || hasFinalizingItem) { - for (final item in currentItems) { - if (firstDownloading == null && - item.status == DownloadStatus.downloading) { - firstDownloading = item; - } - if (finalizingTrackName == null && - item.status == DownloadStatus.finalizing) { - hasFinalizingItem = true; - finalizingTrackName = item.track.name; - finalizingArtistName = item.track.artistName; - } - if ((downloadingCount == 0 || firstDownloading != null) && - (!hasFinalizingItem || finalizingTrackName != null)) { - break; - } - } - } + String? finalizingTrackName = firstFinalizing?.track.name; + String? finalizingArtistName = firstFinalizing?.track.artistName; final replacements = {}; for (final entry in items.entries) { diff --git a/lib/providers/download_queue_state.dart b/lib/providers/download_queue_state.dart index eb2c7847..6e2315ee 100644 --- a/lib/providers/download_queue_state.dart +++ b/lib/providers/download_queue_state.dart @@ -248,8 +248,16 @@ class DownloadQueueLookup { final int failedCount; final int activeDownloadsCount; final int finalizingCount; + final List _items; + final int? _firstDownloadingIndex; + final int? _firstFinalizingIndex; final Map>? _duplicateTrackPositions; + DownloadItem? get firstDownloading => + _firstDownloadingIndex == null ? null : _items[_firstDownloadingIndex]; + DownloadItem? get firstFinalizing => + _firstFinalizingIndex == null ? null : _items[_firstFinalizingIndex]; + const DownloadQueueLookup.empty() : byTrackId = const {}, byItemId = const {}, @@ -261,6 +269,9 @@ class DownloadQueueLookup { failedCount = 0, activeDownloadsCount = 0, finalizingCount = 0, + _items = const [], + _firstDownloadingIndex = null, + _firstFinalizingIndex = null, _duplicateTrackPositions = null; DownloadQueueLookup._({ @@ -274,8 +285,55 @@ class DownloadQueueLookup { required this.failedCount, required this.activeDownloadsCount, required this.finalizingCount, + required List items, + required int? firstDownloadingIndex, + required int? firstFinalizingIndex, Map>? duplicateTrackPositions, - }) : _duplicateTrackPositions = duplicateTrackPositions; + }) : _items = items, + _firstDownloadingIndex = firstDownloadingIndex, + _firstFinalizingIndex = firstFinalizingIndex, + _duplicateTrackPositions = duplicateTrackPositions; + + static int? _firstAfterRemoval( + int? first, + int removedIndex, + DownloadStatus status, + int count, + List nextItems, + ) { + if (first == null) return null; + if (first < removedIndex) return first; + if (first > removedIndex) return first - 1; + if (count > 1) { + for (var index = removedIndex; index < nextItems.length; index++) { + if (nextItems[index].status == status) return index; + } + } + return null; + } + + // Progress-only snapshots retain the first positions. Search beyond an old + // first item only when it leaves the status, not on every native sample. + static int? _firstAfterUpdates( + int? first, + DownloadStatus status, + int count, + List nextItems, + Set changedIndices, + ) { + if (count == 0) return null; + for (final index in changedIndices) { + if ((first == null || index < first) && + nextItems[index].status == status) { + first = index; + } + } + if (first == null || nextItems[first].status == status) return first; + for (var index = first + 1; index < nextItems.length; index++) { + if (nextItems[index].status == status) return index; + } + return null; + } DownloadQueueLookup withoutIndex( List previousItems, @@ -356,6 +414,21 @@ class DownloadQueueLookup { finalizingCount: finalizingCount - (removed.status == DownloadStatus.finalizing ? 1 : 0), + items: nextItems, + firstDownloadingIndex: _firstAfterRemoval( + _firstDownloadingIndex, + index, + DownloadStatus.downloading, + activeDownloadsCount, + nextItems, + ), + firstFinalizingIndex: _firstAfterRemoval( + _firstFinalizingIndex, + index, + DownloadStatus.finalizing, + finalizingCount, + nextItems, + ), duplicateTrackPositions: duplicates, ); } @@ -370,6 +443,8 @@ class DownloadQueueLookup { var failedCount = 0; var activeDownloadsCount = 0; var finalizingCount = 0; + int? firstDownloadingIndex; + int? firstFinalizingIndex; for (var index = 0; index < items.length; index++) { final item = items[index]; byTrackIndex.putIfAbsent(item.track.id, () => index); @@ -381,8 +456,14 @@ class DownloadQueueLookup { if (_countsAsQueued(item.status)) queuedCount++; if (item.status == DownloadStatus.completed) completedCount++; if (item.status == DownloadStatus.failed) failedCount++; - if (item.status == DownloadStatus.downloading) activeDownloadsCount++; - if (item.status == DownloadStatus.finalizing) finalizingCount++; + if (item.status == DownloadStatus.downloading) { + activeDownloadsCount++; + firstDownloadingIndex ??= index; + } + if (item.status == DownloadStatus.finalizing) { + finalizingCount++; + firstFinalizingIndex ??= index; + } } final snapshot = ChunkedList.from(items); final itemIndex = Map.unmodifiable(indexByItemId); @@ -397,6 +478,9 @@ class DownloadQueueLookup { failedCount: failedCount, activeDownloadsCount: activeDownloadsCount, finalizingCount: finalizingCount, + items: snapshot, + firstDownloadingIndex: firstDownloadingIndex, + firstFinalizingIndex: firstFinalizingIndex, ); } @@ -505,6 +589,21 @@ class DownloadQueueLookup { failedCount: nextFailedCount, activeDownloadsCount: nextActiveDownloadsCount, finalizingCount: nextFinalizingCount, + items: snapshot, + firstDownloadingIndex: _firstAfterUpdates( + _firstDownloadingIndex, + DownloadStatus.downloading, + nextActiveDownloadsCount, + snapshot, + normalizedChanged, + ), + firstFinalizingIndex: _firstAfterUpdates( + _firstFinalizingIndex, + DownloadStatus.finalizing, + nextFinalizingCount, + snapshot, + normalizedChanged, + ), duplicateTrackPositions: _duplicateTrackPositions, ); } diff --git a/test/download_queue_removal_test.dart b/test/download_queue_removal_test.dart index c7322894..d98ddf0d 100644 --- a/test/download_queue_removal_test.dart +++ b/test/download_queue_removal_test.dart @@ -39,6 +39,15 @@ void _check(DownloadQueueState state) { expect(actual.failedCount, expected.failedCount); expect(actual.activeDownloadsCount, expected.activeDownloadsCount); expect(actual.finalizingCount, expected.finalizingCount); + for (final entry in { + DownloadStatus.downloading: actual.firstDownloading, + DownloadStatus.finalizing: actual.firstFinalizing, + }.entries) { + expect( + entry.value, + same(state.items.where((item) => item.status == entry.key).firstOrNull), + ); + } expect(actual.byItemId['absent'], isNull); expect(actual.byTrackId['absent'], isNull); expect(actual.indexByItemId['absent'], isNull); diff --git a/test/download_queue_sparse_update_test.dart b/test/download_queue_sparse_update_test.dart index 2a5112f7..8621ffb2 100644 --- a/test/download_queue_sparse_update_test.dart +++ b/test/download_queue_sparse_update_test.dart @@ -98,6 +98,8 @@ void main() { expect(state.lookup.failedCount, rebuilt.failedCount); expect(state.lookup.activeDownloadsCount, rebuilt.activeDownloadsCount); expect(state.lookup.finalizingCount, rebuilt.finalizingCount); + expect(state.lookup.firstDownloading, same(rebuilt.firstDownloading)); + expect(state.lookup.firstFinalizing, same(rebuilt.firstFinalizing)); expect(state.lookup.notCompletedItemIds, rebuilt.notCompletedItemIds); expect( old.lookup.byItemId[old.items[index].id], @@ -112,4 +114,102 @@ void main() { expect(() => state.lookup.byItemId.clear(), throwsUnsupportedError); }, ); + + test( + 'notification candidates follow batch transitions and keep old values', + () { + void check(DownloadQueueState state) { + expect( + state.lookup.firstDownloading, + same( + state.items + .where((item) => item.status == DownloadStatus.downloading) + .firstOrNull, + ), + ); + expect( + state.lookup.firstFinalizing, + same( + state.items + .where((item) => item.status == DownloadStatus.finalizing) + .firstOrNull, + ), + ); + } + + var state = const DownloadQueueState().copyWith( + items: List.generate(200, _item), + ); + void update(Map changes) { + final old = state; + final next = ChunkedList.from(old.items).updated(changes); + state = old.copyWith( + items: next, + lookup: old.lookup.updatedForIndices( + previousItems: old.items, + nextItems: next, + changedIndices: changes.keys.toList().reversed, + ), + ); + check(state); + check(old); + } + + final random = Random(506); + for (var tick = 0; tick < 300; tick++) { + update({ + for (var i = 0; i < 7; i++) + random.nextInt(200): state.items[random.nextInt(200)], + }); + // Above exercises identity/reordering fallback. Keep IDs stable here to + // exercise the incremental path with multiple simultaneous transitions. + final changes = {}; + for (var i = 0; i < 7; i++) { + final index = random.nextInt(200); + changes[index] = state.items[index].copyWith( + status: DownloadStatus + .values[random.nextInt(DownloadStatus.values.length)], + progress: tick / 300, + track: state.items[index].track.copyWith(name: 'Updated $tick'), + ); + } + // Restore unique request IDs after the intentionally malformed batch. + state = state.copyWith( + items: [ + for (var i = 0; i < state.items.length; i++) + state.items[i].copyWith(id: 'item-$i'), + ], + ); + update({ + for (final entry in changes.entries) + entry.key: entry.value.copyWith(id: 'item-${entry.key}'), + }); + } + update({ + for (var i = 0; i < state.items.length; i++) + i: state.items[i].copyWith(status: DownloadStatus.completed), + }); + expect(state.lookup.firstDownloading, isNull); + expect(state.lookup.firstFinalizing, isNull); + update({ + 190: state.items[190].copyWith(status: DownloadStatus.downloading), + 195: state.items[195].copyWith(status: DownloadStatus.finalizing), + }); + update({ + 195: state.items[195].copyWith(status: DownloadStatus.downloading), + 190: state.items[190].copyWith(status: DownloadStatus.finalizing), + 1: state.items[1].copyWith(status: DownloadStatus.downloading), + 0: state.items[0].copyWith(status: DownloadStatus.finalizing), + }); + expect(state.lookup.firstDownloading, same(state.items[1])); + expect(state.lookup.firstFinalizing, same(state.items[0])); + update({ + 0: state.items[0].copyWith(status: DownloadStatus.completed), + 1: state.items[1].copyWith(status: DownloadStatus.completed), + }); + expect(state.lookup.firstDownloading, same(state.items[195])); + expect(state.lookup.firstFinalizing, same(state.items[190])); + check(const DownloadQueueState()); + }, + ); }