mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-07-28 23:08:59 +02:00
fix(library): persist records before download completion
This commit is contained in:
@@ -44,6 +44,16 @@ bool historyItemsReferToSameStoredFile(
|
||||
).any((key) => firstKeys.contains(key));
|
||||
}
|
||||
|
||||
String? resolvePersistedHistoryQuality({
|
||||
required String? incoming,
|
||||
required String? existing,
|
||||
}) {
|
||||
return nonPlaceholderQuality(incoming) ??
|
||||
nonPlaceholderQuality(existing) ??
|
||||
normalizeOptionalString(incoming) ??
|
||||
normalizeOptionalString(existing);
|
||||
}
|
||||
|
||||
class DownloadHistoryItem {
|
||||
final String id;
|
||||
final String trackName;
|
||||
@@ -1034,6 +1044,16 @@ class DownloadHistoryNotifier extends Notifier<DownloadHistoryState> {
|
||||
final mergedItem = existing == null
|
||||
? incomingItem
|
||||
: incomingItem.copyWith(
|
||||
quality: resolvePersistedHistoryQuality(
|
||||
incoming: item.quality,
|
||||
existing: existing.quality,
|
||||
),
|
||||
bitDepth: item.bitDepth ?? existing.bitDepth,
|
||||
sampleRate: item.sampleRate ?? existing.sampleRate,
|
||||
bitrate: item.bitrate ?? existing.bitrate,
|
||||
format:
|
||||
normalizeOptionalString(item.format) ??
|
||||
normalizeOptionalString(existing.format),
|
||||
trackNumber: item.trackNumber ?? existing.trackNumber,
|
||||
totalTracks: item.totalTracks ?? existing.totalTracks,
|
||||
discNumber: item.discNumber ?? existing.discNumber,
|
||||
@@ -1223,6 +1243,21 @@ class DownloadHistoryNotifier extends Notifier<DownloadHistoryState> {
|
||||
return DownloadHistoryItem.fromJson(json);
|
||||
}
|
||||
|
||||
Future<DownloadHistoryItem?> getByFilePathAsync(String filePath) async {
|
||||
final targetKeys = buildPathMatchKeys(filePath);
|
||||
if (targetKeys.isEmpty) return null;
|
||||
|
||||
for (final item in state.lookupItems) {
|
||||
if (buildPathMatchKeys(item.filePath).any(targetKeys.contains)) {
|
||||
return item;
|
||||
}
|
||||
}
|
||||
|
||||
final json = await _db.findByFilePath(filePath);
|
||||
if (json == null) return null;
|
||||
return DownloadHistoryItem.fromJson(json);
|
||||
}
|
||||
|
||||
Future<DownloadHistoryItem?> findByTrackAndArtistAsync(
|
||||
String trackName,
|
||||
String artistName,
|
||||
|
||||
@@ -51,6 +51,18 @@ const String safPermissionLostErrorMessage =
|
||||
const String downloadFolderAccessLostErrorMessage =
|
||||
'Download folder access lost. Please re-select your download folder in Settings.';
|
||||
|
||||
/// Keeps a download in its finalizing state until its durable Library record
|
||||
/// has been written. If persistence fails, completion is deliberately not
|
||||
/// published so the queue can surface the error instead of losing the file
|
||||
/// from the app while it still exists on disk.
|
||||
Future<void> persistBeforePublishingDownloadCompletion({
|
||||
required Future<void> Function() persist,
|
||||
required void Function() publish,
|
||||
}) async {
|
||||
await persist();
|
||||
publish();
|
||||
}
|
||||
|
||||
final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]');
|
||||
final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$');
|
||||
final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$');
|
||||
@@ -3705,12 +3717,11 @@ class DownloadQueueNotifier extends Notifier<DownloadQueueState> {
|
||||
}
|
||||
}
|
||||
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: filePath,
|
||||
);
|
||||
if (normalizeOptionalString(filePath) == null) {
|
||||
throw StateError(
|
||||
'Download backend reported success without a final file path',
|
||||
);
|
||||
}
|
||||
|
||||
if (effectiveSafMode && filePath != null && isContentUri(filePath)) {
|
||||
await _saveExternalLrc(
|
||||
@@ -3754,37 +3765,19 @@ class DownloadQueueNotifier extends Notifier<DownloadQueueState> {
|
||||
_log.w('Album ReplayGain check failed: $e');
|
||||
}
|
||||
|
||||
_completedInSession++;
|
||||
|
||||
final historyNotifier = ref.read(downloadHistoryProvider.notifier);
|
||||
final existingInHistory =
|
||||
await historyNotifier.getBySpotifyIdAsync(trackToDownload.id) ??
|
||||
(trackToDownload.isrc != null
|
||||
? await historyNotifier.getByIsrcAsync(trackToDownload.isrc!)
|
||||
: null);
|
||||
final existingInHistory = filePath == null
|
||||
? null
|
||||
: await historyNotifier.getByFilePathAsync(filePath);
|
||||
|
||||
if (wasExisting && existingInHistory != null) {
|
||||
_log.i('Track already in library, skipping history update');
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
completedCount: _completedInSession,
|
||||
totalCount: _totalQueuedAtStart,
|
||||
alreadyInLibrary: true,
|
||||
_log.i(
|
||||
'Track file already exists in library; refreshing its history metadata',
|
||||
);
|
||||
removeItem(item.id);
|
||||
return;
|
||||
}
|
||||
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
completedCount: _completedInSession,
|
||||
totalCount: _totalQueuedAtStart,
|
||||
alreadyInLibrary: wasExisting,
|
||||
);
|
||||
|
||||
if (filePath != null) {
|
||||
final historyFilePath = filePath;
|
||||
final backendBitDepth = result['actual_bit_depth'] as int?;
|
||||
final backendSampleRate = result['actual_sample_rate'] as int?;
|
||||
final backendFormat =
|
||||
@@ -3893,32 +3886,50 @@ class DownloadQueueNotifier extends Notifier<DownloadQueueState> {
|
||||
final historySampleRate = isLossyOutput ? null : finalSampleRate;
|
||||
final historyBitrate = isLossyOutput ? finalBitrateKbps : null;
|
||||
|
||||
if (settings.saveDownloadHistory) {
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.addToHistory(
|
||||
_historyItemFromResult(
|
||||
item: item,
|
||||
trackToDownload: trackToDownload,
|
||||
result: result,
|
||||
filePath: filePath,
|
||||
quality: actualQuality,
|
||||
useSaf: effectiveSafMode,
|
||||
downloadTreeUri: settings.downloadTreeUri,
|
||||
safRelativeDir: effectiveOutputDir,
|
||||
safFileName: finalSafFileName ?? safFileName,
|
||||
bitDepth: historyBitDepth,
|
||||
sampleRate: historySampleRate,
|
||||
bitrate: historyBitrate,
|
||||
format: finalFormat,
|
||||
genre: effectiveGenre,
|
||||
label: effectiveLabel,
|
||||
copyright: effectiveCopyright,
|
||||
),
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
}
|
||||
|
||||
await persistBeforePublishingDownloadCompletion(
|
||||
persist: () async {
|
||||
if (!settings.saveDownloadHistory) return;
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.addToHistory(
|
||||
_historyItemFromResult(
|
||||
item: item,
|
||||
trackToDownload: trackToDownload,
|
||||
result: result,
|
||||
filePath: historyFilePath,
|
||||
quality: actualQuality,
|
||||
useSaf: effectiveSafMode,
|
||||
downloadTreeUri: settings.downloadTreeUri,
|
||||
safRelativeDir: effectiveOutputDir,
|
||||
safFileName: finalSafFileName ?? safFileName,
|
||||
bitDepth: historyBitDepth,
|
||||
sampleRate: historySampleRate,
|
||||
bitrate: historyBitrate,
|
||||
format: finalFormat,
|
||||
genre: effectiveGenre,
|
||||
label: effectiveLabel,
|
||||
copyright: effectiveCopyright,
|
||||
),
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
},
|
||||
publish: () {
|
||||
_completedInSession++;
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: filePath,
|
||||
);
|
||||
},
|
||||
);
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
completedCount: _completedInSession,
|
||||
totalCount: _totalQueuedAtStart,
|
||||
alreadyInLibrary: wasExisting,
|
||||
);
|
||||
removeItem(item.id);
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -926,48 +926,55 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
}
|
||||
|
||||
if (result['native_finalized'] == true) {
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: filePath,
|
||||
);
|
||||
if (settings.saveDownloadHistory) {
|
||||
final historyItem = result['history_item'];
|
||||
if (historyItem is Map) {
|
||||
try {
|
||||
final nativeFinalizedFilePath = filePath;
|
||||
await persistBeforePublishingDownloadCompletion(
|
||||
persist: () async {
|
||||
if (!settings.saveDownloadHistory) return;
|
||||
final historyItem = result['history_item'];
|
||||
if (historyItem is Map) {
|
||||
try {
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.adoptNativeHistoryItem(
|
||||
DownloadHistoryItem.fromJson(
|
||||
Map<String, dynamic>.from(historyItem),
|
||||
),
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
} catch (e) {
|
||||
_log.w('Failed to adopt native history item: $e');
|
||||
await _persistNativeFinalizedHistoryFallback(
|
||||
context,
|
||||
result,
|
||||
nativeFinalizedFilePath,
|
||||
);
|
||||
}
|
||||
} else if (result['history_written'] == true ||
|
||||
result['already_exists'] == true) {
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.adoptNativeHistoryItem(
|
||||
DownloadHistoryItem.fromJson(
|
||||
Map<String, dynamic>.from(historyItem),
|
||||
),
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
} catch (e) {
|
||||
_log.w('Failed to adopt native history item: $e');
|
||||
.reloadFromStorage();
|
||||
} else {
|
||||
_log.w(
|
||||
'Native finalizer completed without history; persisting Dart fallback',
|
||||
);
|
||||
await _persistNativeFinalizedHistoryFallback(
|
||||
context,
|
||||
result,
|
||||
filePath,
|
||||
nativeFinalizedFilePath,
|
||||
);
|
||||
}
|
||||
} else if (result['history_written'] == true) {
|
||||
await ref.read(downloadHistoryProvider.notifier).reloadFromStorage();
|
||||
} else if (result['already_exists'] == true) {
|
||||
await ref.read(downloadHistoryProvider.notifier).reloadFromStorage();
|
||||
} else {
|
||||
_log.w(
|
||||
'Native finalizer completed without history; persisting Dart fallback',
|
||||
},
|
||||
publish: () {
|
||||
_completedInSession++;
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: nativeFinalizedFilePath,
|
||||
);
|
||||
await _persistNativeFinalizedHistoryFallback(
|
||||
context,
|
||||
result,
|
||||
filePath,
|
||||
);
|
||||
}
|
||||
}
|
||||
_completedInSession++;
|
||||
},
|
||||
);
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
@@ -1130,12 +1137,6 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
if (finalQuality != null) actualQuality = finalQuality;
|
||||
}
|
||||
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: filePath,
|
||||
);
|
||||
await _saveExternalLrc(
|
||||
result: result,
|
||||
settings: settings,
|
||||
@@ -1160,15 +1161,6 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
onFetchError: (e) =>
|
||||
_log.w('Failed to fetch native-worker external LRC: $e'),
|
||||
);
|
||||
_completedInSession++;
|
||||
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
completedCount: _completedInSession,
|
||||
totalCount: _totalQueuedAtStart,
|
||||
alreadyInLibrary: result['already_exists'] == true,
|
||||
);
|
||||
|
||||
final resultSafFileName = result['file_name'] as String?;
|
||||
final lowerFilePath = filePath.toLowerCase();
|
||||
@@ -1186,36 +1178,56 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
lowerFilePath.endsWith('.opus') ||
|
||||
lowerFilePath.endsWith('.ogg');
|
||||
|
||||
if (settings.saveDownloadHistory) {
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.addToHistory(
|
||||
_historyItemFromResult(
|
||||
item: item,
|
||||
trackToDownload: trackToDownload,
|
||||
result: result,
|
||||
filePath: filePath,
|
||||
quality: actualQuality,
|
||||
useSaf: context.storageMode == 'saf',
|
||||
downloadTreeUri: context.downloadTreeUri,
|
||||
safRelativeDir: context.safRelativeDir,
|
||||
safFileName:
|
||||
(resultSafFileName != null && resultSafFileName.isNotEmpty)
|
||||
? resultSafFileName
|
||||
: context.safFileName,
|
||||
bitDepth: isLossyOutput ? null : actualBitDepth,
|
||||
sampleRate: isLossyOutput ? null : actualSampleRate,
|
||||
bitrate: isLossyOutput ? actualBitrate : null,
|
||||
format: historyFormat,
|
||||
genre: normalizeOptionalString(result['genre'] as String?),
|
||||
label: normalizeOptionalString(result['label'] as String?),
|
||||
copyright: normalizeOptionalString(
|
||||
result['copyright'] as String?,
|
||||
final completedFilePath = filePath;
|
||||
await persistBeforePublishingDownloadCompletion(
|
||||
persist: () async {
|
||||
if (!settings.saveDownloadHistory) return;
|
||||
await ref
|
||||
.read(downloadHistoryProvider.notifier)
|
||||
.addToHistory(
|
||||
_historyItemFromResult(
|
||||
item: item,
|
||||
trackToDownload: trackToDownload,
|
||||
result: result,
|
||||
filePath: completedFilePath,
|
||||
quality: actualQuality,
|
||||
useSaf: context.storageMode == 'saf',
|
||||
downloadTreeUri: context.downloadTreeUri,
|
||||
safRelativeDir: context.safRelativeDir,
|
||||
safFileName:
|
||||
(resultSafFileName != null && resultSafFileName.isNotEmpty)
|
||||
? resultSafFileName
|
||||
: context.safFileName,
|
||||
bitDepth: isLossyOutput ? null : actualBitDepth,
|
||||
sampleRate: isLossyOutput ? null : actualSampleRate,
|
||||
bitrate: isLossyOutput ? actualBitrate : null,
|
||||
format: historyFormat,
|
||||
genre: normalizeOptionalString(result['genre'] as String?),
|
||||
label: normalizeOptionalString(result['label'] as String?),
|
||||
copyright: normalizeOptionalString(
|
||||
result['copyright'] as String?,
|
||||
),
|
||||
),
|
||||
),
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
}
|
||||
preserveTrackVariant: item.preserveQualityVariant,
|
||||
);
|
||||
},
|
||||
publish: () {
|
||||
_completedInSession++;
|
||||
updateItemStatus(
|
||||
item.id,
|
||||
DownloadStatus.completed,
|
||||
progress: 1.0,
|
||||
filePath: completedFilePath,
|
||||
);
|
||||
},
|
||||
);
|
||||
await _notificationService.showDownloadComplete(
|
||||
trackName: item.track.name,
|
||||
artistName: item.track.artistName,
|
||||
completedCount: _completedInSession,
|
||||
totalCount: _totalQueuedAtStart,
|
||||
alreadyInLibrary: result['already_exists'] == true,
|
||||
);
|
||||
|
||||
removeItem(item.id);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:spotiflac_android/providers/download_history_provider.dart';
|
||||
import 'package:spotiflac_android/providers/download_queue_provider.dart';
|
||||
|
||||
DownloadHistoryItem _historyItem({
|
||||
required String id,
|
||||
@@ -82,6 +82,44 @@ void main() {
|
||||
);
|
||||
expect(state.items, hasLength(2));
|
||||
});
|
||||
|
||||
test('actual audio quality survives history serialization', () {
|
||||
final item =
|
||||
_historyItem(
|
||||
id: 'quality',
|
||||
filePath: '/music/Album/Hi-Res Song.flac',
|
||||
downloadedAt: DateTime.utc(2026, 7, 23),
|
||||
).copyWith(
|
||||
quality: '24-bit/96kHz',
|
||||
bitDepth: 24,
|
||||
sampleRate: 96000,
|
||||
format: 'flac',
|
||||
);
|
||||
|
||||
final restored = DownloadHistoryItem.fromJson(item.toJson());
|
||||
|
||||
expect(restored.quality, '24-bit/96kHz');
|
||||
expect(restored.bitDepth, 24);
|
||||
expect(restored.sampleRate, 96000);
|
||||
expect(restored.format, 'flac');
|
||||
});
|
||||
|
||||
test('placeholder refresh cannot erase an existing measured quality', () {
|
||||
expect(
|
||||
resolvePersistedHistoryQuality(
|
||||
incoming: 'HI_RES_LOSSLESS',
|
||||
existing: '24-bit/96kHz',
|
||||
),
|
||||
'24-bit/96kHz',
|
||||
);
|
||||
expect(
|
||||
resolvePersistedHistoryQuality(
|
||||
incoming: '24-bit/192kHz',
|
||||
existing: '24-bit/96kHz',
|
||||
),
|
||||
'24-bit/192kHz',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('startup orphan reconciliation', () {
|
||||
@@ -130,4 +168,37 @@ void main() {
|
||||
expect(decision.pendingIds, isEmpty);
|
||||
});
|
||||
});
|
||||
|
||||
group('download completion persistence', () {
|
||||
test('publishes completion only after history persistence', () async {
|
||||
final events = <String>[];
|
||||
|
||||
await persistBeforePublishingDownloadCompletion(
|
||||
persist: () async {
|
||||
events.add('persist');
|
||||
},
|
||||
publish: () => events.add('publish'),
|
||||
);
|
||||
|
||||
expect(events, ['persist', 'publish']);
|
||||
});
|
||||
|
||||
test(
|
||||
'does not publish completion when history persistence fails',
|
||||
() async {
|
||||
var published = false;
|
||||
|
||||
await expectLater(
|
||||
persistBeforePublishingDownloadCompletion(
|
||||
persist: () =>
|
||||
Future<void>.error(StateError('database unavailable')),
|
||||
publish: () => published = true,
|
||||
),
|
||||
throwsStateError,
|
||||
);
|
||||
|
||||
expect(published, isFalse);
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user