Fix Member Presence #607
Fix Member Presence #607
Conversation
sacOO7
commented
Sep 29, 2020
•
sacOO7
commented
- Updated wait logic during presence sync
- Added check for the attached channel state while requesting for members
- Added tests to make sure, the client returns the right members during intermittent detach cycles
- Updated a few other tests
- Fixes Presence blocking get sometimes has missing members #467
|
Conflicts against main now on this one, @sacOO7 ... likely as a result of my recent changes in main. They were necessary however to get us wholesale moved over from tabs to spaces. Plus we now have Checkstyle! 🎉 (early days, only a few rules, but from acorns grow oak trees!) |
|
Hi @QuintinWillison I will fix the conflicts and make PR again |
# Conflicts: # lib/src/main/java/io/ably/lib/realtime/Presence.java # lib/src/test/java/io/ably/lib/test/realtime/RealtimePresenceTest.java
vzhikserg
added
the
bug
| @@ -1508,7 +1537,7 @@ public void onPresenceMessage(PresenceMessage message) { | |||
| leavePresenceWaiter.waitFor(ably1.options.clientId, Action.leave); | |||
|
|
|||
| /* Validate that, | |||
| * - we received all actions | |||
| *- we received all actions | |||
There was a problem hiding this comment.
This change (and a few others) add noise. And, actually, in this case I think reduces readability of the comment.
There was a problem hiding this comment.
I don't usually add unnecessary code. I think this was added as a part of checkstyle run.
| @@ -2598,7 +2627,7 @@ public void presence_state_change () { | |||
| try { | |||
| DebugOptions opts = new DebugOptions(testVars.keys[0].keyStr); | |||
| fillInOptions(opts); | |||
| opts.autoConnect = false; /* to queue presence messages */ | |||
| opts.autoConnect = false; /* to queue presence messages */ | |||
There was a problem hiding this comment.
why this change? It's unrelated to the fix you've made as far as I can see. Same goes for one below. Just adds noise for reviewers. Please try to keep PRs clean. 🙂
There was a problem hiding this comment.
Checkstyle related change
