Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement missing presence spec + refactor presence #407

Open
sacOO7 opened this issue May 23, 2024 · 4 comments
Open

Implement missing presence spec + refactor presence #407

sacOO7 opened this issue May 23, 2024 · 4 comments

Comments

@sacOO7
Copy link
Contributor

sacOO7 commented May 23, 2024

  • Use of statemachine is not need as a part of channel presence since it's not mentioned in the spec and not implemented in other SDKs.
    e.g. you can find
    STATE = ruby_enum('STATE',
      :initialized,
      :entering,
      :entered,
      :leaving,
      :left
    )
  • Remove use of presence statemachine to make implementation in sync with spec and other SDKs
  • There are only 3 states for presence sync as per spec but ruby presence map mentions 6 states
    e.g. instead of
	syncInitial
	syncInProgress
	syncComplete

ruby presence map has

      STATE = ruby_enum('STATE',
        :initialized,
        :sync_starting, # Indicates the client is waiting for SYNC ProtocolMessages from Ably
        :sync_none, # Indicates the ATTACHED ProtocolMessage had no presence flag and thus no members on the channel
        :finalizing_sync,
        :in_sync,
        :failed
      )
  • It feels there is overuse of statemachine in ruby sdk. Need to be refactored at a certain point to make ruby implementation in sync with spec and for better readability.

┆Issue is synchronized with this Jira Task by Unito

@sacOO7
Copy link
Contributor Author

sacOO7 commented May 24, 2024

  • Implementation for independent presence queue is missing as per spec. Currently, all of the messages (channel and presence)are queued in connection queue instead.
  • Presence implementation doesn't follow standard control flow as per spec. So, same spec logic can be found in splits in different part of the codebase.

@sacOO7
Copy link
Contributor Author

sacOO7 commented May 28, 2024

  1. Refactor Presence onMessage/ onSyncMessage implementation like => https://github.com/ably/ably-go/blob/dd8236fc301eb078803e83efee7615f3ba133341/ably/realtime_presence.go#L291
  2. Refactor should_update_member check while adding or removing member from the map =>
    members[presence_message.member_key] = { present: present, message: presence_message, sync_session_id: sync_session_id }
    . It's not implemented as per RTP2b, RTP2c. There seems to be a bug introduced where member is not added when no existing member is found.
  3. Remove use of _syncSessionId. Currently, it's being used for marking members as updated. ( doesn't go as per spec ). We don't have syncSessionId in any other SDK's, seems this is there since very old spec.
  4. Implement presence code with respect to ably-go implementation => https://github.com/ably/ably-go/blob/main/ably/realtime_presence.go

@sacOO7
Copy link
Contributor Author

sacOO7 commented May 30, 2024

Remove use of client initiated sync ->

. This method is only needed as a part of testing and client never initiates a sync as per spec.

@sacOO7 sacOO7 changed the title Refactor presence Implement missing presence spec + refactor presence Jun 3, 2024
@sacOO7
Copy link
Contributor Author

sacOO7 commented Jun 3, 2024

Update missing connectionId, timestamp and id for inner messages / presence messages based on parent protocol message same as => https://github.com/ably/ably-go/blob/dd8236fc301eb078803e83efee7615f3ba133341/ably/proto_message.go#L60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant