Refactor UserSetting property wrapper to support Codable (#814)

* Add UserPreference property wrapper

* Delete UserSetting property wrapper

* Add PlistRepresentable support

* Add more convenience inits

* Add documentation

* Add UTs

* Fix flaky tests

* Fix more flaky tests

* Fix double optional problem in UserPreference

* Refine UTs

* Delete cache in UserPreference

* Conform Date to PlistRepresentable

ElementX/Sources/Application/AppSettings.swift

@@ -26,7 +26,7 @@ final class AppSettings: ObservableObject {
         case enableInAppNotifications
         case pusherProfileTag
         case shouldCollapseRoomStateEvents
-        case startChatFlowEnabled = "showStartChatFlow"
+        case startChatFlowEnabled
         case startChatUserSuggestionsEnabled
         case mediaUploadingFlowEnabled
         case invitesFlowEnabled
@@ -66,7 +66,7 @@ final class AppSettings: ObservableObject {
     /// The last known version of the app that was launched on this device, which is
     /// used to detect when migrations should be run. When `nil` the app may have been
     /// deleted between runs so should clear data in the shared container and keychain.
-    @UserSetting(key: UserDefaultsKeys.lastVersionLaunched.rawValue, defaultValue: nil, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.lastVersionLaunched, storageType: .userDefaults(store))
     var lastVersionLaunched: String?
     
     /// The default homeserver address used. This is intentionally a string without a scheme
@@ -126,24 +126,24 @@ final class AppSettings: ObservableObject {
     }
     
     /// `true` when the user has opted in to send analytics.
-    @UserSetting(key: UserDefaultsKeys.enableAnalytics.rawValue, defaultValue: false, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.enableAnalytics, defaultValue: false, storageType: .userDefaults(store))
     var enableAnalytics
         
     // MARK: - Room Screen
     
-    @UserSettingRawRepresentable(key: UserDefaultsKeys.timelineStyle.rawValue, defaultValue: TimelineStyle.bubbles, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.timelineStyle, defaultValue: TimelineStyle.bubbles, storageType: .userDefaults(store))
     var timelineStyle
     
-    @UserSetting(key: UserDefaultsKeys.shouldCollapseRoomStateEvents.rawValue, defaultValue: true, persistIn: nil)
+    @UserPreference(key: UserDefaultsKeys.shouldCollapseRoomStateEvents, defaultValue: true, storageType: .volatile)
     var shouldCollapseRoomStateEvents
     
     // MARK: - Notifications
 
-    @UserSetting(key: UserDefaultsKeys.timelineStyle.rawValue, defaultValue: true, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.timelineStyle, defaultValue: true, storageType: .userDefaults(store))
     var enableInAppNotifications
 
     /// Tag describing which set of device specific rules a pusher executes.
-    @UserSetting(key: UserDefaultsKeys.pusherProfileTag.rawValue, defaultValue: nil, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.pusherProfileTag, storageType: .userDefaults(store))
     var pusherProfileTag: String?
         
     // MARK: - Other
@@ -154,19 +154,19 @@ final class AppSettings: ObservableObject {
     
     // MARK: Start Chat
     
-    @UserSetting(key: UserDefaultsKeys.startChatFlowEnabled.rawValue, defaultValue: false, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.startChatFlowEnabled, defaultValue: false, storageType: .userDefaults(store))
     var startChatFlowEnabled
     
-    @UserSetting(key: UserDefaultsKeys.startChatUserSuggestionsEnabled.rawValue, defaultValue: false, persistIn: nil)
+    @UserPreference(key: UserDefaultsKeys.startChatUserSuggestionsEnabled, defaultValue: false, storageType: .volatile)
     var startChatUserSuggestionsEnabled
     
     // MARK: Media Uploading
     
-    @UserSetting(key: UserDefaultsKeys.mediaUploadingFlowEnabled.rawValue, defaultValue: false, persistIn: nil)
+    @UserPreference(key: UserDefaultsKeys.mediaUploadingFlowEnabled, defaultValue: false, storageType: .volatile)
     var mediaUploadingFlowEnabled
     
     // MARK: Invites
     
-    @UserSetting(key: UserDefaultsKeys.invitesFlowEnabled.rawValue, defaultValue: false, persistIn: store)
+    @UserPreference(key: UserDefaultsKeys.invitesFlowEnabled, defaultValue: false, storageType: .userDefaults(store))
     var invitesFlowEnabled
 }

ElementX/Sources/Other/UserPreference.swift

@@ -0,0 +1,194 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Combine
+import Foundation
+
+/// Property wrapper that allows to store data into a keyed storage.
+/// It also exposes a Combine publisher for listening to value changes.
+/// The publisher isn't supposed to skip consecutive duplicates if any,
+/// there is no concept of Equatable at this level.
+@propertyWrapper
+final class UserPreference<T: Codable> {
+    private let key: String
+    private var keyedStorage: any KeyedStorage<T>
+    private let defaultValue: T
+    private let subject: PassthroughSubject<T, Never> = .init()
+    
+    init(key: String, defaultValue: T, keyedStorage: any KeyedStorage<T>) {
+        self.key = key
+        self.defaultValue = defaultValue
+        self.keyedStorage = keyedStorage
+    }
+    
+    var wrappedValue: T {
+        get {
+            keyedStorage[key] ?? defaultValue
+        }
+        set {
+            keyedStorage[key] = newValue
+            subject.send(wrappedValue)
+        }
+    }
+    
+    var projectedValue: AnyPublisher<T, Never> {
+        subject
+            .prepend(wrappedValue)
+            .eraseToAnyPublisher()
+    }
+}
+
+// MARK: - UserPreference convenience initializers
+
+extension UserPreference {
+    enum StorageType {
+        case userDefaults(UserDefaults = .standard)
+        case volatile
+    }
+    
+    convenience init(key: String, defaultValue: T, storageType: StorageType) {
+        let storage: any KeyedStorage<T>
+        
+        switch storageType {
+        case .userDefaults(let userDefaults):
+            storage = UserDefaultsStorage(userDefaults: userDefaults)
+        case .volatile:
+            storage = [String: T]()
+        }
+        
+        self.init(key: key, defaultValue: defaultValue, keyedStorage: storage)
+    }
+    
+    convenience init<R: RawRepresentable>(key: R, defaultValue: T, storageType: StorageType) where R.RawValue == String {
+        self.init(key: key.rawValue, defaultValue: defaultValue, storageType: storageType)
+    }
+    
+    convenience init(key: String, storageType: StorageType) where T: ExpressibleByNilLiteral {
+        self.init(key: key, defaultValue: nil, storageType: storageType)
+    }
+    
+    convenience init<R: RawRepresentable>(key: R, storageType: StorageType) where R: RawRepresentable, R.RawValue == String, T: ExpressibleByNilLiteral {
+        self.init(key: key.rawValue, storageType: storageType)
+    }
+}
+
+// MARK: - Storage
+
+protocol KeyedStorage<Value> {
+    associatedtype Value: Codable
+    
+    subscript(key: String) -> Value? { get set }
+}
+
+/// An implementation of KeyedStorage on the UserDefaults.
+///
+/// When used with a `Value` that conforms to `PlistRepresentable` the Codable encode/decode
+/// phase is skipped, and values are stored natively in the plist.
+final class UserDefaultsStorage<Value: Codable>: KeyedStorage {
+    private let userDefaults: UserDefaults
+    
+    init(userDefaults: UserDefaults) {
+        self.userDefaults = userDefaults
+    }
+    
+    subscript(key: String) -> Value? {
+        get {
+            let value: Value?
+            if Value.self is PlistRepresentable.Type {
+                value = decodePlistRepresentableValue(for: key)
+            } else {
+                value = decodeValue(for: key)
+            }
+            
+            return value
+        }
+        set {
+            if Value.self is PlistRepresentable.Type {
+                encodePlistRepresentable(value: newValue, for: key)
+            } else {
+                encode(value: newValue, for: key)
+            }
+        }
+    }
+    
+    private func decodeValue(for key: String) -> Value? {
+        userDefaults
+            .data(forKey: key)
+            .flatMap {
+                try? JSONDecoder().decode(Value.self, from: $0)
+            }
+    }
+    
+    private func decodePlistRepresentableValue(for key: String) -> Value? {
+        userDefaults.object(forKey: key) as? Value
+    }
+    
+    private func encode(value: Value?, for key: String) {
+        // Detects correctly double optionals like this: String?? = .some(nil)
+        if value.isNil {
+            userDefaults.removeObject(forKey: key)
+        } else {
+            let encodedValue = try? JSONEncoder().encode(value)
+            userDefaults.setValue(encodedValue, forKey: key)
+        }
+    }
+    
+    private func encodePlistRepresentable(value: Value?, for key: String) {
+        // Detects correctly double optionals like this: String?? = .some(nil)
+        if value.isNil {
+            userDefaults.removeObject(forKey: key)
+        } else {
+            userDefaults.set(value, forKey: key)
+        }
+    }
+}
+
+private protocol Nullable {
+    var isNil: Bool { get }
+}
+
+extension Optional: Nullable {
+    var isNil: Bool {
+        switch self {
+        case .none:
+            return true
+        case .some(let nullable as Nullable):
+            return nullable.isNil
+        case .some:
+            return false
+        }
+    }
+}
+
+extension Dictionary: KeyedStorage where Key == String, Value: Codable { }
+
+// MARK: - PlistRepresentable
+
+/// A protocol to mark types as being plist compliant.
+/// UserDefaultsStorage uses this protocol to avoid to encode/decode with Codable plist compliant values.
+protocol PlistRepresentable { }
+
+extension Bool: PlistRepresentable { }
+extension String: PlistRepresentable { }
+extension Int: PlistRepresentable { }
+extension Float: PlistRepresentable { }
+extension Double: PlistRepresentable { }
+extension Date: PlistRepresentable { }
+extension Data: PlistRepresentable { }
+
+extension Array: PlistRepresentable where Element: PlistRepresentable { }
+extension Dictionary: PlistRepresentable where Key == String, Value: PlistRepresentable { }
+extension Optional: PlistRepresentable where Wrapped: PlistRepresentable { }

ElementX/Sources/Other/UserSettingPropertyWrapper.swift

@@ -1,139 +0,0 @@
-//
-// Copyright 2022 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-// Taken from https://www.swiftbysundell.com/articles/property-wrappers-in-swift/
-
-import Combine
-import Foundation
-
-/// Property wrapper that allows transparent access to user defaults and exposes
-/// a combine publisher for listening to value changes
-///
-/// Please use `UserSettingRawRepresentable` for storing RawRepresentable values
-@propertyWrapper
-struct UserSetting<Value: Equatable> {
-    private let key: String
-    private var defaultValue: Value
-    private let storage: UserDefaults?
-    private let publisher: CurrentValueSubject<Value, Never>
-    
-    init(key: String, defaultValue: Value, persistIn storage: UserDefaults?) {
-        self.key = key
-        self.defaultValue = defaultValue
-        self.storage = storage
-        
-        let value = storage?.value(forKey: key) as? Value ?? defaultValue
-        publisher = CurrentValueSubject<Value, Never>(value)
-    }
-    
-    var wrappedValue: Value {
-        get {
-            let value = storage?.value(forKey: key) as? Value
-            return value ?? defaultValue
-        }
-        set {
-            guard let storage else {
-                defaultValue = newValue
-                publisher.send(defaultValue)
-                return
-            }
-            
-            if let optional = newValue as? AnyOptional, optional.isNil {
-                storage.removeObject(forKey: key)
-                publisher.send(defaultValue)
-            } else {
-                storage.setValue(newValue, forKey: key)
-                publisher.send(newValue)
-            }
-        }
-    }
-    
-    var projectedValue: AnyPublisher<Value, Never> {
-        publisher.removeDuplicates(by: { $0 == $1 }).eraseToAnyPublisher()
-    }
-}
-
-extension UserSetting where Value: ExpressibleByNilLiteral {
-    init(key: String, persistIn storage: UserDefaults?) {
-        self.init(key: key, defaultValue: nil, persistIn: storage)
-    }
-}
-
-/// Property wrapper that allows transparent access to user defaults for RawRepresentable types
-/// and exposes a combine publisher for listening to value changes
-///
-/// Tried extending UserSetting with RawRepresentable conformance but in that case the non-restricted
-/// method takes precedence. Decided to go with with the simple solution instead of fighting the system
-@propertyWrapper
-struct UserSettingRawRepresentable<Value: RawRepresentable & Equatable> {
-    private let key: String
-    private var defaultValue: Value
-    private let storage: UserDefaults?
-    private let publisher: CurrentValueSubject<Value, Never>
-    
-    init(key: String, defaultValue: Value, persistIn storage: UserDefaults?) {
-        self.key = key
-        self.defaultValue = defaultValue
-        self.storage = storage
-        
-        let value = (storage?.value(forKey: key) as? Value.RawValue).flatMap { Value(rawValue: $0) } ?? defaultValue
-        publisher = CurrentValueSubject<Value, Never>(value)
-    }
-    
-    var wrappedValue: Value {
-        get {
-            guard let value = storage?.value(forKey: key) as? Value.RawValue else {
-                return defaultValue
-            }
-            
-            return Value(rawValue: value) ?? defaultValue
-        }
-        set {
-            guard let storage else {
-                defaultValue = newValue
-                publisher.send(defaultValue)
-                return
-            }
-            
-            if let optional = newValue as? AnyOptional, optional.isNil {
-                storage.removeObject(forKey: key)
-                publisher.send(newValue)
-            } else {
-                storage.setValue(newValue.rawValue, forKey: key)
-                publisher.send(newValue)
-            }
-        }
-    }
-    
-    var projectedValue: AnyPublisher<Value, Never> {
-        publisher.removeDuplicates(by: { $0 == $1 }).eraseToAnyPublisher()
-    }
-}
-
-extension UserSettingRawRepresentable where Value: ExpressibleByNilLiteral {
-    init(key: String, persistIn storage: UserDefaults?) {
-        self.init(key: key, defaultValue: nil, persistIn: storage)
-    }
-}
-
-// Casting to AnyOptional will fail for any types that are not Optional (below)
-private protocol AnyOptional {
-    var isNil: Bool { get }
-}
-
-extension Optional: AnyOptional {
-    var isNil: Bool { self == nil }
-}

ElementX/Sources/Screens/RoomScreen/View/Style/TimelineStyle.swift

@@ -17,7 +17,7 @@
 import Foundation
 import SwiftUI
 
-enum TimelineStyle: String, CaseIterable {
+enum TimelineStyle: String, CaseIterable, Codable {
     case plain
     case bubbles
 

UnitTests/Sources/InvitesViewModelTests.swift

@@ -34,14 +34,14 @@ class InvitesViewModelTests: XCTestCase {
 
     func testEmptyState() async throws {
         setupViewModel()
-        _ = await context.nextViewState()
+        _ = await context.$viewState.values.first(where: { $0.invites != nil })
         let invites = try XCTUnwrap(context.viewState.invites)
         XCTAssertTrue(invites.isEmpty)
     }
     
     func testListState() async throws {
         setupViewModel(roomSummaries: .mockInvites)
-        _ = await context.nextViewState()
+        _ = await context.$viewState.values.first(where: { $0.invites != nil })
         let invites = try XCTUnwrap(context.viewState.invites)
         XCTAssertEqual(invites.count, 2)
     }

UnitTests/Sources/RoomDetailsViewModelTests.swift

@@ -111,7 +111,7 @@ class RoomDetailsScreenViewModelTests: XCTestCase {
     func testIgnoreFailure() async throws {
         let recipient = RoomMemberProxyMock.mockDan
         recipient.ignoreUserClosure = {
-            try? await Task.sleep(for: .milliseconds(10))
+            try? await Task.sleep(for: .milliseconds(100))
             return .failure(.ignoreUserFailed)
         }
         let mockedMembers: [RoomMemberProxyMock] = [.mockMe, recipient]
@@ -132,7 +132,7 @@ class RoomDetailsScreenViewModelTests: XCTestCase {
     func testUnignoreSuccess() async throws {
         let recipient = RoomMemberProxyMock.mockIgnored
         recipient.unignoreUserClosure = {
-            try? await Task.sleep(for: .milliseconds(10))
+            try? await Task.sleep(for: .milliseconds(100))
             return .success(())
         }
         let mockedMembers: [RoomMemberProxyMock] = [.mockMe, recipient]

UnitTests/Sources/UserPreferenceTests.swift

@@ -0,0 +1,168 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+@testable import ElementX
+import Foundation
+import XCTest
+
+final class UserPreferenceTests: XCTestCase {
+    override func setUpWithError() throws {
+        UserDefaults.testDefaults.removeVolatileDomain(forName: .userDefaultsSuiteName)
+    }
+
+    func testStorePlistValue() throws {
+        let setPreference = {
+            let value = TestPreferences()
+            value.plist = "Hello"
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        
+        XCTAssertEqual(value.plist, "Hello")
+        XCTAssertNotNil(UserDefaults.testDefaults.string(forKey: .key2), "Hello")
+        XCTAssertNil(UserDefaults.testDefaults.data(forKey: .key2), "Hello")
+    }
+    
+    func testStoreCodableValue() throws {
+        let storedType = CodableTestType(a: "some", b: [1, 2, 3])
+        
+        let setPreference = {
+            let value = TestPreferences()
+            value.codable = storedType
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        
+        XCTAssertEqual(value.codable, storedType)
+        XCTAssertNotNil(UserDefaults.testDefaults.data(forKey: .key3))
+    }
+    
+    func testStorePlistValueOnVolatileStorage() throws {
+        let setPreference = {
+            let value = TestPreferences()
+            value.volatileVar = "Hello"
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        
+        XCTAssertNil(value.volatileVar)
+    }
+    
+    func testStoreCodableValueOnVolatileStorage() throws {
+        let storedType = CodableTestType(a: "some", b: [1, 2, 3])
+        
+        let setPreference = {
+            let value = TestPreferences()
+            value.volatileCodable = storedType
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        
+        XCTAssertNil(value.volatileCodable)
+        XCTAssertNil(UserDefaults.testDefaults.data(forKey: .key4))
+    }
+    
+    func testStorePlistArray() throws {
+        let setPreference = {
+            let value = TestPreferences()
+            value.plistArray = [1, 2, 3]
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        
+        XCTAssertEqual(value.plistArray, [1, 2, 3])
+        XCTAssertEqual(UserDefaults.testDefaults.array(forKey: .key5) as? [Int], [1, 2, 3])
+        XCTAssertNil(UserDefaults.testDefaults.data(forKey: .key5), "Hello")
+    }
+    
+    func testAssignNilToPlistType() throws {
+        let setPreference = {
+            let value = TestPreferences()
+            value.plist = "Hello"
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        value.plist = nil
+        
+        XCTAssertNil(value.plist)
+        XCTAssertNil(UserDefaults.testDefaults.string(forKey: .key2))
+    }
+    
+    func testAssignNilToCodableType() throws {
+        let storedType = CodableTestType(a: "some", b: [1, 2, 3])
+        
+        let setPreference = {
+            let value = TestPreferences()
+            value.codable = storedType
+        }
+        
+        setPreference()
+        
+        let value = TestPreferences()
+        value.codable = nil
+
+        XCTAssertNil(value.codable)
+        XCTAssertNil(UserDefaults.testDefaults.data(forKey: .key3))
+    }
+}
+
+private struct TestPreferences {
+    @UserPreference(key: .key1, storageType: .volatile)
+    var volatileVar: String?
+    
+    @UserPreference(key: .key2, storageType: .userDefaults(.testDefaults))
+    var plist: String?
+    
+    @UserPreference(key: .key3, storageType: .userDefaults(.testDefaults))
+    var codable: CodableTestType?
+    
+    @UserPreference(key: .key4, storageType: .volatile)
+    var volatileCodable: CodableTestType?
+    
+    @UserPreference(key: .key5, storageType: .userDefaults(.testDefaults))
+    var plistArray: [Int]?
+}
+
+private struct CodableTestType: Equatable, Codable {
+    let a: String
+    let b: [Int]
+}
+
+private extension String {
+    static let key1 = "foo.volatile"
+    static let key2 = "foo.plist"
+    static let key3 = "foo.codable"
+    static let key4 = "foo.volatile.codable"
+    static let key5 = "foo.plist.array"
+    static let userDefaultsSuiteName = "io.element.elementx.unitests"
+}
+
+private extension UserDefaults {
+    // swiftlint:disable:next force_unwrapping
+    static let testDefaults = UserDefaults(suiteName: .userDefaultsSuiteName)!
+}