Perform property migrations in rbx_xml serializer (#340)

rojo-rbx · Aug 4, 2023 · 0805cf6 · 0805cf6
1 parent bd7fa90
commit 0805cf6
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 15 deletions.
diff --git a/rbx_xml/src/core.rs b/rbx_xml/src/core.rs
@@ -50,16 +50,7 @@ pub fn find_serialized_property_descriptor(
     class_name: &str,
     property_name: &str,
 ) -> Option<&'static PropertyDescriptor<'static>> {
-    find_property_descriptors(class_name, property_name).and_then(|(_canonical, serialized)| {
-        if let PropertyKind::Canonical {
-            serialization: PropertySerialization::Migrate(_),
-        } = serialized.kind
-        {
-            None
-        } else {
-            Some(serialized)
-        }
-    })
+    find_property_descriptors(class_name, property_name).map(|(_canonical, serialized)| serialized)
 }
 
 /// Find both the canonical and serialized property descriptors for a given

diff --git a/rbx_xml/src/serializer.rs b/rbx_xml/src/serializer.rs
@@ -1,4 +1,5 @@
 use std::{
+    borrow::Cow,
     collections::{BTreeMap, HashMap},
     io::Write,
 };
@@ -7,7 +8,7 @@ use rbx_dom_weak::{
     types::{Ref, SharedString, SharedStringHash, Variant, VariantType},
     WeakDom,
 };
-use rbx_reflection::DataType;
+use rbx_reflection::{DataType, PropertyKind, PropertySerialization};
 
 use crate::{
     conversion::ConvertVariant,
@@ -194,7 +195,9 @@ fn serialize_instance<'a, W: Write>(
                 _ => unimplemented!(),
             };
 
-            let converted_value = match value.try_convert_ref(data_type) {
+            let mut serialized_name = serialized_descriptor.name.as_ref();
+
+            let mut converted_value = match value.try_convert_ref(data_type) {
                 Ok(value) => value,
                 Err(message) => {
                     return Err(
@@ -209,7 +212,20 @@ fn serialize_instance<'a, W: Write>(
                 }
             };
 
-            write_value_xml(writer, state, &serialized_descriptor.name, &converted_value)?;
+            // Perform migrations during serialization
+            if let PropertyKind::Canonical {
+                serialization: PropertySerialization::Migrate(migration),
+            } = &serialized_descriptor.kind
+            {
+                // If the migration fails, there's no harm in us doing nothing
+                // since old values will still load in Studio.
+                if let Ok(new_value) = migration.perform(&converted_value) {
+                    converted_value = Cow::Owned(new_value);
+                    serialized_name = &migration.new_property_name
+                }
+            }
+
+            write_value_xml(writer, state, serialized_name, &converted_value)?;
         } else {
             match state.options.property_behavior {
                 EncodePropertyBehavior::IgnoreUnknown => {}

diff --git a/rbx_xml/src/tests/basic.rs b/rbx_xml/src/tests/basic.rs
@@ -2,8 +2,9 @@
 
 use rbx_dom_weak::types::{
     Attributes, BinaryString, BrickColor, Color3, Color3uint8, ColorSequence,
-    ColorSequenceKeypoint, MaterialColors, NumberRange, NumberSequence, NumberSequenceKeypoint,
-    Rect, Tags, TerrainMaterials, UDim, UDim2, UniqueId, Variant, Vector2, Vector3,
+    ColorSequenceKeypoint, Enum, Font, MaterialColors, NumberRange, NumberSequence,
+    NumberSequenceKeypoint, Rect, Tags, TerrainMaterials, UDim, UDim2, UniqueId, Variant, Vector2,
+    Vector3,
 };
 use rbx_dom_weak::{InstanceBuilder, WeakDom};
 
@@ -316,3 +317,31 @@ fn number_widening() {
         Some(&Variant::Float64(1337.0))
     );
 }
+
+#[test]
+fn migrated_properties() {
+    let tree = WeakDom::new(InstanceBuilder::new("Folder").with_children([
+        InstanceBuilder::new("ScreenGui").with_property("ScreenInsets", Enum::from_u32(0)),
+        InstanceBuilder::new("ScreenGui").with_property("IgnoreGuiInset", true),
+        InstanceBuilder::new("Part").with_property("Color", Color3::new(1.0, 1.0, 1.0)),
+        InstanceBuilder::new("Part").with_property("BrickColor", BrickColor::Alder),
+        InstanceBuilder::new("Part").with_property("brickColor", BrickColor::Alder),
+        InstanceBuilder::new("TextLabel").with_property("FontFace", Font::default()),
+        InstanceBuilder::new("TextLabel").with_property("Font", Enum::from_u32(8)),
+    ]));
+
+    let mut encoded = Vec::new();
+    crate::to_writer_default(&mut encoded, &tree, &[tree.root_ref()]).unwrap();
+    insta::assert_snapshot!(std::str::from_utf8(&encoded).unwrap());
+}
+
+#[test]
+fn bad_migrated_property() {
+    let tree = WeakDom::new(InstanceBuilder::new("Folder").with_children([
+        InstanceBuilder::new("TextLabel").with_property("Font", Enum::from_u32(u32::MAX)),
+    ]));
+
+    let mut encoded = Vec::new();
+    crate::to_writer_default(&mut encoded, &tree, &[tree.root_ref()]).unwrap();
+    insta::assert_snapshot!(std::str::from_utf8(&encoded).unwrap());
+}
diff --git a/rbx_xml/src/tests/snapshots/rbx_xml__tests__basic__bad_migrated_property.snap b/rbx_xml/src/tests/snapshots/rbx_xml__tests__basic__bad_migrated_property.snap
@@ -0,0 +1,17 @@
+---
+source: rbx_xml/src/tests/basic.rs
+expression: "std::str::from_utf8(&encoded).unwrap()"
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">Folder</string>
+    </Properties>
+    <Item class="TextLabel" referent="1">
+      <Properties>
+        <string name="Name">TextLabel</string>
+        <token name="Font">4294967295</token>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>
diff --git a/rbx_xml/src/tests/snapshots/rbx_xml__tests__basic__migrated_properties.snap b/rbx_xml/src/tests/snapshots/rbx_xml__tests__basic__migrated_properties.snap
@@ -0,0 +1,65 @@
+---
+source: rbx_xml/src/tests/basic.rs
+expression: "std::str::from_utf8(&encoded).unwrap()"
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">Folder</string>
+    </Properties>
+    <Item class="ScreenGui" referent="1">
+      <Properties>
+        <string name="Name">ScreenGui</string>
+        <token name="ScreenInsets">0</token>
+      </Properties>
+    </Item>
+    <Item class="ScreenGui" referent="2">
+      <Properties>
+        <string name="Name">ScreenGui</string>
+        <token name="ScreenInsets">1</token>
+      </Properties>
+    </Item>
+    <Item class="Part" referent="3">
+      <Properties>
+        <string name="Name">Part</string>
+        <Color3uint8 name="Color3uint8">16777215</Color3uint8>
+      </Properties>
+    </Item>
+    <Item class="Part" referent="4">
+      <Properties>
+        <string name="Name">Part</string>
+        <Color3uint8 name="Color">11829503</Color3uint8>
+      </Properties>
+    </Item>
+    <Item class="Part" referent="5">
+      <Properties>
+        <string name="Name">Part</string>
+        <Color3uint8 name="Color">11829503</Color3uint8>
+      </Properties>
+    </Item>
+    <Item class="TextLabel" referent="6">
+      <Properties>
+        <string name="Name">TextLabel</string>
+        <Font name="FontFace">
+          <Family>
+            <url>rbxasset://fonts/families/SourceSansPro.json</url>
+          </Family>
+          <Weight>400</Weight>
+          <Style>Normal</Style>
+        </Font>
+      </Properties>
+    </Item>
+    <Item class="TextLabel" referent="7">
+      <Properties>
+        <string name="Name">TextLabel</string>
+        <Font name="FontFace">
+          <Family>
+            <url>rbxasset://fonts/families/Guru.json</url>
+          </Family>
+          <Weight>400</Weight>
+          <Style>Normal</Style>
+        </Font>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>