Skip to content

Commit

Permalink
Serialize and deserialize tuple struct with one field as newtype stru…
Browse files Browse the repository at this point in the history
…ct (#15628)

# Objective

- fix #15623
## Solution

- Checking field length of tuple struct before ser/der

## Testing

- CI should pass

## Migration Guide

- Reflection now will serialize and deserialize tuple struct with single
field as newtype struct. Consider this code.
```rs
#[derive(Reflect, Serialize)]
struct Test(usize);
let reflect = Test(3);
let serializer = TypedReflectSerializer::new(reflect.as_partial_reflect(), &registry);
return serde_json::to_string(&serializer)
```
Old behavior will return `["3"]`. New behavior will return `"3"`. If you
were relying on old behavior you need to update your logic. Especially
with `serde_json`. `ron` doesn't affect from this.
  • Loading branch information
notmd authored Oct 7, 2024
1 parent 0c959f7 commit cab0076
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 25 deletions.
34 changes: 24 additions & 10 deletions crates/bevy_reflect/src/serde/de/deserializer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#[cfg(feature = "debug_stack")]
use crate::serde::de::error_utils::TYPE_INFO_STACK;
use crate::serde::ReflectDeserializeWithRegistry;
use crate::serde::{ReflectDeserializeWithRegistry, SerializationData};
use crate::{
serde::{
de::{
Expand Down Expand Up @@ -305,15 +305,29 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
Ok(Box::new(dynamic_struct))
}
TypeInfo::TupleStruct(tuple_struct_info) => {
let mut dynamic_tuple_struct = deserializer.deserialize_tuple_struct(
tuple_struct_info.type_path_table().ident().unwrap(),
tuple_struct_info.field_len(),
TupleStructVisitor::new(
tuple_struct_info,
self.registration,
self.registry,
),
)?;
let mut dynamic_tuple_struct = if tuple_struct_info.field_len() == 1
&& self.registration.data::<SerializationData>().is_none()
{
deserializer.deserialize_newtype_struct(
tuple_struct_info.type_path_table().ident().unwrap(),
TupleStructVisitor::new(
tuple_struct_info,
self.registration,
self.registry,
),
)?
} else {
deserializer.deserialize_tuple_struct(
tuple_struct_info.type_path_table().ident().unwrap(),
tuple_struct_info.field_len(),
TupleStructVisitor::new(
tuple_struct_info,
self.registration,
self.registry,
),
)?
};

dynamic_tuple_struct.set_represented_type(Some(self.registration.type_info()));
Ok(Box::new(dynamic_tuple_struct))
}
Expand Down
15 changes: 8 additions & 7 deletions crates/bevy_reflect/src/serde/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,20 +472,21 @@ mod tests {
#[test]
fn should_deserialize_self_describing_binary() {
let expected = get_my_struct();

let registry = get_registry();

let input = vec![
129, 217, 40, 98, 101, 118, 121, 95, 114, 101, 102, 108, 101, 99, 116, 58, 58, 115,
101, 114, 100, 101, 58, 58, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121,
83, 116, 114, 117, 99, 116, 220, 0, 20, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111,
114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0,
1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 64, 145, 206, 59, 154, 201, 255, 145,
172, 84, 117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105,
116, 129, 167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101,
146, 202, 63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99,
116, 145, 180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32,
118, 97, 108, 117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165,
84, 117, 112, 108, 101, 144, 146, 100, 145, 101,
1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 64, 145, 206, 59, 154, 201, 255, 172,
84, 117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105, 116,
129, 167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146,
202, 63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116,
145, 180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97,
108, 117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165, 84, 117,
112, 108, 101, 144, 146, 100, 145, 101,
];

let mut reader = std::io::BufReader::new(input.as_slice());
Expand Down
37 changes: 34 additions & 3 deletions crates/bevy_reflect/src/serde/de/tuple_structs.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use crate::{
serde::de::tuple_utils::visit_tuple, DynamicTupleStruct, TupleStructInfo, TypeRegistration,
TypeRegistry,
serde::{de::tuple_utils::visit_tuple, SerializationData},
DynamicTupleStruct, TupleStructInfo, TypeRegistration, TypeRegistry,
};
use core::{fmt, fmt::Formatter};
use serde::de::{SeqAccess, Visitor};
use serde::de::{DeserializeSeed, SeqAccess, Visitor};

use super::{registration_utils::try_get_registration, TypedReflectDeserializer};

/// A [`Visitor`] for deserializing [`TupleStruct`] values.
///
Expand Down Expand Up @@ -47,4 +49,33 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> {
)
.map(DynamicTupleStruct::from)
}

fn visit_newtype_struct<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
where
D: serde::Deserializer<'de>,
{
let mut tuple = DynamicTupleStruct::default();
let serialization_data = self.registration.data::<SerializationData>();

if let Some(value) = serialization_data.and_then(|data| data.generate_default(0)) {
tuple.insert_boxed(value.into_partial_reflect());
return Ok(tuple);
}

let registration = try_get_registration(
*self
.tuple_struct_info
.field_at(0)
.ok_or(serde::de::Error::custom("Field at index 0 not found"))?
.ty(),
self.registry,
)?;
let reflect_deserializer =
TypedReflectDeserializer::new_internal(registration, self.registry);
let value = reflect_deserializer.deserialize(deserializer)?;

tuple.insert_boxed(value.into_partial_reflect());

Ok(tuple)
}
}
59 changes: 58 additions & 1 deletion crates/bevy_reflect/src/serde/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,11 @@ mod tests {
use crate::{ReflectFromReflect, TypePath};
use alloc::sync::Arc;
use bevy_reflect_derive::reflect_trait;
use core::any::TypeId;
use core::fmt::{Debug, Formatter};
use serde::de::{SeqAccess, Visitor};
use serde::ser::SerializeSeq;
use serde::{Deserializer, Serializer};
use serde::{Deserializer, Serialize, Serializer};

#[reflect_trait]
trait Enemy: Reflect + Debug {
Expand Down Expand Up @@ -398,5 +399,61 @@ mod tests {
// Poor man's comparison since we can't derive PartialEq for Arc<dyn Enemy>
assert_ne!(format!("{:?}", unexpected), format!("{:?}", output));
}

#[test]
fn should_serialize_single_tuple_struct_as_newtype() {
#[derive(Reflect, Serialize, PartialEq, Debug)]
struct TupleStruct(u32);

#[derive(Reflect, Serialize, PartialEq, Debug)]
struct TupleStructWithSkip(
u32,
#[reflect(skip_serializing)]
#[serde(skip)]
u32,
);

#[derive(Reflect, Serialize, PartialEq, Debug)]
enum Enum {
TupleStruct(usize),
NestedTupleStruct(TupleStruct),
NestedTupleStructWithSkip(TupleStructWithSkip),
}

let mut registry = TypeRegistry::default();
registry.register::<TupleStruct>();
registry.register::<TupleStructWithSkip>();
registry.register::<Enum>();

let tuple_struct = TupleStruct(1);
let tuple_struct_with_skip = TupleStructWithSkip(2, 3);
let tuple_struct_enum = Enum::TupleStruct(4);
let nested_tuple_struct = Enum::NestedTupleStruct(TupleStruct(5));
let nested_tuple_struct_with_skip =
Enum::NestedTupleStructWithSkip(TupleStructWithSkip(6, 7));

fn assert_serialize<T: Reflect + FromReflect + Serialize + PartialEq + Debug>(
value: &T,
registry: &TypeRegistry,
) {
let serializer = TypedReflectSerializer::new(value, registry);
let reflect_serialize = serde_json::to_string(&serializer).unwrap();
let serde_serialize = serde_json::to_string(value).unwrap();
assert_eq!(reflect_serialize, serde_serialize);

let registration = registry.get(TypeId::of::<T>()).unwrap();
let reflect_deserializer = TypedReflectDeserializer::new(registration, registry);

let mut deserializer = serde_json::Deserializer::from_str(&serde_serialize);
let reflect_value = reflect_deserializer.deserialize(&mut deserializer).unwrap();
let _ = T::from_reflect(&*reflect_value).unwrap();
}

assert_serialize(&tuple_struct, &registry);
assert_serialize(&tuple_struct_with_skip, &registry);
assert_serialize(&tuple_struct_enum, &registry);
assert_serialize(&nested_tuple_struct, &registry);
assert_serialize(&nested_tuple_struct_with_skip, &registry);
}
}
}
8 changes: 4 additions & 4 deletions crates/bevy_reflect/src/serde/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,10 @@ mod tests {
121, 83, 116, 114, 117, 99, 116, 220, 0, 20, 123, 172, 72, 101, 108, 108, 111, 32, 119,
111, 114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255,
0, 1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 64, 145, 206, 59, 154, 201, 255,
145, 172, 84, 117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110,
105, 116, 129, 167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108,
101, 146, 202, 63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117,
99, 116, 145, 180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32,
172, 84, 117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105,
116, 129, 167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101,
146, 202, 63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99,
116, 145, 180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32,
118, 97, 108, 117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165,
84, 117, 112, 108, 101, 144, 146, 100, 145, 101,
];
Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_reflect/src/serde/ser/tuple_structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ impl<'a> Serialize for TupleStructSerializer<'a> {
.get(type_info.type_id())
.and_then(|registration| registration.data::<SerializationData>());
let ignored_len = serialization_data.map(SerializationData::len).unwrap_or(0);

if self.tuple_struct.field_len() == 1 && serialization_data.is_none() {
let field = self.tuple_struct.field(0).unwrap();
return serializer.serialize_newtype_struct(
tuple_struct_info.type_path_table().ident().unwrap(),
&TypedReflectSerializer::new_internal(field, self.registry),
);
}

let mut state = serializer.serialize_tuple_struct(
tuple_struct_info.type_path_table().ident().unwrap(),
self.tuple_struct.field_len() - ignored_len,
Expand Down

0 comments on commit cab0076

Please sign in to comment.