rumqttc: unable to specify subscription options in v5

[swanandx]

The current API of v5 client doesn't let us specify subscription options while subscribing.

• rumqttc version: "0.22.0"

Context

MQTTv5 introduces subscription options. This options like preserve_retain, retain_forward_rule are preset in Filter defined here:

pub struct Filter {
    pub path: String,
    pub qos: QoS,
    pub nolocal: bool,
    pub preserve_retain: bool,
    pub retain_forward_rule: RetainForwardRule,
}

but currently we can't pass this options to Filter::new(_), instead it uses default values:

impl Filter {
    pub fn new<T: Into<String>>(topic: T, qos: QoS) -> Self {
        Self {
            path: topic.into(),
            qos,
            ..Default::default()
        }
    }
    //...
}

we should be able to pass down those options while creating Filter.

Possible solutions ideas / challenges

We can either just add more arguments to new(_) or use a builder pattern for creating Filter, whichever is better.

We also need to think upon how the users will specify them:

• Do we pass this subscription options as arguments to subscribe(_)? ( would make API very verbose unnecessary as most users won't use these )
• Do we introduce a separate function? ( the we will have way too many fns just for subscribing haha ).

What I would prefer is: Instead of passing `topic`, `qos` to `subscribe(_)`, user will build `Filter` using builder pattern and the pass it to `subscribe(filter: Filter)`. Something like:

- pub async fn subscribe<S: Into<String>>(&self, topic: S, qos: QoS) -> Result<(), ClientError> {
+ pub async fn subscribe(&self, filter: Filter) -> Result<(), ClientError> {

Please feel free to comment your ideas / suggestions :rocket:

[ecarrara]

@swanandx I made a PR that:

• Create the FilterBuilder
• Refactor the subscribe fns to receive a instance of Filter

It looks like a big breaking change. I'm not sure if this is the best way to do that, but I liked how the API looks receiving a Filter instance built with the builder pattern:

client
    .subscribe(
        Filter::builder()
            .path("hello/world")
            .qos(Some(QoS::AtLeastOnce))
            .build(),
    )
    .await
    .unwrap();

[swanandx]

hey @ecarrara , thank you so much for the PR! ( I didn't expect PR this fast haha :rocket: )

It looks like a big breaking change

Yes, I do agree with that. We are planning to reconsider the APIs and integrate builder pattern wherever required in rumqttc, then we can introduce breaking changes at once, but currently we are totally occupied with rumqttd related tasks :sweat_smile: , so it might take a while to review #698 .

Thank you for understanding.

[brianmay]

It is possible to construct a Filter object by hand, the values are public. But unfortunately the RetainForwardRule type is in the subscribe.rs which is not public, and as a result it is not possible to create the retain_forward_rule value.

[drauschenbach]

It is possible to construct a Filter object by hand, the values are public. But unfortunately the RetainForwardRule type is in the subscribe.rs which is not public, and as a result it is not possible to create the retain_forward_rule value.

And likewise the Strategy enum is private, even though it is used in the public member RouterConfig.shared_subscriptions_strategy, which is interfering with my ability to construct a RouterConfig from a unit test.

[swanandx]

Hey @drauschenbach , good catch! would you like to open PR to solve the issue? ( just need to add pub use Strategy in router/mod.rs ig )

Otherwise, I will do it first thing tomorrow!

Thanks 😁