a2be433d8dcd

cache downloaded wallpapers persistently instead of deleting them right after NSWorkspace reads them

time
2026-07-16 23:11 UTC · 2 months ago
machine
koh2
agent
claude-sonnet-5
session
fix wallpaper set regression and download cache
parent
b11e22357970
4 files Sources/Haven/Managers/WallpaperManager.swift+57 −1Sources/Haven/UI/AppState.swift+4 −2Tests/HavenTests/AppStateTests.swift+17 −0Tests/HavenTests/ManagerTests.swift+95 −0
Sources/Haven/Managers/WallpaperManager.swift+57 −1
@@ -3,6 +3,7 @@33 @MainActor44 protocol WallpaperManaging {55   func download(from url: URL) async throws -> URL6+  func cachedLocalFile(for url: URL) async throws -> URL67   func setDesktopImage(from fileURL: URL, screen: NSScreen?) throws78   func saveToFolder(from sourceURL: URL, to folderURL: URL) throws89 }@@ -23,13 +24,16 @@2324   private let desktopSetter: @MainActor (URL, NSScreen) throws -> Void2425   private let mainScreen: () -> NSScreen?2526   private let firstScreen: () -> NSScreen?27+  private let cacheDirectoryProvider: @MainActor () -> URL?2628 2729   init(2830     session: URLSession = .shared,2931     desktopSetter: (@MainActor (URL, NSScreen) throws -> Void)? = nil,3032     workspace: DesktopImageSetting = NSWorkspace.shared,3133     mainScreen: @escaping () -> NSScreen? = { NSScreen.main },32-    firstScreen: @escaping () -> NSScreen? = { NSScreen.screens.first }34+    firstScreen: @escaping () -> NSScreen? = { NSScreen.screens.first },35+    cacheDirectoryProvider: @escaping @MainActor () -> URL? = WallpaperManager36+      .defaultCacheDirectory3337   ) {3438     self.session = session3539     self.desktopSetter =@@ -40,8 +44,17 @@4044       }4145     self.mainScreen = mainScreen4246     self.firstScreen = firstScreen47+    self.cacheDirectoryProvider = cacheDirectoryProvider4348   }4449 50+  private static func defaultCacheDirectory() -> URL? {51+    guard52+      let base = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first53+    else { return nil }54+    let bundleID = Bundle.main.bundleIdentifier ?? "Haven"55+    return base.appendingPathComponent(bundleID).appendingPathComponent("WallpaperCache")56+  }57+4558   /// Downloads the file to a temporary location and returns the local URL.4659   ///4760   /// Each call writes to a fresh, unique temp filename so that distinct@@ -60,6 +73,49 @@6073     let fileURL = Self.makeTempURL(for: url)6174     try data.write(to: fileURL)6275     return fileURL76+  }77+78+  /// Returns a persistent, on-disk copy of `url`, downloading it only if no79+  /// cached copy already exists.80+  ///81+  /// Setting the desktop picture via `NSWorkspace.setDesktopImageURL` does82+  /// not copy the image data — AppKit re-reads the file from the path we83+  /// hand it (e.g. on Space switches, wake, or Dock restarts), so the source84+  /// file must keep living at that path. Deleting it right after the call85+  /// returns (as a "clean up my temp file" pass once did) raced AppKit's own86+  /// read and made wallpapers silently fail to apply. Caching under a stable,87+  /// content-addressed filename fixes that permanently and, as a side88+  /// benefit, means re-setting a wallpaper you've already applied doesn't89+  /// re-download it.90+  func cachedLocalFile(for url: URL) async throws -> URL {91+    guard let cacheDirectory = cacheDirectoryProvider() else {92+      return try await download(from: url)93+    }94+    let destination = cacheDirectory.appendingPathComponent(Self.cacheFilename(for: url))95+    if FileManager.default.fileExists(atPath: destination.path) {96+      return destination97+    }98+99+    let downloadedURL = try await download(from: url)100+    try FileManager.default.createDirectory(101+      at: cacheDirectory, withIntermediateDirectories: true)102+    if FileManager.default.fileExists(atPath: destination.path) {103+      try? FileManager.default.removeItem(at: destination)104+    }105+    try FileManager.default.moveItem(at: downloadedURL, to: destination)106+    return destination107+  }108+109+  /// Stable, deterministic cache filename for `url`. Wallhaven's direct110+  /// image URLs already carry a unique per-wallpaper filename111+  /// (`wallhaven-<id>.<ext>`), so reusing `lastPathComponent` is enough to112+  /// dedupe; the percent-encoded fallback only matters for malformed/local113+  /// URLs without a path component.114+  static func cacheFilename(for url: URL) -> String {115+    let name = url.lastPathComponent116+    if !name.isEmpty { return name }117+    return url.absoluteString.addingPercentEncoding(withAllowedCharacters: .alphanumerics)118+      ?? "wallpaper"63119   }64120 65121   /// Returns a temp URL unique to this call. The original `url.pathExtension`

