Add support for connection and subscription headers

[snaggen]

This is a first draft at supporting connection and subscription headers, to be able to use the ActiveMq stomp extentions
https://activemq.apache.org/stomp

Not sure about having both connect and connect_with_headers, but I didn't want to add them to the regular connect function since the headers are a corner case. Same with subscribe.

Also the to_client_msg will always have empty headers, just like for the Send... not sure how correct this is.

So, please have a look and see what you think.

[codecov]

Codecov Report

Attention: Patch coverage is 76.66667% with 35 lines in your changes missing coverage. Please review.

Project coverage is 67.31%. Comparing base (8bc7416) to head (487c0d5).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/client.rs	74.26%	35 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #2      +/-   ##
==========================================
+ Coverage   59.86%   67.31%   +7.45%     
==========================================
  Files           3        3              
  Lines         578      716     +138     
==========================================
+ Hits          346      482     +136     
- Misses        232      234       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[snaggen]

I have ported this to the latest main branch, and removed the _with_headers functions by using builders instead.
Not sure how to improve coverage, since tests requires a server.
But, let me know what you think about the builder approach

[snaggen]

Maybe @zuisong have an opinion on this? Is this something that is generally desirable, or is it just a weird corner case feature? And is builders better or worse than adding another connect_with_headers method?

[zuisong]

let connect = Message {
    content: ToServer::Connect {
        accept_version: "1.2".into(),
        host: virtualhost,
        login,
        passcode,
        heartbeat: None,
        headers, //<-- header at here
    },
    extra_headers: vec![], //<-- header appear again
};

Message<*> was designed to support extra_headers, no need to add headers property in each enum.

---

If you want to increase the test coverage, one possible solution is to use testcontainers , which starts an stomp server during the tests

[snaggen]

Thanks for the reply, I will have a look into this

[snaggen]

@zuisong I now use the extra_headers field, by letting the Message to_frame method add the extra headers if they are not already present in the frame.
I also did a rebase and squashed all commits in to one.

[zuisong]

I don't have any other questions.

To increase coverage, you can try doing this.

diff --git a/Cargo.toml b/Cargo.toml
index a68113d..9582438 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,3 +23,7 @@ typed-builder = "0.18.0"
 
 [dev-dependencies]
 tokio = { version = "1", features = ["time", "macros", "rt-multi-thread"] }
+testcontainers = { version = "0.15" }
+
+[features]
+integration-test = []
diff --git a/src/client.rs b/src/client.rs
index f9e4195..e5635c6 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -331,3 +331,40 @@ fn connection_message() {
 
     assert_eq!(expected_buffer, actual_buffer);
 }
+
+#[cfg(feature = "integration-test")]
+#[cfg(test)]
+mod integration_test {
+
+    use std::net::Ipv4Addr;
+    use testcontainers::{core::WaitFor, clients, GenericImage};
+    use crate::client;
+
+    #[tokio::test]
+    async fn test_connect() {
+        let docker = clients::Cli::default();
+        let activemq = docker.run(
+            GenericImage::new("apache/activemq-artemis", "latest-alpine").with_wait_for(
+                WaitFor::StdOutMessage {
+                    message: "0.0.0.0:8161".into(),
+                },
+            ),
+        );
+        dbg!(&activemq.ports());
+
+        let stomp_port = activemq.ports().map_to_host_port_ipv4(61613).unwrap();
+        dbg!(stomp_port);
+
+        // now you got stomp server at 127.0.0.1:${stomp_port}
+
+        //  write your test here
+        let mut conn = client::connect(
+            (Ipv4Addr::LOCALHOST, stomp_port),
+            "/".to_string(),
+            "artemis".to_string().into(),
+            "artemis".to_string().into(),
+        )
+        .await
+        .unwrap();
+    }
+}

[snaggen]

Well, I increased the coverage already, so I think the testcontainer is a bit overkill. But, it is good to have in mind if we really want to test the last pieces of the code. But, I leave that to @alexkunde to do that.

[snaggen]

Closing due to inactivity and this is included in my async-stomp fork