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

Update subscription option handling to account no local #814

Merged

Conversation

andsel
Copy link
Collaborator

@andsel andsel commented Feb 3, 2024

Release notes

Implements handling of noLocal subscription option on MQTT5 connections.

What does this PR do?

  • Updates the Callable signature used by routing to return Void instead of String.
  • Updates the publishing of messages to subscribers to exclude or not from the target the clientId of the sender, depending on the status of noLocal in the target subscription.
  • Added integration test to proof the change

Why is it important/What is the impact to the user?

noLocal option is handled.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the Changelog if it's a feature or a fix that has to be reported

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

@andsel andsel self-assigned this Feb 3, 2024
@andsel andsel mentioned this pull request Feb 3, 2024
5 tasks
Comment on lines +685 to +693
if (sub.option().isNoLocal()) {
if (publisherClientId.equals(sub.getClientId())) {
// if noLocal do not publish to the publisher
continue;
}
collector.add(sub);
} else {
collector.add(sub);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main part of the change.

@andsel andsel marked this pull request as ready for review February 3, 2024 10:34
Copy link
Collaborator Author

@andsel andsel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andsel andsel requested a review from hylkevds February 3, 2024 10:39
@andsel
Copy link
Collaborator Author

andsel commented Feb 3, 2024

@hylkevds sorry for the review ping, I thought that assigning a reviewer would trigger the CI.

@andsel andsel removed the request for review from hylkevds February 3, 2024 10:41
@andsel andsel changed the title Upadate subscription option handling to account no local Update subscription option handling to account no local Feb 3, 2024
@andsel andsel merged commit e0d0609 into moquette-io:main Feb 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant