* [pbs-devel] [PATCH] metrics: influxdb test uri creation
@ 2024-04-30 10:10 Gabriel Goller
2024-05-23 10:22 ` Fabian Grünbichler
0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Goller @ 2024-04-30 10:10 UTC (permalink / raw)
To: pbs-devel
Extract the URI creation for write and health URIs. Add unit test to
test the encoding of special characters in the organization and bucket
parameters.
This is a follow-up to:
https://lists.proxmox.com/pipermail/pbs-devel/2024-April/009096.html
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
proxmox-metrics/src/influxdb/http.rs | 93 ++++++++++++++++++++--------
1 file changed, 67 insertions(+), 26 deletions(-)
diff --git a/proxmox-metrics/src/influxdb/http.rs b/proxmox-metrics/src/influxdb/http.rs
index 8167b59..1d53794 100644
--- a/proxmox-metrics/src/influxdb/http.rs
+++ b/proxmox-metrics/src/influxdb/http.rs
@@ -84,6 +84,25 @@ impl InfluxDbHttp {
Client::with_ssl_connector(ssl_connector.build(), HttpOptions::default())
};
+ let (writeuri, healthuri) = Self::create_uris(uri, organization, bucket)?;
+
+ Ok(InfluxDbHttp {
+ client,
+ writeuri,
+ healthuri,
+ token: token.map(String::from),
+ max_body_size,
+ data: String::new(),
+ channel,
+ })
+ }
+
+ /// Return a tuple with the write_uri and the health_uri
+ fn create_uris(
+ uri: &str,
+ org: &str,
+ bucket: &str,
+ ) -> Result<(http::uri::Uri, http::uri::Uri), anyhow::Error> {
let uri: http::uri::Uri = uri.parse()?;
let uri_parts = uri.into_parts();
@@ -93,35 +112,25 @@ impl InfluxDbHttp {
""
};
- let encoded_org: String =
- url::form_urlencoded::byte_serialize(organization.as_bytes()).collect();
+ let encoded_org: String = url::form_urlencoded::byte_serialize(org.as_bytes()).collect();
let encoded_bucket: String =
url::form_urlencoded::byte_serialize(bucket.as_bytes()).collect();
- let writeuri = http::uri::Builder::new()
- .scheme(uri_parts.scheme.clone().unwrap())
- .authority(uri_parts.authority.clone().unwrap())
- .path_and_query(format!(
- "{}/api/v2/write?org={}&bucket={}",
- base_path, encoded_org, encoded_bucket
- ))
- .build()?;
-
- let healthuri = http::uri::Builder::new()
- .scheme(uri_parts.scheme.unwrap())
- .authority(uri_parts.authority.unwrap())
- .path_and_query(format!("{}/health", base_path))
- .build()?;
-
- Ok(InfluxDbHttp {
- client,
- writeuri,
- healthuri,
- token: token.map(String::from),
- max_body_size,
- data: String::new(),
- channel,
- })
+ Ok((
+ http::uri::Builder::new()
+ .scheme(uri_parts.scheme.clone().unwrap())
+ .authority(uri_parts.authority.clone().unwrap())
+ .path_and_query(format!(
+ "{}/api/v2/write?org={}&bucket={}",
+ base_path, encoded_org, encoded_bucket
+ ))
+ .build()?,
+ http::uri::Builder::new()
+ .scheme(uri_parts.scheme.unwrap())
+ .authority(uri_parts.authority.unwrap())
+ .path_and_query(format!("{}/health", base_path))
+ .build()?,
+ ))
}
async fn test_connection(&self) -> Result<(), Error> {
@@ -188,3 +197,35 @@ impl InfluxDbHttp {
Ok(())
}
}
+
+#[cfg(test)]
+mod test {
+ use super::InfluxDbHttp;
+
+ #[test]
+ fn uri_encoding() {
+ let (writeuri, healthuri) =
+ InfluxDbHttp::create_uris("http://localhost/", "c ool/org", "a🔒nother&bu=ck?et")
+ .unwrap();
+ assert_eq!(writeuri.host(), Some("localhost"));
+ assert_eq!(writeuri.path(), "/api/v2/write");
+ assert_eq!(
+ writeuri.query(),
+ Some("org=c+ool%2Forg&bucket=a%F0%9F%94%92nother%26bu%3Dck%3Fet")
+ );
+
+ assert_eq!(healthuri.host(), Some("localhost"));
+ assert_eq!(healthuri.path(), "/health");
+ assert_eq!(healthuri.query(), None);
+
+ let (writeuri, healthuri) =
+ InfluxDbHttp::create_uris("http://localhost/", "org", "bucket").unwrap();
+ assert_eq!(writeuri.host(), Some("localhost"));
+ assert_eq!(writeuri.path(), "/api/v2/write");
+ assert_eq!(writeuri.query(), Some("org=org&bucket=bucket"));
+
+ assert_eq!(healthuri.host(), Some("localhost"));
+ assert_eq!(healthuri.path(), "/health");
+ assert_eq!(healthuri.query(), None);
+ }
+}
--
2.43.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH] metrics: influxdb test uri creation
2024-04-30 10:10 [pbs-devel] [PATCH] metrics: influxdb test uri creation Gabriel Goller
@ 2024-05-23 10:22 ` Fabian Grünbichler
2024-05-23 11:07 ` Gabriel Goller
0 siblings, 1 reply; 3+ messages in thread
From: Fabian Grünbichler @ 2024-05-23 10:22 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
doesn't apply anymore.
two small things:
- form_urlencoded is just a re-export in url, maybe we want to actually
use it instead since we don't otherwise need url?
- IMHO it would be more elegant to use the provided Serializer instead
of encoding manually, something like this:
let writeuri_query = url::form_urlencoded::Serializer::new(String::new())
.append_pair("org", organization)
.append_pair("bucket", bucket)
.finish();
let writeuri = http::uri::Builder::new()
.scheme(uri_parts.scheme.clone().unwrap())
.authority(uri_parts.authority.clone().unwrap())
.path_and_query(format!("{base_path}/api/v2/write?{writeuri_query}",))
.build()?;
AFAICT, the output is the same, but this is a much nicer interface?
On April 30, 2024 12:10 pm, Gabriel Goller wrote:
> Extract the URI creation for write and health URIs. Add unit test to
> test the encoding of special characters in the organization and bucket
> parameters.
> This is a follow-up to:
> https://lists.proxmox.com/pipermail/pbs-devel/2024-April/009096.html
>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> proxmox-metrics/src/influxdb/http.rs | 93 ++++++++++++++++++++--------
> 1 file changed, 67 insertions(+), 26 deletions(-)
>
> diff --git a/proxmox-metrics/src/influxdb/http.rs b/proxmox-metrics/src/influxdb/http.rs
> index 8167b59..1d53794 100644
> --- a/proxmox-metrics/src/influxdb/http.rs
> +++ b/proxmox-metrics/src/influxdb/http.rs
> @@ -84,6 +84,25 @@ impl InfluxDbHttp {
> Client::with_ssl_connector(ssl_connector.build(), HttpOptions::default())
> };
>
> + let (writeuri, healthuri) = Self::create_uris(uri, organization, bucket)?;
> +
> + Ok(InfluxDbHttp {
> + client,
> + writeuri,
> + healthuri,
> + token: token.map(String::from),
> + max_body_size,
> + data: String::new(),
> + channel,
> + })
> + }
> +
> + /// Return a tuple with the write_uri and the health_uri
> + fn create_uris(
> + uri: &str,
> + org: &str,
> + bucket: &str,
> + ) -> Result<(http::uri::Uri, http::uri::Uri), anyhow::Error> {
> let uri: http::uri::Uri = uri.parse()?;
> let uri_parts = uri.into_parts();
>
> @@ -93,35 +112,25 @@ impl InfluxDbHttp {
> ""
> };
>
> - let encoded_org: String =
> - url::form_urlencoded::byte_serialize(organization.as_bytes()).collect();
> + let encoded_org: String = url::form_urlencoded::byte_serialize(org.as_bytes()).collect();
> let encoded_bucket: String =
> url::form_urlencoded::byte_serialize(bucket.as_bytes()).collect();
>
> - let writeuri = http::uri::Builder::new()
> - .scheme(uri_parts.scheme.clone().unwrap())
> - .authority(uri_parts.authority.clone().unwrap())
> - .path_and_query(format!(
> - "{}/api/v2/write?org={}&bucket={}",
> - base_path, encoded_org, encoded_bucket
> - ))
> - .build()?;
> -
> - let healthuri = http::uri::Builder::new()
> - .scheme(uri_parts.scheme.unwrap())
> - .authority(uri_parts.authority.unwrap())
> - .path_and_query(format!("{}/health", base_path))
> - .build()?;
> -
> - Ok(InfluxDbHttp {
> - client,
> - writeuri,
> - healthuri,
> - token: token.map(String::from),
> - max_body_size,
> - data: String::new(),
> - channel,
> - })
> + Ok((
> + http::uri::Builder::new()
> + .scheme(uri_parts.scheme.clone().unwrap())
> + .authority(uri_parts.authority.clone().unwrap())
> + .path_and_query(format!(
> + "{}/api/v2/write?org={}&bucket={}",
> + base_path, encoded_org, encoded_bucket
> + ))
> + .build()?,
> + http::uri::Builder::new()
> + .scheme(uri_parts.scheme.unwrap())
> + .authority(uri_parts.authority.unwrap())
> + .path_and_query(format!("{}/health", base_path))
> + .build()?,
> + ))
> }
>
> async fn test_connection(&self) -> Result<(), Error> {
> @@ -188,3 +197,35 @@ impl InfluxDbHttp {
> Ok(())
> }
> }
> +
> +#[cfg(test)]
> +mod test {
> + use super::InfluxDbHttp;
> +
> + #[test]
> + fn uri_encoding() {
> + let (writeuri, healthuri) =
> + InfluxDbHttp::create_uris("http://localhost/", "c ool/org", "a🔒nother&bu=ck?et")
> + .unwrap();
> + assert_eq!(writeuri.host(), Some("localhost"));
> + assert_eq!(writeuri.path(), "/api/v2/write");
> + assert_eq!(
> + writeuri.query(),
> + Some("org=c+ool%2Forg&bucket=a%F0%9F%94%92nother%26bu%3Dck%3Fet")
> + );
> +
> + assert_eq!(healthuri.host(), Some("localhost"));
> + assert_eq!(healthuri.path(), "/health");
> + assert_eq!(healthuri.query(), None);
> +
> + let (writeuri, healthuri) =
> + InfluxDbHttp::create_uris("http://localhost/", "org", "bucket").unwrap();
> + assert_eq!(writeuri.host(), Some("localhost"));
> + assert_eq!(writeuri.path(), "/api/v2/write");
> + assert_eq!(writeuri.query(), Some("org=org&bucket=bucket"));
> +
> + assert_eq!(healthuri.host(), Some("localhost"));
> + assert_eq!(healthuri.path(), "/health");
> + assert_eq!(healthuri.query(), None);
> + }
> +}
> --
> 2.43.0
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [PATCH] metrics: influxdb test uri creation
2024-05-23 10:22 ` Fabian Grünbichler
@ 2024-05-23 11:07 ` Gabriel Goller
0 siblings, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2024-05-23 11:07 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On 23.05.2024 12:22, Fabian Grünbichler wrote:
>doesn't apply anymore.
>
>two small things:
>- form_urlencoded is just a re-export in url, maybe we want to actually
> use it instead since we don't otherwise need url?
Sure, we can do this.
>- IMHO it would be more elegant to use the provided Serializer instead
> of encoding manually, something like this:
>
> let writeuri_query = url::form_urlencoded::Serializer::new(String::new())
> .append_pair("org", organization)
> .append_pair("bucket", bucket)
> .finish();
> let writeuri = http::uri::Builder::new()
> .scheme(uri_parts.scheme.clone().unwrap())
> .authority(uri_parts.authority.clone().unwrap())
> .path_and_query(format!("{base_path}/api/v2/write?{writeuri_query}",))
> .build()?;
>
>AFAICT, the output is the same, but this is a much nicer interface?
Right, yes, this is cleaner :)
Will submit a new patch shortly!
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-23 11:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-30 10:10 [pbs-devel] [PATCH] metrics: influxdb test uri creation Gabriel Goller
2024-05-23 10:22 ` Fabian Grünbichler
2024-05-23 11:07 ` Gabriel Goller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox