Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't return error on unexpected UDT field in SerializeCql macro #885

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Don't return error on unexpected UDT field in SerializeCql macro #885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb8a1e1
macros: Add `force_exact_match` attribute to SerializeCql
Lorak-mmk Dec 18, 2023
77fb6e2
scylla-cql: Test new behavior of SerializeCql
Lorak-mmk Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions scylla-cql/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,18 @@ pub use scylla_macros::ValueList;
///
Lorak-mmk marked this conversation as resolved.
Show resolved Hide resolved
/// At the moment, only structs with named fields are supported.
///
/// Serialization will fail if there are some fields in the UDT that don't match
/// to any of the Rust struct fields, _or vice versa_.
/// Serialization will fail if there are some fields in the Rust struct that don't match
/// to any of the UDT fields.
///
/// If there are fields in UDT that are not present in Rust definition:
/// - serialization will succeed in "match_by_name" flavor (default). Missing
/// fields in the middle of UDT will be sent as NULLs, missing fields at the end will not be sent
/// at all.
/// - serialization will succed if suffix of UDT fields is missing. If there are missing fields in the
/// middle it will fail. Note that if "skip_name_checks" is enabled, and the types happen to match,
/// it is possible for serialization to succeed with unexpected result.
/// This behavior is the default to support ALTERing UDTs by adding new fields.
/// You can require exact match of fields using `force_exact_match` attribute.
///
/// In case of failure, either [`BuiltinTypeCheckError`](crate::types::serialize::value::BuiltinTypeCheckError)
/// or [`BuiltinSerializationError`](crate::types::serialize::value::BuiltinSerializationError)
Expand All @@ -42,7 +52,7 @@ pub use scylla_macros::ValueList;
/// struct MyUdt {
/// a: i32,
/// b: Option<String>,
/// c: Vec<u8>,
Lorak-mmk marked this conversation as resolved.
Show resolved Hide resolved
/// // No "c" field - it is not mandatory by default for all fields to be present
/// }
/// ```
///
Expand Down Expand Up @@ -87,7 +97,7 @@ pub use scylla_macros::ValueList;
/// macro itself, so in those cases the user must provide an alternative path
/// to either the `scylla` or `scylla-cql` crate.
///
/// `#[scylla(skip_name_checks)]
/// `#[scylla(skip_name_checks)]`
///
/// _Specific only to the `enforce_order` flavor._
///
Expand All @@ -96,6 +106,11 @@ pub use scylla_macros::ValueList;
/// struct field names and UDT field names, i.e. it's OK if i-th field has a
/// different name in Rust and in the UDT. Fields are still being type-checked.
///
/// `#[scylla(force_exact_match)]`
///
/// Forces Rust struct to have all the fields present in UDT, otherwise
/// serialization fails.
///
/// # Field attributes
///
/// `#[scylla(rename = "name_in_the_udt")]`
Expand Down
277 changes: 228 additions & 49 deletions scylla-cql/src/types/serialize/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,7 @@ mod tests {
use std::collections::BTreeMap;

use crate::frame::response::result::{ColumnType, CqlValue};
use crate::frame::value::{MaybeUnset, Unset, Value, ValueTooBig};
use crate::frame::value::{Counter, MaybeUnset, Unset, Value, ValueTooBig};
use crate::types::serialize::value::{
BuiltinSerializationError, BuiltinSerializationErrorKind, BuiltinTypeCheckError,
BuiltinTypeCheckErrorKind, MapSerializationErrorKind, MapTypeCheckErrorKind,
Expand Down Expand Up @@ -2109,6 +2109,95 @@ mod tests {
assert_eq!(reference, udt);
}

#[test]
Lorak-mmk marked this conversation as resolved.
Show resolved Hide resolved
fn test_udt_serialization_with_missing_rust_fields_at_end() {
let udt = TestUdtWithFieldSorting::default();

let typ_normal = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
],
};

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected fields
("d".to_string(), ColumnType::Counter),
("e".to_string(), ColumnType::Counter),
],
};

let result_normal = do_serialize(&udt, &typ_normal);
let result_additional_field = do_serialize(&udt, &typ_unexpected_field);

assert_eq!(result_normal, result_additional_field);
}
Lorak-mmk marked this conversation as resolved.
Show resolved Hide resolved

#[derive(SerializeCql, Debug, PartialEq, Default)]
#[scylla(crate = crate)]
struct TestUdtWithFieldSorting2 {
a: String,
b: i32,
d: Option<Counter>,
c: Vec<i64>,
}

#[derive(SerializeCql, Debug, PartialEq, Default)]
#[scylla(crate = crate)]
struct TestUdtWithFieldSorting3 {
a: String,
b: i32,
d: Option<Counter>,
e: Option<f32>,
c: Vec<i64>,
}

#[test]
fn test_udt_serialization_with_missing_rust_field_in_middle() {
let udt = TestUdtWithFieldSorting::default();
let udt2 = TestUdtWithFieldSorting2::default();
let udt3 = TestUdtWithFieldSorting3::default();

let typ = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
// Unexpected fields
("d".to_string(), ColumnType::Counter),
("e".to_string(), ColumnType::Float),
// Remaining normal field
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
],
};

let result_1 = do_serialize(udt, &typ);
let result_2 = do_serialize(udt2, &typ);
let result_3 = do_serialize(udt3, &typ);

assert_eq!(result_1, result_2);
assert_eq!(result_2, result_3);
}

#[test]
fn test_udt_serialization_failing_type_check() {
let typ_not_udt = ColumnType::Ascii;
Expand Down Expand Up @@ -2145,30 +2234,6 @@ mod tests {
)
));

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected field
("d".to_string(), ColumnType::Counter),
],
};

let err = udt
.serialize(&typ_unexpected_field, CellWriter::new(&mut data))
.unwrap_err();
let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
assert!(matches!(
err.kind,
BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. })
));

let typ_wrong_type = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
Expand Down Expand Up @@ -2292,6 +2357,44 @@ mod tests {
assert_eq!(reference, udt);
}

#[test]
fn test_udt_serialization_with_enforced_order_additional_field() {
let udt = TestUdtWithEnforcedOrder::default();

let typ_normal = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
],
};

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected field
("d".to_string(), ColumnType::Counter),
],
};

let result_normal = do_serialize(&udt, &typ_normal);
let result_additional_field = do_serialize(&udt, &typ_unexpected_field);

assert_eq!(result_normal, result_additional_field);
}

#[test]
fn test_udt_serialization_with_enforced_order_failing_type_check() {
let typ_not_udt = ColumnType::Ascii;
Expand Down Expand Up @@ -2349,30 +2452,6 @@ mod tests {
)
));

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected field
("d".to_string(), ColumnType::Counter),
],
};

let err =
<_ as SerializeCql>::serialize(&udt, &typ_unexpected_field, CellWriter::new(&mut data))
.unwrap_err();
let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
assert!(matches!(
err.kind,
BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. })
));

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
Expand Down Expand Up @@ -2513,4 +2592,104 @@ mod tests {

assert_eq!(reference, udt);
}

#[derive(SerializeCql, Debug, PartialEq, Eq, Default)]
#[scylla(crate = crate, force_exact_match)]
struct TestStrictUdtWithFieldSorting {
a: String,
b: i32,
c: Vec<i64>,
}

#[test]
fn test_strict_udt_with_field_sorting_rejects_additional_field() {
let udt = TestStrictUdtWithFieldSorting::default();
let mut data = Vec::new();

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected field
("d".to_string(), ColumnType::Counter),
],
};

let err = udt
.serialize(&typ_unexpected_field, CellWriter::new(&mut data))
.unwrap_err();
let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
assert!(matches!(
err.kind,
BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. })
));

let typ_unexpected_field_middle = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
// Unexpected field
("b_c".to_string(), ColumnType::Counter),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
],
};

let err = udt
.serialize(&typ_unexpected_field_middle, CellWriter::new(&mut data))
.unwrap_err();
let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
assert!(matches!(
err.kind,
BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. })
));
}

#[derive(SerializeCql, Debug, PartialEq, Eq, Default)]
#[scylla(crate = crate, flavor = "enforce_order", force_exact_match)]
struct TestStrictUdtWithEnforcedOrder {
a: String,
b: i32,
c: Vec<i64>,
}

#[test]
fn test_strict_udt_with_enforced_order_rejects_additional_field() {
let udt = TestStrictUdtWithEnforcedOrder::default();
let mut data = Vec::new();

let typ_unexpected_field = ColumnType::UserDefinedType {
type_name: "typ".to_string(),
keyspace: "ks".to_string(),
field_types: vec![
("a".to_string(), ColumnType::Text),
("b".to_string(), ColumnType::Int),
(
"c".to_string(),
ColumnType::List(Box::new(ColumnType::BigInt)),
),
// Unexpected field
("d".to_string(), ColumnType::Counter),
],
};

let err =
<_ as SerializeCql>::serialize(&udt, &typ_unexpected_field, CellWriter::new(&mut data))
.unwrap_err();
let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
assert!(matches!(
err.kind,
BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. })
));
}
}
Loading