diff --git a/lib/models/download_history.dart b/lib/models/download_history.dart index 20f99428..2e28cd75 100644 --- a/lib/models/download_history.dart +++ b/lib/models/download_history.dart @@ -282,6 +282,16 @@ class DownloadHistoryState { (item) => _trackArtistKey(item.trackName, item.artistName), ); + DownloadHistoryState._withExistingLookup({ + required DownloadHistoryState source, + required this.items, + required this.totalCount, + required this.loadedIndexVersion, + }) : _lookupItems = source._lookupItems, + _bySpotifyId = source._bySpotifyId, + _byIsrc = source._byIsrc, + _byTrackArtistKey = source._byTrackArtistKey; + static String _trackArtistKey(String trackName, String artistName) { final normalizedTrack = trackName.trim().toLowerCase(); if (normalizedTrack.isEmpty) return ''; @@ -313,11 +323,19 @@ class DownloadHistoryState { int? loadedIndexVersion, List? lookupItems, }) { + if (lookupItems == null || identical(lookupItems, _lookupItems)) { + return DownloadHistoryState._withExistingLookup( + source: this, + items: items ?? this.items, + totalCount: totalCount ?? this.totalCount, + loadedIndexVersion: loadedIndexVersion ?? this.loadedIndexVersion, + ); + } return DownloadHistoryState( items: items ?? this.items, totalCount: totalCount ?? this.totalCount, loadedIndexVersion: loadedIndexVersion ?? this.loadedIndexVersion, - lookupItems: lookupItems ?? _lookupItems, + lookupItems: lookupItems, ); } } diff --git a/test/download_history_logic_test.dart b/test/download_history_logic_test.dart index e734f5bd..e0c81eab 100644 --- a/test/download_history_logic_test.dart +++ b/test/download_history_logic_test.dart @@ -20,6 +20,37 @@ DownloadHistoryItem _historyItem({ ); } +class _CountingHistoryItem extends Fake implements DownloadHistoryItem { + _CountingHistoryItem(this.item); + + final DownloadHistoryItem item; + int keyReads = 0; + + @override + String? get spotifyId { + keyReads++; + return item.spotifyId; + } + + @override + String? get isrc { + keyReads++; + return item.isrc; + } + + @override + String get trackName { + keyReads++; + return item.trackName; + } + + @override + String get artistName { + keyReads++; + return item.artistName; + } +} + void main() { group('download history identity', () { test('serializes download timestamps with an explicit UTC offset', () { @@ -64,7 +95,7 @@ void main() { expect(historyItemsReferToSameStoredFile(original, converted), isTrue); }); - test('duplicate track identities keep the newest lookup item', () { + test('duplicate track identities keep the first lookup item', () { final newest = _historyItem( id: 'newest', filePath: '/music/New Album/Same Song.flac', @@ -84,11 +115,20 @@ void main() { totalCount: 2, ); - expect(state.getBySpotifyId('source-track-same')?.id, 'newest'); - expect(state.getByIsrc('SAMEISRC')?.id, 'newest'); + for (final current in [state, state.copyWith(loadedIndexVersion: 1)]) { + expect(current.getBySpotifyId('source-track-same'), same(newest)); + expect(current.getByIsrc('SAMEISRC'), same(newest)); + expect( + current.findByTrackAndArtist(' SAME SONG ', ' same artist '), + same(newest), + ); + } + final reordered = state.copyWith(lookupItems: [older, newest]); + expect(reordered.getBySpotifyId('source-track-same'), same(older)); + expect(reordered.getByIsrc('SAMEISRC'), same(older)); expect( - state.findByTrackAndArtist('Same Song', 'Same Artist')?.id, - 'newest', + reordered.findByTrackAndArtist('Same Song', 'Same Artist'), + same(older), ); expect(state.items, hasLength(2)); }); @@ -177,6 +217,145 @@ void main() { }); }); + group('download history state copies', () { + test('unchanged lookup snapshots do not reread index keys', () { + final item = _CountingHistoryItem( + _historyItem( + id: 'indexed', + filePath: '/music/Same Song.flac', + downloadedAt: DateTime.utc(2026, 7, 23), + ), + ); + final state = DownloadHistoryState(items: [item], totalCount: 1); + final initialReads = item.keyReads; + expect(initialReads, greaterThan(0)); + + final revisionOnly = state.copyWith(loadedIndexVersion: 2); + final countOnly = revisionOnly.copyWith(totalCount: 10); + final visibleItems = []; + final itemsOnly = countOnly.copyWith(items: visibleItems); + final sameLookup = itemsOnly.copyWith(lookupItems: state.lookupItems); + + for (final current in [revisionOnly, countOnly, itemsOnly, sameLookup]) { + expect(current.lookupItems, same(state.lookupItems)); + expect(current.isDownloaded('source-track-same'), isTrue); + expect(current.getBySpotifyId('source-track-same'), same(item)); + expect(current.getByIsrc('SAMEISRC'), same(item)); + expect( + current.findByTrackAndArtist(' Same Song ', 'SAME ARTIST'), + same(item), + ); + } + expect(item.keyReads, initialReads); + expect(revisionOnly.totalCount, 1); + expect(revisionOnly.items, same(state.items)); + expect(countOnly.loadedIndexVersion, 2); + expect(countOnly.items, same(state.items)); + expect(itemsOnly.totalCount, 10); + expect(itemsOnly.loadedIndexVersion, 2); + expect(itemsOnly.items, same(visibleItems)); + expect(state.totalCount, 1); + expect(state.loadedIndexVersion, 0); + + visibleItems.add(item); + expect(itemsOnly.items, [item]); + expect(item.keyReads, initialReads); + }); + + test('constructor lookup snapshots survive mutable input changes', () { + final original = _historyItem( + id: 'original', + filePath: '/music/Same Song.flac', + downloadedAt: DateTime.utc(2026, 7, 23), + ); + final replacement = original.copyWith( + spotifyId: 'replacement', + isrc: 'REPLACEMENT', + trackName: 'Replacement', + ); + + for (final explicitLookup in [false, true]) { + final input = [original]; + final state = DownloadHistoryState( + items: input, + lookupItems: explicitLookup ? input : null, + ); + input[0] = replacement; + + expect(state.items, same(input)); + expect(state.items, [replacement]); + expect(state.lookupItems, [original]); + expect(() => state.lookupItems.clear(), throwsUnsupportedError); + final copied = state.copyWith(totalCount: 2); + expect(copied.lookupItems, same(state.lookupItems)); + expect(copied.getBySpotifyId('source-track-same'), same(original)); + expect(copied.getBySpotifyId('replacement'), isNull); + expect(copied.getByIsrc('SAMEISRC'), same(original)); + expect(copied.getByIsrc('REPLACEMENT'), isNull); + expect( + copied.findByTrackAndArtist('Same Song', 'Same Artist'), + same(original), + ); + expect( + copied.findByTrackAndArtist('Replacement', 'Same Artist'), + isNull, + ); + } + }); + + test('replacement lookup rebuilds all indexes from a new snapshot', () { + final original = _historyItem( + id: 'original', + filePath: '/music/Same Song.flac', + downloadedAt: DateTime.utc(2026, 7, 23), + ); + final replacement = original.copyWith( + spotifyId: 'replacement', + isrc: 'REPLACEMENT', + trackName: ' Replacement ', + artistName: ' NEW ARTIST ', + ); + final state = DownloadHistoryState(items: [original], totalCount: 1); + final input = [replacement]; + final copied = state.copyWith( + lookupItems: input, + totalCount: 2, + loadedIndexVersion: 3, + ); + input.clear(); + + expect(copied.items, same(state.items)); + expect(copied.totalCount, 2); + expect(copied.loadedIndexVersion, 3); + expect(copied.lookupItems, [replacement]); + expect(() => copied.lookupItems.clear(), throwsUnsupportedError); + expect(copied.isDownloaded('source-track-same'), isFalse); + expect(copied.isDownloaded('replacement'), isTrue); + expect(copied.getBySpotifyId('replacement'), same(replacement)); + expect(copied.getByIsrc('SAMEISRC'), isNull); + expect(copied.getByIsrc('REPLACEMENT'), same(replacement)); + expect(copied.findByTrackAndArtist('Same Song', 'Same Artist'), isNull); + expect( + copied.findByTrackAndArtist('replacement', 'new artist'), + same(replacement), + ); + expect(state.lookupItems, [original]); + expect(state.getBySpotifyId('source-track-same'), same(original)); + expect(state.getByIsrc('SAMEISRC'), same(original)); + expect( + state.findByTrackAndArtist('Same Song', 'Same Artist'), + same(original), + ); + + final cleared = copied.copyWith(lookupItems: []); + expect(cleared.items, same(state.items)); + expect(cleared.lookupItems, isEmpty); + expect(cleared.isDownloaded('replacement'), isFalse); + expect(cleared.getByIsrc('REPLACEMENT'), isNull); + expect(cleared.findByTrackAndArtist('replacement', 'new artist'), isNull); + }); + }); + group('startup orphan reconciliation', () { test('requires consecutive missing checks before confirmation', () { final firstMissing = reconcileStartupOrphanSuspects(