[Bug] Can not resubscribe to the same channel after unsubscribing from it

[fedulvtubudul]

Environment

SwiftCentrifuge version: 0.0.3

Steps to reproduce

1. Set token client token and connect.
2. Create a new subscription to channel A and call subscribe (this step is optional).
3. Create a new subscription to channel B and call subscribe.
4. Call unsubscribe on channel B subscription from step 3.
5. Create a new subscription to channel B and call subscribe.

Expected behavior

Client is successfully resubscribed to channel B, by means of either an existing subscription or a new one.

Actual behavior

On step 5 newSubscription method throws a ".duplicateSub" error. However, there is no way to resubscribe to a given channel having only the channel name, and not having previously created subscription. subscriptions array of the CentrifugeClient is private, and there are no accessors to it.

Currently, the only way to overcome this limitation is storing all the subscriptions ever created outside of the CentrifugeClient. It seems not elegant: such collection will duplicate the contents of subscriptions array. It also may become inefficient if client subscribes and unsubscribes a lot to some random channels. We'll have to store all the subscriptions, even if quite a few of them will be subscribed for the second time.

Proposed solution A

Introduce a public getter for existing subscriptions. It may look like

func existingSubscription(channel: String) -> CentrifugeSubscription?

Proposed solution B

Allow reusing existing subscription after it was unsubscribed already. I've implemented this in our fork as the easiest but still working option, so I can create a PR if you like this option.

Proposed solution C

Remove unsubscribed subscriptions from subscriptions array of the CentrifugeClient. That could be done inside of func unsubscribe(sub: CentrifugeSubscription) I guess. It would solve not only the discussed issue, but also make CentrifugeClient more scalable. Сurrent implementation looks suspiciously: wouldn't the subscription look up time degrade after connecting and disconnecting from lots of channels? At least we could store subscriptions in a map (with channel name used as a key), not in array.

[FZambia]

@fedulvtubudul I know about this behaviour and understand that in current form it's a bit frustrating. I'd really want to solve this forever and for all clients but this behaviour exists because of reasons. Maybe we can cooperate and decide what is a safe and general way to deal with subscriptions.

In general we keep _subs internally to automatically resubscribe them after reconnect. Each subscription object has some data attached to it to recover missed messages upon reconnect (sequence number in channel). Centrifugo allows only one subscription to the same channel from the same client at moment.

First of all - our Java client has additional API to work around this case. This is actually like your solution A. I suppose this is the most simple way to fix this problem at least until we find a better solution.

Your thoughts from solution C were already raised in https://github.com/centrifugal/centrifuge-js/issues/98. And if you can see from comments this looks like a good idea but requires POC and some consideration that this won't break existing code. Just had no time yet to think more about this. Adding possibility to removeSubscription manually (like in our Java client) solves growing _subs for apps where many different subscriptions dynamically created/unsubscribed. But results into not very simple/convenient API, I understand.

Solution B looks quite dangerous because we don't know whether application keeps reference to original Subscription object or not. In this case we can occasionally change its delegate method so when user call sub.subscribe() on original subscription it will result into malformed behaviour - messages will be processed by possibly another delegate class with different behaviour.

What do you think?

UPD. Thanks for a great issue description btw

[fedulvtubudul]

In general we keep _subs internally to automatically resubscribe them after reconnect.

OK, got that point. Still, there is one thing I can't understand: why would you want to resume storing a subscription after user explicitly calls unsubscribe on it?

I've looked into Java implementation by your link, and I like it more than current Swift implementation in several aspects. First, user can fetch an existing subscription; second, user can remove existing subscription; and third, subscriptions are stored in an associated collection, which can give performance advantages if we actively manage lots of subscriptions.

I'll try to implement the same. It solves our problems with resubscribing to some previously unsubscribed channels. As this solution will look homogeneous with other Centrifuge implementations on different platforms, I hope you consider to merge it into your repo, cause we don't like having too much differences in our fork, so we can always easily update to the upstream.

[FZambia]

@fedulvtubudul ok, just send pr as it will be ready and I'll look. Regarding to data structure that keeps subscriptions: I suppose that client should not be subscribed to many channels at the same moment (usually there is a reasonable amount of channels) so search for required subscription in array instead of hash table does not introduce a lot of overhead. So when keeping _subs actual and clearing subscriptions access will always stay performant. But I dont mind changing it to map actually.

[FZambia]

Closed by #36

[DanialV]

I still got the error: Subscription to the channel already exists even though I unsubscribed from the channel before Version 3.0.1