view file

Sources/Haven/UI/AppState.swift+4 −2
@@ -322,8 +322,10 @@322322       return323323     }324324     do {325-      let localURL = try await wallpaperManager.download(from: remoteURL)326-      defer { try? removeTempFile(localURL) }325+      // Cached rather than a throwaway temp download: NSWorkspace re-reads326+      // this file from disk on Space switches/wake, and repeated clicks on327+      // the same wallpaper should reuse it instead of re-downloading.328+      let localURL = try await wallpaperManager.cachedLocalFile(for: remoteURL)327329 328330       // AppState is @MainActor isolated; the prior MainActor.run hop was329331       // redundant because the call site already runs on the main actor.

view file

Tests/HavenTests/AppStateTests.swift+17 −0
@@ -71,8 +71,10 @@7171   var error: Error?7272   var downloadedURL = URL(fileURLWithPath: "/tmp/downloaded.jpg")7373   var downloads: [URL] = []74+  var cacheLookups: [URL] = []7475   var desktopURLs: [URL] = []7576   var saves: [(URL, URL)] = []77+  private var cached: Set<URL> = []7678 7779   func download(from url: URL) async throws -> URL {7880     if let error { throw error }@@ -80,6 +82,18 @@8082     return downloadedURL8183   }8284 85+  /// Mirrors `WallpaperManager.cachedLocalFile`: only calls through to86+  /// `download(from:)` the first time a given URL is requested.87+  func cachedLocalFile(for url: URL) async throws -> URL {88+    cacheLookups.append(url)89+    if cached.contains(url) {90+      return downloadedURL91+    }92+    let result = try await download(from: url)93+    cached.insert(url)94+    return result95+  }96+8397   func setDesktopImage(from fileURL: URL, screen: NSScreen?) throws {8498     if let error { throw error }8599     desktopURLs.append(fileURL)@@ -225,6 +239,9 @@225239     state.saveToFolderEnabled = true226240     await state.setWallpaper(wallpaper)227241     #expect(wallpaperManager.saves.count == 1)242+    // Same wallpaper set twice: only the first call should hit the network.243+    #expect(wallpaperManager.cacheLookups.count == 2)244+    #expect(wallpaperManager.downloads.count == 1)228245 229246     await state.downloadWallpaper(wallpaper)230247     #expect(wallpaperManager.saves.count == 2)

view file

Tests/HavenTests/ManagerTests.swift+95 −0
@@ -33,6 +33,30 @@3333   return URLSession(configuration: configuration)3434 }3535 36+/// Wraps `ManagerURLProtocol`'s canned responses with a per-request counter,37+/// so cache tests can assert a repeat request never touched the network.38+private final class CountingURLProtocol: URLProtocol, @unchecked Sendable {39+  nonisolated(unsafe) static var onLoad: (() -> Void)?40+  override class func canInit(with request: URLRequest) -> Bool { true }41+  override class func canonicalRequest(for request: URLRequest) -> URLRequest { request }42+  override func startLoading() {43+    Self.onLoad?()44+    let response = HTTPURLResponse(45+      url: request.url!, statusCode: 200, httpVersion: nil, headerFields: nil)!46+    client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)47+    client?.urlProtocol(self, didLoad: Data("wallpaper".utf8))48+    client?.urlProtocolDidFinishLoading(self)49+  }50+  override func stopLoading() {}51+}52+53+private func countingSession(onLoad: @escaping () -> Void) -> URLSession {54+  CountingURLProtocol.onLoad = onLoad55+  let configuration = URLSessionConfiguration.ephemeral56+  configuration.protocolClasses = [CountingURLProtocol.self]57+  return URLSession(configuration: configuration)58+}59+3660 private let onePixelPNG = Data(3761   base64Encoded:3862     "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII="@@ -291,6 +315,77 @@291315 292316     let errorURL = URL(string: "https://example.com/fail-http.jpg")!293317     await #expect(throws: WallhavenError.self) { try await manager.download(from: errorURL) }318+  }319+320+  @Test("Cached local file persists on disk and is reused without re-downloading")321+  func cachedLocalFile() async throws {322+    var downloadCount = 0323+    let cacheDir = FileManager.default.temporaryDirectory.appendingPathComponent(324+      UUID().uuidString)325+    defer { try? FileManager.default.removeItem(at: cacheDir) }326+    let manager = WallpaperManager(327+      session: countingSession { downloadCount += 1 },328+      cacheDirectoryProvider: { cacheDir })329+    let url = URL(string: "https://example.com/wallhaven-abc123.jpg")!330+331+    let first = try await manager.cachedLocalFile(for: url)332+    #expect(downloadCount == 1)333+    #expect(first.deletingLastPathComponent().path == cacheDir.path)334+    #expect(try Data(contentsOf: first) == Data("wallpaper".utf8))335+336+    // Same URL again: served from disk, no second network hit, and the337+    // cached file is never deleted out from under a caller (unlike a plain338+    // temp download, which a caller may clean up right after use).339+    let second = try await manager.cachedLocalFile(for: url)340+    #expect(downloadCount == 1)341+    #expect(second == first)342+    #expect(FileManager.default.fileExists(atPath: first.path))343+  }344+345+  @Test("Default cache directory lives under Library/Caches and is reused across instances")346+  func defaultCacheDirectoryIsUnderLibraryCaches() async throws {347+    var downloadCount = 0348+    let url = URL(string: "https://example.com/wallhaven-\(UUID().uuidString).jpg")!349+    let manager = WallpaperManager(session: countingSession { downloadCount += 1 })350+    defer {351+      if let cachesRoot = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)352+        .first353+      {354+        let bundleID = Bundle.main.bundleIdentifier ?? "Haven"355+        try? FileManager.default.removeItem(356+          at: cachesRoot.appendingPathComponent(bundleID).appendingPathComponent(357+            "WallpaperCache"358+          ).appendingPathComponent(url.lastPathComponent))359+      }360+    }361+362+    let first = try await manager.cachedLocalFile(for: url)363+    #expect(downloadCount == 1)364+    #expect(try Data(contentsOf: first) == Data("wallpaper".utf8))365+366+    // A second manager built with the same default provider finds the same367+    // file on disk instead of re-downloading.368+    let anotherManager = WallpaperManager(session: countingSession { downloadCount += 1 })369+    let second = try await anotherManager.cachedLocalFile(for: url)370+    #expect(downloadCount == 1)371+    #expect(second == first)372+  }373+374+  @Test("A caching directory that is unavailable falls back to a plain temp download")375+  func cacheDirectoryUnavailableFallsBackToPlainDownload() async throws {376+    var downloadCount = 0377+    let manager = WallpaperManager(378+      session: countingSession { downloadCount += 1 }, cacheDirectoryProvider: { nil })379+    let url = URL(string: "https://example.com/wallhaven-abc999.jpg")!380+381+    let first = try await manager.cachedLocalFile(for: url)382+    defer { try? FileManager.default.removeItem(at: first) }383+    #expect(downloadCount == 1)384+385+    // No persistent cache directory available: every call re-downloads.386+    let second = try await manager.cachedLocalFile(for: url)387+    defer { try? FileManager.default.removeItem(at: second) }388+    #expect(downloadCount == 2)294389   }295390 296391   @Test("Desktop setter receives explicit and default screens")

view file