mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-19 22:17:29 +02:00
fix(history): prevent maintenance from restoring orphan downloads
This commit is contained in:
1 parent
c91d12dbf0
commit
7584daad88
5 files changed
+263
-18
No files matched your search
@@ -19,6 +19,27 @@ part 'download_history_provider_maintenance.dart';
|
||||
|
||||
final _historyLog = AppLogger('DownloadHistory');
|
||||
|
||||
/// Merge a delayed maintenance result into the live lists, never its old
|
||||
/// snapshot. Deletions, new downloads, ordering and counts stay authoritative.
|
||||
DownloadHistoryState mergeHistoryMaintenanceUpdates(
|
||||
DownloadHistoryState current,
|
||||
Iterable<DownloadHistoryItem> updates,
|
||||
Set<String> persistedIds,
|
||||
) {
|
||||
final byId = {
|
||||
for (final item in updates)
|
||||
if (persistedIds.contains(item.id)) item.id: item,
|
||||
};
|
||||
if (byId.isEmpty) return current;
|
||||
return current.copyWith(
|
||||
items: [for (final item in current.items) byId[item.id] ?? item],
|
||||
lookupItems: [
|
||||
for (final item in current.lookupItems) byId[item.id] ?? item,
|
||||
],
|
||||
loadedIndexVersion: current.loadedIndexVersion + 1,
|
||||
);
|
||||
}
|
||||
|
||||
typedef StartupOrphanDecision = ({
|
||||
Set<String> confirmedIds,
|
||||
Set<String> pendingIds,
|
||||
|
||||
@@ -4,6 +4,17 @@ part of 'download_history_provider.dart';
|
||||
/// Startup maintenance: SAF repair, orphan cleanup, and audio-metadata
|
||||
/// backfill that run staggered after the initial history load.
|
||||
extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
Future<void> _commitMaintenanceUpdates(List<Map<String, dynamic>> updates) =>
|
||||
_enqueueHistoryWrite(() async {
|
||||
final persistedIds = await _db.updateExistingBatch(updates);
|
||||
if (!ref.mounted || persistedIds.isEmpty) return;
|
||||
state = mergeHistoryMaintenanceUpdates(
|
||||
state,
|
||||
updates.map(DownloadHistoryItem.fromJson),
|
||||
persistedIds,
|
||||
);
|
||||
});
|
||||
|
||||
void _scheduleStartupMaintenance(List<DownloadHistoryItem> initialItems) {
|
||||
if (_startupMaintenanceScheduled) {
|
||||
return;
|
||||
@@ -159,7 +170,6 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
return;
|
||||
}
|
||||
|
||||
final updatedItems = [...items];
|
||||
final persistedUpdates = <Map<String, dynamic>>[];
|
||||
var changed = false;
|
||||
var repairedCount = 0;
|
||||
@@ -218,7 +228,6 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
: item.safFileName,
|
||||
safRepaired: true,
|
||||
);
|
||||
updatedItems[index] = updated;
|
||||
changed = true;
|
||||
if (newUri == item.filePath) {
|
||||
verifiedCount++;
|
||||
@@ -229,12 +238,7 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
}
|
||||
|
||||
if (changed) {
|
||||
await _db.upsertBatch(persistedUpdates);
|
||||
state = state.copyWith(
|
||||
items: updatedItems,
|
||||
loadedIndexVersion: state.loadedIndexVersion + 1,
|
||||
lookupItems: _lookupItemsWithUpdates(updatedItems),
|
||||
);
|
||||
await _commitMaintenanceUpdates(persistedUpdates);
|
||||
_historyLog.i(
|
||||
'SAF repair pass: verified=$verifiedCount, repaired=$repairedCount, checked=${selectedIndexes.length}',
|
||||
);
|
||||
@@ -558,7 +562,6 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
return;
|
||||
}
|
||||
|
||||
List<DownloadHistoryItem>? updatedItems;
|
||||
final persistedUpdates = <Map<String, dynamic>>[];
|
||||
var refreshedCount = 0;
|
||||
|
||||
@@ -690,19 +693,12 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier {
|
||||
hasReplayGain: resolvedHasReplayGain,
|
||||
replayGainMetadataScanVersion: resolvedReplayGainScanVersion,
|
||||
);
|
||||
updatedItems ??= [...items];
|
||||
updatedItems[index] = updated;
|
||||
persistedUpdates.add(updated.toJson());
|
||||
refreshedCount++;
|
||||
}
|
||||
|
||||
if (persistedUpdates.isNotEmpty && updatedItems != null) {
|
||||
await _db.upsertBatch(persistedUpdates);
|
||||
state = state.copyWith(
|
||||
items: updatedItems,
|
||||
loadedIndexVersion: state.loadedIndexVersion + 1,
|
||||
lookupItems: _lookupItemsWithUpdates(updatedItems),
|
||||
);
|
||||
if (persistedUpdates.isNotEmpty) {
|
||||
await _commitMaintenanceUpdates(persistedUpdates);
|
||||
}
|
||||
|
||||
await _writeStartupCursor(
|
||||
|
||||
@@ -4,6 +4,7 @@ import 'package:sqflite/sqflite.dart';
|
||||
import 'package:path_provider/path_provider.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import 'package:spotiflac_android/services/sqlite_helpers.dart' as sqlite;
|
||||
import 'package:spotiflac_android/services/history_maintenance.dart';
|
||||
import 'package:spotiflac_android/utils/isrc_utils.dart' as isrc;
|
||||
import 'package:spotiflac_android/utils/ios_container_paths.dart';
|
||||
import 'package:spotiflac_android/utils/logger.dart';
|
||||
@@ -806,6 +807,16 @@ class HistoryDatabase {
|
||||
});
|
||||
}
|
||||
|
||||
Future<Set<String>> updateExistingBatch(
|
||||
List<Map<String, dynamic>> items,
|
||||
) async {
|
||||
if (items.isEmpty) return const {};
|
||||
final db = await database;
|
||||
return db.transaction(
|
||||
(txn) => updateExistingHistoryRows(txn, items.map(_jsonToDbRow)),
|
||||
);
|
||||
}
|
||||
|
||||
Future<List<Map<String, dynamic>>> getAll({int? limit, int? offset}) async {
|
||||
final db = await database;
|
||||
final rows = await db.query(
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import 'package:sqflite/sqflite.dart';
|
||||
import 'package:spotiflac_android/services/sqlite_helpers.dart' as sqlite;
|
||||
|
||||
/// Called inside a transaction. Delayed metadata/SAF probes must never insert
|
||||
/// rows deleted by orphan cleanup while the probe was running.
|
||||
Future<Set<String>> updateExistingHistoryRows(
|
||||
DatabaseExecutor db,
|
||||
Iterable<Map<String, dynamic>> rows,
|
||||
) async {
|
||||
final updatedIds = <String>{};
|
||||
final batch = db.batch();
|
||||
for (final row in rows) {
|
||||
final id = row['id'] as String;
|
||||
final changed = await db.update(
|
||||
'history',
|
||||
row,
|
||||
where: 'id = ?',
|
||||
whereArgs: [id],
|
||||
);
|
||||
if (changed == 0) continue;
|
||||
updatedIds.add(id);
|
||||
sqlite.putPathKeysInBatch(
|
||||
batch,
|
||||
'history_path_keys',
|
||||
id,
|
||||
row['file_path'] as String?,
|
||||
);
|
||||
}
|
||||
if (updatedIds.isNotEmpty) await batch.commit(noResult: true);
|
||||
return updatedIds;
|
||||
}
|
||||
@@ -0,0 +1,186 @@
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:sqflite/sqflite.dart';
|
||||
import 'package:spotiflac_android/providers/download_history_provider.dart';
|
||||
import 'package:spotiflac_android/services/history_maintenance.dart';
|
||||
|
||||
class _HistoryRows implements DatabaseExecutor {
|
||||
final statements = <Map<String, Object?>>[];
|
||||
final _keys = _PathKeys();
|
||||
|
||||
@override
|
||||
Future<int> update(
|
||||
String table,
|
||||
Map<String, Object?> values, {
|
||||
String? where,
|
||||
List<Object?>? whereArgs,
|
||||
ConflictAlgorithm? conflictAlgorithm,
|
||||
}) async {
|
||||
statements.add({
|
||||
'sql':
|
||||
'UPDATE $table SET ${values.keys.map((key) => '$key = ?').join(', ')} WHERE $where',
|
||||
'args': [...values.values, ...?whereArgs],
|
||||
});
|
||||
return whereArgs!.single == 'kept' ? 1 : 0;
|
||||
}
|
||||
|
||||
@override
|
||||
Batch batch() => _keys;
|
||||
|
||||
@override
|
||||
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||
}
|
||||
|
||||
class _PathKeys implements Batch {
|
||||
final statements = <Map<String, Object?>>[];
|
||||
|
||||
@override
|
||||
void delete(String table, {String? where, List<Object?>? whereArgs}) {
|
||||
statements.add({
|
||||
'sql': 'DELETE FROM $table WHERE $where',
|
||||
'args': whereArgs,
|
||||
});
|
||||
}
|
||||
|
||||
@override
|
||||
void insert(
|
||||
String table,
|
||||
Map<String, Object?> values, {
|
||||
String? nullColumnHack,
|
||||
ConflictAlgorithm? conflictAlgorithm,
|
||||
}) {
|
||||
expect(conflictAlgorithm, ConflictAlgorithm.ignore);
|
||||
statements.add({
|
||||
'sql':
|
||||
'INSERT OR IGNORE INTO $table (${values.keys.join(', ')}) VALUES (${values.keys.map((_) => '?').join(', ')})',
|
||||
'args': values.values.toList(),
|
||||
});
|
||||
}
|
||||
|
||||
@override
|
||||
Future<List<Object?>> commit({
|
||||
bool? exclusive,
|
||||
bool? noResult,
|
||||
bool? continueOnError,
|
||||
}) async => [];
|
||||
|
||||
@override
|
||||
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||
}
|
||||
|
||||
DownloadHistoryItem _track(String id) => DownloadHistoryItem(
|
||||
id: id,
|
||||
trackName: 'Song $id',
|
||||
artistName: 'Artist',
|
||||
albumName: 'Album',
|
||||
filePath: '/music/$id.flac',
|
||||
service: 'test',
|
||||
downloadedAt: DateTime.utc(2026),
|
||||
);
|
||||
|
||||
void main() {
|
||||
test(
|
||||
'late maintenance cannot recreate deleted rows or their path keys',
|
||||
() async {
|
||||
final db = _HistoryRows();
|
||||
final updated = await updateExistingHistoryRows(db, [
|
||||
{
|
||||
'id': 'removed',
|
||||
'file_path': '/music/removed.flac',
|
||||
'quality': '24-bit',
|
||||
},
|
||||
{'id': 'kept', 'file_path': '/music/kept.opus', 'quality': 'Opus'},
|
||||
]);
|
||||
expect(updated, {'kept'});
|
||||
final result = await Process.run('python3', [
|
||||
'-c',
|
||||
r'''
|
||||
import json, sqlite3, sys
|
||||
db = sqlite3.connect(':memory:')
|
||||
db.executescript("""
|
||||
CREATE TABLE history(id TEXT PRIMARY KEY, file_path TEXT, quality TEXT);
|
||||
CREATE TABLE history_path_keys(item_id TEXT, path_key TEXT, UNIQUE(item_id,path_key));
|
||||
INSERT INTO history VALUES ('removed','/music/removed.flac','16-bit'),('kept','/music/kept.flac','16-bit');
|
||||
INSERT INTO history_path_keys SELECT id,file_path FROM history;
|
||||
DELETE FROM history WHERE id='removed';
|
||||
DELETE FROM history_path_keys WHERE item_id='removed';
|
||||
""")
|
||||
with db:
|
||||
for statement in json.loads(sys.argv[1]):
|
||||
db.execute(statement['sql'], statement['args'])
|
||||
print(json.dumps({
|
||||
'rows': list(db.execute('SELECT id,file_path,quality FROM history')),
|
||||
'keys': list(db.execute('SELECT item_id,path_key FROM history_path_keys'))
|
||||
}))
|
||||
''',
|
||||
jsonEncode([...db.statements, ...db._keys.statements]),
|
||||
]);
|
||||
expect(result.exitCode, 0, reason: result.stderr.toString());
|
||||
final data = jsonDecode(result.stdout as String) as Map<String, dynamic>;
|
||||
expect(data['rows'], [
|
||||
['kept', '/music/kept.opus', 'Opus'],
|
||||
]);
|
||||
expect(
|
||||
(data['keys'] as List).every((row) => (row as List).first == 'kept'),
|
||||
isTrue,
|
||||
);
|
||||
expect(data['keys'], isNotEmpty);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'20 removed downloads stay removed when an old metadata probe finishes',
|
||||
() {
|
||||
final oldSnapshot = [for (var i = 0; i < 20; i++) _track('$i')];
|
||||
final fresh = _track('new-download');
|
||||
final current = DownloadHistoryState(
|
||||
items: [fresh],
|
||||
totalCount: 1,
|
||||
loadedIndexVersion: 7,
|
||||
);
|
||||
final result = mergeHistoryMaintenanceUpdates(current, [
|
||||
oldSnapshot.last.copyWith(quality: '24-bit'),
|
||||
], {});
|
||||
expect(identical(result, current), isTrue);
|
||||
expect(result.items.map((item) => item.id), ['new-download']);
|
||||
expect(result.lookupItems.map((item) => item.id), ['new-download']);
|
||||
expect(result.totalCount, 1);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'live ordering, new downloads and lookup-only items survive maintenance',
|
||||
() {
|
||||
final kept = _track('kept');
|
||||
final fresh = _track('fresh');
|
||||
final lookupOnly = _track('lookup');
|
||||
final current = DownloadHistoryState(
|
||||
items: [fresh, kept],
|
||||
lookupItems: [lookupOnly, fresh, kept],
|
||||
totalCount: 42,
|
||||
loadedIndexVersion: 3,
|
||||
);
|
||||
final result = mergeHistoryMaintenanceUpdates(
|
||||
current,
|
||||
[
|
||||
kept.copyWith(quality: '24-bit'),
|
||||
lookupOnly.copyWith(quality: 'Opus'),
|
||||
_track('deleted'),
|
||||
],
|
||||
{'kept', 'lookup'},
|
||||
);
|
||||
expect(result.items.map((item) => item.id), ['fresh', 'kept']);
|
||||
expect(result.items.last.quality, '24-bit');
|
||||
expect(result.lookupItems.map((item) => item.id), [
|
||||
'lookup',
|
||||
'fresh',
|
||||
'kept',
|
||||
]);
|
||||
expect(result.lookupItems.first.quality, 'Opus');
|
||||
expect(result.totalCount, 42);
|
||||
expect(result.loadedIndexVersion, 4);
|
||||
},
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user