No-connection-serial implementation #1496

ikbalkaya · 2022-09-14T09:15:43Z

No connection-serial implementation for ably-cocoa
Closes #1494

This also includes changes here
However, in ably-cocoa there is no mandatory version param for rest request so no change there.

Including all references. Also some tests has been removed as new spec rendered them useless. Some methods with connection serial parameters are refactored to function without it. Subsequent commit will make any change that's necessary, following the spec document

Also remove _lastPayloadProtocolMessageChannelSerial and perform updates based on RTL15b

This is RTN15c6. Also removes RTN15c3 (that will be replaced by RTN15c7 later)

This also refactors reattach behaviour for RTN15c6 and RTN15c7

Also implement RTN16f, RTN16j and potentially RTN16i. Also updated the references to the recovery key so that key is now a JSON string

Also add fromJson method and turn toJson into a method. I did not throw exception in case of serialization / deserialization failure and instead logged the error

As described in RTN16g getRecoveryKey() will return a string rather than an object. Object will be used only internally

As described in RTN16k. Also recovery key is being used as provided by client options and not from connection

As described in RTL4c1

As described with RTP5a1, Switch branch seems a bit fragile here with some returns for some cases - Needs to re-checked

As described in RTP17f. However the method of reentry needs to be checked to see if it should rather be through event emitter. There are so many redirections in this repo

As described in RTP17g. Also remove reentries on sync and sync completion. I might need to check whether reentry can be invoked from presenceMap

It was marked as nonull instead of nonatomic

Was calling state directly which caused data race issues

…connections to assert against version 2.0

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTRealtimeChannel.m # Source/PrivateHeaders/Ably/ARTRealtimeChannel+Private.h # Source/PrivateHeaders/Ably/ARTWebSocketTransport+Private.h

maratal · 2023-04-10T22:40:16Z

@lawrence-forooghian I've incorporated ARTResumeRequestResponse (somewhat) - d573560, couldn't you comment on these changes whether I'm on the right track with them. Also I would suggest to rename this new class to ARTConnectionRequestResponse since it's not always resume - the very first is not a resume and a failed one is not a resume as well. WDYT?

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTWebSocketTransport.h

lawrence-forooghian · 2023-04-11T11:45:50Z

Thanks @maratal, I'll take a look at it. As for your question, it's called ARTResumeRequestResponse because that's the language used by RTN15c, which talks about "the system’s response to a resume request".

the very first is not a resume and a failed one is not a resume as well

That's correct. That means that it is not appropriate to use ARTResumeRequestResponse in those cases. As its documentation says, to create it you need "the ID of the connection that we are trying to resume" and "the first protocol message received on a transport which is trying to resume a connection with ID currentConnectionID", so in a scenario where it's not possible to provide those things then we shouldn't be using this class.

Does that help?

lawrence-forooghian · 2023-04-12T20:34:03Z

Source/ARTRealtime.m

