This is an automated email from the ASF dual-hosted git repository. kriskras99 pushed a commit to branch fix/simplify_deserializer in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit 5c47475da9de62d011d186b0eddd2cd20a3ba5ec Author: Kriskras99 <[email protected]> AuthorDate: Tue Jan 20 09:06:26 2026 +0100 fix: Simplify deserializer Removed an unnecessary indirection through a reference. Also replaced `Union` matches with a recursive call. --- avro/src/serde/de.rs | 271 +++++++++++++++++++++------------------------------ 1 file changed, 111 insertions(+), 160 deletions(-) diff --git a/avro/src/serde/de.rs b/avro/src/serde/de.rs index 257dd6a..6779502 100644 --- a/avro/src/serde/de.rs +++ b/avro/src/serde/de.rs @@ -216,7 +216,7 @@ impl<'de> de::VariantAccess<'de> for EnumDeserializer<'de> { Err(de::Error::custom( "Expected a newtype variant, got nothing instead.", )), - |item| seed.deserialize(&Deserializer::new(&item.1)), + |item| seed.deserialize(Deserializer::new(&item.1)), ) } @@ -228,7 +228,7 @@ impl<'de> de::VariantAccess<'de> for EnumDeserializer<'de> { Err(de::Error::custom( "Expected a tuple variant, got nothing instead.", )), - |item| de::Deserializer::deserialize_seq(&Deserializer::new(&item.1), visitor), + |item| de::Deserializer::deserialize_seq(Deserializer::new(&item.1), visitor), ) } @@ -244,7 +244,7 @@ impl<'de> de::VariantAccess<'de> for EnumDeserializer<'de> { Err(de::Error::custom("Expected a struct variant, got nothing")), |item| { de::Deserializer::deserialize_struct( - &Deserializer::new(&item.1), + Deserializer::new(&item.1), "", fields, visitor, @@ -286,7 +286,7 @@ impl<'de> de::VariantAccess<'de> for UnionDeserializer<'de> { where T: DeserializeSeed<'de>, { - seed.deserialize(&Deserializer::new(self.value)) + seed.deserialize(Deserializer::new(self.value)) } fn tuple_variant<V>(self, len: usize, visitor: V) -> Result<V::Value, Self::Error> @@ -309,7 +309,7 @@ impl<'de> de::VariantAccess<'de> for UnionDeserializer<'de> { } } -impl<'de> de::Deserializer<'de> for &Deserializer<'de> { +impl<'de> de::Deserializer<'de> for Deserializer<'de> { type Error = Error; fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error> @@ -330,40 +330,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { | Value::LocalTimestampNanos(i) => visitor.visit_i64(*i), &Value::Float(f) => visitor.visit_f32(f), &Value::Double(d) => visitor.visit_f64(d), - Value::Union(_i, u) => match **u { - Value::Null => visitor.visit_unit(), - Value::Boolean(b) => visitor.visit_bool(b), - Value::Int(i) | Value::Date(i) | Value::TimeMillis(i) => visitor.visit_i32(i), - Value::Long(i) - | Value::TimeMicros(i) - | Value::TimestampMillis(i) - | Value::TimestampMicros(i) - | Value::TimestampNanos(i) - | Value::LocalTimestampMillis(i) - | Value::LocalTimestampMicros(i) - | Value::LocalTimestampNanos(i) => visitor.visit_i64(i), - Value::Float(f) => visitor.visit_f32(f), - Value::Double(d) => visitor.visit_f64(d), - 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::Bytes(ref bytes) | Value::Fixed(_, ref bytes) => visitor.visit_bytes(bytes), - Value::Decimal(ref d) => visitor.visit_bytes(&d.to_vec()?), - Value::Enum(_, ref s) => visitor.visit_borrowed_str(s), - Value::BigDecimal(ref big_decimal) => { - visitor.visit_str(big_decimal.to_plain_string().as_str()) - } - Value::Duration(ref duration) => { - let duration_bytes: [u8; 12] = duration.into(); - visitor.visit_bytes(&duration_bytes[..]) + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_any(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as any: {e:?}" + )) + }) } - Value::Union(_, _) => Err(de::Error::custom(format!( - "Directly nested union types are not supported. Got {:?}", - &**u - ))), - }, + } Value::Record(fields) => visitor.visit_map(RecordDeserializer::new(fields)), Value::Array(fields) => visitor.visit_seq(SeqDeserializer::new(fields)), Value::String(s) => visitor.visit_borrowed_str(s), @@ -410,31 +389,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { let n = u64::from_le_bytes(bytes.as_slice().try_into().expect("Size is 8")); visitor.visit_u64(n) } - Value::Union(_i, x) => match x.deref() { - Value::Int(i) | Value::Date(i) | Value::TimeMillis(i) => { - let n = u64::try_from(*i).map_err(|e| Details::ConvertI32ToU64(e, *i))?; - visitor.visit_u64(n) - } - Value::Long(i) - | Value::TimeMicros(i) - | Value::TimestampMillis(i) - | Value::TimestampMicros(i) - | Value::TimestampNanos(i) - | Value::LocalTimestampMillis(i) - | Value::LocalTimestampMicros(i) - | Value::LocalTimestampNanos(i) => { - let n = u64::try_from(*i).map_err(|e| Details::ConvertI64ToU64(e, *i))?; - visitor.visit_u64(n) - } - Value::Fixed(8, bytes) => { - let n = u64::from_le_bytes(bytes.as_slice().try_into().expect("Size is 8")); - visitor.visit_u64(n) + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_u64(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as u64: {e:?}" + )) + }) } - _ => Err(de::Error::custom(format!( - "Expected a Int|Long|Fixed(8), but got {:?}", - self.input - ))), - }, + } _ => Err(de::Error::custom(format!( "Expected a Int|Long|Fixed(8), but got {:?}", self.input @@ -466,31 +433,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { let n = u128::from_le_bytes(bytes.as_slice().try_into().expect("Size is 16")); visitor.visit_u128(n) } - Value::Union(_i, x) => match x.deref() { - Value::Int(i) | Value::Date(i) | Value::TimeMillis(i) => { - let n = u128::try_from(*i).map_err(|e| Details::ConvertI32ToU128(e, *i))?; - visitor.visit_u128(n) - } - Value::Long(i) - | Value::TimeMicros(i) - | Value::TimestampMillis(i) - | Value::TimestampMicros(i) - | Value::TimestampNanos(i) - | Value::LocalTimestampMillis(i) - | Value::LocalTimestampMicros(i) - | Value::LocalTimestampNanos(i) => { - let n = u128::try_from(*i).map_err(|e| Details::ConvertI64ToU128(e, *i))?; - visitor.visit_u128(n) - } - Value::Fixed(16, bytes) => { - let n = u128::from_le_bytes(bytes.as_slice().try_into().expect("Size is 16")); - visitor.visit_u128(n) + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_u128(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as u128: {e:?}" + )) + }) } - _ => Err(de::Error::custom(format!( - "Expected a Int|Long|Fixed(16), but got {:?}", - self.input - ))), - }, + } _ => Err(de::Error::custom(format!( "Expected a Int|Long|Fixed(16), but got {:?}", self.input @@ -518,27 +473,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { let n = i128::from_le_bytes(bytes.as_slice().try_into().expect("Size is 16")); visitor.visit_i128(n) } - Value::Union(_i, x) => match x.deref() { - Value::Int(i) | Value::Date(i) | Value::TimeMillis(i) => { - visitor.visit_i128(i128::from(*i)) - } - Value::Long(i) - | Value::TimeMicros(i) - | Value::TimestampMillis(i) - | Value::TimestampMicros(i) - | Value::TimestampNanos(i) - | Value::LocalTimestampMillis(i) - | Value::LocalTimestampMicros(i) - | Value::LocalTimestampNanos(i) => visitor.visit_i128(i128::from(*i)), - Value::Fixed(16, bytes) => { - let n = i128::from_le_bytes(bytes.as_slice().try_into().expect("Size is 16")); - visitor.visit_i128(n) + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_i128(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as i128: {e:?}" + )) + }) } - _ => Err(de::Error::custom(format!( - "Expected a Int|Long|Fixed(16), but got {:?}", - self.input - ))), - }, + } _ => Err(de::Error::custom(format!( "Expected a Int|Long|Fixed(16), but got {:?}", self.input @@ -571,28 +518,18 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { Value::Fixed(4, bytes) => { visitor.visit_char(char::from_u32(u32::from_le_bytes(bytes.as_slice().try_into().expect("Size is 4"))).ok_or_else(|| <Self::Error as de::Error>::custom("Tried to deserialize char from fixed, but was invalid value"))?) } - Value::Union(_i, x) => match x.deref() { - Value::String(s) => { - if s.chars().count() == 1 { - visitor.visit_char(s.chars().next().expect("There is exactly one char")) - } else { - Err(de::Error::custom(format!("Tried to deserialize char from string, but the string was longer than one char: {s}"))) - } - } - Value::Bytes(bytes) => std::str::from_utf8(bytes) - .map_err(|e| de::Error::custom(e.to_string())) - .and_then(|s| { - if s.chars().count() == 1 { - visitor.visit_char(s.chars().next().expect("There is exactly one char")) - } else { - Err(de::Error::custom(format!("Tried to deserialize char from a byte array, but the byte array was longer than one char: {}", s.len()))) - } - } - ), - Value::Fixed(4, bytes) => { - visitor.visit_char(char::from_u32(u32::from_le_bytes(bytes.as_slice().try_into().expect("Size is 4"))).ok_or_else(|| <Self::Error as de::Error>::custom("Tried to deserialize char from fixed, but was invalid value"))?) + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_char(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as char: {e:?}" + )) + }) } - _ => Err(de::Error::custom(format!("Expected a String|Bytes|Fixed(4) for char, but got {:?}", self.input))) }, _ => Err(de::Error::custom(format!("Expected a String|Bytes|Fixed(4) for char, but got {:?}", self.input))) } @@ -608,16 +545,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { .map_err(|e| de::Error::custom(e.to_string())) .and_then(|s| visitor.visit_borrowed_str(s)), Value::Uuid(u) => visitor.visit_str(&u.to_string()), - Value::Union(_i, x) => match x.deref() { - Value::String(s) => visitor.visit_borrowed_str(s), - Value::Bytes(bytes) | Value::Fixed(_, bytes) => std::str::from_utf8(bytes) - .map_err(|e| de::Error::custom(e.to_string())) - .and_then(|s| visitor.visit_borrowed_str(s)), - Value::Uuid(u) => visitor.visit_str(&u.to_string()), - _ => Err(de::Error::custom(format!( - "Expected a String|Bytes|Fixed|Uuid, but got {x:?}" - ))), - }, + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_str(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as str: {e:?}" + )) + }) + } + } _ => Err(de::Error::custom(format!( "Expected a String|Bytes|Fixed|Uuid, but got {:?}", self.input @@ -635,16 +575,21 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { .map_err(|e| de::Error::custom(e.to_string())) .and_then(|s| visitor.visit_string(s)), Value::Uuid(u) => visitor.visit_str(&u.to_string()), - Value::Union(_i, x) => match x.deref() { - Value::String(s) => visitor.visit_borrowed_str(s), - Value::Bytes(bytes) | Value::Fixed(_, 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(u) => visitor.visit_str(&u.to_string()), - _ => Err(de::Error::custom(format!( - "Expected a String|Bytes|Fixed|Uuid, but got {x:?}" - ))), - }, + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()) + .deserialize_string(visitor) + .map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as string: {e:?}" + )) + }) + } + } _ => Err(de::Error::custom(format!( "Expected a String|Bytes|Fixed|Uuid|Union|Enum, but got {:?}", self.input @@ -706,7 +651,7 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { { match self.input { Value::Union(_i, inner) if inner.as_ref() == &Value::Null => visitor.visit_none(), - Value::Union(_i, inner) => visitor.visit_some(&Deserializer::new(inner)), + Value::Union(_i, inner) => visitor.visit_some(Deserializer::new(inner)), _ => Err(de::Error::custom(format!( "Expected a Union, but got {:?}", self.input @@ -720,13 +665,19 @@ impl<'de> de::Deserializer<'de> for &Deserializer<'de> { { match self.input { Value::Null => visitor.visit_unit(), - Value::Union(_i, x) => match **x { - Value::Null => visitor.visit_unit(), - _ => Err(de::Error::custom(format!( - "Expected a Null, but got {:?}", - self.input - ))), - }, + Value::Union(i, x) => { + if matches!(x.deref(), Value::Union(_, _)) { + Err(de::Error::custom(format!( + "Directly nested union types are not supported. Got Value::Union({i}, {x:?})" + ))) + } else { + Self::new(x.deref()).deserialize_unit(visitor).map_err(|e| { + de::Error::custom(format!( + "Attempted to deserialize Value::Union({i}, {x:?}) as unit: {e:?}" + )) + }) + } + } _ => Err(de::Error::custom(format!( "Expected a Null|Union, but got {:?}", self.input @@ -897,7 +848,7 @@ impl<'de> de::SeqAccess<'de> for SeqDeserializer<'de> { T: DeserializeSeed<'de>, { match self.input.next() { - Some(item) => seed.deserialize(&Deserializer::new(item)).map(Some), + Some(item) => seed.deserialize(Deserializer::new(item)).map(Some), None => Ok(None), } } @@ -925,7 +876,7 @@ impl<'de> de::MapAccess<'de> for MapDeserializer<'de> { V: DeserializeSeed<'de>, { match self.input_values.next() { - Some(value) => seed.deserialize(&Deserializer::new(value)), + Some(value) => seed.deserialize(Deserializer::new(value)), None => Err(de::Error::custom("should not happen - too many values")), } } @@ -955,7 +906,7 @@ impl<'de> de::MapAccess<'de> for RecordDeserializer<'de> { V: DeserializeSeed<'de>, { match self.value.take() { - Some(value) => seed.deserialize(&Deserializer::new(value)), + Some(value) => seed.deserialize(Deserializer::new(value)), None => Err(de::Error::custom("should not happen - too many values")), } } @@ -989,7 +940,7 @@ impl<'de> de::Deserializer<'de> for StringDeserializer { /// structure expected by `D`. pub fn from_value<'de, D: Deserialize<'de>>(value: &'de Value) -> Result<D, Error> { let de = Deserializer::new(value); - D::deserialize(&de) + D::deserialize(de) } #[cfg(test)] @@ -1774,7 +1725,7 @@ mod tests { assert!(!crate::util::is_human_readable()); - let deser = &Deserializer::new(&Value::Null); + let deser = Deserializer::new(&Value::Null); assert!(!deser.is_human_readable());
