{"action":"created","issue":{"url":"https://api.github.com/repos/danthorpe/Operations/issues/395","repository_url":"https://api.github.com/repos/danthorpe/Operations","labels_url":"https://api.github.com/repos/danthorpe/Operations/issues/395/labels{/name}","comments_url":"https://api.github.com/repos/danthorpe/Operations/issues/395/comments","events_url":"https://api.github.com/repos/danthorpe/Operations/issues/395/events","html_url":"https://github.com/danthorpe/Operations/pull/395","id":168546012,"number":395,"title":"CloudKitOperation: ability to add configuration to the next (retry) operation","user":{"login":"swiftlyfalling","id":18540905,"avatar_url":"https://avatars.githubusercontent.com/u/18540905?v=3","gravatar_id":"","url":"https://api.github.com/users/swiftlyfalling","html_url":"https://github.com/swiftlyfalling","followers_url":"https://api.github.com/users/swiftlyfalling/followers","following_url":"https://api.github.com/users/swiftlyfalling/following{/other_user}","gists_url":"https://api.github.com/users/swiftlyfalling/gists{/gist_id}","starred_url":"https://api.github.com/users/swiftlyfalling/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swiftlyfalling/subscriptions","organizations_url":"https://api.github.com/users/swiftlyfalling/orgs","repos_url":"https://api.github.com/users/swiftlyfalling/repos","events_url":"https://api.github.com/users/swiftlyfalling/events{/privacy}","received_events_url":"https://api.github.com/users/swiftlyfalling/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-08-01T00:08:36Z","updated_at":"2016-08-02T21:32:47Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/danthorpe/Operations/pulls/395","html_url":"https://github.com/danthorpe/Operations/pull/395","diff_url":"https://github.com/danthorpe/Operations/pull/395.diff","patch_url":"https://github.com/danthorpe/Operations/pull/395.patch"},"body":"This PR adds two new public methods:\r\n```diff\r\nclass CloudKitOperation {\r\n+    func setPrepareForRetryHandler()\r\n}\r\n```\r\n```diff\r\nclass BatchedCloudKitOperation {\r\n+    func setPrepareForNextOperationHandler()\r\n}\r\n```\r\n\r\n## CloudKitOperation:\r\n\r\n### **CloudKitOperation.setPrepareForRetryHandler()**:\r\nSets a handler block which receives a closure that can be used to add configuration blocks to the retry operation.\r\n\r\nThis will be useful for some of the new (iOS 10 / macOS 10.12) CloudKit operations that use pipelining, like [CKFetchRecordZoneChangesOperation](https://developer.apple.com/reference/cloudkit/ckfetchrecordzonechangesoperation) (where the client code may want to update the previousServerChangeToken for a zone in the event of prior partial success that was persisted).\r\n\r\nHypothetical pseudo-code example (doesn't work yet because support for the new CloudKit operations isn't yet included):\r\n```swift\r\nvar recordsChanged: [CKRecord] = []\r\nvar recordIDsDeleted: [CKRecordID] = []\r\n\r\nlet zone1 = CKRecordZoneID(zoneName: \"zone1\", ownerName: CKCurrentUserDefaultName)\r\nlet zone2 = CKRecordZoneID(zoneName: \"zone2\", ownerName: CKCurrentUserDefaultName)\r\nvar currentChangeTokens: [CKRecordZoneID: CKServerChangeToken] = [zone1: nil, zone2: nil] // fresh/first download of changes\r\nvar zoneCompleted: [CKRecordZoneID: Bool] = [zone1: false, zone2: false]\r\n\r\nlet zone1Options = CKFetchRecordZoneChangesOptions()\r\nlet zone2Options = CKFetchRecordZoneChangesOptions()\r\nzone1Options.previousServerChangeToken = currentChangeTokens[zone1]\r\nzone2Options.previousServerChangeToken = currentChangeTokens[zone2]\r\nlet zones = [zone1, zone2]\r\n\r\nlet operation: CloudKitOperation<CKFetchRecordZoneChangesOperation> = CloudKitOperation { CKFetchRecordZoneChangesOperation() }\r\n\r\noperation.recordZoneIDs = zones\r\noperation.optionsByRecordZoneID = [zone1: zone1Options, zone2: zone2Options]\r\n\r\noperation.recordChangedBlock = { record in\r\n    recordsChanged.append(record)\r\n}\r\noperation.recordWithIDWasDeletedBlock = { recordID in\r\n    recordIDsDeleted.append(recordID)\r\n}\r\noperation.recordZoneChangeTokensUpdatedBlock = { (recordZoneID, serverChangeToken, clientChangeTokenData) in\r\n    currentChangeTokens.updateValue(serverChangeToken, forKey: recordZoneID)\r\n}\r\noperation.recordZoneFetchCompletionBlock = { (recordZoneID, serverChangeToken, clientChangeTokenData, moreComing, recordZoneError) in\r\n    currentChangeTokens.updateValue(serverChangeToken, forKey: recordZoneID)\r\n    zoneCompleted.updateValue(true, forKey: recordZoneID)\r\n    // handle error if needed here, but shouldn't be required - error should bubble up to an operationError in fetchRecordZoneChangesCompletionBlock\r\n    // moreComing is always false unless fetchAllChanges is set to false (fetchAllChanges default is true)\r\n}\r\noperation.fetchRecordZoneChangesCompletionBlock = {\r\n    // if reached here, success - handle the fact that the fetch of all changes is complete\r\n}\r\n\r\noperation.setPrepareForRetryHandler { addConfigureBlock in\r\n    addConfigureBlock { nextOperation in\r\n        // only retry with zones that haven't completed\r\n        // and ensure that the per-zone previousServerChangeToken is the last-processed one\r\n        let incompleteZones = zones.filter { (recordZoneID) -> Bool in\r\n            return !(zoneCompleted[recordZoneID] ?? false)\r\n        }\r\n        nextOperation.recordZoneIDs = incompleteZones\r\n\r\n        var optionsForIncompleteZones: [CKRecordZoneID: CKFetchRecordZoneChangesOptions] = [:]\r\n        for zone in incompleteZones {\r\n            let options = CKFetchRecordZoneChangesOptions()\r\n            options.previousServerChangeToken = currentChangeTokens[zone]\r\n            optionsForIncompleteZones[zone] = options\r\n        }\r\n        nextOperation.optionsByRecordZoneID = optionsForIncompleteZones\r\n    }\r\n}\r\n```\r\n\r\n## BatchedCloudKitOperation:\r\n\r\n### **BatchedCloudKitOperation.setPrepareForNextOperationHandler()**:\r\n\r\nSets a handler block which receives a closure that can be used to add configuration blocks to the next CloudKitOperation<T> (i.e. when the current CloudKitOperation returns with moreComing == true)\r\n\r\nFor example, if the \"next\" operation needs to use the updated serverChangeToken:\r\n```swift\r\nbatchedOperation.setPrepareForNextOperationHandler { addConfigureBlock in\r\n    addConfigureBlock { nextOperation in\r\n        nextOperation.previousServerChangeToken = currentServerChangeToken // <-- (retrieved/persisted elsewhere)\r\n    }\r\n}\r\n```"},"comment":{"url":"https://api.github.com/repos/danthorpe/Operations/issues/comments/237051407","html_url":"https://github.com/danthorpe/Operations/pull/395#issuecomment-237051407","issue_url":"https://api.github.com/repos/danthorpe/Operations/issues/395","id":237051407,"user":{"login":"swiftlyfalling","id":18540905,"avatar_url":"https://avatars.githubusercontent.com/u/18540905?v=3","gravatar_id":"","url":"https://api.github.com/users/swiftlyfalling","html_url":"https://github.com/swiftlyfalling","followers_url":"https://api.github.com/users/swiftlyfalling/followers","following_url":"https://api.github.com/users/swiftlyfalling/following{/other_user}","gists_url":"https://api.github.com/users/swiftlyfalling/gists{/gist_id}","starred_url":"https://api.github.com/users/swiftlyfalling/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swiftlyfalling/subscriptions","organizations_url":"https://api.github.com/users/swiftlyfalling/orgs","repos_url":"https://api.github.com/users/swiftlyfalling/repos","events_url":"https://api.github.com/users/swiftlyfalling/events{/privacy}","received_events_url":"https://api.github.com/users/swiftlyfalling/received_events","type":"User","site_admin":false},"created_at":"2016-08-02T21:32:47Z","updated_at":"2016-08-02T21:32:47Z","body":"Will make the suggested code tweaks later today.\r\n\r\nRegarding the structure of this addition:\r\n\r\nIf the goal is simply to permit changing the next operation's parameters, the \"simpler\" method you proposed could work. (Although it would seem to require more code changes to other things, like Repeated/RetryOperation.)\r\n\r\nBut this method could also be extended to support other functionality.\r\n\r\nFor example:\r\nWhat if a consumer wants to override the default retry behavior, regardless of the cause of the retry (i.e. regardless of the specific CKError) based on some other state?\r\n\r\nInstead of them having to set an error handler for *every single error code that needs to potentially retry* (and thus duplicate the built-in retry support), they can just set this handler, once. (It would obviously need to be changed to also provide the suggested delay & configure, and return a new one.)"}}
{"actor":{"display_login":"swiftlyfalling"}}