+                [self.logger warn:@"RT:%p connection \"%@\" has resumed with non-fatal error \"%@\"", self, message.connectionId, message.error.message];
+                [self resetSerials];
+            }
+            else if (response.type == ARTResumeRequestResponseTypeFatalError || response.type == ARTResumeRequestResponseTypeTokenError) {


Why not a switch here?

Good point, but I'm rewriting this piece anyway.

lawrence-forooghian · 2023-04-12T20:34:50Z

.github/workflows/check-pod.yaml

Why have all of the workflows been deleted on this branch?

I temporarily remove other workflows to not to wait while all of them finish when I try things (yes, I should move PR to draft in this case, but here I needed some feedback from you).

…tion_time_serial # Conflicts: # Source/ARTPresenceMap.m # Source/ARTRealtime.m # Source/ARTRealtimeChannel.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTConnection+Private.h # Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h # Source/PrivateHeaders/Ably/ARTRealtime+Private.h # Source/PrivateHeaders/Ably/ARTRealtimeChannel+Private.h # Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h # Source/PrivateHeaders/Ably/ARTRealtimeTransport.h # Source/PrivateHeaders/Ably/ARTWebSocketTransport.h # Source/include/Ably/ARTProtocolMessage.h

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTRealtimeTransport.h # Test/Tests/RealtimeClientConnectionTests.swift

…tion_time_serial # Conflicts: # .github/workflows/integration-test-iOS16_2.yaml # .github/workflows/integration-test-tvOS16_1.yaml # Source/ARTRealtime.m # Source/PrivateHeaders/Ably/ARTRealtimeTransport.h # Test/Tests/RealtimeClientChannelTests.swift # Test/Tests/RealtimeClientConnectionTests.swift # Test/Tests/RealtimeClientPresenceTests.swift

…tion_time_serial # Conflicts: # Source/ARTRealtimeTransportFactory.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTWebSocketTransport.h # Test/Test Utilities/TestProxyTransportFactory.swift # Test/Test Utilities/TestUtilities.swift

lawrence-forooghian · 2023-07-11T21:31:49Z

Closing this — replaced by #1714.

Increase API version to 2.0

37b9e50

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 14, 2022 09:23 Inactive

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 14, 2022 13:27 Inactive

Add channelSerial to ARTRealtimeChannel

7e4afe8

Also remove _lastPayloadProtocolMessageChannelSerial and perform updates based on RTL15b

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 14, 2022 17:14 Inactive

Implement reattaching channels on resume

59b7f83

This is RTN15c6. Also removes RTN15c3 (that will be replaced by RTN15c7 later)

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 15, 2022 14:05 Inactive

ikbalkaya added 2 commits September 15, 2022 17:37

Reset message serial if error in protocol message

0384fde

Set channel state to attaching on reattach

eb26d07

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 15, 2022 16:53 Inactive

Implement RTN15c7

e9725de

This also refactors reattach behaviour for RTN15c6 and RTN15c7

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 15, 2022 17:20 Inactive

Replace old recovervy key with an object

b699b94

Also implement RTN16f, RTN16j and potentially RTN16i. Also updated the references to the recovery key so that key is now a JSON string

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 16, 2022 14:25 Inactive

ikbalkaya added 2 commits September 20, 2022 10:50

Add connectionKey to recovery key

3287599

Also add fromJson method and turn toJson into a method. I did not throw exception in case of serialization / deserialization failure and instead logged the error

Implement getRecoveryKey as a computed property

5139cd2

As described in RTN16g getRecoveryKey() will return a string rather than an object. Object will be used only internally

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc September 20, 2022 13:08 Inactive

ikbalkaya added 12 commits September 20, 2022 18:22

Add recover as a querystring to websocket request

23f12c1

As described in RTN16k. Also recovery key is being used as provided by client options and not from connection

Update getRecoveryKey function based on RTN16h

0bb1551

Formatting

f7695a9

set up channelSerial for ProtocolMessage only on ATTACH

a989801

As described in RTL4c1

Fix spec number comment

738e00a

Reset channelSerial ATTACH

a1944c9

As described with RTP5a1, Switch branch seems a bit fragile here with some returns for some cases - Needs to re-checked

Re-enter on attach if not already attached

25d45b2

As described in RTP17f. However the method of reentry needs to be checked to see if it should rather be through event emitter. There are so many redirections in this repo

Re-enter using previous map function

3d2d394

As described in RTP17g. Also remove reentries on sync and sync completion. I might need to check whether reentry can be invoked from presenceMap

Add spec label for channel serial nullification

27a427e

Remove nonull marker from recoveryKey

ff8a2a1

It was marked as nonull instead of nonatomic

Change state ref calls to state_nosync

8239a27

Was calling state directly which caused data race issues

Fix test__003__Connection__API_version_param_must_be_included_in_all_…

7dfc85e

…connections to assert against version 2.0

maratal force-pushed the integration/no_connection_time_serial branch from c00b0b6 to 4c5fd67 Compare April 3, 2023 12:08

github-actions bot temporarily deployed to staging/pull/1496/features April 3, 2023 12:09 Inactive

github-actions bot temporarily deployed to staging/pull/1496/jazzydoc April 3, 2023 12:15 Inactive

github-actions bot deployed to staging/pull/1496/features April 3, 2023 13:02 View deployment

github-actions bot deployed to staging/pull/1496/jazzydoc April 3, 2023 13:11 View deployment

maratal force-pushed the integration/no_connection_time_serial branch 4 times, most recently from 857efcb to 4c5fd67 Compare April 3, 2023 21:53

maratal added 2 commits April 3, 2023 23:55

Temporarily removed other workflows

555b7c0

Merge remote-tracking branch 'origin/main' into integration/no_connec…

9708959

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTRealtimeChannel.m # Source/PrivateHeaders/Ably/ARTRealtimeChannel+Private.h # Source/PrivateHeaders/Ably/ARTWebSocketTransport+Private.h

maratal force-pushed the integration/no_connection_time_serial branch from 0b58eee to 9708959 Compare April 9, 2023 13:57

maratal added 2 commits April 10, 2023 16:17

Formatting

1eb0e83

Incorporate ARTResumeRequestResponse.

d573560

Merge remote-tracking branch 'origin/main' into integration/no_connec…

d5557e8

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTWebSocketTransport.h

Reverse changes in ARTResumeRequestResponse.

8b0c65a

lawrence-forooghian reviewed Apr 12, 2023

View reviewed changes

maratal marked this pull request as draft April 12, 2023 20:48

maratal added 2 commits April 12, 2023 23:30

ARTRealtimeConnecting case cleanup

5ad2f43

maratal force-pushed the integration/no_connection_time_serial branch from 29c21c1 to 5ad2f43 Compare April 12, 2023 22:53

maratal added 3 commits April 14, 2023 20:51

Merge remote-tracking branch 'origin/main' into integration/no_connec…

516568e

…tion_time_serial # Conflicts: # Source/ARTRealtime.m # Source/ARTWebSocketTransport.m # Source/PrivateHeaders/Ably/ARTRealtimeTransport.h # Test/Tests/RealtimeClientConnectionTests.swift

maratal mentioned this pull request May 21, 2023

[SDK-1494] No connection serial #1714

Merged

lawrence-forooghian closed this Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No-connection-serial implementation #1496

No-connection-serial implementation #1496

ikbalkaya commented Sep 14, 2022 •

edited

Loading

maratal commented Apr 10, 2023

lawrence-forooghian commented Apr 11, 2023

lawrence-forooghian Apr 12, 2023

maratal Apr 12, 2023

lawrence-forooghian Apr 12, 2023

maratal Apr 12, 2023

lawrence-forooghian commented Jul 11, 2023

No-connection-serial implementation #1496

No-connection-serial implementation #1496

Conversation

ikbalkaya commented Sep 14, 2022 • edited Loading

maratal commented Apr 10, 2023

lawrence-forooghian commented Apr 11, 2023

lawrence-forooghian Apr 12, 2023

Choose a reason for hiding this comment

maratal Apr 12, 2023

Choose a reason for hiding this comment

lawrence-forooghian Apr 12, 2023

Choose a reason for hiding this comment

maratal Apr 12, 2023

Choose a reason for hiding this comment

lawrence-forooghian commented Jul 11, 2023

ikbalkaya commented Sep 14, 2022 •

edited

Loading