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.

Sign up for GitHub

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

Jump to bottom

[ECO-4988] Implement (some of) Room Lifecycle Monitoring spec #67

Merged
merged 5 commits into from
Oct 9, 2024
Merged

[ECO-4988] Implement (some of) Room Lifecycle Monitoring spec #67

merged 5 commits into from
Oct 9, 2024

Commits on Oct 8, 2024

  1. Fix comment 

    Mistake in 25e5052.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 8, 2024
    Configuration menu
    Copy the full SHA
    09f07e6 View commit details
    Browse the repository at this point in the history

  2. Make RoomLifecycleManager initializer async 

    In an upcoming commit I’ll need to perform some async work (subscribing
to contributor state changes) which needs to complete before the manager
can be used.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 8, 2024
    Configuration menu
    Copy the full SHA
    e3342c5 View commit details
    Browse the repository at this point in the history

  3. Change room lifecycle contributor into a protocol 

    For an upcoming feature, I’m going to want to add methods to the
contributor itself, and hence want to be able to mock it.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 8, 2024
    Configuration menu
    Copy the full SHA
    cba991d View commit details
    Browse the repository at this point in the history

  4. Implement (some of) Room Lifecycle Monitoring spec 

    Implements points CHA-RL4* from same spec as referenced in e70ee44.

In addition to the TODOs added in the code (all of which refer either to
existing GitHub issues or questions on the spec, for which we have #66
as a catch-all issue), I’ve also not done:

- CHA-RL4a2 — I don’t understand the meaning of “has not yet
  successfully managed to attach its Realtime Channel”, asked about it
  in [1]

- CHA-RL4b2 — seems redundant, asked about it in [2]

- CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3]

- CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect
  timeouts, so will do them in #48

Something which I didn’t think about in 25e5052, and which I haven’t
thought about here, is how actor reentrancy (i.e. the fact that when an
actor-isolated method suspends via `await`, another actor-isolated
method can be scheduled instead, which can potentially cause issues like
state updates being interleaved in unexpected ways) might affect the
room lifecycle manager. I would like to first of all implement the whole
thing, specifically all of the spec points that might provide us with a
mutual exclusion mechanism (i.e. the ones that tell us to wait until the
current operation is complete), before assessing the situation. Have
created #75 for this.

Created [4] to address the `@preconcurrency import Ably` introduced by
this commit.

Aside: I have not been consistent with the way that I’ve named the
tests; the existing lifecycle manager test names have a part that
describes the expected side effects. But I haven’t done that here
because some of the spec points tested here have multiple side effects
and the test names would become really long and hard to read. So for
those ones I’ve only described the expected side effects inside the
tests. I think we can live with the inconsistency for now.

Part of #53.

[1] https://github.com/ably/specification/pull/200/files#r1775552624
[2] https://github.com/ably/specification/pull/200/files#r1777212960
[3] https://github.com/ably/specification/pull/200/files#r1777365677
[4] ably/ably-cocoa#1987
    @lawrence-forooghian
    lawrence-forooghian committed Oct 8, 2024
    Configuration menu
    Copy the full SHA
    b6c3ddb View commit details
    Browse the repository at this point in the history

  5. Implement CHA-RL1g2 

    Resolves #53.
    @lawrence-forooghian
    lawrence-forooghian committed Oct 8, 2024
    Configuration menu
    Copy the full SHA
    05836a6 View commit details
    Browse the repository at this point in the history