[WIP] [CAP-49] no connection serials
(Note I've updated the original PR to try to make it clearer which commits implement which spec items - so should be easier to walk though commit by commit if thats useful when implementing the other SDKs - and I've moved the ably-js specific upgrade stuff into the last two commits so those can be ignored)
Changes
- updates the protocol version to
2.0(RSC7a)-
testing: covered already by
initbase0
-
testing: covered already by
- fix channel serial type
-
message.channelSerialis a string not a number (eg'e02Tut8QwBFsQM05921682:5')
-
- tracks channel serial of last message/presence (RTL15b)
- note unlike with connection serials we arn't checking for duplicates by comparing serials - this was only needed since the old upgrade scheme could potentially lead to duplicates which is no longer the case, and we want to keep the
channelSerialopaque rather than having to parse it to compare -
testing: covered already by
resume_active
- note unlike with connection serials we arn't checking for duplicates by comparing serials - this was only needed since the old upgrade scheme could potentially lead to duplicates which is no longer the case, and we want to keep the
- includes
channelSerialinATTACH(RTL4c1)-
testing: covered already by
resume_active
-
testing: covered already by
- remove
connectionSerialfrom resume request (RTN15b2)-
testing: covered already by
resume_active
-
testing: covered already by
- reattaches channels once connected
- reattaches
attachedchannels once connected (RTN15c6/RTN15c7) -
attachingandsuspendedstates aleady go though the attach sequence on connected (RTN15c6/RTN15c7) - no longer need to reattach all
attachedchannels inConnectionManageras this is now always done when we get a new connection, regardless of whether the connection id changed (RTN15c3) -
msgSerialis already reset anderrorReasonis already set (RTN15c7) - already processes queued messages upon connection (RTN15c6/RTN15c7)
- reattaches
- clears
channelSerialwhen detached/suspended/failed (RTP5a1)-
testing: covered by channel
rewind works on channel after reattachingtest (if we don't clear the rewind will fail)
-
testing: covered by channel
- adds recovery with channel serials
- RTN16f is already implemented - this just updates to use the new serialised recovery key
- loads the recovery channel serials, applies to any existing channels (which are not yet attached) and stores for later (RTN16j)
- note currently this doesn't clear the recovery channel serials, so if the developer releases the channel then reattaches it uses the recovered channel serial again (not defined in the spec)
- RTN16k is already implemented - this just updates to use the new serialised recovery key
- encodes the recovery key as JSON (RTN16g)
-
RTN16d is implemented already since in those states the
connectionKeyisundefined - channel recovery keys are only loaded in
connect, since therecoverparam may be a user defined callback it can't be loaded in the constructor, and by the time we get toconnectwe know theres no attached channels butrecoverhas been resolved into a string (bygetTransportParams) - testing: added a test for recovering multiple channels
- presence changes
- sends the member ID on re-entry (
dataandchannelIdalready sent) (RTP17g) - updates presencemessage to include the id when encoding
- re-enter whenever we become
ATTACHED(RTP17f) (regardless of ifHAS_PRESENCEhas been set (RTP17c2)) - no longer re-enter when we complete sync (RTP17c1)
- when re-entering sends all members with our
connectionId, regardless of whether its in the main presence map, and no longer removes from the 'my members' presence map (RTP17g/RTP17d) -
testing: already tested by
presence_auth_reenter
- sends the member ID on re-entry (
- removes connection serial (RTN10/RTN15b2)
- this breaks upgrade - which will be fixed later - though prefered to group all the spec changes together (then add the ably-js specific upgrade stuff later)
- removes
duplicateConnectionIdtest as now redundant
- removes
checkChannelsOnResumeoptions- given we're already reattaching each channel on resume, reattaching again after 30s is redundant
Upgrade Changes (ably-js only - from https://github.com/ably/realtime/issues/4216#issuecomment-1221407649)
- replaces upgrade
SYNCwithACTIVATE - doesn't wait for no channels pending before upgrading
- removes
mostRecentMessageas this is no longer needed in the new upgrade scheme (and causes a bug - see thread)- removes
duplicateMsgIdtest as now redundant
- removes
Broken Tests Listing the tests that are currently breaking due to missing realtime changes. Excluding tests failing due to missing upgrade in realtime.
- connection -
connectionQueuing- fails with
Invalid connection serialsince we no longer send a serial - will be fixed by the realtime changes
- fails with
- resume -
channel_resumed_flag- fails with
Invalid connection serialsince we no longer send a serial - will be fixed by the realtime changes
- fails with
- presence -
presence_auto_reenter- failing since we get a new connection due to
Invalid connection serial- fixed by realtime changes
- failing since we get a new connection due to
Thanks for adding new test, it's more readable now : )
Can we add spec id as a comment next to code onwards ... it's really hard to refer to the code and relevant spec item when looking at the PR
Can we add spec id as a comment next to code onwards ... it's really hard to refer to the code and relevant spec item when looking at the PR
Each commit is labeled with the spec ID - so probably best to walk though commit by commit
We will be merging this PR to integration-2.0. rather than main right?
Given this PR is now old and the commit history quite messy I've replaced with https://github.com/ably/ably-js/pull/1110