I marked this as 0.4, hopefully paho.mqtt new version will be released before our new version and we can get this one in. I'll review this PR asap

Seems like musl doesn't like paho.mqtt? Is there a way to fix it? I see this issue is open here: eclipse/paho.mqtt.rust#57

One thing I wanted with the integration between sdk-rust and mqtt is a crate supporting no_std (given we're also investing a lot of time in supporting no std #94). Is there any particular reason why @sbcd90 you chose paho.mqtt? Did you considered other mqtt clients supporting no_std?

I did evaluate https://github.com/bytebeamio/rumqtt . I can use any other library.
However, i went with paho mqtt because I was not sure of future maintenance guarantees for rumqtt library.

Please let me know if i should try switching the library.

A Paho release went out last week (v0.8).

Another (v0.9) is in active development and should be out by the end of the year. The initial purpose for the next release was to get HTTP/S proxy support for websocket connections, but a few other additions popped up, and now we're also going to push for better cross-compiling support and fully static linking. That includes (optional) static OpenSSL linking and getting that musl support stable.

@sbcd90 I think we can go ahead and work on this one, but let's be explicit we're supporting paho, so maybe rename it to cloudevents-sdk-paho-mqtt. Can you set the version to 0.8?

@sbcd90 I think we can go ahead and work on this one, but let's be explicit we're supporting paho, so maybe rename it to cloudevents-sdk-paho-mqtt. Can you set the version to 0.8?

@slinkydeveloper I have set version to 0.8

So i have some concerns about the whole v3/v5 difference, why did you defined an extension for it? do we need it?

Hi @slinkydeveloper , currently, v3 supports only structured mode. v5 supports both structured as well as binary mode according to spec.
This is because mqtt v3 has no support for headers while mqtt v5 has.
Now, in the current approach the default mode is structured mode which is supported by both v3 & v5. If binary mode needs to be used, then we need a flag to tell us if the message has to be deserialized using BinaryDeserializer.

The spec doesnt define how to set this mqtt version info.

Earlier, I was passing the flag as a parameter, now I pass it as an extension field. paho-mqtt passes version info as a param.
See

Another approach, we can follow is, ignore the mqtt version, & just trust the Content-Type header. This leaves us with a risk that we're depending on library api behavior a little bit.

match (
            self.headers
                .get(headers::CONTENT_TYPE)
                .map(|s| String::from_utf8(s.to_vec()).ok())
                .flatten()
                .map(|s| s.starts_with(headers::CLOUDEVENTS_JSON_HEADER))
                .unwrap_or(false),
            self.headers.len() > 0,
        ) {
            // v5 & structured
            (true, true) => Encoding::STRUCTURED,
            // v3 & structured
            (false, false) => Encoding::STRUCTURED,
            // v5 & binary
            (false, true) => Encoding::BINARY,
            _ => Encoding::UNKNOWN
        }

Kindly let me know according to you which is the best approach.

Ok so one step at the time.

MQTT -> CloudEvent

First of all, i think you should remove the headers map you create and you should directly access to the Property borrow, in order to avoid allocating intermediate memory, which in this case it's not required.

For figuring out v3 vs v5 messages, I think we should simply do best effort: from the paho.mqtt rustdocs i see that you can always get the properties from a message https://docs.rs/paho-mqtt/0.8.0/paho_mqtt/message/struct.Message.html#method.properties and in case the UserProperty is empty, then we can assume the properties are not there so it's not mqtt v5 (hence we should just deserialize as structured).

CloudEvent -> MQTT

In this case, we should have an additional input in the serialization that explicits the mqtt version, like an enum MqttVersion created by us with MQTT_3 and MQTT_5 variants. I thought there was such enum in paho.mqtt, but there isn't. No extensions, the event must not be mutated.

Also note that i don't expect the user providing MessageRecord, this should be done under the hood:

pub trait MessageBuilderExt {
    fn event(self, Event, MqttVersion) -> MessageBuilder;
}

And the implementation would look like:

impl MessageBuilderExt for MessageBuilder {
    fn event(self, Event, MqttVersion) -> MessageBuilder {
        // mutate directly MessageBuilder here using MessageRecord
    }
}

Check out the actix-web, because it's a better one (I see that you probably looked at the MessageRecord thingy from Kafka, but that's kinda bad because of the rdkafka apis themselves)

Does that makes sense? Did I understood the problems here?

Hi @slinkydeveloper , Thanks a lot for your detailed review. Refactored the code accordingly.
Added a sample README.

Can you please have a look now?

hi @slinkydeveloper , i fixed the remaining issues.
For musl, i get the error

Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

@sbcd90 try to modify the github action here: https://github.com/cloudevents/sdk-rust/blob/master/.github/workflows/rust_tests.yml#L31 adding openssl dev package setup

hi @slinkydeveloper , the builds are fixed now.