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.

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

Topic branch renaming in bridge not consistent #40

Open
ralight opened this issue Mar 15, 2016 · 1 comment
Open

Topic branch renaming in bridge not consistent #40

ralight opened this issue Mar 15, 2016 · 1 comment

Comments

@ralight
Copy link
Contributor

ralight commented Mar 15, 2016

migrated from Bugzilla #473286
status NEW severity normal in component Mosquitto for 1.4
Reported in version 1.4 on platform PC
Assigned to: Roger Light

On 2015-07-22 10:07:25 -0400, Jan-Piet Mens wrote:

I'm trying to rename a topic branch from, say, owntracks/jpm/5s to RAL/name.

Using 1.4.2 and 1.5-devel from Git, I see the following when publishing to owntracks/jpm/5s:

a. Using topic # in 0 RAL/ owntracks/ clients get RAL/jpm/5s which is OK.
b. Usingtopic # in 0 RAL/two/ owntracks/jpm/ clients get RAL/two/5s which is OK.
c. Usingtopic # in 0 RAL/three/ owntracks/jpm/5s/clients get RAL/three/owntracks/jpm/5s which is a BUG.

The last is quite unexpected, and should work.

On 2015-07-22 10:13:04 -0400, Jan-Piet Mens wrote:

We're seeing this behaviour both for in' and forout' connections.

On 2015-07-25 18:02:50 -0400, Roger Light wrote:

The behaviour you're expecting on is that foo/# matches against foo. Bridge topic remapping is simpler than that I'm afraid. It is literally trying to remove "owntracks/jpm/5s/" from "owntracks/jpm/5s" and failing because the former isn't a substring of the latter.

I'm not sure what to do about it at the moment. Good choice of target topic name though.

@PierreF
Copy link
Contributor

PierreF commented Dec 29, 2017

We could add a case when topic is the remote/local prefix without the ending slash and do a substitution without the ending slash. This would fix this issue.

But I would like to suggest something slightly different: instead of just prepending the prefix as a string to the topic, I propose to consider the prefix as a hierarchy and prepend this hierarchy to the topic.
What this would means is that for both local and remote prefix, ending with or without a slash would be the same.

All of the following would yield the same result:

topic # 0 local/prefix/ remote/prefix/
topic # 0 local/prefix/ remote/prefix
topic # 0 local/prefix remote/prefix/
topic # 0 local/prefix remote/prefix

Under the hood, the ending slash if present will be str
Under the hood, the subscribe pattern would be prefix_with_ending_slash/topic and the substitution would be remote_prefix_without_ending_slash to local_prefix_without_ending_slash.

This would break rules that don't have slash between prefix and topic:

topic fix/# 0 local/pre remote/pre

The new proposed solution would subscribe to "remote/pre/fix/#" while currently is would subscribe to "remote/prefix/#".
Note that writing rule like this should probably no be used, as it could be better written like the very first rules (topic # 0 local/prefix/ remote/prefix/).

It would also break rules that don't have same slash on both side (remote vs local):

topic clients/total in 0 test/mosquitto/org $SYS/broker/

The new proposed solution would remap topic $SYS/broker/clients/total to test/mosquitto/org/clients/total while currently it is remapped to test/mosquitto/orgclients/total.

For this case, I'm think the proposed behavior is better (the example is taken from manpage and the manpage said that result is the proposed behavior).

If this proposed solution is accepted, I could work on implementing it for the develop branch. In the meantime, I've added tests (on fixes branch) to make sure no regression happen. Check that exhibit this issue is commented to avoid failing test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants