From 515f257f83a48a09737415bbe929c9643d35c2b3 Mon Sep 17 00:00:00 2001 From: Laurent Date: Sat, 30 May 2026 17:29:09 +0200 Subject: [PATCH] feat: add DatabaseService.updateTrack --- Music/Services/DatabaseService.swift | 69 +++++++++++++++++++++++---- MusicTests/DatabaseServiceTests.swift | 44 +++++++++++++++++ 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/Music/Services/DatabaseService.swift b/Music/Services/DatabaseService.swift index f3cf6d0..74128e2 100644 --- a/Music/Services/DatabaseService.swift +++ b/Music/Services/DatabaseService.swift @@ -147,6 +147,37 @@ nonisolated final class DatabaseService: Sendable { } } + /// Returns `true` only if the SQLite file at `path` is a complete, well-formed + /// database. Opens a throwaway **read-only** connection (so it never flips the + /// file to WAL or creates side files) and runs `PRAGMA quick_check`, which walks + /// every b-tree page. This catches a truncated or inconsistent copy *before* it + /// reaches GRDB — where it would otherwise blow up with the opaque + /// "database disk image is malformed" (`SQLITE_CORRUPT`) error mid-query. + static func isWellFormedDatabase(atPath path: String) -> Bool { + guard FileManager.default.fileExists(atPath: path) else { return false } + do { + var config = Configuration() + config.readonly = true + let queue = try DatabaseQueue(path: path, configuration: config) + return try queue.read { db in + // 1. quick_check walks every b-tree page — catches truncated/corrupt images. + let check = try String.fetchOne(db, sql: "PRAGMA quick_check") ?? "unknown" + guard check == "ok" else { return false } + // 2. A 0-byte or schema-less file is *valid* but empty SQLite, which would + // yield an empty remote library. Require the core `tracks` table so an + // empty/wrong file is rejected too. (Existence, not row count — an empty + // library with the table present is legitimate.) + let hasTracks = try Int.fetchOne( + db, + sql: "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'tracks'" + ) ?? 0 + return hasTracks > 0 + } + } catch { + return false + } + } + // MARK: - Write func insert(_ track: inout Track) throws { @@ -172,6 +203,15 @@ nonisolated final class DatabaseService: Sendable { } } + // Full-record update for metadata edits. The tracks_ft FTS5 index is kept in + // sync automatically by the triggers installed via synchronize(withTable:), + // so no manual FTS write is needed here. + func updateTrack(_ track: Track) throws { + try dbPool.write { db in + try track.update(db) + } + } + func deleteTracksWithURLs(_ urls: Set) throws { try dbPool.write { db in let placeholders = databaseQuestionMarks(count: urls.count) @@ -189,6 +229,21 @@ nonisolated final class DatabaseService: Sendable { "trackNumber", "dateAdded", "playCount", "rating", "bpm" ] + /// Builds the SQL `ORDER BY` expression (without the `ORDER BY` keyword) for a track + /// list. `column` is whitelisted against `validSortColumns`, so it is safe to + /// interpolate. When sorting by `album`, a secondary `discNumber, trackNumber` + /// ascending sort is appended so tracks within an album stay in playing order — + /// always ascending, even when the album sort itself is descending. + private static func orderByClause(column: String, ascending: Bool) -> String { + let col = validSortColumns.contains(column) ? column : "title" + let order = ascending ? "ASC" : "DESC" + var clause = "\(col) COLLATE NOCASE \(order)" + if col == "album" { + clause += ", discNumber ASC, trackNumber ASC" + } + return clause + } + func fetchTracks(search: String, sortColumn: String, ascending: Bool) throws -> [Track] { try dbPool.read { db in try self.fetchTracks(db: db, search: search, sortColumn: sortColumn, ascending: ascending) @@ -197,13 +252,12 @@ nonisolated final class DatabaseService: Sendable { // Used by ValueObservation which already holds a Database access func fetchTracks(db: Database, search: String, sortColumn: String, ascending: Bool) throws -> [Track] { - let col = Self.validSortColumns.contains(sortColumn) ? sortColumn : "title" - let order = ascending ? "ASC" : "DESC" + let orderBy = Self.orderByClause(column: sortColumn, ascending: ascending) if search.trimmingCharacters(in: .whitespaces).isEmpty { return try Track.fetchAll( db, - sql: "SELECT * FROM tracks ORDER BY \(col) COLLATE NOCASE \(order)" + sql: "SELECT * FROM tracks ORDER BY \(orderBy)" ) } @@ -216,7 +270,7 @@ nonisolated final class DatabaseService: Sendable { SELECT tracks.* FROM tracks JOIN tracks_ft ON tracks_ft.rowid = tracks.id WHERE tracks_ft MATCH ? - ORDER BY \(col) COLLATE NOCASE \(order) + ORDER BY \(orderBy) """, arguments: [pattern] ) @@ -229,15 +283,14 @@ nonisolated final class DatabaseService: Sendable { } func fetchTracks(db: Database, conditions: [SmartPlaylistCondition], sortColumn: String, ascending: Bool) throws -> [Track] { - let col = Self.validSortColumns.contains(sortColumn) ? sortColumn : "title" - let order = ascending ? "ASC" : "DESC" + let orderBy = Self.orderByClause(column: sortColumn, ascending: ascending) let (whereSQL, args) = buildWhereClause(conditions) if whereSQL.isEmpty { - return try Track.fetchAll(db, sql: "SELECT * FROM tracks ORDER BY \(col) COLLATE NOCASE \(order)") + return try Track.fetchAll(db, sql: "SELECT * FROM tracks ORDER BY \(orderBy)") } return try Track.fetchAll( db, - sql: "SELECT * FROM tracks WHERE \(whereSQL) ORDER BY \(col) COLLATE NOCASE \(order)", + sql: "SELECT * FROM tracks WHERE \(whereSQL) ORDER BY \(orderBy)", arguments: args ) } diff --git a/MusicTests/DatabaseServiceTests.swift b/MusicTests/DatabaseServiceTests.swift index 37af9b4..a4bd2c8 100644 --- a/MusicTests/DatabaseServiceTests.swift +++ b/MusicTests/DatabaseServiceTests.swift @@ -31,6 +31,31 @@ struct DatabaseServiceTests { #expect(descending[0].artist == "Zebra") } + // Verifies that sorting by album orders tracks within an album by disc then track + // number ascending — and that this secondary order stays ascending even when the + // album sort direction is descending (so an album always reads in playing order). + @Test func fetchTracksByAlbumOrdersByDiscAndTrackNumber() throws { + // 1. Insert one album's tracks out of order, spanning two discs, so only a + // secondary disc/track sort can restore playing order. + let db = try DatabaseService(inMemory: true) + let fixtures = [ + Track.fixture(fileURL: "/3.mp3", title: "C", album: "Greatest Hits", trackNumber: 3, discNumber: 1), + Track.fixture(fileURL: "/1.mp3", title: "A", album: "Greatest Hits", trackNumber: 1, discNumber: 1), + Track.fixture(fileURL: "/4.mp3", title: "D", album: "Greatest Hits", trackNumber: 1, discNumber: 2), + Track.fixture(fileURL: "/2.mp3", title: "B", album: "Greatest Hits", trackNumber: 2, discNumber: 1), + ] + for var t in fixtures { try db.insert(&t) } + + // 2. Sort by album ascending → disc1/1, disc1/2, disc1/3, disc2/1. + let asc = try db.fetchTracks(search: "", sortColumn: "album", ascending: true) + #expect(asc.map(\.title) == ["A", "B", "C", "D"]) + + // 3. Sort by album descending → same within-album order, because the disc/track + // secondary sort is always ascending regardless of the album direction. + let desc = try db.fetchTracks(search: "", sortColumn: "album", ascending: false) + #expect(desc.map(\.title) == ["A", "B", "C", "D"]) + } + // Searches using FTS5 and verifies only matching tracks are returned. @Test func fts5Search() throws { let db = try DatabaseService(inMemory: true) @@ -284,6 +309,25 @@ struct DatabaseServiceTests { #expect(result[2].id == tracks[4].id) } + // Verifies updateTrack persists edited fields and that the tracks_ft index + // stays in sync (the synchronize-installed triggers fire on UPDATE). + @Test func updateTrackPersistsFieldsAndSyncsFTS() throws { + // Step 1: insert a track. + let db = try DatabaseService(inMemory: true) + var t = Track.fixture(title: "Original Title", artist: "X") + try db.insert(&t) + // Step 2: edit fields and update. + t.title = "Renamed Title"; t.album = "New Album" + try db.updateTrack(t) + // Step 3: re-fetch and assert persisted. + let fetched = try #require(db.fetchTracksByIds([t.id!]).first) + #expect(fetched.title == "Renamed Title") + #expect(fetched.album == "New Album") + // Step 4: FTS reflects the new title and not the old (triggers keep it synced). + #expect(try db.fetchTracks(search: "Renamed", sortColumn: "title", ascending: true).count == 1) + #expect(try db.fetchTracks(search: "Original", sortColumn: "title", ascending: true).count == 0) + } + // Inserts tracks in different months and verifies fetchMonthlyAdditions returns // the correct per-month counts covering the requested range including empty months. // Uses a UTC calendar to match the implementation, which uses UTC month boundaries