[pdm-devel] [RFC PATCH 1/3] pdm-client: add query builder

* [pdm-devel] [RFC PATCH 1/3] pdm-client: add query builder
@ 2025-02-14 14:11 Maximiliano Sandoval
  2025-02-14 14:11 ` [pdm-devel] [RFC PATCH 2/3] pdm-client: make use of " Maximiliano Sandoval
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Maximiliano Sandoval @ 2025-02-14 14:11 UTC (permalink / raw)
  To: pdm-devel

A big proportion of the consumers of this API use the `format!` macro to
define the base of the url, hence why we use a `Cow<'_, str>` on the
constructor.

A `perl-api-path-builder` feature was added to satisfy the needs of
pve-api-types.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

This is marked as RFC as I think this is better suited for proxmox-client and
here it is easier to showcase in the second and third commit how the API falls
into place.

 lib/pdm-client/Cargo.toml              |   1 +
 lib/pdm-client/src/api_path_builder.rs | 207 +++++++++++++++++++++++++
 2 files changed, 208 insertions(+)
 create mode 100644 lib/pdm-client/src/api_path_builder.rs

diff --git a/lib/pdm-client/Cargo.toml b/lib/pdm-client/Cargo.toml
index fe5ddf7..af42c43 100644
--- a/lib/pdm-client/Cargo.toml
+++ b/lib/pdm-client/Cargo.toml
@@ -26,3 +26,4 @@ pbs-api-types.workspace = true
 [features]
 default = []
 hyper-client = [ "proxmox-client/hyper-client" ]
