From cab00766d99c2bf0519464a60e8a91e05c83d0fd Mon Sep 17 00:00:00 2001 From: notmd <33456881+notmd@users.noreply.github.com> Date: Tue, 8 Oct 2024 06:40:03 +0700 Subject: [PATCH] Serialize and deserialize tuple struct with one field as newtype struct (#15628) # Objective - fix https://github.com/bevyengine/bevy/issues/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(), ®istry); 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. --- .../bevy_reflect/src/serde/de/deserializer.rs | 34 +++++++---- crates/bevy_reflect/src/serde/de/mod.rs | 15 ++--- .../src/serde/de/tuple_structs.rs | 37 +++++++++++- crates/bevy_reflect/src/serde/mod.rs | 59 ++++++++++++++++++- crates/bevy_reflect/src/serde/ser/mod.rs | 8 +-- .../src/serde/ser/tuple_structs.rs | 9 +++ 6 files changed, 137 insertions(+), 25 deletions(-) diff --git a/crates/bevy_reflect/src/serde/de/deserializer.rs b/crates/bevy_reflect/src/serde/de/deserializer.rs index 1379fb3768d2f..a1a97e3877ef3 100644 --- a/crates/bevy_reflect/src/serde/de/deserializer.rs +++ b/crates/bevy_reflect/src/serde/de/deserializer.rs @@ -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::{ @@ -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::().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)) } diff --git a/crates/bevy_reflect/src/serde/de/mod.rs b/crates/bevy_reflect/src/serde/de/mod.rs index 318cb8d42671f..92a43ed17565d 100644 --- a/crates/bevy_reflect/src/serde/de/mod.rs +++ b/crates/bevy_reflect/src/serde/de/mod.rs @@ -472,6 +472,7 @@ mod tests { #[test] fn should_deserialize_self_describing_binary() { let expected = get_my_struct(); + let registry = get_registry(); let input = vec![ @@ -479,13 +480,13 @@ mod tests { 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()); diff --git a/crates/bevy_reflect/src/serde/de/tuple_structs.rs b/crates/bevy_reflect/src/serde/de/tuple_structs.rs index e070142ab71c8..c9703cb504f75 100644 --- a/crates/bevy_reflect/src/serde/de/tuple_structs.rs +++ b/crates/bevy_reflect/src/serde/de/tuple_structs.rs @@ -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. /// @@ -47,4 +49,33 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> { ) .map(DynamicTupleStruct::from) } + + fn visit_newtype_struct(self, deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let mut tuple = DynamicTupleStruct::default(); + let serialization_data = self.registration.data::(); + + 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) + } } diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index b0f0d7e5c6910..3fcaa6aafc7b3 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -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 { @@ -398,5 +399,61 @@ mod tests { // Poor man's comparison since we can't derive PartialEq for Arc 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::(); + registry.register::(); + registry.register::(); + + 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( + 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::()).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, ®istry); + assert_serialize(&tuple_struct_with_skip, ®istry); + assert_serialize(&tuple_struct_enum, ®istry); + assert_serialize(&nested_tuple_struct, ®istry); + assert_serialize(&nested_tuple_struct_with_skip, ®istry); + } } } diff --git a/crates/bevy_reflect/src/serde/ser/mod.rs b/crates/bevy_reflect/src/serde/ser/mod.rs index 0a0d006b989fc..ead36d4819898 100644 --- a/crates/bevy_reflect/src/serde/ser/mod.rs +++ b/crates/bevy_reflect/src/serde/ser/mod.rs @@ -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, ]; diff --git a/crates/bevy_reflect/src/serde/ser/tuple_structs.rs b/crates/bevy_reflect/src/serde/ser/tuple_structs.rs index 388cdebe5718f..55e171feae473 100644 --- a/crates/bevy_reflect/src/serde/ser/tuple_structs.rs +++ b/crates/bevy_reflect/src/serde/ser/tuple_structs.rs @@ -48,6 +48,15 @@ impl<'a> Serialize for TupleStructSerializer<'a> { .get(type_info.type_id()) .and_then(|registration| registration.data::()); 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,