This is an automated email from the ASF dual-hosted git repository. mgrigorov pushed a commit to branch avro-3646-improve-deserialization in repository https://gitbox.apache.org/repos/asf/avro.git
commit f73232a51a0949a22732edff466f397381cdc66b Author: Martin Tzvetanov Grigorov <[email protected]> AuthorDate: Thu Feb 9 15:33:37 2023 +0200 AVRO-3646: [Rust] Add missing pattern match branches These improvements were identified while working on AVRO-3646 but they are not related to the changes related to this ticket. This commit was part of https://github.com/apache/avro/pull/1921 Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> --- lang/rust/avro/src/de.rs | 191 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 143 insertions(+), 48 deletions(-) diff --git a/lang/rust/avro/src/de.rs b/lang/rust/avro/src/de.rs index 75ac229c8..a5bc14b4f 100644 --- a/lang/rust/avro/src/de.rs +++ b/lang/rust/avro/src/de.rs @@ -250,7 +250,7 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Union(_i, u) => match **u { Value::Null => visitor.visit_unit(), Value::Boolean(b) => visitor.visit_bool(b), - Value::Int(i) => visitor.visit_i32(i), + Value::Int(i) | Value::Date(i) | Value::TimeMillis(i) => visitor.visit_i32(i), Value::Long(i) | Value::TimeMicros(i) | Value::TimestampMillis(i) @@ -260,6 +260,7 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Record(ref fields) => visitor.visit_map(RecordDeserializer::new(fields)), Value::Array(ref fields) => visitor.visit_seq(SeqDeserializer::new(fields)), Value::String(ref s) => visitor.visit_borrowed_str(s), + Value::Uuid(uuid) => visitor.visit_str(&uuid.to_string()), Value::Map(ref items) => visitor.visit_map(MapDeserializer::new(items)), _ => Err(de::Error::custom(format!( "unsupported union: {:?}", @@ -269,6 +270,7 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Record(ref fields) => visitor.visit_map(RecordDeserializer::new(fields)), Value::Array(ref fields) => visitor.visit_seq(SeqDeserializer::new(fields)), Value::String(ref s) => visitor.visit_borrowed_str(s), + Value::Uuid(uuid) => visitor.visit_str(&uuid.to_string()), Value::Map(ref items) => visitor.visit_map(MapDeserializer::new(items)), value => Err(de::Error::custom(format!( "incorrect value of type: {:?}", @@ -298,7 +300,10 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { .map_err(|e| de::Error::custom(e.to_string())) .and_then(|s| visitor.visit_borrowed_str(s)), Value::Uuid(ref u) => visitor.visit_str(&u.to_string()), - _ => Err(de::Error::custom("not a string|bytes|fixed")), + _ => Err(de::Error::custom(format!( + "Expected a String|Bytes|Fixed|Uuid, but got {:?}", + self.input + ))), } } @@ -313,11 +318,23 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { .map_err(|e| de::Error::custom(e.to_string())) .and_then(|s| visitor.visit_string(s)) } + Value::Uuid(ref u) => visitor.visit_str(&u.to_string()), Value::Union(_i, ref x) => match **x { Value::String(ref s) => visitor.visit_borrowed_str(s), - _ => Err(de::Error::custom("not a string|bytes|fixed")), + Value::Bytes(ref bytes) | Value::Fixed(_, ref bytes) => { + String::from_utf8(bytes.to_owned()) + .map_err(|e| de::Error::custom(e.to_string())) + .and_then(|s| visitor.visit_string(s)) + } + Value::Uuid(ref u) => visitor.visit_str(&u.to_string()), + _ => Err(de::Error::custom(format!( + "Expected a String|Bytes|Fixed|Uuid, but got {x:?}" + ))), }, - _ => Err(de::Error::custom("not a string|bytes|fixed")), + _ => Err(de::Error::custom(format!( + "Expected a String|Bytes|Fixed|Uuid|Union, but got {:?}", + self.input + ))), } } @@ -329,7 +346,10 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::String(ref s) => visitor.visit_bytes(s.as_bytes()), Value::Bytes(ref bytes) | Value::Fixed(_, ref bytes) => visitor.visit_bytes(bytes), Value::Uuid(ref u) => visitor.visit_bytes(u.as_bytes()), - _ => Err(de::Error::custom("not a string|bytes|fixed")), + _ => Err(de::Error::custom(format!( + "Expected a String|Bytes|Fixed|Uuid, but got {:?}", + self.input + ))), } } @@ -342,7 +362,10 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Bytes(ref bytes) | Value::Fixed(_, ref bytes) => { visitor.visit_byte_buf(bytes.to_owned()) } - _ => Err(de::Error::custom("not a string|bytes|fixed")), + _ => Err(de::Error::custom(format!( + "Expected a String|Bytes|Fixed, but got {:?}", + self.input + ))), } } @@ -353,7 +376,10 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { match *self.input { Value::Union(_i, ref inner) if inner.as_ref() == &Value::Null => visitor.visit_none(), Value::Union(_i, ref inner) => visitor.visit_some(&Deserializer::new(inner)), - _ => Err(de::Error::custom("not a union")), + _ => Err(de::Error::custom(format!( + "Expected a Union, but got {:?}", + self.input + ))), } } @@ -365,15 +391,21 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Null => visitor.visit_unit(), Value::Union(_i, ref x) => match **x { Value::Null => visitor.visit_unit(), - _ => Err(de::Error::custom("not a null")), + _ => Err(de::Error::custom(format!( + "Expected a Null, but got {:?}", + self.input + ))), }, - _ => Err(de::Error::custom("not a null")), + _ => Err(de::Error::custom(format!( + "Expected a Null|Union, but got {:?}", + self.input + ))), } } fn deserialize_unit_struct<V>( self, - _: &'static str, + _struct_name: &'static str, visitor: V, ) -> Result<V::Value, Self::Error> where @@ -384,7 +416,7 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { fn deserialize_newtype_struct<V>( self, - _: &'static str, + _struct_name: &'static str, visitor: V, ) -> Result<V::Value, Self::Error> where @@ -401,9 +433,15 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Array(ref items) => visitor.visit_seq(SeqDeserializer::new(items)), Value::Union(_i, ref inner) => match **inner { Value::Array(ref items) => visitor.visit_seq(SeqDeserializer::new(items)), - _ => Err(de::Error::custom("not an array")), + Value::Null => visitor.visit_seq(SeqDeserializer::new(&[])), + _ => Err(de::Error::custom(format!( + "Expected an Array or Null, but got: {inner:?}" + ))), }, - _ => Err(de::Error::custom("not an array")), + _ => Err(de::Error::custom(format!( + "Expected an Array or Union, but got: {:?}", + self.input + ))), } } @@ -416,8 +454,8 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { fn deserialize_tuple_struct<V>( self, - _: &'static str, - _: usize, + _struct_name: &'static str, + _len: usize, visitor: V, ) -> Result<V::Value, Self::Error> where @@ -442,8 +480,8 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { fn deserialize_struct<V>( self, - _: &'static str, - _: &'static [&'static str], + _struct_name: &'static str, + _fields: &'static [&'static str], visitor: V, ) -> Result<V::Value, Self::Error> where @@ -453,15 +491,21 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Record(ref fields) => visitor.visit_map(RecordDeserializer::new(fields)), Value::Union(_i, ref inner) => match **inner { Value::Record(ref fields) => visitor.visit_map(RecordDeserializer::new(fields)), - _ => Err(de::Error::custom("not a record")), + Value::Null => visitor.visit_map(RecordDeserializer::new(&[])), + _ => Err(de::Error::custom(format!( + "Expected a Record or Null, got: {inner:?}" + ))), }, - _ => Err(de::Error::custom("not a record")), + _ => Err(de::Error::custom(format!( + "Expected a Record or Union, got: {:?}", + self.input + ))), } } fn deserialize_enum<V>( self, - _: &'static str, + _enum_name: &'static str, _variants: &'static [&'static str], visitor: V, ) -> Result<V::Value, Self::Error> @@ -473,7 +517,10 @@ impl<'a, 'de> de::Deserializer<'de> for &'a Deserializer<'de> { Value::Record(ref fields) => visitor.visit_enum(EnumDeserializer::new(fields)), // This has to be a unit Enum Value::Enum(_index, ref field) => visitor.visit_enum(EnumUnitDeserializer::new(field)), - _ => Err(de::Error::custom("not an enum")), + _ => Err(de::Error::custom(format!( + "Expected a Record|Enum, but got {:?}", + self.input + ))), } } @@ -663,17 +710,6 @@ mod tests { Val2, } - #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] - struct TestNullExternalEnum { - a: NullExternalEnum, - } - - #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] - enum NullExternalEnum { - Val1(()), - Val2(u64), - } - #[derive(Debug, Serialize, Deserialize, PartialEq)] struct TestSingleValueExternalEnum { a: SingleValueExternalEnum, @@ -735,6 +771,7 @@ mod tests { let final_value: TestInner = from_value(&test_inner).unwrap(); assert_eq!(final_value, expected_inner) } + #[test] fn test_from_value_unit_enum() { let expected = TestUnitExternalEnum { @@ -787,23 +824,81 @@ mod tests { } #[test] - fn test_from_value_null_enum() { - let expected = TestNullExternalEnum { - a: NullExternalEnum::Val1(()), - }; + fn avro_3645_3646_test_from_value_enum() { + #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] + struct TestNullExternalEnum { + a: NullExternalEnum, + } - let test = Value::Record(vec![( - "a".to_owned(), - Value::Record(vec![ - ("type".to_owned(), Value::String("Val1".to_owned())), - ("value".to_owned(), Value::Union(0, Box::new(Value::Null))), - ]), - )]); - let final_value: TestNullExternalEnum = from_value(&test).unwrap(); - assert_eq!( - final_value, expected, - "Error deserializing null external enum" - ); + #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] + enum NullExternalEnum { + Val1, + Val2(), + Val3(()), + Val4(u64), + } + + let data = vec![ + ( + TestNullExternalEnum { + a: NullExternalEnum::Val1, + }, + Value::Record(vec![("a".to_owned(), Value::Enum(0, "Val1".to_owned()))]), + ), + ( + TestNullExternalEnum { + a: NullExternalEnum::Val2(), + }, + Value::Record(vec![( + "a".to_owned(), + Value::Record(vec![ + ("type".to_owned(), Value::Enum(1, "Val2".to_owned())), + ("value".to_owned(), Value::Union(1, Box::new(Value::Null))), + ]), + )]), + ), + ( + TestNullExternalEnum { + a: NullExternalEnum::Val2(), + }, + Value::Record(vec![( + "a".to_owned(), + Value::Record(vec![ + ("type".to_owned(), Value::Enum(1, "Val2".to_owned())), + ("value".to_owned(), Value::Array(vec![])), + ]), + )]), + ), + ( + TestNullExternalEnum { + a: NullExternalEnum::Val3(()), + }, + Value::Record(vec![( + "a".to_owned(), + Value::Record(vec![ + ("type".to_owned(), Value::Enum(2, "Val3".to_owned())), + ("value".to_owned(), Value::Union(2, Box::new(Value::Null))), + ]), + )]), + ), + ( + TestNullExternalEnum { + a: NullExternalEnum::Val4(123), + }, + Value::Record(vec![( + "a".to_owned(), + Value::Record(vec![ + ("type".to_owned(), Value::Enum(3, "Val4".to_owned())), + ("value".to_owned(), Value::Union(3, Value::Long(123).into())), + ]), + )]), + ), + ]; + + for (expected, test) in data.iter() { + let actual: TestNullExternalEnum = from_value(test).unwrap(); + assert_eq!(actual, *expected); + } } #[test]
