diff --git a/Packages/ClementineKit/Sources/ClementineKit/Sync/PhonePlayer.swift b/Packages/ClementineKit/Sources/ClementineKit/Sync/PhonePlayer.swift index db0f51d..086dd2f 100644 --- a/Packages/ClementineKit/Sources/ClementineKit/Sync/PhonePlayer.swift +++ b/Packages/ClementineKit/Sources/ClementineKit/Sync/PhonePlayer.swift @@ -11,6 +11,9 @@ public struct KeptSong: Sendable, Hashable, Identifiable { /// As Clementine writes it, such as "5:36"; empty if it didn't say. public var length: String public var file: URL + /// The cover Clementine sent with the song, if it did. Otherwise the cover is the one in + /// [file], if it has one. + public var artFile: URL? public var id: String { itemKey } @@ -24,6 +27,7 @@ public struct KeptSong: Sendable, Hashable, Identifiable { album = song.album length = song.prettyLength self.file = file + artFile = item.artKey.flatMap(SyncFiles.artPath(key:)).map { folder.appending(path: $0) } } /// [song], downloaded to this phone, to play with what's kept; its file is its key. @@ -50,7 +54,7 @@ public final class PhonePlayer { public private(set) var isShuffled = false /// Nothing (.off), all the songs (.playlist) or the one playing (.track). public private(set) var repeatMode = RepeatMode.off - /// The cover in the song's file, once read, if it has one. + /// The song's cover, once read, if it has one. public private(set) var artwork: Data? @ObservationIgnored public var onUpdate: (() -> Void)? @@ -247,20 +251,29 @@ public final class PhonePlayer { onUpdate?() } - /// Reads the cover in the playing song's file, when the song changes. + /// Reads the playing song's cover, when the song changes: the one Clementine sent, or else the + /// one in its file. private func readArtwork() { guard song?.itemKey != artworkFor else { return } artworkFor = song?.itemKey artwork = nil guard let key = artworkFor, let file = song?.file else { return } + let artFile = song?.artFile Task { - let data = await Self.artwork(in: file) + let data = await Self.artwork(artFile, orIn: file) guard artworkFor == key, let data else { return } artwork = data onUpdate?() } } + nonisolated private static func artwork(_ artFile: URL?, orIn file: URL) async -> Data? { + if let artFile, let data = try? Data(contentsOf: artFile), !data.isEmpty { + return data + } + return await artwork(in: file) + } + nonisolated private static func artwork(in file: URL) async -> Data? { guard let metadata = try? await AVURLAsset(url: file).load(.commonMetadata) else { return nil } for item in AVMetadataItem.metadataItems(from: metadata, filteredByIdentifier: .commonIdentifierArtwork) { diff --git a/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncEngine.swift b/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncEngine.swift index 3bfc2a5..2ba9593 100644 --- a/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncEngine.swift +++ b/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncEngine.swift @@ -32,9 +32,9 @@ public enum RuleResult: Sendable, Equatable { } /// Brings what's on the phone in line with the rules for one Clementine: asks for each rule's -/// manifest, downloads the songs that are new or changed into `folder`, and deletes the songs that -/// no rule wants any more. A song in several rules is downloaded once. Covers and conversions come -/// later. +/// manifest, downloads the songs that are new or changed into `folder`, with their covers, and +/// deletes the songs that no rule wants any more. A song in several rules is downloaded once, and a +/// cover many songs share is too. Conversions come later. /// /// `supportedFormats` are the types the phone plays, as for streaming (`Renderer.formats`). /// `progress` hears how many of a rule's songs to download have been, before the first and after @@ -90,6 +90,10 @@ public struct SyncEngine: Sendable { try await store.removeItem(serverID: serverID, itemKey: item.itemKey) removed += 1 } + for art in try await store.unusedArt() { + try? FileManager.default.removeItem(at: folder.appending(path: art.path)) + try await store.removeArt(key: art.artKey) + } return removed } @@ -117,6 +121,10 @@ public struct SyncEngine: Sendable { try await download(item, serverID: rule.serverID, replacing: have, over: connection) downloaded += 1 await progress(rule.id, SyncProgress(done: downloaded, total: total)) + } else if var have, have.artKey != (item.hasArt ? item.art.key : nil) { + // The same song with another cover, or one kept before covers were. + have.artKey = await art(item, over: connection) + try await store.putItem(have) } keys.append(item.itemKey) } @@ -189,7 +197,35 @@ public struct SyncEngine: Sendable { SyncedItem( serverID: serverID, itemKey: item.itemKey, version: item.version, path: path, mimeType: item.mimeType, sizeBytes: item.sizeBytes, delivery: item.delivery, - artKey: nil, metadata: try song.serializedData(), pinned: have?.pinned ?? false, - added: have?.added ?? now())) + artKey: await art(item, over: connection), metadata: try song.serializedData(), + pinned: have?.pinned ?? false, added: have?.added ?? now())) + } + + /// The key of `item`'s cover, downloading it if the phone doesn't have it yet; nil if it has + /// none, or it couldn't be downloaded, so it's tried again next time. A song without a cover + /// of its own from Clementine shows the one in its file, if any. + private func art(_ item: Pb_Remote_SyncItem, over connection: some SyncConnection) async -> String? { + guard item.hasArt, let path = SyncFiles.artPath(key: item.art.key) else { return nil } + let key = item.art.key + if (try? await store.art(key: key)) != nil { return key } + + let file = folder.appending(path: path) + let files = FileManager.default + let partial = file.deletingLastPathComponent().appending(path: ".\(UUID().uuidString).part") + do { + try files.createDirectory(at: file.deletingLastPathComponent(), withIntermediateDirectories: true) + try await connection.download(item.art.url, to: partial) + if files.fileExists(atPath: file.path) { + _ = try files.replaceItemAt(file, withItemAt: partial) + } else { + try files.moveItem(at: partial, to: file) + } + let size = (try? file.resourceValues(forKeys: [.fileSizeKey]))?.fileSize ?? 0 + try await store.putArt(SyncedArt(artKey: key, path: path, sizeBytes: Int64(size))) + return key + } catch { + try? files.removeItem(at: partial) + return nil + } } } diff --git a/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncFiles.swift b/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncFiles.swift index 5578b05..80f7ee8 100644 --- a/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncFiles.swift +++ b/Packages/ClementineKit/Sources/ClementineKit/Sync/SyncFiles.swift @@ -11,6 +11,14 @@ enum SyncFiles { return part(artist, otherwise: "Unknown artist") + "/" + part(song.album, otherwise: "Unknown album") + "/" } + /// Where the cover with `key` goes, relative to the synced songs' folder: hidden, since it's + /// the app's, not one of the songs. Nil for a key that isn't a hash, so it can't name a file + /// somewhere else. + static func artPath(key: String) -> String? { + guard !key.isEmpty, key.count <= 128, key.allSatisfy(\.isHexDigit) else { return nil } + return ".covers/" + key.lowercased() + } + static func fileName(_ song: SongMetadata, mimeType: String) -> String { let title = part(song.title, otherwise: "Untitled") let name = song.track > 0 ? (song.track < 10 ? "0" : "") + String(song.track) + " " + title : title diff --git a/Packages/ClementineKit/Tests/ClementineKitTests/SyncEngineTests.swift b/Packages/ClementineKit/Tests/ClementineKitTests/SyncEngineTests.swift index 2682539..d997a49 100644 --- a/Packages/ClementineKit/Tests/ClementineKitTests/SyncEngineTests.swift +++ b/Packages/ClementineKit/Tests/ClementineKitTests/SyncEngineTests.swift @@ -84,7 +84,8 @@ struct SyncEngineTests { return item } - /// The files in the synced songs' folder, by path, leaving out hidden ones. + /// The files in the synced songs' folder, by path, leaving out hidden ones and those in hidden + /// folders. func files() throws -> [String: String] { var files: [String: String] = [:] guard let paths = FileManager.default.enumerator(atPath: folder.path) else { return files } @@ -92,7 +93,7 @@ struct SyncEngineTests { let url = folder.appending(path: path) var isDirectory: ObjCBool = false guard FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory), !isDirectory.boolValue, - !url.lastPathComponent.hasPrefix(".") else { continue } + !path.split(separator: "/").contains(where: { $0.hasPrefix(".") }) else { continue } files[path] = try String(decoding: Data(contentsOf: url), as: UTF8.self) } return files @@ -308,6 +309,91 @@ struct SyncEngineTests { #expect(await clementine.opened == ["/d/token/c-1"]) } + /// `item` with the cover `key`, which Clementine serves. + func withArt(_ item: Pb_Remote_SyncItem, _ key: String) async -> Pb_Remote_SyncItem { + var item = item + item.art.key = key + item.art.url = "/a/token/\(key)" + await clementine.put(item.art.url, Data("cover \(key)".utf8)) + return item + } + + func cover(_ key: String) throws -> String { + try String(decoding: Data(contentsOf: folder.appending(path: ".covers/\(key)")), as: UTF8.self) + } + + @Test func downloadsEachCoverOnce() async throws { + _ = try await rule("smart/library/0b6e") + await clementine.serve( + "smart/library/0b6e", + [await withArt(await song("a"), "c0ffee"), await withArt(await song("b", title: "Tame"), "c0ffee"), + await song("c", title: "Hey")]) + _ = try await engine.sync(clementine) + + #expect(await clementine.opened.filter { $0.hasPrefix("/a/") } == ["/a/token/c0ffee"]) + #expect(try await store.item(serverID: Self.server, itemKey: "a")?.artKey == "c0ffee") + #expect(try await store.item(serverID: Self.server, itemKey: "b")?.artKey == "c0ffee") + #expect(try await store.item(serverID: Self.server, itemKey: "c")?.artKey == nil) + #expect(try await store.art(key: "c0ffee")?.path == ".covers/c0ffee") + #expect(try cover("c0ffee") == "cover c0ffee") + // Hidden: they're the app's, not songs. + #expect(try files().count == 3) + + let kept = KeptSong(try #require(await store.item(serverID: Self.server, itemKey: "a")), in: folder) + #expect(kept.artFile == folder.appending(path: ".covers/c0ffee")) + } + + @Test func songsKeptBeforeTheirCoverGetIt() async throws { + _ = try await rule("playlist/1") + await clementine.serve("playlist/1", [await song("a")]) + _ = try await engine.sync(clementine) + + // The same song, now with a cover: only the cover's downloaded. + await clementine.serve("playlist/1", [await withArt(await song("a"), "beef")]) + await clementine.clearOpened() + let result = try await engine.sync(clementine) + #expect(await clementine.opened == ["/a/token/beef"]) + #expect(result.rules.values.first == .synced(songs: 1, downloaded: 0, skipped: 0)) + #expect(try await store.item(serverID: Self.server, itemKey: "a")?.artKey == "beef") + } + + @Test func aCoverThatCantBeDownloadedIsTriedAgain() async throws { + _ = try await rule("playlist/1") + let item = await withArt(await song("a"), "beef") + await clementine.remove(item.art.url) + await clementine.serve("playlist/1", [item]) + let result = try await engine.sync(clementine) + // The song's kept all the same. + #expect(result.rules.values.first == .synced(songs: 1, downloaded: 1, skipped: 0)) + #expect(try await store.item(serverID: Self.server, itemKey: "a")?.artKey == nil) + + await clementine.put(item.art.url, Data("cover beef".utf8)) + _ = try await engine.sync(clementine) + #expect(try await store.item(serverID: Self.server, itemKey: "a")?.artKey == "beef") + } + + @Test func coversNoSongUsesAreDeleted() async throws { + _ = try await rule("playlist/1") + await clementine.serve( + "playlist/1", [await withArt(await song("a"), "beef"), await withArt(await song("b", title: "Tame"), "f00d")]) + _ = try await engine.sync(clementine) + + // One song goes, and the other's cover changes. + await clementine.serve("playlist/1", [await withArt(await song("b", title: "Tame"), "cafe")]) + _ = try await engine.sync(clementine) + #expect(try await store.art(key: "beef") == nil) + #expect(try await store.art(key: "f00d") == nil) + #expect(try cover("cafe") == "cover cafe") + #expect(!FileManager.default.fileExists(atPath: folder.appending(path: ".covers/beef").path)) + #expect(!FileManager.default.fileExists(atPath: folder.appending(path: ".covers/f00d").path)) + } + + @Test func coverKeysMustBeHashes() { + #expect(SyncFiles.artPath(key: "C0FFEE") == ".covers/c0ffee") + #expect(SyncFiles.artPath(key: "../songs") == nil) + #expect(SyncFiles.artPath(key: "") == nil) + } + @Test func namesFilesForPeople() { var song = SongMetadata() song.title = "What/Ever?"