Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVCloudKitDao doesn't automatically detect iCloud account status changes #31

Open
tbaggett opened this issue Nov 17, 2015 · 13 comments
Open

Comments

@tbaggett
Copy link
Contributor

Code could be added to detect changes to the user's iCloud account status so the connectStatus property value is automatically updated and (possibly) severed connections can be automatically restored.

I would like your opinion on adding this to your library. It makes sense to me, since it seems your library is meant to avoid the need to directly work with CloudKit or manage error conditions related to the user's iCloud account status. Basically, some code like I have in my tvOS app would need to be added:

// NotificationManager class from http://moreindirection.blogspot.com/2014/08/nsnotificationcenter-swift-and-blocks.html. Could modify to use NSNotificationManager directly in this case.
private let notificationManager = NotificationManager()

notificationManager.registerObserver(NSUbiquityIdentityDidChangeNotification) { [unowned self] note in
  // Verify iCloud account status hasn't changed
  self.verifyiCloudAccountStatus()
}

func verifyiCloudAccountStatus() {
    Async.main { [unowned self] in
        var reset: Bool = false
        let tokenKey = "NL.EVICT.CloudKit.UbiquityIdentityToken"
        // Get the current iCloud token
        let currentToken = NSFileManager.defaultManager().ubiquityIdentityToken
        if currentToken != nil {
            // Check for a previous token being written to user defaults and compare tokens if found
            let existingToken = NSUserDefaults.standardUserDefaults().objectForKey(tokenKey)
            if (existingToken == nil || currentToken!.isEqual(existingToken)) {
                let newTokenData: NSData = NSKeyedArchiver.archivedDataWithRootObject(currentToken!)
                NSUserDefaults.standardUserDefaults().setObject(newTokenData, forKey: tokenKey)
                reset = true
            }
        } else {
            NSUserDefaults.standardUserDefaults().removeObjectForKey(tokenKey)
            reset = true
        }

        if reset {
            // TODO: Update connectStatus property and possibly reestablish connections as needed
        }
    }
}

Also, FYI, this will probably be the last change I will be suggesting for a while. Your library works great overall. The effort you've put into it is greatly appreciated. I don't know of anything else I need from it that it doesn't already deliver at this point.

@tbaggett
Copy link
Contributor Author

Oh, I did forget one thing: I would also like to add a NSNotificationManager notification whenever the accountStatus property value changes. I display a modal error message when iCloud isn't accessible, so it will be useful to be notified when it changes if the above code is being moved into your library.

@tbaggett
Copy link
Contributor Author

FYI, I accidentally left out the call to the verifyiCloudAccountStatus method whenever the app becomes active. It is needed as well.

func applicationDidBecomeActive(application: UIApplication) {
      // Verify iCloud account status hasn't changed
      verifyiCloudAccountStatus()
}

@evermeer
Copy link
Owner

When caching data, it should also cache to a separate folder for each user.
Then the cache does not have to be cleared after an account change.

@tbaggett
Copy link
Contributor Author

That makes sense. You're saying that needs to be part of the changes, not that it is already being done, correct?

Also, I haven't looked closely enough at how your library and iCloud works to know the answer to this. Is there anything that has to be done for all the connections that were made before the account change to work? It seems like all would be well if per-user cache folders were in place, but I don't want to overlook anything when making the changes if you want me to proceed.

Except maybe the in-memory stuff hanging off of the "data" property. I will review things more thoroughly before jumping into changes.

@evermeer
Copy link
Owner

Its something that also should be done. It was already on my todo list.
with this change it gets more important.

I also think the EVCloudData.disconnectAll() should be called to clear all open connections and remove all data from memory. Don't reconnect, because often userID's can be part of the connection. The application should get a notification to reestablish all connections.

My head is a little fuzzy right know... Maybe this will work? ... I think there will be a need to always call the initialize function with a callback code block for handling user changes (NSUbiquityIdentityDidChangeNotification). That would also eliminate the need for the semaphores there. You then will continue with your app after the callback was made (for the initial user or for the newly logedin user).

@tbaggett
Copy link
Contributor Author

Okay, gotcha. My head is also fuzzy, as I didn't get a lot of sleep last night (this morning, actually). I will probably look at this in more detail tomorrow after a good night of rest. I will comment again here when I have a game plan together.

@tbaggett
Copy link
Contributor Author

FYI, I haven't forgotten about this, just got wrapped up in some issues in my app. I should get to this later today or tomorrow.

@tbaggett
Copy link
Contributor Author

Began working on this late yesterday, hope to send a PR your way by the end of my day today.

@evermeer
Copy link
Owner

Great, there is no rush, just play around with it until you are happy with it to be part of the project.

@tbaggett
Copy link
Contributor Author

Created PR #34 for your amusement. I look forward to discussing it with you once you've reviewed it.

@tbaggett
Copy link
Contributor Author

Hey there, and Merry Christmas! Any thoughts on what you're going to do with this and my associated PR at this point? Thanks.

@evermeer
Copy link
Owner

A couple of bug fixes / improvements for EVReflection took most of my time last week. Im now almost finished with the nested objects issue. The nest 4 days my time will be consumed with all kind of chrisms parties. This is high on my priority list. I think I will come to this around next Wednesday. I will probably do some 'cherry picking' from your PR.

@tbaggett
Copy link
Contributor Author

Cool, thanks for the detailed update. I hope you have a great Christmas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants