From b9065a75f4ec9951d1d86ebc7b10329e21d32a20 Mon Sep 17 00:00:00 2001 From: Simon Chen Date: Sat, 23 Dec 2023 19:48:05 +0000 Subject: [PATCH] Improve bytes serde. --- .github/workflows/regression_test.yml | 2 +- .github/workflows/test_coverage.yml | 2 +- Cargo.lock | 52 ++++++++++++++------ Cargo.toml | 4 ++ amqp_serde/Cargo.toml | 1 + amqp_serde/src/de.rs | 6 +-- amqp_serde/src/types.rs | 68 +-------------------------- amqprs/Cargo.toml | 1 + amqprs/src/api/channel/basic.rs | 1 - amqprs/src/api/channel/confim.rs | 2 +- amqprs/src/api/channel/mod.rs | 6 +-- amqprs/src/api/channel/tx.rs | 2 +- amqprs/src/api/connection.rs | 14 +++--- amqprs/src/frame/content_body.rs | 1 + 14 files changed, 61 insertions(+), 101 deletions(-) diff --git a/.github/workflows/regression_test.yml b/.github/workflows/regression_test.yml index 8e42d42..1e9c979 100644 --- a/.github/workflows/regression_test.yml +++ b/.github/workflows/regression_test.yml @@ -13,7 +13,7 @@ on: - 'prepare_release.sh' - 'regression_test.sh' workflow_dispatch: - branches: ["main"] + pull_request: branches: ["main"] paths-ignore: diff --git a/.github/workflows/test_coverage.yml b/.github/workflows/test_coverage.yml index 0642a9f..0d5de6e 100644 --- a/.github/workflows/test_coverage.yml +++ b/.github/workflows/test_coverage.yml @@ -13,7 +13,7 @@ on: - 'prepare_release.sh' - 'regression_test.sh' workflow_dispatch: - branches: ["main"] + pull_request: branches: ["main"] paths-ignore: diff --git a/Cargo.lock b/Cargo.lock index 3ebc450..f39c691 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,6 +56,7 @@ version = "0.4.0" dependencies = [ "bytes", "serde", + "serde_bytes_ng", ] [[package]] @@ -67,6 +68,7 @@ dependencies = [ "bytes", "rustls-pemfile", "serde", + "serde_bytes_ng", "tokio", "tokio-rustls", "tracing", @@ -188,7 +190,7 @@ checksum = "1e805d94e6b5001b651426cf4cd446b1ab5f319d27bab5c644f61de0a804360c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.103", ] [[package]] @@ -983,7 +985,7 @@ checksum = "069bdb1e05adc7a8990dce9cc75370895fbe4e3d58b9b73bf1aee56359344a55" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.103", ] [[package]] @@ -1048,18 +1050,18 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.47" +version = "1.0.71" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ea3d908b0e36316caf9e9e2c4625cdde190a7e6f440d794667ed17a1855e725" +checksum = "75cb1540fadbd5b8fbccc4dddad2734eba435053f725621c070711a14bb5f4b8" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.21" +version = "1.0.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bbe448f377a7d6961e30f5955f9b8d106c3f5e449d493ee1b125c1d43c2b5179" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" dependencies = [ "proc-macro2", ] @@ -1264,22 +1266,31 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.147" +version = "1.0.193" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d193d69bae983fc11a79df82342761dfbf28a99fc8d203dca4c3c1b590948965" +checksum = "25dd9975e68d0cb5aa1120c288333fc98731bd1dd12f561e468ea4728c042b89" dependencies = [ "serde_derive", ] +[[package]] +name = "serde_bytes_ng" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bdb0ebce8684e2253f964e8b6ce51f0ccc6666bbb448fb4a6788088bda6544b6" +dependencies = [ + "serde", +] + [[package]] name = "serde_derive" -version = "1.0.147" +version = "1.0.193" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f1d362ca8fc9c3e3a7484440752472d68a6caa98f1ab81d99b5dfe517cec852" +checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.42", ] [[package]] @@ -1379,6 +1390,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "2.0.42" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b7d0a2c048d661a1a59fcd7355baa232f7ed34e0ee4df2eef3c1c1c0d3852d8" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "tcp-stream" version = "0.24.4" @@ -1470,7 +1492,7 @@ checksum = "9724f9a975fb987ef7a3cd9be0350edcbe130698af5b8f7a631e23d42d052484" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.103", ] [[package]] @@ -1529,7 +1551,7 @@ checksum = "4017f8f45139870ca7e672686113917c71c7a6e02d4924eda67186083c03081a" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.103", ] [[package]] @@ -1681,7 +1703,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 1.0.103", "wasm-bindgen-shared", ] @@ -1703,7 +1725,7 @@ checksum = "07bc0c051dc5f23e307b13285f9d75df86bfdf816c5721e573dec1f9b8aa193c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.103", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/Cargo.toml b/Cargo.toml index c3ba6b9..5acbd66 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,3 +9,7 @@ resolver = "2" # debug = false # lto = true # codegen-units = 1 + + +[workspace.dependencies] +serde_bytes_ng = { version = "0.1.1" } diff --git a/amqp_serde/Cargo.toml b/amqp_serde/Cargo.toml index 3321a63..4c29485 100644 --- a/amqp_serde/Cargo.toml +++ b/amqp_serde/Cargo.toml @@ -14,5 +14,6 @@ documentation = "https://docs.rs/amqp_serde/latest/amqp_serde/" [dependencies] serde = { version = "1.0", features = ["derive"] } bytes = { version = "1.0" } +serde_bytes_ng = { workspace = true } [dev-dependencies] diff --git a/amqp_serde/src/de.rs b/amqp_serde/src/de.rs index 000ded4..6026252 100644 --- a/amqp_serde/src/de.rs +++ b/amqp_serde/src/de.rs @@ -737,7 +737,7 @@ mod tests { fn test_eof() { #[derive(Deserialize)] struct Frame(u32); - let input = vec![0x00]; + let input = [0x00]; let _: Frame = from_bytes(&input[..]).unwrap(); } @@ -746,7 +746,7 @@ mod tests { fn test_missing_length() { #[derive(Deserialize)] struct Frame(Vec); - let input = vec![0, 1, 2]; + let input = [0, 1, 2]; let _: Frame = from_bytes(&input[..]).unwrap(); } @@ -755,7 +755,7 @@ mod tests { fn test_syntax_err() { #[derive(Deserialize)] struct Frame(u8, Vec); - let input = vec![9, 0, 0]; + let input = [9, 0, 0]; let _: Frame = from_bytes(&input[..]).unwrap(); } diff --git a/amqp_serde/src/types.rs b/amqp_serde/src/types.rs index 9adcfc1..6b68fdc 100644 --- a/amqp_serde/src/types.rs +++ b/amqp_serde/src/types.rs @@ -169,7 +169,7 @@ impl fmt::Display for DecimalValue { /// AMQP byte array type. #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] -pub struct ByteArray(LongUint, Vec); +pub struct ByteArray(LongUint, #[serde(with = "serde_bytes_ng")] Vec); impl TryFrom> for ByteArray { type Error = TryFromIntError; @@ -492,72 +492,6 @@ impl AsRef> for FieldTable { } } -///////////////////////////////////////////////////////////////////////////// -// #[derive(Serialize, Deserialize, PartialEq, Debug, Clone)] -// pub struct FieldTable(HashMap); -// impl Default for FieldTable { -// fn default() -> Self { -// Self(HashMap::new()) -// } -// } - -// impl Deref for FieldTable { -// type Target = HashMap; - -// fn deref(&self) -> &Self::Target { -// &self.0 -// } -// } - -// impl FieldTable { -// pub fn new() -> Self { -// Self(HashMap::new()) -// } - -// pub fn insert(&mut self, k: String, v: V) -// where -// V: Any, -// { -// let k: FieldName = k.try_into().unwrap(); -// let v = &v as &dyn Any; - -// if v.is::() { -// let v = v.downcast_ref::().unwrap(); -// let old = self.0.insert(k, FieldValue::t(v.clone())); -// } else if v.is::() { -// let v = v.downcast_ref::().unwrap(); -// let old = self.0.insert(k, FieldValue::b(v.clone())); -// } else if v.is::() { -// let v = v.downcast_ref::().unwrap(); -// let old = self.0.insert(k, FieldValue::B(v.clone())); -// } else if v.is::() { -// let v = v.downcast_ref::().unwrap(); -// let old = self.0.insert(k, FieldValue::t(v.clone())); -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// } else if v.is::() { -// // RabbitMQ does not have "short string" type in field value, -// let v = v.downcast_ref::().unwrap(); -// let old = self -// .0 -// .insert(k, FieldValue::S(v.clone().try_into().unwrap())); -// } else if v.is::() { -// } else if v.is::() { // RabbitMQ do not have "Unsigned 64-bit" field value, so `u64` can be uniquely mapped to TimeStamp -// } else if v.is::() { -// } else if v.is::<()>() { -// } else if v.is::() { -// } else { -// panic!("unsupported value type {:?} ", v); -// } - -// } -// } - ///////////////////////////////////////////////////////////////////////////// // AMQP domains /// Note: it is different from definition in [`RabbitMQ Definition`]. diff --git a/amqprs/Cargo.toml b/amqprs/Cargo.toml index c7b9811..c03831b 100644 --- a/amqprs/Cargo.toml +++ b/amqprs/Cargo.toml @@ -42,6 +42,7 @@ amqp_serde = { path = "../amqp_serde", version = "0.4" } async-trait = "0.1" tracing = { version = "0.1", optional = true } uriparse = { version = "0.6", optional = true } +serde_bytes_ng = { workspace = true } # SSL/TLS dependencies tokio-rustls = { version = "0.23", optional = true } diff --git a/amqprs/src/api/channel/basic.rs b/amqprs/src/api/channel/basic.rs index 07b6c8f..b37fa6d 100644 --- a/amqprs/src/api/channel/basic.rs +++ b/amqprs/src/api/channel/basic.rs @@ -970,7 +970,6 @@ mod tests { consumer::DefaultConsumer, }, frame::BasicProperties, - DELIVERY_MODE_TRANSIENT, }; use tokio::time; diff --git a/amqprs/src/api/channel/confim.rs b/amqprs/src/api/channel/confim.rs index f0db8f5..a9b1e5d 100644 --- a/amqprs/src/api/channel/confim.rs +++ b/amqprs/src/api/channel/confim.rs @@ -64,7 +64,7 @@ mod tests { channel::BasicPublishArguments, connection::{Connection, OpenConnectionArguments}, test_utils::setup_logging, - BasicProperties, DELIVERY_MODE_TRANSIENT, + BasicProperties, }; use super::ConfirmSelectArguments; diff --git a/amqprs/src/api/channel/mod.rs b/amqprs/src/api/channel/mod.rs index 35d3837..8bb21df 100644 --- a/amqprs/src/api/channel/mod.rs +++ b/amqprs/src/api/channel/mod.rs @@ -424,7 +424,7 @@ mod tests { #[ignore = "https://github.com/gftea/amqprs/issues/69"] #[tokio::test] - async fn test_channel_is_not_cloneable() { + async fn test_channel_cloneable() { // default: `IS_CLONEABLE = false` for all types trait NotCloneable { const IS_CLONEABLE: bool = false; @@ -438,8 +438,8 @@ mod tests { impl Wrapper { const IS_CLONEABLE: bool = true; } - - assert_eq!(false, >::IS_CLONEABLE); + // Prevent clippy to report assertion on const value. + assert_eq!(>::IS_CLONEABLE.to_string(), "true"); } #[tokio::test] diff --git a/amqprs/src/api/channel/tx.rs b/amqprs/src/api/channel/tx.rs index 894d60a..2908ff5 100644 --- a/amqprs/src/api/channel/tx.rs +++ b/amqprs/src/api/channel/tx.rs @@ -96,7 +96,7 @@ mod tests { callbacks::{DefaultChannelCallback, DefaultConnectionCallback}, channel::BasicPublishArguments, connection::{Connection, OpenConnectionArguments}, - BasicProperties, DELIVERY_MODE_TRANSIENT, + BasicProperties, }; #[tokio::test] diff --git a/amqprs/src/api/connection.rs b/amqprs/src/api/connection.rs index 3e8ce1d..bd7acc9 100644 --- a/amqprs/src/api/connection.rs +++ b/amqprs/src/api/connection.rs @@ -607,13 +607,11 @@ impl Connection { FieldValue::S("0.1".try_into().unwrap()), ); let mut client_properties_capabilities = FieldTable::new(); - client_properties_capabilities.insert( - "consumer_cancel_notify".try_into().unwrap(), - true.into() - ); + client_properties_capabilities + .insert("consumer_cancel_notify".try_into().unwrap(), true.into()); client_properties.insert( "capabilities".try_into().unwrap(), - FieldValue::F(client_properties_capabilities) + FieldValue::F(client_properties_capabilities), ); // S: `Start` C: `StartOk` @@ -1301,9 +1299,9 @@ mod tests { let conn1 = Connection::open(&args).await.unwrap(); let conn2 = conn1.clone(); tokio::spawn(async move { - assert_eq!(true, conn2.is_open()); + assert!(conn2.is_open()); }); - assert_eq!(true, conn1.is_open()); + assert!(conn1.is_open()); } // wait for finished, otherwise runtime exit before all tasks are done time::sleep(time::Duration::from_millis(100)).await; @@ -1348,7 +1346,7 @@ mod tests { jh.push(thread::spawn(|| generate_connection_name("testdomain"))); } for h in jh { - assert_eq!(true, res.insert(h.join().unwrap())); + assert!(res.insert(h.join().unwrap())); } } diff --git a/amqprs/src/frame/content_body.rs b/amqprs/src/frame/content_body.rs index a73a5b3..3c83233 100644 --- a/amqprs/src/frame/content_body.rs +++ b/amqprs/src/frame/content_body.rs @@ -4,6 +4,7 @@ use super::Frame; #[derive(Debug, Serialize)] pub struct ContentBody { + #[serde(with = "serde_bytes_ng")] pub(crate) inner: Vec, }