Skip to content

Commit 6a38368

Browse files
Lorak-mmkLorak-mmk
authored
Merge pull request #1378 from Lorak-mmk/fix-serialize-renaming
Fix renaming in serialization
2 parents af5fd91 + c2e2c97 commit 6a38368

File tree

2 files changed

+186
-11
lines changed

2 files changed

+186
-11
lines changed

‎scylla-cql/src/serialize/value.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ impl<V: SerializeValue + secrecy_08::Zeroize> SerializeValue for secrecy_08::Sec
223223
writer: CellWriter<'b>,
224224
) -> Result<WrittenCellProof<'b>, SerializationError> {
225225
use secrecy_08::ExposeSecret;
226-
V::serialize(self.expose_secret(), typ, writer)
226+
V::serialize(self.expose_secret(), typ, writer).map_err(fix_rust_name_in_err::<Self>)
227227
}
228228
}
229229
impl SerializeValue for bool {
@@ -350,7 +350,9 @@ impl<T: SerializeValue> SerializeValue for Option<T> {
350350
writer: CellWriter<'b>,
351351
) -> Result<WrittenCellProof<'b>, SerializationError> {
352352
match self {
353-
Some(v) => v.serialize(typ, writer),
353+
Some(v) => v
354+
.serialize(typ, writer)
355+
.map_err(fix_rust_name_in_err::<Self>),
354356
None => Ok(writer.set_null()),
355357
}
356358
}
@@ -382,7 +384,9 @@ impl<V: SerializeValue> SerializeValue for MaybeUnset<V> {
382384
writer: CellWriter<'b>,
383385
) -> Result<WrittenCellProof<'b>, SerializationError> {
384386
match self {
385-
MaybeUnset::Set(v) => v.serialize(typ, writer),
387+
MaybeUnset::Set(v) => v
388+
.serialize(typ, writer)
389+
.map_err(fix_rust_name_in_err::<Self>),
386390
MaybeUnset::Unset => Ok(writer.set_unset()),
387391
}
388392
}
@@ -393,7 +397,7 @@ impl<T: SerializeValue + ?Sized> SerializeValue for &T {
393397
typ: &ColumnType,
394398
writer: CellWriter<'b>,
395399
) -> Result<WrittenCellProof<'b>, SerializationError> {
396-
T::serialize(*self, typ, writer)
400+
T::serialize(*self, typ, writer).map_err(fix_rust_name_in_err::<Self>)
397401
}
398402
}
399403
impl<T: SerializeValue + ?Sized> SerializeValue for Box<T> {
@@ -402,7 +406,7 @@ impl<T: SerializeValue + ?Sized> SerializeValue for Box<T> {
402406
typ: &ColumnType,
403407
writer: CellWriter<'b>,
404408
) -> Result<WrittenCellProof<'b>, SerializationError> {
405-
T::serialize(&**self, typ, writer)
409+
T::serialize(&**self, typ, writer).map_err(fix_rust_name_in_err::<Self>)
406410
}
407411
}
408412
impl<V: SerializeValue, S: BuildHasher + Default> SerializeValue for HashSet<V, S> {
@@ -549,7 +553,7 @@ impl SerializeValue for CqlValue {
549553
typ: &ColumnType,
550554
writer: CellWriter<'b>,
551555
) -> Result<WrittenCellProof<'b>, SerializationError> {
552-
serialize_cql_value(self, typ, writer).map_err(fix_cql_value_name_in_err)
556+
serialize_cql_value(self, typ, writer).map_err(fix_rust_name_in_err::<Self>)
553557
}
554558
}
555559

@@ -631,13 +635,13 @@ fn serialize_cql_value<'b>(
631635
}
632636
}
633637

