[pbs-devel] [PATCH v2 proxmox-backup 1/3] api types: version: drop unused `repoid` field

[pbs-devel] [PATCH v2 proxmox-backup 1/3] api types: version: drop unused `repoid` field

[Christian Ebner]

The `ApiVersion` type was introduced in commit a926803b
("api/api-types: refactor api endpoint version, add api types")
including the `repoid`, added for completeness when converting from
a pre-existing `ApiVersionInfo` instance, as returned by the
`version` api endpoint.

Drop the additional `repoid` field, since this is currently not used,
can be obtained fro the `ApiVersionInfo` as well and only hinders the
implementation for easy api version comparison.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- not present in previous version

 pbs-api-types/src/version.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/pbs-api-types/src/version.rs b/pbs-api-types/src/version.rs
index 80f87e372..bd4c517da 100644
--- a/pbs-api-types/src/version.rs
+++ b/pbs-api-types/src/version.rs
@@ -37,7 +37,6 @@ pub struct ApiVersion {
     pub major: ApiVersionMajor,
     pub minor: ApiVersionMinor,
     pub release: ApiVersionRelease,
-    pub repoid: String,
 }
 
 impl TryFrom<ApiVersionInfo> for ApiVersion {
@@ -64,7 +63,6 @@ impl TryFrom<ApiVersionInfo> for ApiVersion {
             major,
             minor,
             release,
-            repoid: value.repoid.clone(),
         })
     }
 }
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 2/3] api types: version: implement traits to allow for version comparison

[Christian Ebner]

Derive and implement the traits to allow comparison of two
`ApiVersion` instances for more direct and easy api version
comparisons. Further, add some basic test cases to reduce risk of
regressions.

This is useful for e.g. feature compatibility checks by comparing api
versions of remote instances.

Example comparison:
```
api_version >= ApiVersion::new(3, 3, 0)
```

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- implement traits for operator based version comparison
- add basic regression tests

 pbs-api-types/src/version.rs | 122 +++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/pbs-api-types/src/version.rs b/pbs-api-types/src/version.rs
index bd4c517da..09e725eb6 100644
--- a/pbs-api-types/src/version.rs
+++ b/pbs-api-types/src/version.rs
@@ -1,4 +1,5 @@
 //! Defines the types for the api version info endpoint
+use std::cmp::Ordering;
 use std::convert::TryFrom;
 
 use anyhow::{format_err, Context};
@@ -33,6 +34,7 @@ pub type ApiVersionMajor = u64;
 pub type ApiVersionMinor = u64;
 pub type ApiVersionRelease = u64;
 
+#[derive(PartialEq, Eq)]
 pub struct ApiVersion {
     pub major: ApiVersionMajor,
     pub minor: ApiVersionMinor,
@@ -66,3 +68,123 @@ impl TryFrom<ApiVersionInfo> for ApiVersion {
         })
     }
 }