+perl-api-path-builder = []
diff --git a/lib/pdm-client/src/api_path_builder.rs b/lib/pdm-client/src/api_path_builder.rs
new file mode 100644
index 0000000..0058489
--- /dev/null
+++ b/lib/pdm-client/src/api_path_builder.rs
@@ -0,0 +1,207 @@
+use std::borrow::Cow;
+
+/// Builder for urls with a query.
+///
+/// The [Self::arg] method can be used to add multiple arguments to the query.
+///
+/// ```rust
+/// use pdm_client::api_path_builder::ApiPathBuilder;
+///
+/// let query = ApiPathBuilder::new("/api2/extjs/cluster/resources")
+///     .arg("type", pve_api_types::ClusterResourceKind::Vm)
+///     .build();
+///
+/// assert_eq!(&query, "/api2/extjs/cluster/resources?type=vm");
+/// ```
+///
+/// ## Compatibility with perl clients
+///
+/// The methods `bool_arg` and `list_arg` were added to translate booleans as
+/// `"0"`/`"1"` and split lists so that they can be feed to perl's
+/// `split_list()` respectively. These methods require the
+/// `perl-api-path-builder` feature.
+pub struct ApiPathBuilder {
+    url: String,
+    separator: char,
+}
+
+impl ApiPathBuilder {
+    /// Creates a new builder.
+    pub fn new<'a>(base: impl Into<Cow<'a, str>>) -> Self {
+        Self {
+            url: base.into().into_owned(),
+            separator: '?',
+        }
+    }
+
+    /// Adds an argument to the query.
+    ///
+    /// The value is percent-encoded.
+    pub fn arg<T: std::fmt::Display>(mut self, name: &str, value: T) -> Self {
+        self.push_name_and_separator(name);
+        self.push_encoded(value);
+        self
+    }
+
+    /// Adds an optional argument to the query.
+    ///
+    /// Does nothing if the value is `None`. See [Self::arg].
+    pub fn maybe_arg<T: std::fmt::Display>(mut self, name: &str, value: &Option<T>) -> Self {
+        if let Some(value) = value {
+            self = self.arg(name, value);
+        }
+        self
+    }
+
+    /// Builds the url.
+    pub fn build(self) -> String {
+        self.url
+    }
+
+    fn push_name_and_separator(&mut self, name: &str) {
+        self.url.push(self.separator);
+        self.separator = '&';
+        self.url.push_str(name);
+        self.url.push('=');
+    }
+
+    fn push_encoded<T: std::fmt::Display>(&mut self, value: T) {
+        let str_value = value.to_string();
+        let enc_value = percent_encoding::percent_encode(
+            str_value.as_bytes(),
+            percent_encoding::NON_ALPHANUMERIC,
+        );
+        self.url.extend(enc_value);
+    }
+}
+
+#[cfg(feature = "perl-api-path-builder")]
+impl ApiPathBuilder {
+    /// Adds a boolean arg in a perl-friendly fashion.
+    ///
+    /// `true` will be converted into `"1"` and `false` to `"0"`.
+    pub fn bool_arg(mut self, name: &str, value: bool) -> Self {
+        self.push_name_and_separator(name);
+        if value {
+            self.url.push('1');
+        } else {
+            self.url.push('0');
+        };
+        self
+    }
+
+    /// Adds an optional boolean arg in a perl-friendly fashion.
+    ///
+    /// Does nothing if `value` is `None`. See [Self::bool_arg].
+    pub fn maybe_bool_arg(mut self, name: &str, value: Option<bool>) -> Self {
+        if let Some(value) = value {
+            self = self.bool_arg(name, value);
+        }
+        self
+    }
+
+    /// Helper for building perl-friendly queries.
+    ///
+    /// For `<type>-list` entries we turn an array into a string ready for
+    /// perl's `split_list()` call.
+    ///
+    /// The values are percent-encoded.
+    pub fn list_arg<T: std::fmt::Display, P: Iterator<Item = T>>(
+        mut self,
+        name: &str,
+        values: P,
+    ) -> Self {
+        self.push_name_and_separator(name);
+        let mut list_separator = "";
+        for entry in values {
+            self.url.push_str(list_separator);
+            list_separator = "%00";
+            self.push_encoded(entry);
+        }
+        self
+    }
+
+    /// Helper for building perl-friendly queries.
+    ///
+    /// See [Self::list_arg].
+    pub fn maybe_list_arg<T: std::fmt::Display>(
+        mut self,
+        name: &str,
+        values: &Option<Vec<T>>,
+    ) -> Self {
+        if let Some(values) = values {
+            self = self.list_arg(name, values.iter());
+        };
+        self
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use pdm_api_types::ConfigurationState;
+    use pve_api_types::ClusterResourceKind;
+
+    use super::*;
+
+    #[test]
+    fn test_builder() {
+        let expected = "/api2/extjs/cluster/resources?type=vm";
+        let ty = ClusterResourceKind::Vm;
+
+        let query = ApiPathBuilder::new("/api2/extjs/cluster/resources")
+            .arg("type", ty)
+            .build();
+
+        assert_eq!(&query, expected);
+
+        let second_expected =
+            "/api2/extjs/pve/remotes/some-remote/qemu/100/config?state=active&node=myNode";
+        let state = ConfigurationState::Active;
+        let node = "myNode";
+        let snapshot = None::<&str>;
+
+        let second_query =
+            ApiPathBuilder::new("/api2/extjs/pve/remotes/some-remote/qemu/100/config")
+                .arg("state", state)
+                .arg("node", node)
+                .maybe_arg("snapshot", &snapshot)
+                .build();
+
+        assert_eq!(&second_query, &second_expected);
+    }
+}
+
+#[cfg(all(test, feature = "perl-api-path-builder"))]
+mod perl_tests {
+    use super::*;
+
+    use pve_api_types::client::{add_query_arg, add_query_bool};
+
+    #[test]
+    fn test_perl_builder() {
+        let history = true;
+        let local_only = false;
+        let start_time = 1000;
+
+        let (mut query, mut sep) = (String::new(), '?');
+        add_query_bool(&mut query, &mut sep, "history", Some(history));
+        add_query_bool(&mut query, &mut sep, "local-only", Some(local_only));
+        add_query_arg(&mut query, &mut sep, "start-time", &Some(start_time));
+        let url = format!("/api2/extjs/cluster/metrics/export{query}");
+
+        let query = ApiPathBuilder::new("/api2/extjs/cluster/metrics/export")
+            .bool_arg("history", history)
+            .bool_arg("local-only", local_only)
+            .arg("start-time", start_time)
+            .build();
+        assert_eq!(url, query);
+
+        let maybe_arg = ApiPathBuilder::new("/api2/extjs/cluster/metrics/export")
+            .maybe_bool_arg("history", Some(history))
+            .maybe_bool_arg("local-only", Some(local_only))
+            .maybe_arg("start-time", &Some(start_time))
+            .build();
+
+        assert_eq!(url, maybe_arg);
+    }
+}
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 4+ messages in thread