634-
fn fix_cql_value_name_in_err(mut err: SerializationError) -> SerializationError {
638+
fn fix_rust_name_in_err<RustT>(mut err: SerializationError) -> SerializationError {
635639
// The purpose of this function is to change the `rust_name` field
636-
// in the error to CqlValue. Most of the time, the `err` given to the
640+
// in the error to the given one. Most of the time, the `err` given to the
637641
// function here will be the sole owner of the data, so theoretically
638642
// we could fix this in place.
639643

640-
let rust_name = std::any::type_name::<CqlValue>();
644+
let rust_name = std::any::type_name::<RustT>();
641645

642646
match Arc::get_mut(&mut err.0) {
643647
Some(err_mut) => {
@@ -713,7 +717,7 @@ fn serialize_udt<'b>(
713717
match fvalue {
714718
None => writer.set_null(),
715719
Some(v) => serialize_cql_value(v, ftyp, writer).map_err(|err| {
716-
let err = fix_cql_value_name_in_err(err);
720+
let err = fix_rust_name_in_err::<CqlValue>(err);
717721
mk_ser_err::<CqlValue>(
718722
typ,
719723
UdtSerializationErrorKind::FieldSerializationFailed {
@@ -756,7 +760,7 @@ fn serialize_tuple_like<'t, 'b>(
756760
match el {
757761
None => sub.set_null(),
758762
Some(el) => serialize_cql_value(el, el_typ, sub).map_err(|err| {
759-
let err = fix_cql_value_name_in_err(err);
763+
let err = fix_rust_name_in_err::<CqlValue>(err);
760764
mk_ser_err::<CqlValue>(
761765
typ,
762766
TupleSerializationErrorKind::ElementSerializationFailed { index, err },

‎scylla-cql/src/serialize/value_tests.rs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use crate::serialize::value::{
55
SetOrListSerializationErrorKind, SetOrListTypeCheckErrorKind, TupleSerializationErrorKind,
66
TupleTypeCheckErrorKind, UdtSerializationErrorKind, UdtTypeCheckErrorKind,
77
};
8+
use crate::serialize::writers::WrittenCellProof;
89
use crate::serialize::{CellWriter, SerializationError};
910
use crate::value::{
1011
Counter, CqlDate, CqlDuration, CqlTime, CqlTimestamp, CqlTimeuuid, CqlValue, CqlVarint,
@@ -20,8 +21,31 @@ use std::str::FromStr;
2021
use std::sync::Arc;
2122

2223
use assert_matches::assert_matches;
24+
use thiserror::Error;
2325
use uuid::Uuid;
2426

27+
#[derive(PartialEq, Eq, PartialOrd, Ord)]
28+
struct SerializeWithCustomError;
29+
30+
#[derive(Error, Debug)]
31+
#[error("Custom serialization error")]
32+
struct CustomSerializationError;
33+
34+
impl SerializeValue for SerializeWithCustomError {
35+
fn serialize<'b>(
36+
&self,
37+
_typ: &ColumnType,
38+
_writer: CellWriter<'b>,
39+
) -> Result<WrittenCellProof<'b>, SerializationError> {
40+
Err(SerializationError::new(CustomSerializationError))
41+
}
42+
}
43+
44+
#[cfg(feature = "secrecy-08")]
45+
impl secrecy_08::Zeroize for SerializeWithCustomError {
46+
fn zeroize(&mut self) {}
47+
}
48+
2549
#[test]
2650
fn test_dyn_serialize_value() {
2751
let v: i32 = 123;
@@ -106,6 +130,70 @@ fn test_native_errors() {
106130
// a value which is at least 2GB in size.
107131
}
108132

133+
/// Helper function for tests of wrappers (Option<T>, Box<T> etc).
134+
/// Its main purpose is to verify that the rust_name in typecheck error
135+
/// is the name of whole container instead of container value (for example
136+
/// `Option<i32>` instead of `i32`).
137+
fn verify_typeck_error_in_wrapper<T: SerializeValue>(v: T) {
138+
let err = do_serialize_err::<T>(v, &ColumnType::Native(NativeType::Text));
139+
let err = get_typeck_err(&err);
140+
assert_eq!(err.rust_name, std::any::type_name::<T>());
141+
assert_eq!(err.got, ColumnType::Native(NativeType::Text));
142+
assert_matches!(
143+
err.kind,
144+
BuiltinTypeCheckErrorKind::MismatchedType {
145+
expected: &[ColumnType::Native(NativeType::Int)]
146+
}
147+
);
148+
}
149+
150+
fn verify_custom_error_in_wrapper<T: SerializeValue>(v: T) {
151+
let err = do_serialize_err::<T>(v, &ColumnType::Native(NativeType::BigInt));
152+
err.0
153+
.downcast_ref::<CustomSerializationError>()
154+
.expect("CustomSerializationError");
155+
}
156+
157+
#[cfg(feature = "secrecy-08")]
158+
#[test]
159+
fn test_secrecy_08_errors() {
160+
use secrecy_08::Secret;
161+
verify_typeck_error_in_wrapper::<Secret<i32>>(Secret::new(123));
162+
verify_custom_error_in_wrapper::<Secret<SerializeWithCustomError>>(Secret::new(
163+
SerializeWithCustomError,
164+
));
165+
}
166+
167+
#[test]
168+
fn test_option_errors() {
169+
verify_typeck_error_in_wrapper::<Option<i32>>(Some(123));
170+
verify_custom_error_in_wrapper::<Option<SerializeWithCustomError>>(Some(
171+
SerializeWithCustomError,
172+
));
173+
}
174+
175+
#[test]
176+
fn test_maybe_unset_errors() {
177+
verify_typeck_error_in_wrapper::<MaybeUnset<i32>>(MaybeUnset::Set(123));
178+
verify_custom_error_in_wrapper::<MaybeUnset<SerializeWithCustomError>>(MaybeUnset::Set(
179+
SerializeWithCustomError,
180+
));
181+
}
182+
183+
#[test]
184+
fn test_ref_errors() {
185+
verify_typeck_error_in_wrapper::<&i32>(&123_i32);
186+
verify_custom_error_in_wrapper::<&SerializeWithCustomError>(&SerializeWithCustomError);
187+
}
188+
189+
#[test]
190+
fn test_box_errors() {
191+
verify_typeck_error_in_wrapper::<Box<i32>>(Box::new(123));
192+
verify_custom_error_in_wrapper::<Box<SerializeWithCustomError>>(Box::new(
193+
SerializeWithCustomError,
194+
));
195+
}
196+
109197
#[cfg(feature = "bigdecimal-04")]
110198
#[test]
111199
fn test_native_errors_bigdecimal_04() {
@@ -177,6 +265,25 @@ fn test_set_or_list_errors() {
177265
expected: &[ColumnType::Native(NativeType::Int)],
178266
}
179267
);
268+
269+
// Test serialization with custom error
270+
let err = do_serialize_err(
271+
vec![SerializeWithCustomError],
272+
&ColumnType::Collection {
273+
frozen: false,
274+
typ: CollectionType::Set(Box::new(ColumnType::Native(NativeType::Double))),
275+
},
276+
);
277+
let err = get_ser_err(&err);
278+
let BuiltinSerializationErrorKind::SetOrListError(
279+
SetOrListSerializationErrorKind::ElementSerializationFailed(err),
280+
) = &err.kind
281+
else {
282+
panic!("unexpected error kind: {}", err.kind)
283+
};
284+
err.0
285+
.downcast_ref::<CustomSerializationError>()
286+
.expect("CustomSerializationError");
180287
}
181288

182289
#[test]
@@ -248,6 +355,51 @@ fn test_map_errors() {
248355
expected: &[ColumnType::Native(NativeType::Int)],
249356
}
250357
);
358+
359+
// Test serialization with custom error
360+
// Value
361+
let err = do_serialize_err(
362+
BTreeMap::from([(123_i32, SerializeWithCustomError)]),
363+
&ColumnType::Collection {
364+
frozen: false,
365+
typ: CollectionType::Map(
366+
Box::new(ColumnType::Native(NativeType::Int)),
367+
Box::new(ColumnType::Native(NativeType::Int)),
368+
),
369+
},
370+
);
371+
let err = get_ser_err(&err);
372+
let BuiltinSerializationErrorKind::MapError(
373+
MapSerializationErrorKind::ValueSerializationFailed(err),
374+
) = &err.kind
375+
else {
376+
panic!("unexpected error kind: {}", err.kind)
377+
};
378+
err.0
379+
.downcast_ref::<CustomSerializationError>()
380+
.expect("CustomSerializationError");
381+
382+
// Key
383+
let err = do_serialize_err(
384+
BTreeMap::from([(SerializeWithCustomError, 123_i32)]),
385+
&ColumnType::Collection {
386+
frozen: false,
387+
typ: CollectionType::Map(
388+
Box::new(ColumnType::Native(NativeType::Int)),
389+
Box::new(ColumnType::Native(NativeType::Int)),
390+
),
391+
},
392+
);
393+
let err = get_ser_err(&err);
394+
let BuiltinSerializationErrorKind::MapError(MapSerializationErrorKind::KeySerializationFailed(
395+
err,
396+
)) = &err.kind
397+
else {
398+
panic!("unexpected error kind: {}", err.kind)
399+
};
400+
err.0
401+
.downcast_ref::<CustomSerializationError>()
402+
.expect("CustomSerializationError");
251403
}
252404

253405
#[test]
@@ -302,6 +454,25 @@ fn test_tuple_errors() {
302454
expected: &[ColumnType::Native(NativeType::Double)],
303455
}
304456
);
457+
458+
// Test serialization with custom error
459+
let err = do_serialize_err(
460+
(SerializeWithCustomError, SerializeWithCustomError),
461+
&ColumnType::Tuple(vec![
462+
ColumnType::Native(NativeType::Double),
463+
ColumnType::Native(NativeType::Double),
464+
]),
465+
);
466+
let err = get_ser_err(&err);
467+
let BuiltinSerializationErrorKind::TupleError(
468+
TupleSerializationErrorKind::ElementSerializationFailed { index: 0, err },
469+
) = &err.kind
470+
else {
471+
panic!("unexpected error kind: {}", err.kind)
472+
};
473+
err.0
474+
.downcast_ref::<CustomSerializationError>()
475+
.expect("CustomSerializationError");
305476
}
306477

307478
#[test]

0 commit comments

Comments
 (0)