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

Presence map is using clientId as its key, rather than memberKey #641

Closed
SimonWoolf opened this issue Oct 26, 2017 · 2 comments
Closed

Presence map is using clientId as its key, rather than memberKey #641

SimonWoolf opened this issue Oct 26, 2017 · 2 comments
Assignees
@ricardopereira
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@SimonWoolf
Copy link
Member

SimonWoolf commented Oct 26, 2017 •
edited
Loading

The lib solemnly set a memberKey for presence messages... and then ignores it and just uses the clientId as the key in the presence map.

https://github.com/ably/ably-ios/blob/7d18a06e190e6bd1fef7b879a258af521d6fcabe/Source/ARTPresenceMap.m#L109 https://github.com/ably/ably-ios/blob/7d18a06e190e6bd1fef7b879a258af521d6fcabe/Source/ARTPresenceMap.m#L128

So presence behaviour for clients which share a connectionId is currently completely broken.

See e.g. customer reporting an issue caused by this here .

Relevant spec items: RTP2, RTP2a
@SimonWoolf SimonWoolf added the bug Something isn't working. It's clear that this does need to be fixed. label Oct 26, 2017
@SimonWoolf
Copy link
Member Author

SimonWoolf commented Oct 26, 2017 •
edited
Loading

Also it looks like presence.get doesn't currently support filtering by clientId/connectionId, so also fails RTP11c2 and RTP11c3. Looks like there was an issue for this but it was closed?

(removed comment about test coverage, I was only looking at the obj-c tests, missed the swift ones)

@mattheworiordan
Copy link
Member

@ricardopereira would you mind looking at this ASAP as this sounds like a pretty major bug? Also, any idea why the issue was closed?

@ricardopereira ricardopereira mentioned this issue Nov 15, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ricardopereira ricardopereira

Labels
bug Something isn't working. It's clear that this does need to be fixed.
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mattheworiordan @ricardopereira @SimonWoolf