search

crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 274 forks source link

Fix proxy controller has_realm() logic #1968

Closed om26er closed 2 years ago

om26er commented 2 years ago

ProxyController.has_realm(), checks if the route for a realm exists or not. Initially that function works correctly when no route exists, however when a route is started and later stopped, that function still returns True because the code only checks for the existence of a key. It also needs to check if the key returns an empty dictionary or not.

Here is the relevant part which actually removes a route https://github.com/crossbario/crossbar/blob/e88876f47f12a830d76d225723e0b8d13fdc589c/crossbar/worker/proxy.py#L1653.

oberstet commented 2 years ago

actually, the PR seems fine, as a "having a realm" == "having at least one route configured for that realm"

https://github.com/crossbario/crossbar/blob/e88876f47f12a830d76d225723e0b8d13fdc589c/crossbar/worker/proxy.py#L1157 https://github.com/crossbario/crossbar/blob/e88876f47f12a830d76d225723e0b8d13fdc589c/crossbar/worker/proxy.py#L1591

so the only thing we might want to add is this hint to the docs of "has_realm": "having a realm" == "having at least one route configured for that realm"

om26er commented 2 years ago

ok, in that case, we can close the PR.

The discrepancy here is that the proxy worker has very basic knowledge of a realm. So when both the proxy connection and the proxy realm route is stopped, then at that point, the proxy worker should just forget the realm. However currently it tells the application code something like

wamp.error.no_such_role message=realm "omer-home" has no role "owner"

So basically when the route is stopped, it "forgets" the role but not the realm itself. expected ?

oberstet commented 2 years ago

no, sorry for the confusion: the PR is fine after the change you did, as:

"having a realm" == "having at least one route configured for that realm"

om26er commented 2 years ago

ah, just saw the other comment, I can do the doc fix

om26er commented 2 years ago

Looking at it again, I think the current doc string is already clear enough

        """
        Check if a route to a realm with the given name is currently running.

        :param realm: Realm name (the WAMP name, _not_ the run-time object ID).
        :type realm: str

        :returns: True if a route to the realm (for any role) exists.
        :rtype: bool
        """