[WIP, AIT-287] Implement new rules for discarding buffered object operations#112
Closed
lawrence-forooghian wants to merge 4 commits intomainfrom
Closed
[WIP, AIT-287] Implement new rules for discarding buffered object operations#112lawrence-forooghian wants to merge 4 commits intomainfrom
lawrence-forooghian wants to merge 4 commits intomainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
11de235 to
5e0c796
Compare
5e0c796 to
45e17cd
Compare
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
Outdated
Show resolved
Hide resolved
45e17cd to
18a9f76
Compare
a9c8eef to
83643ca
Compare
83643ca to
6125bf0
Compare
6125bf0 to
4942c24
Compare
4942c24 to
b8a86e8
Compare
b8a86e8 to
4049153
Compare
Instead of waiting for the server to echo back an operation before applying it locally, operations are now applied immediately upon receiving the ACK from Realtime. Implements the behaviours from spec commit 56a0bba and ports the corresponding integration tests from ably-js commit 6b1c2de, plus the test fix in ably-js commit f9fbe8e (from [1]). [1] ably/ably-js#2175 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Port of ably-js commit b329fd2. Verifies that when an ATTACHED message is received without the HAS_OBJECTS flag, the SDK clears all local LiveObjects state (root map should have no keys). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Per spec commit 997584f, buffered object operations must now be cleared on every ATTACHED message (new RTO4d), rather than when a new OBJECT_SYNC sequence starts (old RTO5a2b) or in the no-HAS_OBJECTS path specifically (old RTO4b5). The key insight is that on ATTACHED, either: - HAS_OBJECTS is set, meaning the subsequent sync sequence includes all operations up to the attach point, making previously buffered operations redundant - HAS_OBJECTS is not set, meaning the client performs an implicit sync and clears local state anyway Production changes: - nosync_onChannelAttached: replace `if` with `switch` to clear buffer on every ATTACHED (RTO4d), even when already SYNCING - nosync_handleObjectSyncProtocolMessage: stop clearing buffer on new sync sequence (RTO5a2b replaced by RTO4d) Test changes: - doesNotModifyStateWhenHasObjectsIsTrue: renamed, now asserts buffer is cleared (RTO4d) whilst pool and sync sequence remain unchanged - newSequenceIdDiscardsInFlightSync: removed RTO5a2b annotation and buffered ops assertion, now only covers RTO5a2a Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Port four test cases from ably-js commit 9b2224d that verify the updated buffered object operations clearing behaviour: 1. bufferedObjectOperationsAreDiscardedOnAttached — starts a sync sequence, injects an operation, receives ATTACHED, injects another operation, completes sync, verifies only the post-ATTACHED operation was applied 2. bufferedObjectOperationsAreDiscardedWhenAlreadySyncingChannel- ReceivesAttached — already SYNCING, injects an operation, receives another ATTACHED, injects another operation, completes sync, verifies only the post-ATTACHED operation was applied 3. bufferedObjectOperationsAreNotDiscardedOnNewObjectSyncSequence — starts a sync, injects an operation, starts a new sync with a different sequence ID, injects another operation, completes the second sync, verifies BOTH operations were applied (buffer was NOT cleared by the new sequence) 4. operationsAreBufferedDuringResyncWithoutPrecedingAttached — completes a sync, then receives OBJECT_SYNC without a preceding ATTACHED, verifies operations are buffered during the resync and applied upon completion Also adds a testsOnly_bufferedObjectOperationsCount accessor to support direct assertions on buffer state in these tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4049153 to
f736ce8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ably/specification@997584f. Integration tests ported from ably/ably-js@9b2224d.