From e9fb357930f9e55da7855a680941f86c6f37e37f Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Mon, 18 Nov 2024 10:25:38 +0100 Subject: [PATCH] feat: channel is serialized without trailing slash (#948) * `ChannelUrl` serializes without the trailing slash. Its information that is not needed and looks better without. * `ChannelUrl::url` returns the `UrlWithTrailingSlash` type. This type is autoderefed into a `Url`. --- .../src/channel/channel_url.rs | 17 ++++++++++-- crates/rattler_conda_types/src/channel/mod.rs | 18 +++++++------ .../src/match_spec/parse.rs | 4 +-- ...arse__tests__test_from_string_Lenient.snap | 26 +++++++++---------- ...parse__tests__test_from_string_Strict.snap | 26 +++++++++---------- .../src/utils/url_with_trailing_slash.rs | 23 ++++++++++++++-- 6 files changed, 74 insertions(+), 40 deletions(-) diff --git a/crates/rattler_conda_types/src/channel/channel_url.rs b/crates/rattler_conda_types/src/channel/channel_url.rs index 52e3697b0..39bf65e50 100644 --- a/crates/rattler_conda_types/src/channel/channel_url.rs +++ b/crates/rattler_conda_types/src/channel/channel_url.rs @@ -11,13 +11,13 @@ use crate::{utils::url_with_trailing_slash::UrlWithTrailingSlash, Platform}; /// * The URL always contains a trailing `/`. /// /// This is useful to be able to compare different channels. -#[derive(Clone, Hash, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Hash, Eq, PartialEq, Deserialize)] #[serde(transparent)] pub struct ChannelUrl(UrlWithTrailingSlash); impl ChannelUrl { /// Returns the base Url of the channel. - pub fn url(&self) -> &Url { + pub fn url(&self) -> &UrlWithTrailingSlash { &self.0 } @@ -34,6 +34,19 @@ impl ChannelUrl { } } +// Override the behavior of the `Serialize` trait to remove the trailing slash. +// In code, we want to ensure that a trailing slash is always present but when +// we serialize the type it can be removed to safe space and make it look better +// for humans. +impl Serialize for ChannelUrl { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.as_str().trim_end_matches('/').serialize(serializer) + } +} + impl Debug for ChannelUrl { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.0.as_str()) diff --git a/crates/rattler_conda_types/src/channel/mod.rs b/crates/rattler_conda_types/src/channel/mod.rs index 7b198f56a..fb533b926 100644 --- a/crates/rattler_conda_types/src/channel/mod.rs +++ b/crates/rattler_conda_types/src/channel/mod.rs @@ -569,7 +569,7 @@ mod tests { let channel = Channel::from_str("conda-forge", &config).unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("https://conda.anaconda.org/conda-forge/").unwrap() + "https://conda.anaconda.org/conda-forge/".parse().unwrap() ); assert_eq!(channel.name.as_deref(), Some("conda-forge")); assert_eq!(channel.name(), "conda-forge"); @@ -586,7 +586,7 @@ mod tests { Channel::from_str("https://conda.anaconda.org/conda-forge/", &config).unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("https://conda.anaconda.org/conda-forge/").unwrap() + "https://conda.anaconda.org/conda-forge/".parse().unwrap() ); assert_eq!(channel.name.as_deref(), Some("conda-forge")); assert_eq!(channel.name(), "conda-forge"); @@ -612,7 +612,7 @@ mod tests { assert_eq!(channel.name(), "file:///var/channels/conda-forge/"); assert_eq!( channel.base_url.url().clone(), - Url::from_str("file:///var/channels/conda-forge/").unwrap() + "file:///var/channels/conda-forge/".parse().unwrap() ); assert_eq!(channel.platforms, None); assert_eq!( @@ -644,7 +644,7 @@ mod tests { let channel = Channel::from_str("http://localhost:1234", &config).unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("http://localhost:1234/").unwrap() + "http://localhost:1234/".parse().unwrap() ); assert_eq!(channel.name, None); assert_eq!(channel.platforms, None); @@ -671,7 +671,7 @@ mod tests { .unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("https://conda.anaconda.org/conda-forge/").unwrap() + "https://conda.anaconda.org/conda-forge/".parse().unwrap() ); assert_eq!(channel.name.as_deref(), Some("conda-forge")); assert_eq!(channel.platforms, Some(vec![platform])); @@ -683,7 +683,7 @@ mod tests { .unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("https://conda.anaconda.org/pkgs/main/").unwrap() + "https://conda.anaconda.org/pkgs/main/".parse().unwrap() ); assert_eq!(channel.name.as_deref(), Some("pkgs/main")); assert_eq!(channel.platforms, Some(vec![platform])); @@ -691,7 +691,9 @@ mod tests { let channel = Channel::from_str("conda-forge/label/rust_dev", &config).unwrap(); assert_eq!( channel.base_url.url().clone(), - Url::from_str("https://conda.anaconda.org/conda-forge/label/rust_dev/").unwrap() + "https://conda.anaconda.org/conda-forge/label/rust_dev/" + .parse() + .unwrap() ); assert_eq!(channel.name.as_deref(), Some("conda-forge/label/rust_dev")); } @@ -726,7 +728,7 @@ mod tests { }; assert_eq!( channel_config - .canonical_name(&Url::from_str("https://conda.anaconda.org/conda-forge/").unwrap()) + .canonical_name(&"https://conda.anaconda.org/conda-forge/".parse().unwrap()) .as_str(), "conda-forge" ); diff --git a/crates/rattler_conda_types/src/match_spec/parse.rs b/crates/rattler_conda_types/src/match_spec/parse.rs index 14027b982..007d86308 100644 --- a/crates/rattler_conda_types/src/match_spec/parse.rs +++ b/crates/rattler_conda_types/src/match_spec/parse.rs @@ -825,11 +825,11 @@ mod tests { build: cpu* - version: "==foobar" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge - version: "==foobar" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge - version: "*" build: foo diff --git a/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap b/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap index 9ae8f69c5..e1dab9eda 100644 --- a/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap +++ b/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap @@ -41,7 +41,7 @@ pytorch=*=cuda*: name: foo version: 1.0.* channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge "conda-forge::foo[version=1.0.*, build_number=\">6\"]": name: foo @@ -50,7 +50,7 @@ pytorch=*=cuda*: op: Gt rhs: 6 channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge python ==2.7.*.*|>=3.6: name: python @@ -65,27 +65,27 @@ python=*: name: python version: "==3.9" channel: - base_url: "https://software.repos.intel.com/python/conda/" + base_url: "https://software.repos.intel.com/python/conda" name: python/conda "https://c.com/p/conda/linux-64::python[version=3.9]": name: python version: "==3.9" channel: - base_url: "https://c.com/p/conda/" + base_url: "https://c.com/p/conda" name: p/conda subdir: linux-64 "https://c.com/p/conda::python[version=3.9, subdir=linux-64]": name: python version: "==3.9" channel: - base_url: "https://c.com/p/conda/" + base_url: "https://c.com/p/conda" name: p/conda subdir: linux-64 "conda-forge/linux-32::python[version=3.9, subdir=linux-64]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 "conda-forge/linux-32::python ==3.9[subdir=linux-64, build_number=\"0\"]": @@ -95,20 +95,20 @@ python=*: op: Eq rhs: 0 channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 "python ==3.9[channel=conda-forge]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge "python ==3.9[channel=conda-forge/linux-64]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 rust ~=1.2.3: @@ -117,24 +117,24 @@ rust ~=1.2.3: "~/channel/dir::package": name: package channel: - base_url: "file:///channel/dir/" + base_url: "file:///channel/dir" name: ~/channel/dir "~\\windows_channel::package": error: invalid channel "./relative/channel::package": name: package channel: - base_url: "file:///relative/channel/" + base_url: "file:///relative/channel" name: "./relative/channel" "python[channel=https://conda.anaconda.org/python/conda,version=3.9]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/python/conda/" + base_url: "https://conda.anaconda.org/python/conda" name: python/conda "channel/win-64::foobar[channel=conda-forge, subdir=linux-64]": name: foobar channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 diff --git a/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap b/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap index 4b00e3e5a..05ea333c6 100644 --- a/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap +++ b/crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap @@ -35,7 +35,7 @@ pytorch=*=cuda*: name: foo version: 1.0.* channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge "conda-forge::foo[version=1.0.*, build_number=\">6\"]": name: foo @@ -44,7 +44,7 @@ pytorch=*=cuda*: op: Gt rhs: 6 channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge python ==2.7.*.*|>=3.6: error: "invalid version constraint: regex constraints are not supported" @@ -57,27 +57,27 @@ python=*: name: python version: "==3.9" channel: - base_url: "https://software.repos.intel.com/python/conda/" + base_url: "https://software.repos.intel.com/python/conda" name: python/conda "https://c.com/p/conda/linux-64::python[version=3.9]": name: python version: "==3.9" channel: - base_url: "https://c.com/p/conda/" + base_url: "https://c.com/p/conda" name: p/conda subdir: linux-64 "https://c.com/p/conda::python[version=3.9, subdir=linux-64]": name: python version: "==3.9" channel: - base_url: "https://c.com/p/conda/" + base_url: "https://c.com/p/conda" name: p/conda subdir: linux-64 "conda-forge/linux-32::python[version=3.9, subdir=linux-64]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 "conda-forge/linux-32::python ==3.9[subdir=linux-64, build_number=\"0\"]": @@ -87,20 +87,20 @@ python=*: op: Eq rhs: 0 channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 "python ==3.9[channel=conda-forge]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge "python ==3.9[channel=conda-forge/linux-64]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 rust ~=1.2.3: @@ -109,24 +109,24 @@ rust ~=1.2.3: "~/channel/dir::package": name: package channel: - base_url: "file:///channel/dir/" + base_url: "file:///channel/dir" name: ~/channel/dir "~\\windows_channel::package": error: invalid channel "./relative/channel::package": name: package channel: - base_url: "file:///relative/channel/" + base_url: "file:///relative/channel" name: "./relative/channel" "python[channel=https://conda.anaconda.org/python/conda,version=3.9]": name: python version: "==3.9" channel: - base_url: "https://conda.anaconda.org/python/conda/" + base_url: "https://conda.anaconda.org/python/conda" name: python/conda "channel/win-64::foobar[channel=conda-forge, subdir=linux-64]": name: foobar channel: - base_url: "https://conda.anaconda.org/conda-forge/" + base_url: "https://conda.anaconda.org/conda-forge" name: conda-forge subdir: linux-64 diff --git a/crates/rattler_conda_types/src/utils/url_with_trailing_slash.rs b/crates/rattler_conda_types/src/utils/url_with_trailing_slash.rs index 470babdf1..f061c36e2 100644 --- a/crates/rattler_conda_types/src/utils/url_with_trailing_slash.rs +++ b/crates/rattler_conda_types/src/utils/url_with_trailing_slash.rs @@ -1,6 +1,11 @@ +use std::{ + fmt::{Display, Formatter}, + ops::Deref, + str::FromStr, +}; + +use rattler_redaction::Redact; use serde::{Deserialize, Deserializer, Serialize}; -use std::fmt::{Display, Formatter}; -use std::ops::Deref; use url::Url; /// A URL that always has a trailing slash. A trailing slash in a URL has @@ -47,6 +52,14 @@ impl<'de> Deserialize<'de> for UrlWithTrailingSlash { } } +impl FromStr for UrlWithTrailingSlash { + type Err = url::ParseError; + + fn from_str(s: &str) -> Result { + Ok(Url::parse(s)?.into()) + } +} + impl From for Url { fn from(value: UrlWithTrailingSlash) -> Self { value.0 @@ -58,3 +71,9 @@ impl Display for UrlWithTrailingSlash { write!(f, "{}", &self.0) } } + +impl Redact for UrlWithTrailingSlash { + fn redact(self) -> Self { + UrlWithTrailingSlash(self.0.redact()) + } +}