From 7ddd57a4d435299fbbfb5bcd33dcf8ef93cdcf45 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Sat, 18 Feb 2023 12:24:15 +0100 Subject: [PATCH 1/8] types: make write_bytes_opt more generic Now, write_bytes_opt is able to serialize an Option where T: AsRef<[u8]> - for example, you can now serialize Option> but also Option<&[u8]>. --- scylla-cql/src/frame/types.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scylla-cql/src/frame/types.rs b/scylla-cql/src/frame/types.rs index 86acb42781..fa47087247 100644 --- a/scylla-cql/src/frame/types.rs +++ b/scylla-cql/src/frame/types.rs @@ -302,11 +302,14 @@ pub fn write_bytes(v: &[u8], buf: &mut impl BufMut) -> Result<(), ParseError> { Ok(()) } -pub fn write_bytes_opt(v: Option<&Vec>, buf: &mut impl BufMut) -> Result<(), ParseError> { +pub fn write_bytes_opt( + v: Option>, + buf: &mut impl BufMut, +) -> Result<(), ParseError> { match v { Some(bytes) => { - write_int_length(bytes.len(), buf)?; - buf.put_slice(bytes); + write_int_length(bytes.as_ref().len(), buf)?; + buf.put_slice(bytes.as_ref()); } None => write_int(-1, buf), } From d0b5888b72df6e9aea62a4960c13e70a8476d3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 7 May 2024 08:19:50 +0200 Subject: [PATCH 2/8] types: return std::io::Error instead of ParseError It makes no sense for the functions to return broader error type while only a proper subset of it can be returned. In the particular case, std::io::Error is needlessly converted to ParseError, which makes pattern matching required in the caller in order to get the std::io::Error back. The functions are changed to return std::io::Error. The previous behaviour of propagating the error as ParseError can be easily achieved using ? operator in the caller: ```rust fn i_return_parse_error(buf: &mut &[u8]) -> Result<(), ParseError) { read_int(buf)?; Ok(()) } ``` --- scylla-cql/src/frame/types.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/scylla-cql/src/frame/types.rs b/scylla-cql/src/frame/types.rs index fa47087247..de311ac63c 100644 --- a/scylla-cql/src/frame/types.rs +++ b/scylla-cql/src/frame/types.rs @@ -173,7 +173,7 @@ fn read_raw_bytes<'a>(count: usize, buf: &mut &'a [u8]) -> Result<&'a [u8], Pars Ok(ret) } -pub fn read_int(buf: &mut &[u8]) -> Result { +pub fn read_int(buf: &mut &[u8]) -> Result { let v = buf.read_i32::()?; Ok(v) } @@ -206,7 +206,7 @@ fn type_int() { } } -pub fn read_long(buf: &mut &[u8]) -> Result { +pub fn read_long(buf: &mut &[u8]) -> Result { let v = buf.read_i64::()?; Ok(v) } @@ -225,7 +225,7 @@ fn type_long() { } } -pub fn read_short(buf: &mut &[u8]) -> Result { +pub fn read_short(buf: &mut &[u8]) -> Result { let v = buf.read_u16::()?; Ok(v) } @@ -234,7 +234,7 @@ pub fn write_short(v: u16, buf: &mut impl BufMut) { buf.put_u16(v); } -pub(crate) fn read_short_length(buf: &mut &[u8]) -> Result { +pub(crate) fn read_short_length(buf: &mut &[u8]) -> Result { let v = read_short(buf)?; let v: usize = v.into(); Ok(v) @@ -514,9 +514,12 @@ fn type_string_multimap() { pub fn read_uuid(buf: &mut &[u8]) -> Result { let raw = read_raw_bytes(16, buf)?; - // It's safe to unwrap here because Uuid::from_slice only fails - // if the argument slice's length is not 16. - Ok(Uuid::from_slice(raw).unwrap()) + // It's safe to unwrap here because the conversion only fails + // if the argument slice's length does not match, which + // `read_raw_bytes` prevents. + let raw_array: &[u8; 16] = raw.try_into().unwrap(); + + Ok(Uuid::from_bytes(*raw_array)) } pub fn write_uuid(uuid: &Uuid, buf: &mut impl BufMut) { @@ -649,7 +652,7 @@ fn unsigned_vint_encode(v: u64, buf: &mut Vec) { buf.put_uint(v, number_of_bytes as usize) } -fn unsigned_vint_decode(buf: &mut &[u8]) -> Result { +fn unsigned_vint_decode(buf: &mut &[u8]) -> Result { let first_byte = buf.read_u8()?; let extra_bytes = first_byte.leading_ones() as usize; @@ -671,7 +674,7 @@ pub(crate) fn vint_encode(v: i64, buf: &mut Vec) { unsigned_vint_encode(zig_zag_encode(v), buf) } -pub(crate) fn vint_decode(buf: &mut &[u8]) -> Result { +pub(crate) fn vint_decode(buf: &mut &[u8]) -> Result { unsigned_vint_decode(buf).map(zig_zag_decode) } From 165e8a86381008d5f57864be0ca91d82080ae5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Wed, 8 May 2024 08:37:27 +0200 Subject: [PATCH 3/8] scylla-cql: add assert_matches as dev-dep This will be helpful in testing deserialization framework, and will also be used in serialization instead of assert!(matches!(...)). As a bonus, Cargo.toml of all crates was auto formatted. --- Cargo.lock.msrv | 1 + scylla-cql/Cargo.toml | 12 ++++++++++-- scylla-macros/Cargo.toml | 4 ++-- scylla-proxy/Cargo.toml | 10 +++++++++- scylla/Cargo.toml | 29 +++++++++++++++++++++++++---- 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/Cargo.lock.msrv b/Cargo.lock.msrv index 776cc09f56..4ddbe9dae1 100644 --- a/Cargo.lock.msrv +++ b/Cargo.lock.msrv @@ -1524,6 +1524,7 @@ dependencies = [ name = "scylla-cql" version = "0.2.0" dependencies = [ + "assert_matches", "async-trait", "bigdecimal", "byteorder", diff --git a/scylla-cql/Cargo.toml b/scylla-cql/Cargo.toml index 75753dded7..b81ecb69c9 100644 --- a/scylla-cql/Cargo.toml +++ b/scylla-cql/Cargo.toml @@ -28,7 +28,8 @@ serde = { version = "1.0", features = ["derive"], optional = true } time = { version = "0.3", optional = true } [dev-dependencies] -criterion = "0.4" # Note: v0.5 needs at least rust 1.70.0 +assert_matches = "1.5.0" +criterion = "0.4" # Note: v0.5 needs at least rust 1.70.0 # Use large-dates feature to test potential edge cases time = { version = "0.3.21", features = ["large-dates"] } @@ -43,7 +44,14 @@ chrono = ["dep:chrono"] num-bigint-03 = ["dep:num-bigint-03"] num-bigint-04 = ["dep:num-bigint-04"] bigdecimal-04 = ["dep:bigdecimal-04"] -full-serialization = ["chrono", "time", "secret", "num-bigint-03", "num-bigint-04", "bigdecimal-04"] +full-serialization = [ + "chrono", + "time", + "secret", + "num-bigint-03", + "num-bigint-04", + "bigdecimal-04", +] [lints.rust] unreachable_pub = "warn" diff --git a/scylla-macros/Cargo.toml b/scylla-macros/Cargo.toml index 9d015b50ea..2b3b954966 100644 --- a/scylla-macros/Cargo.toml +++ b/scylla-macros/Cargo.toml @@ -14,8 +14,8 @@ proc-macro = true [dependencies] darling = "0.20.0" syn = "2.0" -quote = "1.0" +quote = "1.0" proc-macro2 = "1.0" [lints.rust] -unreachable_pub = "warn" \ No newline at end of file +unreachable_pub = "warn" diff --git a/scylla-proxy/Cargo.toml b/scylla-proxy/Cargo.toml index 704c81061d..694fb32a58 100644 --- a/scylla-proxy/Cargo.toml +++ b/scylla-proxy/Cargo.toml @@ -17,7 +17,15 @@ scylla-cql = { version = "0.2.0", path = "../scylla-cql" } byteorder = "1.3.4" bytes = "1.2.0" futures = "0.3.6" -tokio = { version = "1.12", features = ["net", "time", "io-util", "sync", "rt", "macros", "rt-multi-thread"] } +tokio = { version = "1.12", features = [ + "net", + "time", + "io-util", + "sync", + "rt", + "macros", + "rt-multi-thread", +] } uuid = "1.0" thiserror = "1.0.32" bigdecimal = "0.4" diff --git a/scylla/Cargo.toml b/scylla/Cargo.toml index 028406f6ca..5c4667c95e 100644 --- a/scylla/Cargo.toml +++ b/scylla/Cargo.toml @@ -16,14 +16,28 @@ rustdoc-args = ["--cfg", "docsrs"] [features] default = [] ssl = ["dep:tokio-openssl", "dep:openssl"] -cloud = ["ssl", "scylla-cql/serde", "dep:serde_yaml", "dep:serde", "dep:url", "dep:base64"] +cloud = [ + "ssl", + "scylla-cql/serde", + "dep:serde_yaml", + "dep:serde", + "dep:url", + "dep:base64", +] secret = ["scylla-cql/secret"] chrono = ["scylla-cql/chrono"] time = ["scylla-cql/time"] num-bigint-03 = ["scylla-cql/num-bigint-03"] num-bigint-04 = ["scylla-cql/num-bigint-04"] bigdecimal-04 = ["scylla-cql/bigdecimal-04"] -full-serialization = ["chrono", "time", "secret", "num-bigint-03", "num-bigint-04", "bigdecimal-04"] +full-serialization = [ + "chrono", + "time", + "secret", + "num-bigint-03", + "num-bigint-04", + "bigdecimal-04", +] [dependencies] scylla-macros = { version = "0.5.0", path = "../scylla-macros" } @@ -33,7 +47,14 @@ bytes = "1.0.1" futures = "0.3.6" hashbrown = "0.14" histogram = "0.6.9" -tokio = { version = "1.34", features = ["net", "time", "io-util", "sync", "rt", "macros"] } +tokio = { version = "1.34", features = [ + "net", + "time", + "io-util", + "sync", + "rt", + "macros", +] } snap = "1.0" uuid = { version = "1.0", features = ["v4"] } rand = "0.8.3" @@ -62,7 +83,7 @@ num-bigint-04 = { package = "num-bigint", version = "0.4" } bigdecimal-04 = { package = "bigdecimal", version = "0.4" } scylla-proxy = { version = "0.0.3", path = "../scylla-proxy" } ntest = "0.9.0" -criterion = "0.4" # Note: v0.5 needs at least rust 1.70.0 +criterion = "0.4" # Note: v0.5 needs at least rust 1.70.0 tokio = { version = "1.27", features = ["test-util"] } tracing-subscriber = { version = "0.3.14", features = ["env-filter"] } assert_matches = "1.5.0" From 8065379093283c7fcb8ecbe79146c4bb06a8477f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 21 May 2024 09:00:43 +0200 Subject: [PATCH 4/8] codewide: ditch assert!(matches!(...)) As assert_matches!(...) provide superior experience (providing helpful debugging messages with how the mismatched type looked like), all assert!(matches!(...)) occurences across the code were changed to assert_matches!(...). --- scylla-cql/src/frame/value_tests.rs | 6 +- scylla-cql/src/types/serialize/row.rs | 43 ++++---- scylla-cql/src/types/serialize/value.rs | 132 ++++++++++++------------ scylla/src/history.rs | 5 +- scylla/src/transport/query_result.rs | 14 +-- 5 files changed, 99 insertions(+), 101 deletions(-) diff --git a/scylla-cql/src/frame/value_tests.rs b/scylla-cql/src/frame/value_tests.rs index f431aaacce..f15c5793fd 100644 --- a/scylla-cql/src/frame/value_tests.rs +++ b/scylla-cql/src/frame/value_tests.rs @@ -10,6 +10,8 @@ use super::value::{ CqlDate, CqlDuration, CqlTime, CqlTimestamp, LegacyBatchValues, LegacySerializedValues, MaybeUnset, SerializeValuesError, Unset, Value, ValueList, ValueTooBig, }; +#[cfg(test)] +use assert_matches::assert_matches; use bytes::BufMut; use std::collections::hash_map::DefaultHasher; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; @@ -1262,7 +1264,7 @@ fn serialized_values_value_list() { ser_values.add_value(&"qwertyuiop").unwrap(); let ser_ser_values: Cow = ser_values.serialized().unwrap(); - assert!(matches!(ser_ser_values, Cow::Borrowed(_))); + assert_matches!(ser_ser_values, Cow::Borrowed(_)); assert_eq!(&ser_values, ser_ser_values.as_ref()); } @@ -1272,7 +1274,7 @@ fn cow_serialized_values_value_list() { let cow_ser_values: Cow = Cow::Owned(LegacySerializedValues::new()); let serialized: Cow = cow_ser_values.serialized().unwrap(); - assert!(matches!(serialized, Cow::Borrowed(_))); + assert_matches!(serialized, Cow::Borrowed(_)); assert_eq!(cow_ser_values.as_ref(), serialized.as_ref()); } diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index c8f04cc6bc..2e14b75b3e 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -865,6 +865,7 @@ mod tests { }; use super::SerializedValues; + use assert_matches::assert_matches; use scylla_macros::SerializeRow; fn col_spec(name: &str, typ: ColumnType) -> ColumnSpec { @@ -1044,13 +1045,13 @@ mod tests { let err = do_serialize_err(v, &spec); let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::<()>()); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { actual: 0, asked_for: 1, } - )); + ); // Non-unit tuple // Count mismatch @@ -1059,13 +1060,13 @@ mod tests { let err = do_serialize_err(v, &spec); let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::<(&str,)>()); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { actual: 1, asked_for: 2, } - )); + ); // Serialization of one of the element fails let v = ("Ala ma kota", 123_i32); @@ -1086,13 +1087,13 @@ mod tests { let err = do_serialize_err(v, &spec); let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::>()); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { actual: 1, asked_for: 2, } - )); + ); // Serialization of one of the element fails let v = vec!["Ala ma kota", "Kot ma pchły"]; @@ -1214,10 +1215,10 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. } - )); + ); let spec_duplicate_column = [ col("a", ColumnType::Text), @@ -1232,10 +1233,7 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( - err.kind, - BuiltinTypeCheckErrorKind::NoColumnWithName { .. } - )); + assert_matches!(err.kind, BuiltinTypeCheckErrorKind::NoColumnWithName { .. }); let spec_wrong_type = [ col("a", ColumnType::Text), @@ -1248,10 +1246,10 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinSerializationErrorKind::ColumnSerializationFailed { .. } - )); + ); } #[derive(SerializeRow)] @@ -1325,10 +1323,10 @@ mod tests { let ctx = RowSerializationContext { columns: &spec }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::ColumnNameMismatch { .. } - )); + ); let spec_without_c = [ col("a", ColumnType::Text), @@ -1341,10 +1339,10 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. } - )); + ); let spec_duplicate_column = [ col("a", ColumnType::Text), @@ -1359,10 +1357,7 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( - err.kind, - BuiltinTypeCheckErrorKind::NoColumnWithName { .. } - )); + assert_matches!(err.kind, BuiltinTypeCheckErrorKind::NoColumnWithName { .. }); let spec_wrong_type = [ col("a", ColumnType::Text), @@ -1375,10 +1370,10 @@ mod tests { }; let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinSerializationErrorKind::ColumnSerializationFailed { .. } - )); + ); } #[test] diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 6f917f8de4..b896b6a528 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -1536,6 +1536,7 @@ mod tests { }; use crate::types::serialize::{CellWriter, SerializationError}; + use assert_matches::assert_matches; use scylla_macros::SerializeCql; use super::{SerializeCql, UdtSerializationErrorKind, UdtTypeCheckErrorKind}; @@ -1628,12 +1629,12 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Int], - }, - )); + } + ); // str (and also Uuid) are interesting because they accept two types, // also check str here @@ -1642,12 +1643,12 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::<&str>()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Ascii, ColumnType::Text], - }, - )); + } + ); // We'll skip testing for SizeOverflow as this would require producing // a value which is at least 2GB in size. @@ -1665,10 +1666,7 @@ mod tests { let err = get_ser_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, ColumnType::Decimal); - assert!(matches!( - err.kind, - BuiltinSerializationErrorKind::ValueOverflow, - )); + assert_matches!(err.kind, BuiltinSerializationErrorKind::ValueOverflow); } #[test] @@ -1679,10 +1677,10 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::>()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, - BuiltinTypeCheckErrorKind::SetOrListError(SetOrListTypeCheckErrorKind::NotSetOrList), - )); + BuiltinTypeCheckErrorKind::SetOrListError(SetOrListTypeCheckErrorKind::NotSetOrList) + ); // Trick: Unset is a ZST, so [Unset; 1 << 33] is a ZST, too. // While it's probably incorrect to use Unset in a collection, this @@ -1694,12 +1692,12 @@ mod tests { let err = get_ser_err(&err); assert_eq!(err.rust_name, std::any::type_name::<&[Unset]>()); assert_eq!(err.got, typ); - assert!(matches!( + assert_matches!( err.kind, BuiltinSerializationErrorKind::SetOrListError( SetOrListSerializationErrorKind::TooManyElements - ), - )); + ) + ); // Error during serialization of an element let v = vec![123_i32]; @@ -1715,12 +1713,12 @@ mod tests { panic!("unexpected error kind: {}", err.kind) }; let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Int], } - )); + ); } #[test] @@ -1731,10 +1729,10 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::>()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, - BuiltinTypeCheckErrorKind::MapError(MapTypeCheckErrorKind::NotMap), - )); + BuiltinTypeCheckErrorKind::MapError(MapTypeCheckErrorKind::NotMap) + ); // It's not practical to check the TooManyElements error as it would // require allocating a huge amount of memory. @@ -1753,12 +1751,12 @@ mod tests { panic!("unexpected error kind: {}", err.kind) }; let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Int], } - )); + ); // Error during serialization of a value let v = BTreeMap::from([(123_i32, 456_i32)]); @@ -1774,12 +1772,12 @@ mod tests { panic!("unexpected error kind: {}", err.kind) }; let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Int], } - )); + ); } #[test] @@ -1790,10 +1788,10 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::<(i32, i32, i32)>()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, - BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple), - )); + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple) + ); // The Rust tuple has more elements than the CQL type let v = (123_i32, 456_i32, 789_i32); @@ -1802,13 +1800,13 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::<(i32, i32, i32)>()); assert_eq!(err.got, typ); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { actual: 3, asked_for: 2, - }), - )); + }) + ); // Error during serialization of one of the elements let v = (123_i32, "Ala ma kota", 789.0_f64); @@ -1824,12 +1822,12 @@ mod tests { panic!("unexpected error kind: {}", err.kind) }; let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Double], } - )); + ); } #[test] @@ -1840,7 +1838,7 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, ColumnType::Counter); - assert!(matches!(err.kind, BuiltinTypeCheckErrorKind::NotEmptyable)); + assert_matches!(err.kind, BuiltinTypeCheckErrorKind::NotEmptyable); // Handle tuples and UDTs in separate tests, as they have some // custom logic @@ -1858,10 +1856,10 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, - BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple), - )); + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple) + ); // The Rust tuple has more elements than the CQL type let v = CqlValue::Tuple(vec![ @@ -1874,13 +1872,13 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, typ); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { actual: 3, asked_for: 2, - }), - )); + }) + ); // Error during serialization of one of the elements let v = CqlValue::Tuple(vec![ @@ -1900,12 +1898,12 @@ mod tests { panic!("unexpected error kind: {}", err.kind) }; let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Double], } - )); + ); } #[test] @@ -1924,10 +1922,10 @@ mod tests { let err = get_typeck_err(&err); assert_eq!(err.rust_name, std::any::type_name::()); assert_eq!(err.got, ColumnType::Double); - assert!(matches!( + assert_matches!( err.kind, - BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NotUdt), - )); + BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NotUdt) + ); // Wrong type name let v = CqlValue::UserDefinedType { @@ -2027,12 +2025,12 @@ mod tests { }; assert_eq!(field_name, "c"); let err = get_typeck_err(err); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::MismatchedType { expected: &[ColumnType::Int], } - )); + ); } // Do not remove. It's not used in tests but we keep it here to check that @@ -2251,10 +2249,10 @@ mod tests { .serialize(&typ_not_udt, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NotUdt) - )); + ); let typ_without_c = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2270,12 +2268,12 @@ mod tests { .serialize(&typ_without_c, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError( UdtTypeCheckErrorKind::ValueMissingForUdtField { .. } ) - )); + ); let typ_wrong_type = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2291,12 +2289,12 @@ mod tests { .serialize(&typ_wrong_type, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinSerializationErrorKind::UdtError( UdtSerializationErrorKind::FieldSerializationFailed { .. } ) - )); + ); } #[derive(SerializeCql)] @@ -2448,10 +2446,10 @@ mod tests { let err = <_ as SerializeCql>::serialize(&udt, &typ_not_udt, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NotUdt) - )); + ); let typ = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2470,10 +2468,10 @@ mod tests { let err = <_ as SerializeCql>::serialize(&udt, &typ, CellWriter::new(&mut data)).unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::FieldNameMismatch { .. }) - )); + ); let typ_without_c = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2488,12 +2486,12 @@ mod tests { let err = <_ as SerializeCql>::serialize(&udt, &typ_without_c, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError( UdtTypeCheckErrorKind::ValueMissingForUdtField { .. } ) - )); + ); let typ_unexpected_field = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2509,12 +2507,12 @@ mod tests { <_ as SerializeCql>::serialize(&udt, &typ_unexpected_field, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinSerializationErrorKind::UdtError( UdtSerializationErrorKind::FieldSerializationFailed { .. } ) - )); + ); } #[derive(SerializeCql, Debug)] @@ -2669,10 +2667,10 @@ mod tests { .serialize(&typ_unexpected_field, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. }) - )); + ); let typ_unexpected_field_middle = ColumnType::UserDefinedType { type_name: "typ".to_string(), @@ -2693,10 +2691,10 @@ mod tests { .serialize(&typ_unexpected_field_middle, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. }) - )); + ); } #[derive(SerializeCql, Debug, PartialEq, Eq, Default)] @@ -2731,10 +2729,10 @@ mod tests { <_ as SerializeCql>::serialize(&udt, &typ_unexpected_field, CellWriter::new(&mut data)) .unwrap_err(); let err = err.0.downcast_ref::().unwrap(); - assert!(matches!( + assert_matches!( err.kind, BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. }) - )); + ); } #[derive(SerializeCql, Debug)] diff --git a/scylla/src/history.rs b/scylla/src/history.rs index a43d46423a..f7c9acf6d9 100644 --- a/scylla/src/history.rs +++ b/scylla/src/history.rs @@ -465,6 +465,7 @@ mod tests { SpeculativeId, StructuredHistory, TimePoint, }; use crate::test_utils::create_new_session_builder; + use assert_matches::assert_matches; use chrono::{DateTime, NaiveDate, NaiveDateTime, NaiveTime, Utc}; use futures::StreamExt; use scylla_cql::{ @@ -642,10 +643,10 @@ mod tests { assert_eq!(history.queries.len(), 1); assert_eq!(history.queries[0].non_speculative_fiber.attempts.len(), 1); assert!(history.queries[0].speculative_fibers.is_empty()); - assert!(matches!( + assert_matches!( history.queries[0].non_speculative_fiber.attempts[0].result, Some(AttemptResult::Success(_)) - )); + ); let displayed = "Queries History: === Query #0 === diff --git a/scylla/src/transport/query_result.rs b/scylla/src/transport/query_result.rs index 98be873db4..e368a11745 100644 --- a/scylla/src/transport/query_result.rs +++ b/scylla/src/transport/query_result.rs @@ -267,6 +267,8 @@ mod tests { }; use std::convert::TryInto; + use assert_matches::assert_matches; + // Returns specified number of rows, each one containing one int32 value. // Values are 0, 1, 2, 3, 4, ... fn make_rows(rows_num: usize) -> Vec { @@ -483,10 +485,10 @@ mod tests { Ok((0,)) ); - assert!(matches!( + assert_matches!( make_string_rows_query_result(2).first_row_typed::<(i32,)>(), Err(FirstRowTypedError::FromRowError(_)) - )); + ); } #[test] @@ -539,10 +541,10 @@ mod tests { Ok(Some((0,))) ); - assert!(matches!( + assert_matches!( make_string_rows_query_result(1).maybe_first_row_typed::<(i32,)>(), Err(MaybeFirstRowTypedError::FromRowError(_)) - )) + ) } #[test] @@ -594,9 +596,9 @@ mod tests { Err(SingleRowTypedError::BadNumberOfRows(3)) ); - assert!(matches!( + assert_matches!( make_string_rows_query_result(1).single_row_typed::<(i32,)>(), Err(SingleRowTypedError::FromRowError(_)) - )); + ); } } From 5c71314983f4a946513027be2a1fc907bd846740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 7 May 2024 08:26:42 +0200 Subject: [PATCH 5/8] serialize: deduplicate do_serialize[_err] --- scylla-cql/src/types/serialize/value.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index b896b6a528..621ec8f416 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -1574,17 +1574,21 @@ mod tests { assert_eq!(typed_data, erased_data); } - fn do_serialize(t: T, typ: &ColumnType) -> Vec { + fn do_serialize_result( + t: T, + typ: &ColumnType, + ) -> Result, SerializationError> { let mut ret = Vec::new(); let writer = CellWriter::new(&mut ret); - t.serialize(typ, writer).unwrap(); - ret + t.serialize(typ, writer).map(|_| ()).map(|()| ret) + } + + fn do_serialize(t: T, typ: &ColumnType) -> Vec { + do_serialize_result(t, typ).unwrap() } fn do_serialize_err(t: T, typ: &ColumnType) -> SerializationError { - let mut ret = Vec::new(); - let writer = CellWriter::new(&mut ret); - t.serialize(typ, writer).unwrap_err() + do_serialize_result(t, typ).unwrap_err() } #[test] From 406128c5337c734073ce7206a9161fc7f5033122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 7 May 2024 08:29:13 +0200 Subject: [PATCH 6/8] serialize: polish TupleTypeCheckErrorKind::WrongElementCount - a bunch of typos are fixed, - some names are changed to be more explicit. --- scylla-cql/src/types/serialize/value.rs | 31 ++++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 621ec8f416..306853a830 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -48,7 +48,7 @@ pub trait SerializeCql { /// The [`CellWriter`] provided to the method ensures that the value produced /// will be properly framed (i.e. incorrectly written value should not /// cause the rest of the request to be misinterpreted), but otherwise - /// the implementor of the trait is responsible for producing the a value + /// the implementor of the trait is responsible for producing the value /// in a correct format. fn serialize<'b>( &self, @@ -548,8 +548,8 @@ fn serialize_cql_value<'b>( return Err(mk_typck_err::( typ, TupleTypeCheckErrorKind::WrongElementCount { - actual: t.len(), - asked_for: fields.len(), + rust_type_el_count: t.len(), + cql_type_el_count: fields.len(), }, )); } @@ -586,7 +586,7 @@ fn fix_cql_value_name_in_err(mut err: SerializationError) -> SerializationError } } None => { - // The `None` case shouldn't happen consisdering how we are using + // The `None` case shouldn't happen considering how we are using // the function in the code now, but let's provide it here anyway // for correctness. if let Some(err) = err.0.downcast_ref::() { @@ -729,8 +729,8 @@ macro_rules! impl_tuple { _ => return Err(mk_typck_err::( typ, TupleTypeCheckErrorKind::WrongElementCount { - actual: $length, - asked_for: typs.len(), + rust_type_el_count: $length, + cql_type_el_count: typs.len(), } )) } @@ -1352,10 +1352,10 @@ pub enum TupleTypeCheckErrorKind { /// elements will be set to null. WrongElementCount { /// The number of elements that the Rust tuple has. - actual: usize, + rust_type_el_count: usize, /// The number of elements that the CQL tuple type has. - asked_for: usize, + cql_type_el_count: usize, }, } @@ -1366,9 +1366,12 @@ impl Display for TupleTypeCheckErrorKind { f, "the CQL type the tuple was attempted to be serialized to is not a tuple" ), - TupleTypeCheckErrorKind::WrongElementCount { actual, asked_for } => write!( + TupleTypeCheckErrorKind::WrongElementCount { + rust_type_el_count, + cql_type_el_count, + } => write!( f, - "wrong tuple element count: CQL type has {asked_for}, the Rust tuple has {actual}" + "wrong tuple element count: CQL type has {cql_type_el_count}, the Rust tuple has {rust_type_el_count}" ), } } @@ -1807,8 +1810,8 @@ mod tests { assert_matches!( err.kind, BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { - actual: 3, - asked_for: 2, + rust_type_el_count: 3, + cql_type_el_count: 2, }) ); @@ -1879,8 +1882,8 @@ mod tests { assert_matches!( err.kind, BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { - actual: 3, - asked_for: 2, + rust_type_el_count: 3, + cql_type_el_count: 2, }) ); From 20fbd00f2ca6c02bbc7237c5fe21503ef2464422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 7 May 2024 08:34:55 +0200 Subject: [PATCH 7/8] serialize: polish row WrongColumnCount error - names and error messages are changed to better reflect reality. --- scylla-cql/src/types/serialize/row.rs | 36 +++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index 2e14b75b3e..f980064388 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -108,8 +108,8 @@ macro_rules! impl_serialize_row_for_unit { if !ctx.columns().is_empty() { return Err(mk_typck_err::( BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: 0, - asked_for: ctx.columns().len(), + rust_cols: 0, + cql_cols: ctx.columns().len(), }, )); } @@ -142,8 +142,8 @@ macro_rules! impl_serialize_row_for_slice { if ctx.columns().len() != self.len() { return Err(mk_typck_err::( BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: self.len(), - asked_for: ctx.columns().len(), + rust_cols: self.len(), + cql_cols: ctx.columns().len(), }, )); } @@ -289,8 +289,8 @@ macro_rules! impl_tuple { [$($tidents),*] => ($($tidents,)*), _ => return Err(mk_typck_err::( BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: $length, - asked_for: ctx.columns().len(), + rust_cols: $length, + cql_cols: ctx.columns().len(), }, )), }; @@ -582,13 +582,13 @@ fn mk_ser_err_named( #[derive(Debug, Clone)] #[non_exhaustive] pub enum BuiltinTypeCheckErrorKind { - /// The Rust type expects `actual` column, but the statement requires `asked_for`. + /// The Rust type provides `rust_cols` columns, but the statement operates on `cql_cols`. WrongColumnCount { /// The number of values that the Rust type provides. - actual: usize, + rust_cols: usize, - /// The number of columns that the statement requires. - asked_for: usize, + /// The number of columns that the statement operates on. + cql_cols: usize, }, /// The Rust type provides a value for some column, but that column is not @@ -618,8 +618,8 @@ pub enum BuiltinTypeCheckErrorKind { impl Display for BuiltinTypeCheckErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - BuiltinTypeCheckErrorKind::WrongColumnCount { actual, asked_for } => { - write!(f, "wrong column count: the query requires {asked_for} columns, but {actual} were provided") + BuiltinTypeCheckErrorKind::WrongColumnCount { rust_cols, cql_cols } => { + write!(f, "wrong column count: the statement operates on {cql_cols} columns, but the given rust type provides {rust_cols}") } BuiltinTypeCheckErrorKind::NoColumnWithName { name } => { write!( @@ -1048,8 +1048,8 @@ mod tests { assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: 0, - asked_for: 1, + rust_cols: 0, + cql_cols: 1, } ); @@ -1063,8 +1063,8 @@ mod tests { assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: 1, - asked_for: 2, + rust_cols: 1, + cql_cols: 2, } ); @@ -1090,8 +1090,8 @@ mod tests { assert_matches!( err.kind, BuiltinTypeCheckErrorKind::WrongColumnCount { - actual: 1, - asked_for: 2, + rust_cols: 1, + cql_cols: 2, } ); From c00ae720a25ecea7362061156de742a5d9853823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Tue, 7 May 2024 09:43:25 +0200 Subject: [PATCH 8/8] serialize/value errors: various fixes for accuracy - error messages are changed to better reflect reality, - where possible, `write!(f, ...)` is replaced with `f.write_str(...)` for performance. --- scylla-cql/src/types/serialize/value.rs | 60 +++++++------------------ 1 file changed, 16 insertions(+), 44 deletions(-) diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 306853a830..b5e6181d01 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -1149,17 +1149,14 @@ impl Display for BuiltinTypeCheckErrorKind { write!(f, "expected one of the CQL types: {expected:?}") } BuiltinTypeCheckErrorKind::NotEmptyable => { - write!( - f, - "the separate empty representation is not valid for this type" - ) + f.write_str("the separate empty representation is not valid for this type") } BuiltinTypeCheckErrorKind::SetOrListError(err) => err.fmt(f), BuiltinTypeCheckErrorKind::MapError(err) => err.fmt(f), BuiltinTypeCheckErrorKind::TupleError(err) => err.fmt(f), BuiltinTypeCheckErrorKind::UdtError(err) => err.fmt(f), BuiltinTypeCheckErrorKind::CustomTypeUnsupported => { - write!(f, "custom CQL types are unsupported") + f.write_str("custom CQL types are unsupported") } } } @@ -1217,16 +1214,10 @@ impl Display for BuiltinSerializationErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { BuiltinSerializationErrorKind::SizeOverflow => { - write!( - f, - "the Rust value is too big to be serialized in the CQL protocol format" - ) + f.write_str("the Rust value is too big to be serialized in the CQL protocol format") } BuiltinSerializationErrorKind::ValueOverflow => { - write!( - f, - "the Rust value is out of range supported by the CQL type" - ) + f.write_str("the Rust value is out of range supported by the CQL type") } BuiltinSerializationErrorKind::SetOrListError(err) => err.fmt(f), BuiltinSerializationErrorKind::MapError(err) => err.fmt(f), @@ -1247,12 +1238,9 @@ pub enum MapTypeCheckErrorKind { impl Display for MapTypeCheckErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - MapTypeCheckErrorKind::NotMap => { - write!( - f, - "the CQL type the map was attempted to be serialized to was not map" - ) - } + MapTypeCheckErrorKind::NotMap => f.write_str( + "the CQL type the Rust type was attempted to be type checked against was not a map", + ), } } } @@ -1275,10 +1263,7 @@ impl Display for MapSerializationErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { MapSerializationErrorKind::TooManyElements => { - write!( - f, - "the map contains too many elements to fit in CQL representation" - ) + f.write_str("the map contains too many elements to fit in CQL representation") } MapSerializationErrorKind::KeySerializationFailed(err) => { write!(f, "failed to serialize one of the keys: {}", err) @@ -1302,10 +1287,7 @@ impl Display for SetOrListTypeCheckErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { SetOrListTypeCheckErrorKind::NotSetOrList => { - write!( - f, - "the CQL type the tuple was attempted to was neither a set or a list" - ) + f.write_str("the CQL type the Rust type was attempted to be type checked against was neither a set or a list") } } } @@ -1325,12 +1307,9 @@ pub enum SetOrListSerializationErrorKind { impl Display for SetOrListSerializationErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - SetOrListSerializationErrorKind::TooManyElements => { - write!( - f, - "the collection contains too many elements to fit in CQL representation" - ) - } + SetOrListSerializationErrorKind::TooManyElements => f.write_str( + "the collection contains too many elements to fit in CQL representation", + ), SetOrListSerializationErrorKind::ElementSerializationFailed(err) => { write!(f, "failed to serialize one of the elements: {err}") } @@ -1362,14 +1341,10 @@ pub enum TupleTypeCheckErrorKind { impl Display for TupleTypeCheckErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TupleTypeCheckErrorKind::NotTuple => write!( - f, - "the CQL type the tuple was attempted to be serialized to is not a tuple" + TupleTypeCheckErrorKind::NotTuple => f.write_str( + "the CQL type the Rust type was attempted to be type checked against is not a tuple" ), - TupleTypeCheckErrorKind::WrongElementCount { - rust_type_el_count, - cql_type_el_count, - } => write!( + TupleTypeCheckErrorKind::WrongElementCount { rust_type_el_count, cql_type_el_count } => write!( f, "wrong tuple element count: CQL type has {cql_type_el_count}, the Rust tuple has {rust_type_el_count}" ), @@ -1442,10 +1417,7 @@ pub enum UdtTypeCheckErrorKind { impl Display for UdtTypeCheckErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - UdtTypeCheckErrorKind::NotUdt => write!( - f, - "the CQL type the tuple was attempted to be type checked against is not a UDT" - ), + UdtTypeCheckErrorKind::NotUdt => f.write_str("the CQL type the Rust type was attempted to be type checked against is not a UDT"), UdtTypeCheckErrorKind::NameMismatch { keyspace, type_name,