mentrena / SyncKit

Automatic CloudKit synchronization
https://mentrena.github.io/SyncKit/
MIT License
507 stars 59 forks source link

reuploadRecordsForChildrenOf does not handle errors #101

Closed BlixLT closed 4 years ago

BlixLT commented 5 years ago

reuploadRecordsForChildrenOf should handle errors. At least one easy reproducible issue is with limitExceeded error (when records "tree" contains more than 400 objects). Then reuploadRecordsForChildrenOf gets a limitExceededError and future operations fails. To reproduce you can create a company with 400+ employees in the sample app and try to share that company. One more change in the sample project's sharing method is needed:

func share(company: Company) {
        sharingCompany = company
        
        synchronizer.synchronize { [weak self](error) in
            guard error == nil,
                let strongSelf = self,
                let company = strongSelf.sharingCompany,
                let modelObject = strongSelf.interactor.modelObject(for: company) else { return }
            
            strongSelf.synchronizer.reuploadRecordsForChildrenOf(root: modelObject) { (reuploadError) in
                // limitExceededError is being return if modelObject's tree has more than 400 objects
                DispatchQueue.main.async {

                    let share = strongSelf.synchronizer.share(for: modelObject)
                    let container = CKContainer(identifier: strongSelf.synchronizer.containerIdentifier)
                    let sharingController: UICloudSharingController
                    if let share = share {
                        sharingController = UICloudSharingController(share: share, container: container)
                    } else {
                        sharingController = UICloudSharingController(preparationHandler: { (controller, completionHandler) in
                            strongSelf.synchronizer.share(object: modelObject,
                                                          publicPermission: .readOnly,
                                                          participants: [],
                                                          completion: { (share, error) in
                                                            share?[CKShare.SystemFieldKey.title] = company.name
                                                            completionHandler(share, container, error)
                            })
                        })
                    }
                    sharingController.availablePermissions = [.allowPublic, .allowReadOnly, .allowReadWrite]
                    sharingController.delegate = self
                    strongSelf.view?.present(sharingController,
                                             animated: true,
                                             completion: nil)
                }
            }
        }
    }
}

I couldn't find the usage of this method in the sample project, but I suppose something like that is supposed usage - to make sure, that records "tree" has correct children-parent relationships, re-upload them before sharing.

BlixLT commented 5 years ago

My intitial thought, that something like func uploadRecords(adapter: ModelAdapter, completion: @escaping (Error?)->()) could be reused (both methods are dealing with uploading CKRecords). However, it fetches records to upload inside, so not sure how easily refactor it.

mentrena commented 4 years ago

Just to clarify:

My intitial thought, that something like func uploadRecords(adapter: ModelAdapter, completion: @escaping (Error?)->()) could be reused

As you said the use case would be to set the right parent-child hierarchy for records that were created before the library supported CKRecord.parent, to support sharing. If your model classes were conforming to the ParentKey protocol already you wouldn't have any use for this method. If you had used SyncKit some time ago, before CKSharing was supported, and created several records with it, and now you wanted to be able to share some of them, then you might want the .parent property for those records would to be updated before you share the parent record, so the whole record tree is visible by the user accepting the share. In such a case this method might be helpful.

Will add error handling when I get a chance.

mentrena commented 4 years ago

0.7.9 now uses batch uploads for this function.