Download synced songs' covers

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) {

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
+        }
     }
 }

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

Packages/ClementineKit/Tests/ClementineKitTests/SyncEngineTests.swift

@@ -84,15 +84,16 @@ 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 }
         for case let path as String in paths {
             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?"