Fix for #2053

[Skully17]

• Fixed PubSub over RLinks by using "sub_details['id']" instead of "sub_id".
• changed "sub_id" to "sub_session"

These changes follow the fix made to registering over RLinks in #1789

[oberstet]

Wow, kudos for tracking this down! Also thanks for contributing in general=)

---

since this was your first contribution in the repo here: guess you are David, right? in which case, thanks for CAA!

---

rgd the PR: minor whitespace formatting (yapf is unhappy), and the docs failure is network related. I'll just restart that one later.

[oberstet]

ok, the change seems to indeed match how it should work:

• RLinks use BridgeSession https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/worker/rlink.py#L37 which
• sets up a meta API handler https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/worker/rlink.py#L191 for wamp.subscription.on_create and uses wamp.subscription.get to get subscription details which
• returns a subscription object: https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/router/service.py#L642

[oberstet]

Fixed PubSub over RLinks by using "sub_details['id']" instead of "sub_id".

actually, I'm not sure anymore I understand the issue, because: https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/router/service.py#L643

[Skully17]

guess you are David, right?

Yes :)

minor whitespace formatting (yapf is unhappy), and the docs failure is network related. I'll just restart that one later.

Cheers. I have changed the formatting and the yapf test is now passing locally (copied the test's command from the error).

actually, I'm not sure anymore I understand the issue, because:

I don't think I follow what you mean but I have noticed a few things when following the steps on how it works: First, I noticed that "forward_current_subs" passes "sub_id" into "on_subscription_create" which should, I assume, be the same as the id stored in "sub". https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/worker/rlink.py#L199

However, when debugging before, the "sub_id" and "sub_details['id']" were not the same when I printed them from "on_subscription_create". Perhaps "on_subscription_create" was being called elsewhere? This lead me to find that "on_subscription_create" is assigned to a subscription and that the subscription is published to here: https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/router/broker.py#L873

Here "on_subscription_create" is being given "session._session_id", not "sub_id" like in "forward_current_subs".

I propose that "forward_current_subs" should also be changed to pass session id, instead of "sub_id", into "on_subscription_create". This would mean that is works same as the comparable function for regesters: https://github.com/crossbario/crossbar/blob/ccbae0a56ef331fe00192455837aa72578eadc54/crossbar/worker/rlink.py#L397

There are also other slight differences, such as there being an assert in "register_current":

What further changes would be appropriate for me to make in this PR?

1. Change "sub_id" to "self.session_id" in "on_subscription_create"
2. Add assert to check that "sub_id" and "sub['id']" are the same
3. Move check to see if the URI starts with "wamp." to be done at the start of "on_subscription_create" like it is in "on_registration_create"

[Skully17]

I have made the changes I suggested to in my last comment. They are based off of the changes that were made to the handling of registrations but seemed to have be forgotten to be applied to the handling of subscriptions. Please let me know if there is anything I've missed or is not quite right