Refactor album art with promises

Co-authored-by: Adam Sharp <adam@sharplet.me>

Persephone/DataSources/AlbumDataSource.swift

@@ -23,7 +23,7 @@ class AlbumDataSource: NSObject, NSCollectionViewDataSource {
     albumViewItem.setAlbum(albums[indexPath.item])
 
     if albums[indexPath.item].coverArt == nil {
-      AlbumArtService.shared.fetchAlbumArt(for: albums[indexPath.item]) { image in
+      AlbumArtService(album: albums[indexPath.item]).fetchAlbumArt { image in
         self.albums[indexPath.item].coverArt = image
 
         DispatchQueue.main.async {

Persephone/Services/AlbumArtService.swift

@@ -7,23 +7,69 @@
 //
 
 import Cocoa
+import PromiseKit
 
-class AlbumArtService: NSObject {
+class AlbumArtService {
   var preferences = Preferences()
+  let album: AlbumItem
   
   let cachedArtworkSize = 180
   let cachedArtworkQuality: CGFloat = 0.5
 
-  static var shared = AlbumArtService()
-
   var session = URLSession(configuration: .default)
-  let cacheQueue = DispatchQueue(label: "albumArtCacheQueue", attributes: .concurrent)
+  let cacheQueue = DispatchQueue(label: "albumArtCacheQueue")
 
-  func fetchAlbumArt(for album: AlbumItem, callback: @escaping (_ image: NSImage) -> Void) {
+  init(album: AlbumItem) {
-    cacheQueue.async { [unowned self] in
+    self.album = album
-      if !self.getCachedArtwork(for: album, callback: callback) {
+  }
-        self.getArtworkFromFilesystem(for: album, callback: callback)
+
+  func fetchAlbumArt(callback: @escaping (_ image: NSImage?) -> Void) {
+    cacheQueue.async {
+      firstly {
+        self.getCachedArtwork()
+      }.then { artwork -> Promise<NSImage?> in
+        artwork.map(Promise.value) ?? self.cacheIfNecessary(self.getArtworkFromFilesystem())
+      }.then { artwork -> Promise<NSImage?> in
+        artwork.map(Promise.value) ?? self.cacheIfNecessary(self.getArtworkFromMusicBrainz().map(Optional.some))
+      }.tap { result in
+        switch result {
+        case .fulfilled(nil), .rejected(MusicBrainzError.noArtworkAvailable):
+          self.cacheArtwork(data: Data())
+        default:
+          break
+        }
+      }.recover { error in
+        .value(nil)
+      }.done(callback)
+    }
+  }
+
+  func cacheIfNecessary(_ promise: Promise<NSImage?>) -> Promise<NSImage?> {
+    return promise.get { image in
+      if let data = image?.jpegData(compressionQuality: self.cachedArtworkQuality) {
+        self.cacheArtwork(data: data)
       }
     }
   }
 }
+
+
+//getCachedArtwork
+//  .then {
+//    callback($0)
+//  }
+//  .catch {
+//    getFileSystemArtwork
+//  }
+//  .then {
+//    callback($0)
+//  }
+//  .catch {
+//    getRemoteArtwork
+//  }4
+//  .then {
+//    callback($0)
+//  }
+//
+//// [() -> Promise<NSImage?>]
+//// () -> Promise<NSImage>

Persephone/Services/Extensions/AlbumArtService+Caching.swift

@@ -7,30 +7,22 @@
 //
 
 import Cocoa
+import PromiseKit
 
 extension AlbumArtService {
-  func getCachedArtwork(for album: AlbumItem, callback: @escaping (_ image: NSImage) -> Void) -> Bool {
+  static let cacheDir = try! FileManager.default.url(for: .cachesDirectory, in: .userDomainMask, appropriateFor: nil, create: true).appendingPathComponent(Bundle.main.bundleIdentifier!)
-    guard let bundleIdentifier = Bundle.main.bundleIdentifier,
-      let cacheDir = try? FileManager.default.url(for: .cachesDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
-        .appendingPathComponent(bundleIdentifier)
-      else { return false }
 
-    let cacheFilePath = cacheDir.appendingPathComponent(album.hash).path
+  func getCachedArtwork() -> Promise<NSImage?> {
+    return Promise { seal in
+      let cacheFilePath = AlbumArtService.cacheDir.appendingPathComponent(album.hash).path
+      let data = FileManager.default.contents(atPath: cacheFilePath)
+      let image = data.flatMap(NSImage.init(data:))
 
-    if FileManager.default.fileExists(atPath: cacheFilePath) {
+      seal.fulfill(image)
-      guard let data = FileManager.default.contents(atPath: cacheFilePath),
-        let image = NSImage(data: data)
-        else { return true }
-
-      callback(image)
-
-      return true
-    } else {
-      return false
     }
   }
 
-  func cacheArtwork(for album: AlbumItem, data: Data?) {
+  func cacheArtwork(data: Data?) {
     guard let bundleIdentifier = Bundle.main.bundleIdentifier,
       let cacheDir = try? FileManager.default.url(for: .cachesDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
         .appendingPathComponent(bundleIdentifier)

Persephone/Services/Extensions/AlbumArtService+Filesystem.swift

@@ -7,56 +7,36 @@
 //
 
 import Cocoa
+import PromiseKit
 
 extension AlbumArtService {
-  func getArtworkFromFilesystem(
+  func getArtworkFromFilesystem() -> Promise<NSImage?> {
-    for album: AlbumItem,
-    callback: @escaping (_ image: NSImage) -> Void
-  ) {
-    var tryImage: NSImage?
-
     let coverArtFilenames = [
       "folder.jpg",
       "cover.jpg",
       "\(album.artist) - \(album.title).jpg"
     ]
 
-    let callback = { (_ albumURI: String?) in
+    return getAlbumURI().map { albumURI in
-      guard let albumURI = albumURI
+        let musicDir = self.preferences.expandedMpdLibraryDir
-        else { return }
 
-      let musicDir = self.preferences.expandedMpdLibraryDir
+        return coverArtFilenames
-      let fullAlbumURI = "\(musicDir)/\(albumURI)"
+          .lazy
-
+          .map { "\(musicDir)/\($0)" }
-      for coverArtFilename in coverArtFilenames {
+          .compactMap(self.tryImage)
-        let coverArtURI = "\(fullAlbumURI)/\(coverArtFilename)"
+          .first
-
-        tryImage = self.tryImage(coverArtURI)
-
-        if let image = tryImage {
-          self.cacheArtwork(
-            for: album,
-            data: image.jpegData(compressionQuality: self.cachedArtworkQuality)
-          )
-          callback(image)
-          break
-        }
       }
+  }
 
-      if tryImage == nil && self.preferences.fetchMissingArtworkFromInternet {
+  func getAlbumURI() -> Promise<String> {
-        self.getRemoteArtwork(for: album, callback: callback)
+    return Promise { seal in
-      }
+      AppDelegate.mpdClient.getAlbumURI(for: album.album, callback: seal.fulfill)
     }
-
+      .compactMap { $0 }
-    AppDelegate.mpdClient.getAlbumURI(
-      for: album.album,
-      callback: callback
-    )
   }
 
   func tryImage(_ filePath: String) -> NSImage? {
-    guard FileManager.default.fileExists(atPath: filePath),
+    guard let data = FileManager.default.contents(atPath: filePath),
-      let data = FileManager.default.contents(atPath: filePath),
       let image = NSImage(data: data)
       else { return nil }
 

Persephone/Services/Extensions/AlbumArtService+Remote.swift

@@ -12,24 +12,25 @@ import PromiseKit
 import PMKFoundation
 
 extension AlbumArtService {
-  func getRemoteArtwork(for album: AlbumItem, callback: @escaping (_ image: NSImage) -> Void) {
+  enum MusicBrainzError: Error {
-    let albumArtWorkItem = DispatchWorkItem() {
+    case noArtworkAvailable
-      self.getArtworkFromMusicBrainz(for: album, callback: callback)
-    }
-
-    AlbumArtQueue.shared.addToQueue(workItem: albumArtWorkItem)
   }
 
-  func getArtworkFromMusicBrainz(for album: AlbumItem, callback: @escaping (_ image: NSImage) -> Void) {
+  func getRemoteArtwork() -> Promise<NSImage> {
-    guard var urlComponents = URLComponents(string: "https://musicbrainz.org/ws/2/release/")
+    return Promise { seal in
-      else { return }
+      let albumArtWorkItem = DispatchWorkItem {
+        self.getArtworkFromMusicBrainz().pipe(to: seal.resolve)
+      }
 
-    urlComponents.query = "query=artist:\(album.artist) AND release:\(album.title) AND country:US&limit=1&fmt=json"
+      AlbumArtQueue.shared.addToQueue(workItem: albumArtWorkItem)
+    }
+  }
 
-    guard let searchURL = urlComponents.url
+  func getArtworkFromMusicBrainz() -> Promise<NSImage> {
-      else { return }
+    var search = URLComponents(string: "https://musicbrainz.org/ws/2/release/")!
+    search.query = "query=artist:\(album.artist) AND release:\(album.title) AND country:US&limit=1&fmt=json"
 
-    URLSession.shared.dataTask(.promise, with: searchURL).validate()
+    return URLSession.shared.dataTask(.promise, with: search.url!).validate()
       .compactMap {
         JSON($0.data)
       }.compactMap {
@@ -43,25 +44,11 @@ extension AlbumArtService {
         NSImage(data: $0.data)?.toFitBox(
           size: NSSize(width: self.cachedArtworkSize, height: self.cachedArtworkSize)
         )
-      }.compactMap {
+      }.recover { error -> Promise<NSImage> in
-        self.cacheArtwork(
+        if case PMKHTTPError.badStatusCode(404, _, _) = error {
-          for: album,
+          throw MusicBrainzError.noArtworkAvailable
-          data: $0.jpegData(compressionQuality: self.cachedArtworkQuality)
+        } else {
-        )
+          throw error
-        return $0
-      }.done {
-        callback($0)
-      }.catch {
-        if let httpError = $0 as? PMKHTTPError {
-          switch httpError {
-          case let .badStatusCode(statusCode, _, _):
-            switch statusCode {
-            case 404:
-              self.cacheArtwork(for: album, data: Data())
-            default:
-              self.getRemoteArtwork(for: album, callback: callback)
-            }
-          }
         }
       }
   }