fix: fix taking taking serializers for serializing older verions

Author: NathanFlurry

rust/README.md

@@ -60,8 +60,8 @@ pub enum MyTypeVersioned {
 impl OwnedVersionedData for MyTypeVersioned {
     type Latest = schemas::v2::MyType;
 
-    fn latest(latest: Self::Latest) -> Self { Self::V2(latest) }
-    fn into_latest(self) -> Result<Self::Latest> {
+    fn wrap_latest(latest: Self::Latest) -> Self { Self::V2(latest) }
+    fn unwrap_latest(self) -> Result<Self::Latest> {
         match self { Self::V2(x) => Ok(x), _ => bail!("not latest") }
     }
 

rust/vbare/src/lib.rs

@@ -1,4 +1,4 @@
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
 
 pub trait VersionedData<'a>: Sized {
     type Latest;
@@ -29,9 +29,12 @@ pub trait VersionedData<'a>: Sized {
     fn deserialize(payload: &'a [u8], version: u16) -> Result<Self::Latest> {
         let mut data = Self::deserialize_version(payload, version)?;
 
+        let skip_count = version
+            .checked_sub(1)
+            .with_context(|| format!("proto version ({version}) must be > 0"))?;
         for converter in Self::deserialize_converters()
             .iter()
-            .skip(version.saturating_sub(1) as usize)
+            .skip(skip_count as usize)
         {
             data = converter(data)?;
         }
@@ -42,10 +45,16 @@ pub trait VersionedData<'a>: Sized {
     fn serialize(self, version: u16) -> Result<Vec<u8>> {
         let mut data = self;
 
-        for converter in Self::serialize_converters()
-            .iter()
-            .skip(version.saturating_sub(1) as usize)
-        {
+        let converters = Self::serialize_converters();
+        let take_count = (converters.len() + 1)
+            .checked_sub(version as usize)
+            .with_context(|| {
+                format!(
+                    "proto version ({version}) greater than latest version ({})",
+                    converters.len() + 1
+                )
+            })?;
+        for converter in converters.iter().take(take_count) {
             data = converter(data)?;
         }
 
@@ -103,9 +112,12 @@ pub trait OwnedVersionedData: Sized {
     fn deserialize(payload: &[u8], version: u16) -> Result<Self::Latest> {
         let mut data = Self::deserialize_version(payload, version)?;
 
+        let skip_count = version
+            .checked_sub(1)
+            .with_context(|| format!("proto version ({version}) must be > 0"))?;
         for converter in Self::deserialize_converters()
             .iter()
-            .skip(version.saturating_sub(1) as usize)
+            .skip(skip_count as usize)
         {
             data = converter(data)?;
         }
@@ -116,10 +128,16 @@ pub trait OwnedVersionedData: Sized {
     fn serialize(self, version: u16) -> Result<Vec<u8>> {
         let mut data = self;
 
-        for converter in Self::serialize_converters()
-            .iter()
-            .skip(version.saturating_sub(1) as usize)
-        {
+        let converters = Self::serialize_converters();
+        let take_count = (converters.len() + 1)
+            .checked_sub(version as usize)
+            .with_context(|| {
+                format!(
+                    "proto version ({version}) greater than latest version ({})",
+                    converters.len() + 1
+                )
+            })?;
+        for converter in converters.iter().take(take_count) {
             data = converter(data)?;
         }
 

rust/vbare/tests/test.rs

@@ -15,22 +15,31 @@ struct TestDataV2 {
     description: String,
 }
 
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+struct TestDataV3 {
+    id: u32,
+    name: String,
+    description: String,
+    tags: Vec<String>,
+}
+
 #[derive(Clone)]
 enum TestData {
     V1(TestDataV1),
     V2(TestDataV2),
+    V3(TestDataV3),
 }
 
 impl OwnedVersionedData for TestData {
-    type Latest = TestDataV2;
+    type Latest = TestDataV3;
 
-    fn wrap_latest(latest: TestDataV2) -> Self {
-        TestData::V2(latest)
+    fn wrap_latest(latest: TestDataV3) -> Self {
+        TestData::V3(latest)
     }
 
     fn unwrap_latest(self) -> Result<Self::Latest> {
         #[allow(irrefutable_let_patterns)]
-        if let TestData::V2(data) = self {
+        if let TestData::V3(data) = self {
             Ok(data)
         } else {
             bail!("version not latest");
@@ -41,6 +50,7 @@ impl OwnedVersionedData for TestData {
         match version {
             1 => Ok(TestData::V1(serde_bare::from_slice(payload)?)),
             2 => Ok(TestData::V2(serde_bare::from_slice(payload)?)),
+            3 => Ok(TestData::V3(serde_bare::from_slice(payload)?)),
             _ => bail!("invalid version: {version}"),
         }
     }
@@ -49,15 +59,16 @@ impl OwnedVersionedData for TestData {
         match self {
             TestData::V1(data) => serde_bare::to_vec(&data).map_err(Into::into),
             TestData::V2(data) => serde_bare::to_vec(&data).map_err(Into::into),
+            TestData::V3(data) => serde_bare::to_vec(&data).map_err(Into::into),
         }
     }
 
     fn deserialize_converters() -> Vec<impl Fn(Self) -> Result<Self>> {
-        vec![Self::v1_to_v2]
+        vec![Self::v1_to_v2, Self::v2_to_v3]
     }
 
     fn serialize_converters() -> Vec<impl Fn(Self) -> Result<Self>> {
-        vec![Self::v2_to_v1]
+        vec![Self::v3_to_v2, Self::v2_to_v1]
     }
 }
 
@@ -110,7 +121,30 @@ impl TestData {
                 name: v1.name,
                 description: "default".to_string(),
             })),
-            v2 => Ok(v2),
+            other => Ok(other),
+        }
+    }
+
+    fn v2_to_v3(self) -> Result<Self> {
+        match self {
+            TestData::V2(v2) => Ok(TestData::V3(TestDataV3 {
+                id: v2.id,
+                name: v2.name,
+                description: v2.description,
+                tags: vec![],
+            })),
+            other => Ok(other),
+        }
+    }
+
+    fn v3_to_v2(self) -> Result<Self> {
+        match self {
+            TestData::V3(v3) => Ok(TestData::V2(TestDataV2 {
+                id: v3.id,
+                name: v3.name,
+                description: v3.description,
+            })),
+            other => Ok(other),
         }
     }
 
@@ -120,7 +154,7 @@ impl TestData {
                 id: v2.id,
                 name: v2.name,
             })),
-            v1 => Ok(v1),
+            other => Ok(other),
         }
     }
 }
@@ -151,37 +185,112 @@ fn test_v2_to_v2() {
 
     let payload = TestData::V2(data.clone()).serialize(2).unwrap();
 
+    // V2 data deserialized with version 2 will be converted to V3 (latest)
     let deserialized = TestData::deserialize(&payload, 2).unwrap();
     assert_eq!(deserialized.id, 456);
     assert_eq!(deserialized.name, "test");
     assert_eq!(deserialized.description, "data");
+    assert_eq!(deserialized.tags.len(), 0); // Tags default to empty vec
 }
 
 #[test]
 fn test_unsupported_version() {
     assert!(TestData::deserialize(&[], 99).is_err());
 }
 
+#[test]
+fn test_v3_to_v2() {
+    // This is the critical test case: serializing from V3 to V2
+    // With 3 versions and 2 serialize converters (V3->V2, V2->V1),
+    // serializing to version 2 should apply 1 converter (V3->V2)
+    let data = TestDataV3 {
+        id: 789,
+        name: "v3_test".to_string(),
+        description: "test description".to_string(),
+        tags: vec!["tag1".to_string(), "tag2".to_string()],
+    };
+
+    // Serialize V3 data to V2 format (should strip tags)
+    let payload = TestData::V3(data.clone()).serialize(2).unwrap();
+
+    // Deserialize as V2 and verify tags were stripped
+    let deserialized = TestData::deserialize(&payload, 2).unwrap();
+    assert_eq!(deserialized.id, 789);
+    assert_eq!(deserialized.name, "v3_test");
+    assert_eq!(deserialized.description, "test description");
+    // Tags should not be present in V2
+}
+
+#[test]
+fn test_v3_to_v1() {
+    // Test serializing from V3 all the way down to V1
+    // Should apply both converters: V3->V2, then V2->V1
+    let data = TestDataV3 {
+        id: 999,
+        name: "v3_to_v1_test".to_string(),
+        description: "should be stripped".to_string(),
+        tags: vec!["will be removed".to_string()],
+    };
+
+    // Serialize V3 data to V1 format
+    let payload = TestData::V3(data.clone()).serialize(1).unwrap();
+
+    // Deserialize as V1 and verify both description and tags were stripped
+    let deserialized = TestData::deserialize(&payload, 1).unwrap();
+    assert_eq!(deserialized.id, 999);
+    assert_eq!(deserialized.name, "v3_to_v1_test");
+    assert_eq!(deserialized.description, "default");
+    assert_eq!(deserialized.tags.len(), 0);
+}
+
+#[test]
+fn test_v3_to_v3() {
+    // Test that serializing V3 to V3 preserves all data
+    let data = TestDataV3 {
+        id: 123,
+        name: "v3_same".to_string(),
+        description: "preserved".to_string(),
+        tags: vec!["keep".to_string()],
+    };
+
+    let payload = TestData::V3(data.clone()).serialize(3).unwrap();
+
+    let deserialized = TestData::deserialize(&payload, 3).unwrap();
+    assert_eq!(deserialized.id, 123);
+    assert_eq!(deserialized.name, "v3_same");
+    assert_eq!(deserialized.description, "preserved");
+    assert_eq!(deserialized.tags, vec!["keep".to_string()]);
+}
+
 #[test]
 fn test_serialize() {
-    let data = TestData::V2(TestDataV2 {
+    let data = TestData::V3(TestDataV3 {
         id: 456,
         name: "serialize_test".to_string(),
         description: "will be stripped".to_string(),
+        tags: vec!["tag1".to_string()],
     });
 
-    // Test serializing to V1 (should convert V2 -> V1)
+    // Test serializing to V1 (should convert V3 -> V2 -> V1)
     let result = data.clone().serialize(1).unwrap();
     let deserialized: TestDataV1 = serde_bare::from_slice(&result).unwrap();
     assert_eq!(deserialized.id, 456);
     assert_eq!(deserialized.name, "serialize_test");
 
-    // Test serializing to V2
-    let result = data.serialize(2).unwrap();
+    // Test serializing to V2 (should convert V3 -> V2)
+    let result = data.clone().serialize(2).unwrap();
     let deserialized: TestDataV2 = serde_bare::from_slice(&result).unwrap();
     assert_eq!(deserialized.id, 456);
     assert_eq!(deserialized.name, "serialize_test");
     assert_eq!(deserialized.description, "will be stripped");
+
+    // Test serializing to V3 (no conversion)
+    let result = data.serialize(3).unwrap();
+    let deserialized: TestDataV3 = serde_bare::from_slice(&result).unwrap();
+    assert_eq!(deserialized.id, 456);
+    assert_eq!(deserialized.name, "serialize_test");
+    assert_eq!(deserialized.description, "will be stripped");
+    assert_eq!(deserialized.tags, vec!["tag1".to_string()]);
 }
 
 #[test]
@@ -196,14 +305,16 @@ fn test_embedded_v2_to_v1_to_v2() {
         .serialize_with_embedded_version(1)
         .unwrap();
 
-    // First 2 bytes should be the version (2 in little-endian)
+    // First 2 bytes should be the version (1 in little-endian)
     assert_eq!(payload[0], 1u8);
     assert_eq!(payload[1], 0u8);
 
+    // Deserializing V1 data converts it to V3 (latest)
     let deserialized = TestData::deserialize_with_embedded_version(&payload).unwrap();
     assert_eq!(deserialized.id, 456);
     assert_eq!(deserialized.name, "test");
     assert_eq!(deserialized.description, "default");
+    assert_eq!(deserialized.tags.len(), 0);
 }
 
 #[test]
@@ -222,10 +333,12 @@ fn test_embedded_v2_to_v2() {
     assert_eq!(payload[0], 2u8);
     assert_eq!(payload[1], 0u8);
 
+    // Deserializing V2 data converts it to V3 (latest)
     let deserialized = TestData::deserialize_with_embedded_version(&payload).unwrap();
     assert_eq!(deserialized.id, 456);
     assert_eq!(deserialized.name, "test");
     assert_eq!(deserialized.description, "data");
+    assert_eq!(deserialized.tags.len(), 0);
 }
 
 #[test]