sunshinejr / SwiftyUserDefaults

Modern Swift API for NSUserDefaults
http://radex.io/swift/nsuserdefaults/static
MIT License
4.85k stars 366 forks source link

Version 4.0 migration issues #185

Closed fredpi closed 5 years ago

fredpi commented 5 years ago

I have issues converting to version 4.0 without data loss. This probably relates to me having used SwiftyUserDefaults in a weird way before 🙃

For version 3, I have the following extension in my code:

extension UserDefaults {
    subscript(key: DefaultsKey<Data?>) -> Data? {
        get { return unarchive(key) }
        set { archive(key, newValue) }
    }
}

To decode into a type, I use the following code in version 3:

if let myModelData = Defaults[.myModel], // DefaultsKey.myModel is of type DefaultsKey<Data?>
   let myModel = try? JSONDecoder().decode(MyModel.self, from: myModelData)
{
    // ...
}

For version 4, I tried to use a DefaultsKey<MyModel?> directly, but Defaults[.myModel] returns nil instead of a MyModel instance. Am I doing something wrong? Or is there no possible way to migrate from my weird use of version 3 SwiftyUserDefaults to a proper use of version 4 SwiftyUserDefaults without data loss? Thanks in advance!

sunshinejr commented 5 years ago

Hey @fredpi! First of all thank you very much for testing the new version even though it's a beta. I've been working on it a long time and any feedback on the current state of the library is priceless.

Coming back to your issue: from what you've written yeah, it seems like your usage is quite unusual, but since version 3 had many (mostly Swift-based) limitations I don't blame you at all.

What I think is happening here is that you were archiving your already-converted Data to Data again (double-archiving basically). Thus, it can't unarchive to given model as it needs to do it twice.

Good news is that you probably can make it work with version 4. In the newest version we use something called "bridges" to save or fetch values from UserDefaults. For all codable-represented types there is a DefaultsCodableBridge that does the transition:

public final class DefaultsCodableBridge<T: Codable>: DefaultsBridge<T> {

    public override func save(key: String, value: T?, userDefaults: UserDefaults) {
        guard let value = value else {
            userDefaults.removeObject(forKey: key)
            return
        }
        userDefaults.set(encodable: value, forKey: key)
    }

    public override func get(key: String, userDefaults: UserDefaults) -> T? {
        guard let data = userDefaults.data(forKey: key) else {
            return nil
        }
        return deserialize(data)
    }

    public override func isSerialized() -> Bool {
        return true
    }

    public override func deserialize(_ object: Any) -> T? {
        guard let data = object as? Data else { return nil }

        return try? JSONDecoder().decode(T.self, from: data)
    }
}

You can customize these bridges in your types however you want, e.g.:

struct MyModel: DefaultsSerializable {

    static var _defaults: DefaultsBridge<MyModel> { return DefaultsCustomBridge() }
    static var _defaultsArray: DefaultsBridge<[MyModel]> { return DefaultsCustomBridge() }

    let name: String
}

So now we would need to make our own custom bridge that would double deserialize your Data into a custom codable-represented model:

public final class DefaultsArchivedCodableBridge<T: Codable>: DefaultsBridge<T> {

    public override func save(key: String, value: T?, userDefaults: UserDefaults) {
        guard let value = value else {
            userDefaults.removeObject(forKey: key)
            return
        }
        userDefaults.set(encodable: value, forKey: key)
    }

    public override func get(key: String, userDefaults: UserDefaults) -> T? {
        guard let data = userDefaults.data(forKey: key) else {
            return nil
        }
        // Let's try deserialize to Codable first
        if let codable = deserialize(data) {
            return codable
        } else if let unarchivedData = NSKeyedUnarchiver.unarchiveObject(with: data) {
            // in case it didn't succeed, let's unarchive the Data again and then deserialize
            return deserialize(unarchivedData)
        } else {
            // okay now I don't know the reason it failed, you would need to debug what's happening here
            return nil
        }
    }

    public override func isSerialized() -> Bool {
        return true
    }

    public override func deserialize(_ object: Any) -> T? {
        guard let data = object as? Data else { return nil }

        return try? JSONDecoder().decode(T.self, from: data)
    }
}

This bridge would not only get back your Data when it's double-serialized, but it would also save it correctly the next time and fetch it correctly the next time again. At least in theory, didn't test it really 😅

So you'd need to use this new bridge in your model now:

struct MyModel: DefaultsSerializable {

    static var _defaults: DefaultsBridge<MyModel> { return DefaultsArchivedCodableBridge() }
    static var _defaultsArray: DefaultsBridge<[MyModel]> { return DefaultsArchivedCodableBridge() }

    let name: String
}

Oh that's a lot of text! Please let me know if this makes sense and if it helped.

fredpi commented 5 years ago

Hey @sunshinejr, thanks for this fanatastic support!!! I'm quite overwhelmed to receive such valuable ideas in almost no time. 🚀🚀🚀

You pointed me into the right direction, so I could create the following bridge that's almost identical to what you proposed:

final class UserDefaultsv3v4CompatibleBridge<T: Codable>: DefaultsBridge<T> {
    override func save(key: String, value: T?, userDefaults: UserDefaults) {
        guard let value = value else { return userDefaults.removeObject(forKey: key) }

        let encoder = JSONEncoder()
        if let data = try? encoder.encode(value) {
            userDefaults.set(data, forKey: key)
        } else {
            assertionFailure("Encodable \(T.self) is not _actually_ encodable to any data...Please fix 😭")
        }
    }

    override func get(key: String, userDefaults: UserDefaults) -> T? {
        guard let data = userDefaults.data(forKey: key) else { return nil }

        if let codable = deserialize(data) {
            return codable
        } else if let unarchivedData = NSKeyedUnarchiver.unarchiveObject(with: data) {
            return deserialize(unarchivedData)
        } else {
            return nil
        }
    }

    override func isSerialized() -> Bool {
        return true
    }

    override func deserialize(_ object: Any) -> T? {
        guard let data = object as? Data else { return nil }
        return try? JSONDecoder().decode(T.self, from: data)
    }
}

Unfortunately, the set(encodable:forKey:) interface extending UserDefaults has an internal protection level (see here), so I had to copy its functionality into my bridge. I'd propose to make it public, as it may be useful for custom bridges (such as the one I implemented).

While implementing the "migration" bridge, I noticed it wasn't compatible with arrays of Codable. In Version 3, I encoded each of the items in the array separately, so I had an array of Data. To get my data back in Version 4, I implemented the following bridge that works just as expected:

final class CodableArrayBridge<T: Codable>: DefaultsBridge<[T]> {
    override func save(key: String, value: [T]?, userDefaults: UserDefaults) {
        guard let value = (value.flatMap { serializeArrayItems(of: $0) }) else { return userDefaults.removeObject(forKey: key) }
        userDefaults.set(value, forKey: key)
    }

    override func get(key: String, userDefaults: UserDefaults) -> [T]? {
        guard let array = userDefaults.array(forKey: key) else { return nil }

        if let codableArray = deserializeArrayItems(in: array) {
            return codableArray
        } else {
            return nil
        }
    }

    private func serializeArrayItems(of array: [T]) -> [Data]? {
        let encodedArray = array.compactMap { try? JSONEncoder().encode($0) }
        guard encodedArray.count == array.count else { return nil }
        return encodedArray
    }

    private func deserializeArrayItems(in array: [Any]) -> [T]? {
        let decodedArray = array.compactMap { item -> T? in
            guard let data = item as? Data else { return nil }
            return try? JSONDecoder().decode(T.self, from: data)
        }

        guard decodedArray.count == array.count else { return nil }
        return decodedArray
    }
}

I'm however wondering why such a bridge isn't part of the framework in the first place. There is probably a better way to store Codable arrays in version 4, is it?

sunshinejr commented 5 years ago

Hey @fredpi! Glad you figured it out and that it works as expected - I'm happy that the current system is capable of doing something like that, it feels powerful.

So the Codable array issue. If you look at the array bridge for Codable it's basically the same class as for a single value bridge. This is because an array of Codable values is a Codable itself!

And, as you probably know, Codable is first serialized into Data and then saved to the property list that UserDefaults uses underneath the public API layer. This is the reason why it didn't work for you - you had an array of data values saved into property list instead of one data blob that can be deserialized into an array.

Also, great job on the custom array bridge! One thing to note is that you should implement isSerialized and deserialize methods if you want to use KVO in the future with your types.

And yeah I agree, we could not only make set(encodable:) but also probably make all the bridges we have in the library open instead of public to make it easier to create custom bridges. If you would like to make a PR with given updates I would be really grateful!

fredpi commented 5 years ago

@sunshinejr Thanks for your further explanations and suggestions! I adjusted my bridge accordingly, and it still works as expected 👍

I opened https://github.com/radex/SwiftyUserDefaults/issues/186 to track progress on the public / open matter and I will happily create a PR for this soon.

Closing this issue now, as my migration issues are resolved 🎊

sunshinejr commented 5 years ago

Awesome to hear @fredpi 🚀