Associate user connections with channels

[jordoh]

These changes add an association between a user's subscribed channels and the connections that are subscribed to those channels. This has two major impacts:

1. When the user has two (or more) open connections to different channels and closes one of the connections, the user will immediately be unsubscribed from that channel. Previously, channel unsubscription would only happen once all of the user's connections were closed, even if none of the remaining connections were subscribed to the channel. If the user has multiple open connections to one channel and closes one, channel unsubscription will not occur (until all connections subscribed to the channel are closed).
2. All channel-related frames sent by the channel to users are only sent to the users' connections that are subscribed to the channel. Previously, subscribe frames (for example, but this applied to all channel specific frames sent to users) for one channel the user was subscribed to would be sent to all of the user's connections, even those connections that were not subscribed to that channel.

[mcarter]

Hi jordoh, thanks for the continued good work!

I like the use case for this -- a number of users have been interested in this seperation.

However, the original use-case for which the current behavior was designed is that of synchronizing application state across multiple tabs/browsers/computers. For instance, if you subscribe to a new channel in browser A (user 'mcarter') then browser B (which is also connected on user 'mcarter') should be given notification that there is a new channel subscription, and be put in that channel.

What do you think about making the two behaviors somehow optional, or pluggable, or something?

[jordoh]

Makes sense - I can definitely see some use cases for either behavior. The main confusion I had with the original implementation was on the Javascript side, where the onSubscribe/onUnsubscribe callbacks were set on a specific Subscription object, but would be called for all of the user's subscriptions.

Should this be a user option, as returned by the connect web-hook? That seems to make the most sense to me - that is, the addition of a per-user option like "discreet_subscription_connections" (though I don't think that is a very clear name). Given such an option. should the behavior created by these changes or the original behavior be the default?

[mcarter]

first, a side note: I am somewhat confused by the state of pull requests and such. Would you be interested in joining as a committer to the project? Please send a follow up email to [email protected] if so.

"The main confusion I had with the original implementation was on the Javascript side, where the onSubscribe/onUnsubscribe callbacks were set on a specific Subscription object, but would be called for all of the user's subscriptions."

This sounds like a bug to me. The way it should work is this:

• conn comes from hookbox.connect() call
• conn.onSubscribed / conn.onUnsubscribed (Called whenever the user [ the one associated with this connection ] is subscribed or unsubscribed to/from a channel successfully)
• the conn.onSubscribed callback should be passsed a subscription object.
• subscription.onSubscribe / subscription.onUnsubscribe should be called whenever a user not associated with this browser's connection is successfully subscribed or unsubscribed to/from the channel.

Are you seeing different behavior?

"Should this be a user option, as returned by the connect web-hook?"

That seems reasonable. It might also be okay for it to become part of the browser-side hookbox.connect api, unless there is a strong reason to enforce this behavior from the server-side web app logic.

"Given such an option. should the behavior created by these changes or the original behavior be the default?"

I think the current (intended) default should remain the default for now, until we have more feedback from users using the new option. We can switch it if we see a strong reason for the reverse.

[jordoh]

Yeah - in the last step of the way it should work, I'm seeing subscription.onSubscribe and onUnsubscribe get called for all channels that the user is in, not just the specific one that the subscription is for. I can filter on the client side using frame.channel_name - the changes in this pull request don't actually change that behavior.

I'll add in a configuration option sometime today and update this pull request.

I like the pull request system of contribution, as it allows for review - but I can see how it can be time consuming. I'll shoot you an e-mail tonight.

[mcarter]

Sorry that I haven't pulled these in yet. I've been meaning to first investigate and fix the other bug you outlined where the on(un)Subscribe call is issued on all subscriptions, but I haven't gotten to it yet.

Also, don't forget to add the configuration we talked about.

[jordoh]

Option added (defaults to original behavior), along with documentation. It came across a little weird to the branch this pull request is based on (it builds on some changes in a different branch), so the commit looks like it has a lot in it (when it really just has a few lines). It's also the last commit in the master branch of my fork.

[alanjds]

Hi, dudes.

Jordoh, can you please clarify me if, with this pull request applied, the default behavior will be:

• NOT per connection subscribes
• Channels subscriptions BECOME TRACKED and unsubscribes at all connections when all explicit subscribing connections are closed

Sorry my dumbness if some :)

[jordoh]

alanjds, in response to your questions:

• That is correct, per-connection subscriptions are not the default after applying this pull request. The connect webhook must return per_connection_subscriptions => true for the user to enable the new behavior.
• The original behavior still occurs in this regard: a user is only unsubscribed from a channel when all of their connections to the server have been disconnected. Unsubscribing from a channel while still connected to the server requires a call to unsubscribe. If per_connection_subscriptions is set for the user, the behavior is changed as described in the pull request above.

[alanjds]

Lets say that per-connection subscriptions => off.
And say that I have 2 connections, A and B. A subcribes to 'chan_a' and then B subscribes to 'chan_b'
At this point, A and B are subscribed to both channels, right?

If I explicitly unsubscribe 'chan_a' from A and close A, it will not unsubscribe 'chan_a' from B, right?
I wanted that, when unsubscribing at A, B become unsubscribed too.

Can I ask you to create an option to allow this behavior, who means "tracked (un)subscriptions" without "per-connection subscriptions"?

[jordoh]

So the behavior you would want is a "first-out" based unsubscribe? It should be pretty simple to build that off of this branch. That said - I don't have the time at the moment to test that this change would indeed function as desired, but I would be happy to answer any questions you have.

All you should need to do (in addition to adding a per-user option, if desired) is modify line #103 in user.py (in User#remove_connection) to something like "del self.channels[channel_name][:]" (clearing the list of connections to that channel), which should trigger an unsubscribe from the channel (since there are no connections to it left in the list).