mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-09-14 05:49:02 +02:00
fix(library): bound playlist picker SQL parameters
Deduplicate requested track keys and aggregate match counts in batches of 500 parameters. Use scalar preview lookups instead of expanding another playlist ID list. Cover large selections, duplicate keys, empty selections, and cover selection in regression tests.
This commit is contained in:
zarzet
@@ -310,116 +310,7 @@ class LibraryCollectionsDatabase {
|
||||
|
||||
Future<List<PlaylistPickerSummaryRow>> loadPlaylistPickerSummaries(
|
||||
List<String> requestedTrackKeys,
|
||||
) async {
|
||||
final db = await database;
|
||||
final uniqueTrackKeys = requestedTrackKeys
|
||||
.where((key) => key.trim().isNotEmpty)
|
||||
.toSet()
|
||||
.toList(growable: false);
|
||||
|
||||
final playlistRows = await db.rawQuery('''
|
||||
SELECT
|
||||
p.id,
|
||||
p.name,
|
||||
p.cover_image_path,
|
||||
p.created_at,
|
||||
p.updated_at,
|
||||
COUNT(pt.track_key) AS track_count
|
||||
FROM $_tablePlaylists p
|
||||
LEFT JOIN $_tablePlaylistTracks pt ON pt.playlist_id = p.id
|
||||
GROUP BY p.id
|
||||
ORDER BY p.created_at DESC, p.rowid DESC
|
||||
''');
|
||||
|
||||
final matchedCountsByPlaylistId = <String, int>{};
|
||||
if (uniqueTrackKeys.isNotEmpty) {
|
||||
final placeholders = List.filled(uniqueTrackKeys.length, '?').join(', ');
|
||||
final matchedRows = await db.rawQuery('''
|
||||
SELECT playlist_id, COUNT(*) AS matched_count
|
||||
FROM $_tablePlaylistTracks
|
||||
WHERE track_key IN ($placeholders)
|
||||
GROUP BY playlist_id
|
||||
''', uniqueTrackKeys);
|
||||
for (final row in matchedRows) {
|
||||
final playlistId = row['playlist_id']?.toString();
|
||||
if (playlistId == null || playlistId.isEmpty) continue;
|
||||
matchedCountsByPlaylistId[playlistId] =
|
||||
(row['matched_count'] as num?)?.toInt() ?? 0;
|
||||
}
|
||||
}
|
||||
|
||||
final playlistIdsNeedingPreview = playlistRows
|
||||
.where((row) {
|
||||
final coverPath = row['cover_image_path']?.toString();
|
||||
return coverPath == null || coverPath.isEmpty;
|
||||
})
|
||||
.map((row) => row['id']?.toString() ?? '')
|
||||
.where((id) => id.isNotEmpty)
|
||||
.toList(growable: false);
|
||||
|
||||
final previewCoverByPlaylistId = <String, String?>{};
|
||||
if (playlistIdsNeedingPreview.isNotEmpty) {
|
||||
final placeholders = List.filled(
|
||||
playlistIdsNeedingPreview.length,
|
||||
'?',
|
||||
).join(', ');
|
||||
final previewRows = await db.rawQuery('''
|
||||
SELECT outer_tracks.playlist_id, outer_tracks.track_json
|
||||
FROM $_tablePlaylistTracks outer_tracks
|
||||
WHERE outer_tracks.playlist_id IN ($placeholders)
|
||||
AND outer_tracks.rowid = (
|
||||
SELECT inner_tracks.rowid
|
||||
FROM $_tablePlaylistTracks inner_tracks
|
||||
WHERE inner_tracks.playlist_id = outer_tracks.playlist_id
|
||||
ORDER BY inner_tracks.added_at ASC, inner_tracks.rowid ASC
|
||||
LIMIT 1
|
||||
)
|
||||
''', playlistIdsNeedingPreview);
|
||||
|
||||
for (final row in previewRows) {
|
||||
final playlistId = row['playlist_id']?.toString();
|
||||
final trackJson = row['track_json'] as String?;
|
||||
if (playlistId == null ||
|
||||
playlistId.isEmpty ||
|
||||
trackJson == null ||
|
||||
trackJson.isEmpty) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
final decoded = jsonDecode(trackJson);
|
||||
if (decoded is! Map) continue;
|
||||
final coverUrl = decoded['coverUrl']?.toString();
|
||||
if (coverUrl != null && coverUrl.isNotEmpty) {
|
||||
previewCoverByPlaylistId[playlistId] = coverUrl;
|
||||
}
|
||||
} catch (_) {}
|
||||
}
|
||||
}
|
||||
|
||||
return playlistRows
|
||||
.map((row) {
|
||||
final id = row['id']?.toString() ?? '';
|
||||
final createdAt =
|
||||
DateTime.tryParse(row['created_at']?.toString() ?? '') ??
|
||||
DateTime.now();
|
||||
final updatedAt =
|
||||
DateTime.tryParse(row['updated_at']?.toString() ?? '') ??
|
||||
createdAt;
|
||||
return PlaylistPickerSummaryRow(
|
||||
id: id,
|
||||
name: row['name']?.toString() ?? '',
|
||||
coverImagePath: row['cover_image_path'] as String?,
|
||||
previewCover: previewCoverByPlaylistId[id],
|
||||
createdAt: createdAt,
|
||||
updatedAt: updatedAt,
|
||||
trackCount: (row['track_count'] as num?)?.toInt() ?? 0,
|
||||
containsAllRequestedTracks:
|
||||
uniqueTrackKeys.isNotEmpty &&
|
||||
matchedCountsByPlaylistId[id] == uniqueTrackKeys.length,
|
||||
);
|
||||
})
|
||||
.toList(growable: false);
|
||||
}
|
||||
) async => readPlaylistPickerSummaries(await database, requestedTrackKeys);
|
||||
|
||||
Future<void> _upsertEntry(
|
||||
String table,
|
||||
@@ -703,3 +594,92 @@ class LibraryCollectionsDatabase {
|
||||
_log.i('Restored collections from backup');
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads picker rows with bounded selection queries, preserving playlist order.
|
||||
Future<List<PlaylistPickerSummaryRow>> readPlaylistPickerSummaries(
|
||||
DatabaseExecutor db,
|
||||
List<String> requestedTrackKeys,
|
||||
) async {
|
||||
final uniqueTrackKeys = requestedTrackKeys
|
||||
.where((key) => key.trim().isNotEmpty)
|
||||
.toSet()
|
||||
.toList(growable: false);
|
||||
|
||||
final playlistRows = await db.rawQuery('''
|
||||
SELECT
|
||||
p.id,
|
||||
p.name,
|
||||
p.cover_image_path,
|
||||
p.created_at,
|
||||
p.updated_at,
|
||||
COUNT(pt.track_key) AS track_count,
|
||||
CASE WHEN p.cover_image_path IS NULL OR p.cover_image_path = '' THEN (
|
||||
SELECT preview.track_json
|
||||
FROM $_tablePlaylistTracks preview
|
||||
WHERE preview.playlist_id = p.id
|
||||
ORDER BY preview.added_at ASC, preview.rowid ASC
|
||||
LIMIT 1
|
||||
) END AS preview_track_json
|
||||
FROM $_tablePlaylists p
|
||||
LEFT JOIN $_tablePlaylistTracks pt ON pt.playlist_id = p.id
|
||||
GROUP BY p.id
|
||||
ORDER BY p.created_at DESC, p.rowid DESC
|
||||
''');
|
||||
|
||||
final matchedCountsByPlaylistId = <String, int>{};
|
||||
const chunkSize = 500;
|
||||
for (var offset = 0; offset < uniqueTrackKeys.length; offset += chunkSize) {
|
||||
final chunk = uniqueTrackKeys.sublist(
|
||||
offset,
|
||||
(offset + chunkSize).clamp(0, uniqueTrackKeys.length),
|
||||
);
|
||||
final placeholders = List.filled(chunk.length, '?').join(', ');
|
||||
final matchedRows = await db.rawQuery('''
|
||||
SELECT playlist_id, COUNT(*) AS matched_count
|
||||
FROM $_tablePlaylistTracks
|
||||
WHERE track_key IN ($placeholders)
|
||||
GROUP BY playlist_id
|
||||
''', chunk);
|
||||
for (final row in matchedRows) {
|
||||
final playlistId = row['playlist_id']?.toString();
|
||||
if (playlistId == null || playlistId.isEmpty) continue;
|
||||
matchedCountsByPlaylistId[playlistId] =
|
||||
(matchedCountsByPlaylistId[playlistId] ?? 0) +
|
||||
((row['matched_count'] as num?)?.toInt() ?? 0);
|
||||
}
|
||||
}
|
||||
|
||||
return playlistRows
|
||||
.map((row) {
|
||||
final id = row['id']?.toString() ?? '';
|
||||
final createdAt =
|
||||
DateTime.tryParse(row['created_at']?.toString() ?? '') ??
|
||||
DateTime.now();
|
||||
final updatedAt =
|
||||
DateTime.tryParse(row['updated_at']?.toString() ?? '') ?? createdAt;
|
||||
String? previewCover;
|
||||
final previewJson = row['preview_track_json'] as String?;
|
||||
if (previewJson != null && previewJson.isNotEmpty) {
|
||||
try {
|
||||
final decoded = jsonDecode(previewJson);
|
||||
if (decoded is Map) {
|
||||
final cover = decoded['coverUrl']?.toString();
|
||||
if (cover != null && cover.isNotEmpty) previewCover = cover;
|
||||
}
|
||||
} catch (_) {}
|
||||
}
|
||||
return PlaylistPickerSummaryRow(
|
||||
id: id,
|
||||
name: row['name']?.toString() ?? '',
|
||||
coverImagePath: row['cover_image_path'] as String?,
|
||||
previewCover: previewCover,
|
||||
createdAt: createdAt,
|
||||
updatedAt: updatedAt,
|
||||
trackCount: (row['track_count'] as num?)?.toInt() ?? 0,
|
||||
containsAllRequestedTracks:
|
||||
uniqueTrackKeys.isNotEmpty &&
|
||||
matchedCountsByPlaylistId[id] == uniqueTrackKeys.length,
|
||||
);
|
||||
})
|
||||
.toList(growable: false);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
import 'dart:convert';
|
||||
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:sqflite/sqflite.dart';
|
||||
import 'package:spotiflac_android/services/library_collections_database.dart';
|
||||
|
||||
// Exercises the database boundary's bounded requests and aggregation without
|
||||
// requiring a platform SQLite plugin. The production SQL is passed unchanged.
|
||||
class _PickerDatabase implements DatabaseExecutor {
|
||||
final List<Map<String, Object?>> playlistRows;
|
||||
final Map<String, Set<String>> membership;
|
||||
final batches = <List<Object?>>[];
|
||||
int summaryQueries = 0;
|
||||
|
||||
_PickerDatabase(this.playlistRows, this.membership);
|
||||
|
||||
@override
|
||||
Future<List<Map<String, Object?>>> rawQuery(
|
||||
String sql, [
|
||||
List<Object?>? arguments,
|
||||
]) async {
|
||||
if (arguments == null) {
|
||||
summaryQueries++;
|
||||
expect(sql, contains('preview_track_json'));
|
||||
return playlistRows;
|
||||
}
|
||||
// Emulate a database with a conservative parameter budget. Regressing to
|
||||
// one unbounded query must fail even on hosts with a larger SQLite limit.
|
||||
if (arguments.length > 500) throw StateError('too many SQL variables');
|
||||
expect('?'.allMatches(sql).length, arguments.length);
|
||||
batches.add(List.of(arguments));
|
||||
return [
|
||||
for (final entry in membership.entries)
|
||||
{
|
||||
'playlist_id': entry.key,
|
||||
'matched_count': arguments.where(entry.value.contains).length,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
@override
|
||||
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||
}
|
||||
|
||||
Map<String, Object?> _playlist(String id, {String? cover, String? preview}) => {
|
||||
'id': id,
|
||||
'name': id,
|
||||
'cover_image_path': cover,
|
||||
'preview_track_json': preview,
|
||||
'created_at': '2026-01-01T00:00:00Z',
|
||||
'updated_at': '2026-01-02T00:00:00Z',
|
||||
'track_count': 1205,
|
||||
};
|
||||
|
||||
void main() {
|
||||
test(
|
||||
'large selections aggregate across batches and deduplicate keys',
|
||||
() async {
|
||||
final keys = List.generate(1205, (i) => 'track-$i');
|
||||
final db = _PickerDatabase(
|
||||
[
|
||||
_playlist('partial'),
|
||||
_playlist(
|
||||
'full',
|
||||
preview: jsonEncode({'coverUrl': 'https://example.test/cover'}),
|
||||
),
|
||||
_playlist('empty'),
|
||||
],
|
||||
{'full': keys.toSet(), 'partial': keys.take(1204).toSet(), 'empty': {}},
|
||||
);
|
||||
final rows = await readPlaylistPickerSummaries(db, [
|
||||
...keys,
|
||||
...keys.take(10),
|
||||
'',
|
||||
' ',
|
||||
]);
|
||||
expect(db.batches.map((batch) => batch.length), [500, 500, 205]);
|
||||
expect(db.batches.expand((batch) => batch).toList(), keys);
|
||||
expect(db.summaryQueries, 1);
|
||||
expect(rows.map((row) => row.id), ['partial', 'full', 'empty']);
|
||||
expect(rows.map((row) => row.containsAllRequestedTracks), [
|
||||
false,
|
||||
true,
|
||||
false,
|
||||
]);
|
||||
expect(rows[1].previewCover, 'https://example.test/cover');
|
||||
expect(rows[1].trackCount, 1205);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'empty selection performs no matching query or implicit all-match',
|
||||
() async {
|
||||
final db = _PickerDatabase([_playlist('playlist')], {});
|
||||
final rows = await readPlaylistPickerSummaries(db, ['', ' ']);
|
||||
expect(db.batches, isEmpty);
|
||||
expect(rows.single.containsAllRequestedTracks, isFalse);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'many playlist previews need no parameter list and tolerate bad JSON',
|
||||
() async {
|
||||
final db = _PickerDatabase([
|
||||
for (var i = 0; i < 1200; i++) _playlist('$i'),
|
||||
_playlist('custom', cover: '/local/cover.jpg'),
|
||||
_playlist('bad', preview: '{broken'),
|
||||
_playlist('blank', preview: '{"coverUrl":""}'),
|
||||
], {});
|
||||
final rows = await readPlaylistPickerSummaries(db, []);
|
||||
expect(rows, hasLength(1203));
|
||||
expect(db.summaryQueries, 1);
|
||||
expect(db.batches, isEmpty);
|
||||
expect(rows[1200].coverImagePath, '/local/cover.jpg');
|
||||
expect(rows.map((row) => row.previewCover), everyElement(isNull));
|
||||
},
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user