Refactor pendingWrites / write pipeline. #1699
Conversation
[Port of firebase/firebase-js-sdk#972] No functional changes. Just renames, moves, added comments, etc. * pendingWrites => writePipeline * canWriteMutations renamed canAddToWritePipeline * commitBatch => addBatchToWritePipeline * lastBatchSeen removed since we can compute it on demand from writePipeline. * Removed cleanUpWriteStreamState and instead inlined it in disableNetworkInternal since I didn't like the non-symmetry with cleanUpWatchStreamState which is called every time the stream closes. * Misc. comment cleanup.
| * | ||
| * Whenever writePipeline is not empty the RemoteStore will attempt to start or restart the write | ||
| * stream. When the stream is established the writes in the pipeline will be sent in order. | ||
| * |
There was a problem hiding this comment.
I'd insert a comma after "is not empty" (line 99) and "stream is established" (line 100).
There was a problem hiding this comment.
Done, but I'm not backporting to the other platforms. 😛
| - (BOOL)canWriteMutations { | ||
| return [self isNetworkEnabled] && self.pendingWrites.count < kMaxPendingWrites; | ||
| - (BOOL)canAddToWritePipeline { | ||
| return [self isNetworkEnabled] && self.writePipeline.count < kMaxPendingWrites; |
There was a problem hiding this comment.
Should we rename isNetworkEnabled to canUseNetwork like in the other ports?
There was a problem hiding this comment.
Yes, but that happened in a later PR that I'm not porting (yet).
| * Queues additional writes to be sent to the write stream, sending them immediately if the write | ||
| * stream is established, else starting the write stream if it is not yet started. | ||
| */ | ||
| - (void)addBatchToWritePipeline:(FSTMutationBatch *)batch { |
There was a problem hiding this comment.
If I remember the few bits about naming on iOS correctly, then this method name should end in its argument type. I can't come up with any sane suggestions, and since this is a fine C++ name, we may be able to ignore this if you also can't think of a name.
There was a problem hiding this comment.
Yeah, I don't see a good way of doing that and we have other similarly-named methods already (e.g. applyChangesToDocuments:(FSTViewDocumentChanges *)docChanges;)
[Port of https://github.com/firebase/firebase-js-sdk/pull/972]
No functional changes. Just renames, moves, added comments, etc.
writePipeline.
disableNetworkInternal since I didn't like the non-symmetry with
cleanUpWatchStreamState which is called every time the stream closes.