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

Contributor discontinuity criteria are ambiguous re shared channels #240

Open
lawrence-forooghian opened this issue Nov 25, 2024 · 3 comments
Assignees
Labels
chat Related to the Chat SDK.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Nov 25, 2024

*** @(CHA-RL4a2)@ @[Testable]@ If the given contributor has not yet successfully managed to attach its Realtime Channel (i.e. no call to @attach()@ on the channel, per @CHA-RL1f@, has succeeded), then no action should be taken (i.e. @CHA-RL4a3@ and @CHA-RL4a4@ behaviours are not performed).

I find the wording of CHA-RL4a2 ambiguous — is the "has-already-been-attached" state meant to be keyed by the contributor or by its underlying realtime channel?

Given that contributors might share channels, we might receive a state change for a contributor which was actually triggered by calling attach() on some other contributor.

It makes me wonder whether it would be simpler to not allow contributors to share channels. I don't really see what it buys us, and it means that we write

In that sense, implementations of Room Lifecycle @MUST@ make no assumptions over which realtime channels are in use or being shared, and treat every @contributor@ as being entirely standalone.

but then have to introduce exceptions like

*** @(CHA-RL5a1)@ NOTE: As many chat features share channels, the equality of contributors when deciding not to detach is based on their realtime channel, and not the contributor themselves. i.e. if two features share a realtime channel, and that channel is suspended, then that channel should not be detached.

Since, as of #233, the room already now has logic to fetch each channel precisely once, how about we also create only one lifecycle contributor per realtime channel?

@lawrence-forooghian lawrence-forooghian added the chat Related to the Chat SDK. label Nov 25, 2024
lawrence-forooghian added a commit to ably/ably-chat-swift that referenced this issue Nov 25, 2024
This spec point requires us to implement a special behaviour to handle
the fact that multiple contributors can share a channel. I have decided,
instead, to make it so that each channel has precisely one lifecycle
contributor. I think this is a simpler, functionally equivalent approach
and have suggested it in [1].

[1] ably/specification#240
lawrence-forooghian added a commit to ably/ably-chat-swift that referenced this issue Nov 25, 2024
This spec point requires us to implement a special behaviour to handle
the fact that multiple contributors can share a channel. I have decided,
instead, to make it so that each channel has precisely one lifecycle
contributor. I think this is a simpler, functionally equivalent approach
and have suggested it in [1].

[1] ably/specification#240
@lawrence-forooghian
Copy link
Collaborator Author

I’ve implemented CHA-RL5a1 in Swift by making it so that contributors no longer share realtime channels; see ably/ably-chat-swift#147

@AndyTWF
Copy link
Contributor

AndyTWF commented Nov 25, 2024

So the logic when originally implemented is thus:

  • Different chat features can definitely share one channel. It makes a lot more sense to have presence and occupancy living alongside messages than their own channel. It's also more economical with the current pricing model.
  • In the original implementation, we just made it so that different features count as a contributor to room lifecycle (as that seemed like a reasonable abstraction).
  • There needs to be some linkage of feature - lifecycle contributor simply because of the reason of discontinuities.

That said, I think it would be reasonable to define the lifecycle contributor on a per-channel basis, so long as those individual features can be notified of discontinuities on their channels (i.e. we still tie features to contributors in some way).

The thing that this would require us to remove would be the feature attachment/detachment error codes for occupancy and presence, as they share channels with messages. Though I don't think this would necessarily be a problem.

WDYT?

@lawrence-forooghian
Copy link
Collaborator Author

Yep, what you've described is pretty much how it's implemented in Swift in the above PR (the error codes for occupancy and presence are still there but will never get thrown).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat Related to the Chat SDK.
Development

No branches or pull requests

2 participants