Multipeer two-way sync can fail if the receiving device has no new updates

[UberJason]

This is sort of tangentially related to #233, even though that issue is quite old and was resolved a while ago. In summary, if two devices A and B are periodically synced over Multipeer Connectivity but the connection is also periodically broken and changes are happening on either device while the connection is down, I believe there is a scenario in which two-way sync fails. I've outlined it below, as well as a potential fix. Apologies that it's very long; I personally found the data flow complex to follow, so I'm laying it out really carefully to make sure my logic is sound.

• Suppose I have two devices A and B, and they are successfully synced over Multipeer Connectivity (so both devices' CDEMultipeerCloudFileSystems are synchronized).
• The connection is then broken, and device A adds a new Core Data entity.
• The connection is re-established, and device A initiates another sync, directing its CDEMultipeerCloudFileSystem to retrieveFilesFromPeersWithIDs:.
• Over on device B, the method handleFileRetrievalRequestFromPeerWithID:withRemotePaths: only checks to see if A is missing any files from B (as described initially in #233).
  • If there are missing files, the method sends back an archive of the files to device A. Device A can then import the files, merge them in, and later notify device B to sync back with itself using CDEMultipeerMessageTypeNewDataAvailable. (This was PR #234. It's great. 😄)
  • However, if there are no missing files, the method simply returns without ever sending a message back to device A about the lack of new files: https://github.com/drewmccormack/ensembles/blob/c2fb83b4be2cad955ed569279ec20bf640788fc2/Framework/Extensions/CDEMultipeerCloudFileSystem.m#L212 In this situation, device A never hears back and never ends up finishing the two-way sync by telling device B to retrieve device A's files.

This behavior doesn't actually manifest in Idiomatic, although I believe the reason is that Idiomatic completely de-leeches and tears down the CDEMultipeerCloudFileSystem when sync is disabled, and creates it from scratch again when sync is re-enabled: https://github.com/drewmccormack/ensembles/blob/c2fb83b4be2cad955ed569279ec20bf640788fc2/Examples/Idiomatic/Idiomatic/IDMSyncManager.m#L110-L116 https://github.com/drewmccormack/ensembles/blob/c2fb83b4be2cad955ed569279ec20bf640788fc2/Examples/Idiomatic/Idiomatic/IDMSyncManager.m#L80-L84 This is perfectly fine for a demo app, but I'm assuming in real world use that's probably not what I want to be doing.

I'll be submitting a PR soon with a proposed fix.

[drewmccormack]

If I understand this, it is an enhancement, rather than a fix, correct? Seems to me it is not the task of device A to tell device B to do a sync. Ensembles has been designed as a server-less framework, meaning it's every peer for itself. They should behave greedily, getting their own new files, and updating their own local database. The other peer will behave in the same way, pulling its own changes. You should design in enough calls to retrieve files to ensure this happens regularly.

But I agree, having a callback may be an advantage, and I am happy to accept the PR.

Correct me if I have misinterpreted the situation.

[UberJason]

Maybe it’s sort of both? When A makes a file retrieval request, it’s reasonable to expect some sort of response (in my opinion). That response may be “here are some files” or it may be “there are no files” so that in either case A can handle the result appropriately.

Add in the fact that one of the message enum cases was going unused (which I am guessing is an oversight and maybe the result of a refactor in the past) and that #234 was accepted as an enhancement, and maybe this can be seen as a fix to #234 as well as an enhancement.

[drewmccormack]

I guess the requests are setup more like one-way messages, as opposed to a call-response setup. It's more like a trigger, which may or may not trigger other calls. A call of no-files can be included too, but not sure what exactly you would use that for. You are unlikely to merge in that case. Still, I'm happy to include it.

In any case, you should never rely on getting a response, because this type of networking is notoriously unreliable. Don't write your code so that it will break if no response comes at all. It should handle a missing response. (This is one reason to use one-way requests, rather than more fragile two-way.)

I will accept the PR in any case.

[UberJason]

Fair enough - and thank you for accepting the PR anyway!