diff --git a/sentry-types/src/dsn.rs b/sentry-types/src/dsn.rs index 058d0ecc..67806865 100644 --- a/sentry-types/src/dsn.rs +++ b/sentry-types/src/dsn.rs @@ -140,8 +140,8 @@ impl Dsn { } /// Returns the project_id - pub fn project_id(&self) -> ProjectId { - self.project_id + pub fn project_id(&self) -> &ProjectId { + &self.project_id } } @@ -229,7 +229,7 @@ mod test { #[test] fn test_dsn_parsing() { - let url = "https://username:password@domain:8888/23"; + let url = "https://username:password@domain:8888/23%21"; let dsn = url.parse::().unwrap(); assert_eq!(dsn.scheme(), Scheme::Https); assert_eq!(dsn.public_key(), "username"); @@ -237,7 +237,7 @@ mod test { assert_eq!(dsn.host(), "domain"); assert_eq!(dsn.port(), 8888); assert_eq!(dsn.path(), "/"); - assert_eq!(dsn.project_id(), ProjectId::new(23)); + assert_eq!(dsn.project_id(), &ProjectId::new("23%21")); assert_eq!(url, dsn.to_string()); } @@ -303,9 +303,18 @@ mod test { } #[test] - #[should_panic(expected = "InvalidProjectId")] + fn test_dsn_non_integer_project_id() { + let url = "https://username:password@domain:8888/abc123youandme%21%21"; + let dsn = url.parse::().unwrap(); + assert_eq!(dsn.project_id(), &ProjectId::new("abc123youandme%21%21")); + } + + #[test] fn test_dsn_more_than_one_non_integer_path() { - Dsn::from_str("http://username:@domain:8888/path/path2").unwrap(); + let url = "http://username:@domain:8888/pathone/pathtwo/pid"; + let dsn = url.parse::().unwrap(); + assert_eq!(dsn.project_id(), &ProjectId::new("pid")); + assert_eq!(dsn.path(), "/pathone/pathtwo/"); } #[test] diff --git a/sentry-types/src/project_id.rs b/sentry-types/src/project_id.rs index 28aa931e..f55b1282 100644 --- a/sentry-types/src/project_id.rs +++ b/sentry-types/src/project_id.rs @@ -1,4 +1,3 @@ -use std::convert::TryFrom; use std::fmt; use std::str::FromStr; @@ -8,76 +7,37 @@ use thiserror::Error; /// Raised if a project ID cannot be parsed from a string. #[derive(Debug, Error, PartialEq, Eq, PartialOrd, Ord)] pub enum ParseProjectIdError { - /// Raised if the value is not an integer in the supported range. - #[error("invalid value for project id")] - InvalidValue, /// Raised if an empty value is parsed. #[error("empty or missing project id")] EmptyValue, } /// Represents a project ID. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Deserialize, Serialize)] -pub struct ProjectId(u64); +#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Deserialize, Serialize)] +pub struct ProjectId(String); impl ProjectId { - /// Creates a new project ID from its numeric value. + /// Creates a new project ID from its string representation. + /// This assumes that the string is already well-formed and URL + /// encoded/decoded. #[inline] - pub fn new(id: u64) -> Self { - Self(id) + pub fn new(id: &str) -> Self { + Self(id.to_string()) } - /// Returns the numeric value of this project id. + /// Returns the string representation of the project ID. #[inline] - pub fn value(self) -> u64 { - self.0 + pub fn value(&self) -> &str { + self.0.as_ref() } } impl fmt::Display for ProjectId { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.value()) + write!(f, "{}", self.0) } } -macro_rules! impl_from { - ($ty:ty) => { - impl From<$ty> for ProjectId { - #[inline] - fn from(val: $ty) -> Self { - Self::new(val as u64) - } - } - }; -} - -impl_from!(u8); -impl_from!(u16); -impl_from!(u32); -impl_from!(u64); - -macro_rules! impl_try_from { - ($ty:ty) => { - impl TryFrom<$ty> for ProjectId { - type Error = ParseProjectIdError; - - #[inline] - fn try_from(val: $ty) -> Result { - match u64::try_from(val) { - Ok(id) => Ok(Self::new(id)), - Err(_) => Err(ParseProjectIdError::InvalidValue), - } - } - } - }; -} - -impl_try_from!(usize); -impl_try_from!(i8); -impl_try_from!(i16); -impl_try_from!(i32); -impl_try_from!(i64); - impl FromStr for ProjectId { type Err = ParseProjectIdError; @@ -85,11 +45,7 @@ impl FromStr for ProjectId { if s.is_empty() { return Err(ParseProjectIdError::EmptyValue); } - - match s.parse::() { - Ok(val) => Ok(ProjectId::new(val)), - Err(_) => Err(ParseProjectIdError::InvalidValue), - } + Ok(ProjectId::new(s)) } } @@ -100,21 +56,21 @@ mod test { #[test] fn test_basic_api() { let id: ProjectId = "42".parse().unwrap(); - assert_eq!(id, ProjectId::new(42)); - assert_eq!( - "42xxx".parse::(), - Err(ParseProjectIdError::InvalidValue) - ); + assert_eq!(id, ProjectId::new("42")); + assert_eq!("42xxx".parse::().unwrap().value(), "42xxx"); assert_eq!( "".parse::(), Err(ParseProjectIdError::EmptyValue) ); - assert_eq!(ProjectId::new(42).to_string(), "42"); + assert_eq!(ProjectId::new("42").to_string(), "42"); - assert_eq!(serde_json::to_string(&ProjectId::new(42)).unwrap(), "42"); assert_eq!( - serde_json::from_str::("42").unwrap(), - ProjectId::new(42) + serde_json::to_string(&ProjectId::new("42")).unwrap(), + "\"42\"" + ); + assert_eq!( + serde_json::from_str::("\"42\"").unwrap(), + ProjectId::new("42") ); } } diff --git a/sentry/tests/test_client.rs b/sentry/tests/test_client.rs index 984317c4..0687fb8f 100644 --- a/sentry/tests/test_client.rs +++ b/sentry/tests/test_client.rs @@ -5,17 +5,17 @@ use std::sync::Arc; #[test] fn test_into_client() { - let c: sentry::Client = sentry::Client::from_config("https://public@example.com/42"); + let c: sentry::Client = sentry::Client::from_config("https://public@example.com/42%21"); { let dsn = c.dsn().unwrap(); assert_eq!(dsn.public_key(), "public"); assert_eq!(dsn.host(), "example.com"); assert_eq!(dsn.scheme(), sentry::types::Scheme::Https); - assert_eq!(dsn.project_id().value(), 42); + assert_eq!(dsn.project_id().value(), "42%21"); } let c: sentry::Client = sentry::Client::from_config(( - "https://public@example.com/42", + "https://public@example.com/42%21", sentry::ClientOptions { release: Some("foo@1.0".into()), ..Default::default() @@ -26,7 +26,7 @@ fn test_into_client() { assert_eq!(dsn.public_key(), "public"); assert_eq!(dsn.host(), "example.com"); assert_eq!(dsn.scheme(), sentry::types::Scheme::Https); - assert_eq!(dsn.project_id().value(), 42); + assert_eq!(dsn.project_id().value(), "42%21"); assert_eq!(&c.options().release.as_ref().unwrap(), &"foo@1.0"); }