Delete redundant object type check when applying SyncObjectsPool#429
Merged
lawrence-forooghian merged 1 commit intomainfrom Feb 27, 2026
Merged
Delete redundant object type check when applying SyncObjectsPool#429lawrence-forooghian merged 1 commit intomainfrom
SyncObjectsPool#429lawrence-forooghian merged 1 commit intomainfrom
Conversation
lawrence-forooghian
changed the title
SyncObjectsPool
lawrence-forooghian
force-pushed
the
remove-RTO5c1b1c
branch
from
1c53834 to
41aae76
Compare
lawrence-forooghian
added a commit
to ably/ably-liveobjects-swift-plugin
that referenced
this pull request
Feb 26, 2026
Spec commits 1f22417 and 9f4d7de add partial object sync for
protocol v6+. The server can now split a large object across
multiple OBJECT_SYNC protocol messages. This commit implements
the merge logic in SyncObjectsPool.
SyncObjectsPool.swift — full rewrite:
- Changed from Collection (array-backed) to Sequence
(dictionary-backed, keyed by objectId)
- Stores InboundObjectMessage internally; Entry is now a computed
view yielded during iteration
- Entry.state documents the guarantee that either .map or .counter
is always populated
- New accumulate(objectMessage:logger:) method implements RTO5f:
- Skips messages with nil .object
- RTO5f3: Rejects unsupported types (neither map nor counter)
before pool lookup — provides the Entry.state guarantee
- RTO5f1: Stores new entries
- RTO5f2a1: Replaces map entry when incoming tombstone is true
- RTO5f2a2: Merges map entries otherwise
- RTO5f2b: Logs error and skips partial counter
- Removed init(entries:) and append(contentsOf:) (replaced by
accumulate)
InternalDefaultRealtimeObjects.swift — replaced the compactMap +
append(contentsOf:) / init(entries:) pattern with loops calling
pool.accumulate(objectMessage:logger:) for both the multi-message
and single-message paths.
TestFactories.swift — added serialTimestamp parameter to
inboundObjectMessage factory.
SyncObjectsPoolTests.swift — full rewrite with tests for all
RTO5f spec points plus adapted versions of the original tests.
ObjectsPoolTests.swift — added SyncObjectsPool.testsOnly_fromStates
convenience extension; mechanically replaced all .init(entries:)
call sites.
Removed RTO5c1b1c — the ignoresNonMapOrCounterObject test and the
RTO5c1b1c else branch in ObjectsPool.nosync_applySyncObjectsPool
have been removed, replaced with a preconditionFailure referencing
the Entry.state guarantee. A corresponding spec PR
(ably/specification#429) marks RTO5c1b1c
as deleted.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lawrence-forooghian
added a commit
to ably/ably-liveobjects-swift-plugin
that referenced
this pull request
Feb 26, 2026
Spec commits 1f22417 and 9f4d7de add partial object sync for
protocol v6+. The server can now split a large object across
multiple OBJECT_SYNC protocol messages. This commit implements
the merge logic in SyncObjectsPool.
SyncObjectsPool.swift — full rewrite:
- Changed from Collection (array-backed) to Sequence
(dictionary-backed, keyed by objectId)
- Stores InboundObjectMessage internally; Entry is now a computed
view yielded during iteration
- Entry.state documents the guarantee that either .map or .counter
is always populated
- New accumulate(objectMessage:logger:) method implements RTO5f:
- Skips messages with nil .object
- RTO5f3: Rejects unsupported types (neither map nor counter)
before pool lookup — provides the Entry.state guarantee
- RTO5f1: Stores new entries
- RTO5f2a1: Replaces map entry when incoming tombstone is true
- RTO5f2a2: Merges map entries otherwise
- RTO5f2b: Logs error and skips partial counter
- Removed init(entries:) and append(contentsOf:) (replaced by
accumulate)
InternalDefaultRealtimeObjects.swift — replaced the compactMap +
append(contentsOf:) / init(entries:) pattern with loops calling
pool.accumulate(objectMessage:logger:) for both the multi-message
and single-message paths.
TestFactories.swift — added serialTimestamp parameter to
inboundObjectMessage factory.
SyncObjectsPoolTests.swift — full rewrite with tests for all
RTO5f spec points plus adapted versions of the original tests.
ObjectsPoolTests.swift — added SyncObjectsPool.testsOnly_fromStates
convenience extension; mechanically replaced all .init(entries:)
call sites.
ObjectsIntegrationTests.swift — ported the partial OBJECT_SYNC
integration test from ably-js commit 1052acf. (TODO update commit
reference once JS finalised)
Removed RTO5c1b1c — the ignoresNonMapOrCounterObject test and the
RTO5c1b1c else branch in ObjectsPool.nosync_applySyncObjectsPool
have been removed, replaced with a preconditionFailure referencing
the Entry.state guarantee. A corresponding spec PR
(ably/specification#429) marks RTO5c1b1c
as deleted.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
I would argue we should remove RTO5c1b1c instead. |
Collaborator
Author
Sorry @VeskeR I feel I must have missed something — isn't what this PR does precisely what you've suggested? |
Contributor
|
Sorry, my bad 🤦♂️ . I must've misread the PR yesterday, it was a looong day of PR feedback reviews/addressing. |
VeskeR
approved these changes
Feb 27, 2026
lawrence-forooghian
force-pushed
the
remove-RTO5c1b1c
branch
from
41aae76 to
0a8fe99
Compare
RTO5f3 (added in 9f4d7de) rejects unsupported object types before they enter the SyncObjectsPool, so the RTO5c1b1c check during pool application can never be reached. Furthermore, testing RTO5c1b1c would require putting the SDK into an internal state that is inconsistent with the rest of the spec — a SyncObjectsPool containing an unsupported object type — which should not be possible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lawrence-forooghian
force-pushed
the
remove-RTO5c1b1c
branch
from
0a8fe99 to
963ec30
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
2 participants
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.
RTO5f3 (added in 9f4d7de) rejects unsupported object types before they enter the
SyncObjectsPool, so the RTO5c1b1c check during pool application can never be reached. Furthermore, testing RTO5c1b1c would require putting the SDK into an internal state that is inconsistent with the rest of the spec — aSyncObjectsPoolcontaining an unsupported object type — which should not be possible.