Skip to content

Conversation

@SimonWoolf
Copy link
Member

No description provided.

@SimonWoolf SimonWoolf requested a review from paddybyers July 22, 2019 10:06
@mattheworiordan mattheworiordan temporarily deployed to ably-docs-pr-711 July 22, 2019 10:06 Inactive
Copy link
Member

@paddybyers paddybyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but please see comments

@mattheworiordan
Copy link
Member

mattheworiordan commented Jul 22, 2019

Thanks.

Before this is merged, please can you:

  • Create an issue in all libraries that implement realtime presence to correct this
  • Describe what test may be useful to verify the spec change is fixed (making it easier for all client lib devs)
  • Update the spec sheet (or rename existing spec items) with these new spec items as I assume, because numbering has been changed, some of these previously implemented spec items may be partially implemented anyway i.e. if we don't do this now in the sheet, later it will be much harder for each client lib developer to understand the spec items are renumbered and moved and quite similar to previous implementations.

@paddybyers
Copy link
Member

paddybyers commented Jul 23, 2019

@SimonWoolf please confirm that the tests that need to exist for this should cover the following.

Construction of these tests will require the use of a mock transport that is capable of sending specific messages to the backend, suppressing the sending of specific messages to the backend, and suppressing the processing of specific messages received from the backend.

Server-sent ATTACHED indicates hasPresence = false but local PresenceMap has members that must be re-entered.

The sequence of events would be:

  • connect;
  • attach;
  • enter a member (via the API), but suppress the ENTER sent by the client to the backend;
  • simulate the receipt of that member from the backend, so it is entered by the library into the internal PresenceMap;
  • disconnect, and wait for < connectionStateTtl;
  • trigger a resume;
  • await the CONNECTED and then ATTACHED. The received ATTACHED message should have hasPresence=false;
  • verify that the library attempts to re-enter the member;
  • verify that the subsequent expected sequence of events occurs (ie the member is entered in the backend, and subsequently reflected back to the client).

Server-sent ATTACHED indicates hasPresence = true followed by SYNC; local PresenceMap has members that must be reentered.

The sequence of events would be:

  • connect;
  • attach;
  • enter a member, memberA, normally;
  • enter a member, memberB via the API, but suppress the ENTER sent by the client to the backend;
  • simulate the receipt of memberB from the backend, so it is entered by the library into the internal PresenceMap;
  • disconnect, and wait for < connectionStateTtl;
  • trigger a resume;
  • await the CONNECTED and then ATTACHED. The received ATTACHED message should have hasPresence=true;
  • await the SYNC;
  • verify that the library attempts to re-enter memberB;
  • verify that the subsequent expected sequence of events occurs (ie memberB is entered in the backend, and subsequently reflected back to the client).

@SimonWoolf
Copy link
Member Author

SimonWoolf commented Jul 31, 2019

please confirm that the tests that need to exist for this should cover the following.

lgtm

@SimonWoolf SimonWoolf merged commit 43ba2d4 into master Aug 2, 2019
@SimonWoolf SimonWoolf deleted the presence-auto-reenter branch August 2, 2019 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants