feat: add version field to archive and metadata

[b-zee]

No description provided.

[mickvandijke]

The implementation is great, but If I understand it right, the caveat of this approach is that when we update the Metadata struct and files/archives are subsequently being uploaded using the new Metadata struct. Older API versions will not be able to parse the new PrivateArchive or PublicArchive at all. Anyone on an older API version would be forced to update to be able to parse newer archives with updated Metadata structs.

The Metadata struct however may be unlikely to be updated very often, so perhaps we (and most importantly our builders) could live with that?

Did you also try the update Metadata struct with new optional fields approach?

[b-zee]

This pull request doesn't address forward compatibility. But I do agree, we want to support old API/apps for as long as possible!

Yes, Metadata might not be needed at this point, but I'd still say it's nice to be able to introduce a breaking change if we really want to?

I have tried to have a go at forward compatibility (though to note again, not part of this PR!), but I can't seem to make it work with rmp_serde (while it does work as expected with serde_json):

use serde::{Deserialize, Serialize};

#[derive(Debug, Default, Serialize, Deserialize)]
struct MetadataA {
    a: usize,
}

#[derive(Debug, Default, Serialize, Deserialize)]
struct MetadataB {
    a: usize,
    b: Option<usize>,
}

#[test]
fn test() {
    let b_meta = MetadataB { a: 1, b: None };
    let b_meta_ser = rmp_serde::to_vec(&b_meta).unwrap();

    // Does not work: called `Result::unwrap()` on an `Err` value: LengthMismatch(1)
    let _a = rmp_serde::from_slice::<MetadataA>(b_meta_ser.as_slice()).unwrap();

    let b_meta = MetadataB { a: 1, b: None };
    let b_meta_ser = serde_json::to_vec(&b_meta).unwrap();

    // Works
    let _a = serde_json::from_slice::<MetadataA>(b_meta_ser.as_slice()).unwrap();
}

[mickvandijke]

I had a go with the option approach and ended up with a solution that is both forward and backwards compatible. The key change is to use rmp_serde::to_vec_named to include the field keys. That does make this incompatible with the files on the current network, as we used rmp_serde::to_vec for serialisation there. But since we are allowed to do breaking changes for the coming update, it might be worth to consider.

If we were to use this approach. We wouldn't have to use enum variants for different versions. Simply the latest version would be equivalent to Metadata, as it can always be deserialised from all older versions.

use serde::{Deserialize, Serialize};

#[derive(Debug, Default, Serialize, Deserialize)]
struct MetadataV1 {
    size: usize,
}

#[derive(Debug, Default, Serialize, Deserialize)]
struct MetadataV2 {
    size: usize,
    date_uploaded: Option<usize>,
}

#[derive(Debug, Default, Serialize, Deserialize)]
struct MetadataV3 {
    size: usize,
    date_uploaded_str: Option<String>,
    name: Option<String>,
}

#[cfg(test)]
mod tests {
    use crate::files::test::{MetadataV1, MetadataV2, MetadataV3};

    #[test]
    fn conversion_between_metadata_v1_and_v2() {
        // Can serialise V2 from V1
        let v1_meta = MetadataV1 { size: 42 };
        let v1_meta_ser = rmp_serde::to_vec_named(&v1_meta).unwrap();
        let v2_meta: MetadataV2 = rmp_serde::from_slice(&v1_meta_ser).unwrap();

        assert_eq!(v1_meta.size, v2_meta.size);
        assert!(v2_meta.date_uploaded.is_none());

        // Can serialise V1 from V2
        let v2_meta = MetadataV2 {
            size: 42,
            date_uploaded: Some(1698350000),
        };
        let v2_meta_ser = rmp_serde::to_vec_named(&v2_meta).unwrap();
        let v1_meta: MetadataV1 = rmp_serde::from_slice(&v2_meta_ser).unwrap();

        assert_eq!(v1_meta.size, v2_meta.size);
    }

    #[test]
    fn conversion_between_metadata_v1_and_v3() {
        // Can serialise V3 from V1
        let v1_meta = MetadataV1 { size: 42 };
        let v1_meta_ser = rmp_serde::to_vec_named(&v1_meta).unwrap();
        let v3_meta: MetadataV3 = rmp_serde::from_slice(&v1_meta_ser).unwrap();

        assert_eq!(v3_meta.size, v1_meta.size);
        assert!(v3_meta.date_uploaded_str.is_none());
        assert!(v3_meta.name.is_none());

        // Can serialise V1 from V3
        let v3_meta = MetadataV3 {
            size: 42,
            date_uploaded_str: Some("2023-10-01".to_string()),
            name: Some("test_file".to_string()),
        };
        let v3_meta_ser = rmp_serde::to_vec_named(&v3_meta).unwrap();
        let v1_meta: MetadataV1 = rmp_serde::from_slice(&v3_meta_ser).unwrap();

        assert_eq!(v1_meta.size, v3_meta.size);
    }

    #[test]
    fn conversion_between_metadata_v2_and_v3() {
        // Can serialise V2 from V3
        let v2_meta = MetadataV2 {
            size: 42,
            date_uploaded: Some(1698350000),
        };
        let v2_meta_ser = rmp_serde::to_vec_named(&v2_meta).unwrap();
        let v3_meta: MetadataV3 = rmp_serde::from_slice(&v2_meta_ser).unwrap();

        assert_eq!(v3_meta.size, v2_meta.size);
        assert!(v3_meta.date_uploaded_str.is_none());
        assert!(v3_meta.name.is_none());

        // Can serialise V3 from V2
        let v3_meta = MetadataV3 {
            size: 42,
            date_uploaded_str: Some("2023-10-01".to_string()),
            name: Some("test_file".to_string()),
        };
        let v3_meta_ser = rmp_serde::to_vec_named(&v3_meta).unwrap();
        let v2_meta: MetadataV2 = rmp_serde::from_slice(&v3_meta_ser).unwrap();

        assert_eq!(v2_meta.size, v3_meta.size);
        assert!(v2_meta.date_uploaded.is_none());
    }
}

[b-zee]

Great find! I just realized I came across that method when I looked at using #[serde(other)].

That does make this incompatible with the files on the current network, as we used rmp_serde::to_vec for serialisation there. But since we are allowed to do breaking changes for the coming update, it might be worth to consider.

Yes, this PR itself is also a breaking change, so that should not be a problem.

[mickvandijke]

LGTM 🚀