Fix exception when using AppStorageKey with URL Value (#3098)

pointfreeco · May 16, 2024 · 452155f · 452155f
1 parent 3a77664
commit 452155f
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 2 deletions.
diff --git a/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift b/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift
@@ -193,7 +193,7 @@ public struct AppStorageKey<Value> {
 
   public init(_ key: String) where Value == URL {
     @Dependency(\.defaultAppStorage) var store
-    self.lookup = CastableLookup()
+    self.lookup = URLLookup()
     self.key = key
     self.store = store
   }
@@ -251,7 +251,7 @@ public struct AppStorageKey<Value> {
 
   public init(_ key: String) where Value == URL? {
     @Dependency(\.defaultAppStorage) var store
-    self.lookup = OptionalLookup(base: CastableLookup())
+    self.lookup = OptionalLookup(base: URLLookup())
     self.key = key
     self.store = store
   }
@@ -384,6 +384,30 @@ private struct CastableLookup<Value>: Lookup {
   }
 }
 
+/// Lookup implementation tuned for URL values.
+/// For URLs, dedicated UserDefaults APIs for getting/setting need to be called that convert the URL from/to Data.
+/// Calling setValue with a URL causes a NSInvalidArgumentException exception.
+private struct URLLookup: Lookup {
+  typealias Value = URL
+
+  func loadValue(from store: UserDefaults, at key: String, default defaultValue: URL?) -> URL? {
+    guard let value = store.url(forKey: key)
+    else {
+      SharedAppStorageLocals.$isSetting.withValue(true) {
+        store.set(defaultValue, forKey: key)
+      }
+      return defaultValue
+    }
+    return value
+  }
+
+  func saveValue(_ newValue: URL, to store: UserDefaults, at key: String) {
+    SharedAppStorageLocals.$isSetting.withValue(true) {
+      store.set(newValue, forKey: key)
+    }
+  }
+}
+
 private struct RawRepresentableLookup<Value: RawRepresentable, Base: Lookup>: Lookup
 where Value.RawValue == Base.Value {
   let base: Base

diff --git a/Tests/ComposableArchitectureTests/AppStorageTests.swift b/Tests/ComposableArchitectureTests/AppStorageTests.swift
@@ -24,6 +24,33 @@ final class AppStorageTests: XCTestCase {
     XCTAssertEqual(defaults.integer(forKey: "count"), 43)
   }
 
+  func testDefaultsReadURL() {
+    @Dependency(\.defaultAppStorage) var defaults
+    defaults.set(URL(string: "https://pointfree.co"), forKey: "url")
+    @Shared(.appStorage("url")) var url: URL?
+    XCTAssertEqual(url, URL(string: "https://pointfree.co"))
+  }
+
+  func testDefaultsRegistered_URL() {
+    @Dependency(\.defaultAppStorage) var defaults
+    @Shared(.appStorage("url")) var url: URL = URL(string: "https://pointfree.co")!
+    XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://pointfree.co")!)
+
+    url = URL(string: "https://example.com")!
+    XCTAssertEqual(url, URL(string: "https://example.com")!)
+    XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://example.com")!)
+  }
+
+  func testDefaultsRegistered_Optional_URL() {
+    @Dependency(\.defaultAppStorage) var defaults
+    @Shared(.appStorage("url")) var url: URL? = URL(string: "https://pointfree.co")
+    XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://pointfree.co"))
+
+    url = URL(string: "https://example.com")
+    XCTAssertEqual(url, URL(string: "https://example.com"))
+    XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://example.com"))
+  }
+
   func testDefaultsRegistered_Optional() {
     @Dependency(\.defaultAppStorage) var defaults
     @Shared(.appStorage("data")) var data: Data?