+
+impl PartialOrd for ApiVersion {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        let ordering = match (
+            self.major.cmp(&other.major),
+            self.minor.cmp(&other.minor),
+            self.release.cmp(&other.release),
+        ) {
+            (Ordering::Equal, Ordering::Equal, ordering) => ordering,
+            (Ordering::Equal, ordering, _) => ordering,
+            (ordering, _, _) => ordering,
+        };
+
+        Some(ordering)
+    }
+}
+
+impl ApiVersion {
+    pub fn new(major: ApiVersionMajor, minor: ApiVersionMinor, release: ApiVersionRelease) -> Self {
+        Self {
+            major,
+            minor,
+            release,
+        }
+    }
+}
+
+#[test]
+fn same_level_version_comarison() {
+    let major_base = ApiVersion::new(2, 0, 0);
+    let major_less = ApiVersion::new(1, 0, 0);
+    let major_greater = ApiVersion::new(3, 0, 0);
+
+    let minor_base = ApiVersion::new(2, 2, 0);
+    let minor_less = ApiVersion::new(2, 1, 0);
+    let minor_greater = ApiVersion::new(2, 3, 0);
+
+    let release_base = ApiVersion::new(2, 2, 2);
+    let release_less = ApiVersion::new(2, 2, 1);
+    let release_greater = ApiVersion::new(2, 2, 3);
+
+    assert!(major_base == major_base);
+    assert!(minor_base == minor_base);
+    assert!(release_base == release_base);
+
+    assert!(major_base > major_less);
+    assert!(major_base >= major_less);
+    assert!(major_base != major_less);
+
+    assert!(major_base < major_greater);
+    assert!(major_base <= major_greater);
+    assert!(major_base != major_greater);
+
+    assert!(minor_base > minor_less);
+    assert!(minor_base >= minor_less);
+    assert!(minor_base != minor_less);
+
+    assert!(minor_base < minor_greater);
+    assert!(minor_base <= minor_greater);
+    assert!(minor_base != minor_greater);
+
+    assert!(release_base > release_less);
+    assert!(release_base >= release_less);
+    assert!(release_base != release_less);
+
+    assert!(release_base < release_greater);
+    assert!(release_base <= release_greater);
+    assert!(release_base != release_greater);
+}
+
+#[test]
+fn mixed_level_version_comarison() {
+    let major_base = ApiVersion::new(2, 0, 0);
+    let major_less = ApiVersion::new(1, 0, 0);
+    let major_greater = ApiVersion::new(3, 0, 0);
+
+    let minor_base = ApiVersion::new(2, 2, 0);
+    let minor_less = ApiVersion::new(2, 1, 0);
+    let minor_greater = ApiVersion::new(2, 3, 0);
+
+    let release_base = ApiVersion::new(2, 2, 2);
+    let release_less = ApiVersion::new(2, 2, 1);
+    let release_greater = ApiVersion::new(2, 2, 3);
+
+    assert!(major_base < minor_base);
+    assert!(major_base < minor_less);
+    assert!(major_base < minor_greater);
+
+    assert!(major_base < release_base);
+    assert!(major_base < release_less);
+    assert!(major_base < release_greater);
+
+    assert!(major_less < minor_base);
+    assert!(major_less < minor_less);
+    assert!(major_less < minor_greater);
+
+    assert!(major_less < release_base);
+    assert!(major_less < release_less);
+    assert!(major_less < release_greater);
+
+    assert!(major_greater > minor_base);
+    assert!(major_greater > minor_less);
+    assert!(major_greater > minor_greater);
+
+    assert!(major_greater > release_base);
+    assert!(major_greater > release_less);
+    assert!(major_greater > release_greater);
+
+    assert!(minor_base < release_base);
+    assert!(minor_base < release_less);
+    assert!(minor_base < release_greater);
+
+    assert!(minor_greater > release_base);
+    assert!(minor_greater > release_less);
+    assert!(minor_greater > release_greater);
+
+    assert!(minor_less < release_base);
+    assert!(minor_less < release_less);
+    assert!(minor_less < release_greater);
+}
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 3/3] sync: push: use direct api version comparison in compatibility checks

[Christian Ebner]

Use the trait implementations of `ApiVersion` to perform operator
based version comparisons. This makes the comparison more readable
and reduces the risk for errors.

No functional change intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- use operator based version comparison, improving code readability

 src/server/push.rs | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index 95c7c6bff..fdd259aff 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -129,13 +129,11 @@ impl PushParameters {
         let api_version = ApiVersion::try_from(version_info)?;
 
         // push assumes namespace support on the remote side, fail early if missing
-        if api_version.major < 2 || (api_version.major == 2 && api_version.minor < 2) {
+        if api_version < ApiVersion::new(2, 2, 0) {
             bail!("Unsupported remote api version, minimum v2.2 required");
         }
 
-        let supports_prune_delete_stats = api_version.major > 3
-            || (api_version.major == 3 && api_version.minor >= 3)
-            || (api_version.major == 3 && api_version.minor == 2 && api_version.release >= 11);
+        let supports_prune_delete_stats = api_version >= ApiVersion::new(3, 2, 11);
 
         let target = PushTarget {
             remote,
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied-series: [PATCH v2 proxmox-backup 1/3] api types: version: drop unused `repoid` field

[Thomas Lamprecht]

Am 28.11.24 um 17:07 schrieb Christian Ebner:
> The `ApiVersion` type was introduced in commit a926803b
> ("api/api-types: refactor api endpoint version, add api types")
> including the `repoid`, added for completeness when converting from
> a pre-existing `ApiVersionInfo` instance, as returned by the
> `version` api endpoint.
> 
> Drop the additional `repoid` field, since this is currently not used,
> can be obtained fro the `ApiVersionInfo` as well and only hinders the
> implementation for easy api version comparison.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1:
> - not present in previous version
> 
>  pbs-api-types/src/version.rs | 2 --
>  1 file changed, 2 deletions(-)
> 
>

applied all three patches, it's really nicer this way, thanks!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel