Closed crashoverride777 closed 7 years ago
SOLVED: So it turns out I now have to use
.... data(forKey: ...)
with your helper. Not sure this is a swift 3 thing or if you made change to the
...object(forKey: ...)
method. I even doubled checked on a old backup and object for key is what I definitely used to have before.
To give you an here is how I use your helper to retrieve saved data from a NSCoding class.
static let shared: GameData = {
// Check local defaults first, if they exist it means there was an error with the latest keychain saving action
if let decodedData = UserDefaults.standard.object(forKey: Key.encodedData.rawValue) as? Data,
let gameData = NSKeyedUnarchiver.unarchiveObject(with: decodedData) as? GameData {
print("Decoding local defaults backup data")
return gameData
}
if let decodedData = KeychainWrapper.defaultKeychainWrapper.object(forKey: Key.encodedData.rawValue) as? Data,
let gameData = NSKeyedUnarchiver.unarchiveObject(with: decodedData) as? GameData {
print("Decoding keychain data")
return gameData
}
print("No decoded data, creating new")
return GameData()
}()
Because your awesome helper returns a boolean when you save I am making a Userdefaults backup incase it returns false. On the next successful keychain save I will remove this UserDefaults backup.
Its actually kind of better tho because using data I dont have to downcast (as? Data). I didn't realise this before, even UserDefaults has and I will use both from now on.
Using
...object(forKey: ...)
does still work with UserDefaults in Swift 3, its doesn't tho with your helper. Not sure this is a bug or what has changed but I thought I let you know in detail what I experienced.
Thanks
Thanks for digging into this a bit. I will have to setup some tests myself and see whats going on. Definitely leave the issue open till I can get a better answer, but I'm glad there's a work around.
Using data is actually better for me because I dont have to downcast to Data (as? Data). Save myself 8-10 lines in an ugly if let statement, and thats always awesome hahah .
Thought its good to let you know in detail tho because people might do what I have done and use object and wonder why it doesnt work anymore with Swift 3. This could become especially confusing because with UserDefault it still works in my example.
I triple checked my old backups to make sure that I used object before, which I did, so something must have changed. I will defiantly keep you posted if I find more swift 3 weirdness.
Can you actually believe that the Swift 3 migrator did not change the 2 encoding methods in my GameData NSCoding class to this. I didn't get an error with the old method.
convenience init?(coder aDecoder: NSCoder) { ... }
func encode(with aCoder: NSCoder) { ...}
So maybe something weird has happened when you did the upgrade that slipped in or Data behaviour is different than NSData.
I dont understand the keychain API at all, so I cant really offer any suggestions apart from poor guesses.
Thanks for the reply
I did some tests using a small test app I have for the keychain wrapper and using a test object thats NSCoding compliant. Everything seems to be working fine for me.
Its a simple example, but if you want to take a quick glance at it, let me know if you notice anything different from what you're doing that might cause an issue.
Awesome. I will check it out right now and report back
In your sample you are just saving a NSObject compliant object. My problems occur when you are making a whole class NSObject, NSCoding compliant.
You can test this yourself. In your test project create a new .swift file and add this code. This is just a simple example
class GameData: NSObject, NSCoding {
static let shared: GameData = {
/// 1) Old way. This use to work, but doesnt anymore with swift 3. This still works with UserDefaults
if let decodedData = KeychainWrapper.defaultKeychainWrapper.object(forKey: "EncodedData") as? Data,
let gameData = NSKeyedUnarchiver.unarchiveObject(with: decodedData) as? GameData {
print("Decoding keychain data")
return gameData
}
/// 2) New way that works that actually makes more sense because its shorter and more specific.
if let decodedData = KeychainWrapper.defaultKeychainWrapper.data(forKey: "EncodedData"),
let gameData = NSKeyedUnarchiver.unarchiveObject(with: decodedData) as? GameData {
print("Decoding keychain data")
return gameData
}
print("No decoded data, creating new")
return GameData()
}()
var text = "NotSaved"
// MARK: - Init
private override init() {
super.init()
}
// MARK: - Convenience Init
convenience init?(coder aDecoder: NSCoder) {
self.init()
text = aDecoder.decodeObject(forKey: "key") as? String ?? text
}
// MARK: - Encode
func encode(with aCoder: NSCoder) {
aCoder.encode(text, forKey: "key")
}
// MARK: - Save
func save() {
let encodedData = NSKeyedArchiver.archivedData(withRootObject: self)
let savedSucessful = KeychainWrapper.defaultKeychainWrapper.set(encodedData, forKey: "EncodedData")
if savedSucessful {
print("Saved game data to keychain")
} else {
print("Error saving game data to keychain, creating NSUserDefaults backup")
}
}
}
Than change your view controller code to this
class ViewController: UIViewController {
@IBOutlet weak var textfield: UITextField!
@IBOutlet weak var saveButton: UIButton!
override func viewDidLoad() {
super.viewDidLoad()
// Do any additional setup after loading the view, typically from a nib.
_ = GameData.shared
}
override func didReceiveMemoryWarning() {
super.didReceiveMemoryWarning()
// Dispose of any resources that can be recreated.
}
@IBAction func saveTapped(_ sender: AnyObject) {
if let text = textfield.text, !text.isEmpty {
GameData.shared.text = text
GameData.shared.save()
}
}
@IBAction func clearTapped(_ sender: AnyObject) {
textfield.text = ""
GameData.shared.text = ""
}
@IBAction func loadTapped(_ sender: AnyObject) {
textfield.text = GameData.shared.text
}
}
Now if you use object(forKey...) in the GameData class singleton (shared) property it will always load a new instance of GameData, it will never load the saved data. You can see this in the print statements when launching the app. As mentioned this used to work before Swift 3 but now I have to say ...data(forKey...). Using object(forKey...) is still working with UserDefaults.
Hope this helps and thanks for the support
So first off, I apologize it looks like I didn't push my changes to SwiftKeychainWrapperExample. The repo is showing nothing changed for 11 days and I added an NSCoding test example to it last night...thought for sure I had pushed all my changes but it seems they didn't go through. So I will double check that tonight.
Without running any code or testing the example you've pasted, at first glance I see you've got it setup to save and retrieve Data. To work directly with the object your save would simply be:
func save() {
let savedSucessful = KeychainWrapper.defaultKeychainWrapper.set(self, forKey: "EncodedData")
if savedSucessful {
print("Saved game data to keychain")
} else {
print("Error saving game data to keychain, creating NSUserDefaults backup")
}
}
No need to archive first. And likewise on the lookup you don't need to unarchive. Your lookup in the singleton would be something like:
if let gameData = KeychainWrapper.defaultKeychainWrapper.object(forKey: "EncodedData") as? GameData {
print("Decoding keychain data")
return gameData
}
Again, thats me just looking at it and what I think it should be. So I could have the syntax wrong. But if you're using keychain wrapper to save and retrieve an NSCoding compliant object directly, it will do the archiving and unarchive to/from Data for you.
I realize you may have already tried that, but thats just one quick observation. I'll see what happened with my keychain wrapper example update tonight and see if I can get that pushed as well.
Ah thanks for that tip with the archiving, I will try that out. I basically took some old objc ray wunderlich tutorial about game data saving and converted it to swift. Its code that I haven't touched in a while.
Still bear in mind tho that my example is what I used for over 1 year and it worked great until swift 3. Not sure what has changed.
Thanks for the reply
Thanks again, I just tested your new code and it works great. Much appreciated as I save myself more lines of code.
I am still curious to what has changed that my old way stopped working with object(forKey:..) for keychain, but still works with userDefaults. Something must have changed.
Essentially what you were doing is double archiving/unarchiving. Prior to my Swift3 changes you would have called setObject(...) getObject(...). So the wrapper would have re-archived the NSData on set and then unarchived for you on get.
Now you just call set(value:...) and it determines from your value type what to do. So when you call it with Data its like calling setData. Which does not archive. So now on the set you archived once and the wrapper does not archive for you. But then to retrieve it you call .object(...) which assumes its retrieving some NSCoding compliant object and attempts to unarchive for you. And then you also attempted to unarchive. So you were getting the double unarchive, which would fail.
Does that make sense?
Yeah that makes sense, its so much cleaner now as well. Thanks a lot for the help, much appreciated!
Hey,
So I have done some more testing with your updated version and its not saving data anymore.
It returns a successful bool on the set method but on next launch all data is gone again. So ...object(forKey: ...) doesnt work anymore.
Have you made any changes because when I upgraded the project to swift 3 myself I did not have that problem.
Whats going on?