remove all but emptiness checks on project IDs

sentry-types/src/dsn.rs

@@ -229,15 +229,15 @@ 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::<Dsn>().unwrap();
         assert_eq!(dsn.scheme(), Scheme::Https);
         assert_eq!(dsn.public_key(), "username");
         assert_eq!(dsn.secret_key(), Some("password"));
         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::<Dsn>().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::<Dsn>().unwrap();
+        assert_eq!(dsn.project_id(), &ProjectId::new("pid"));
+        assert_eq!(dsn.path(), "/pathone/pathtwo/");
     }
 
     #[test]

sentry-types/src/project_id.rs

@@ -1,4 +1,3 @@
-use std::convert::TryFrom;
 use std::fmt;
 use std::str::FromStr;
 
@@ -8,31 +7,28 @@ 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(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Deserialize, Serialize)]
-#[serde(into = "u64", from = "u64")]
 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 {
+    pub fn new(id: &str) -> Self {
         Self(id.to_string())
     }
 
-    /// Returns the numeric value of this project id. None is returned if a
-    /// valid could not be parsed from the project id.
+    /// Returns the string representation of the project ID.
     #[inline]
-    pub fn value(&self) -> Option<u64> {
-        self.0.parse::<u64>().ok()
+    pub fn value(&self) -> &str {
+        self.0.as_ref()
     }
 }
 
@@ -42,68 +38,14 @@ impl fmt::Display for ProjectId {
     }
 }
 
-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<Self, Self::Error> {
-                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;
 
     fn from_str(s: &str) -> Result<ProjectId, ParseProjectIdError> {
         if s.is_empty() {
             return Err(ParseProjectIdError::EmptyValue);
         }
-
-        match s.parse::<u64>() {
-            Ok(val) => Ok(ProjectId::new(val)),
-            Err(_) => Err(ParseProjectIdError::InvalidValue),
-        }
-    }
-}
-
-// Combined with the serde into/from annotation, this allows the project ID to
-// continue being serialized and deserialized as a u64 until other parts of
-// sentry add in full support for project strings.
-impl From<ProjectId> for u64 {
-    fn from(pid: ProjectId) -> Self {
-        match pid.value() {
-            Some(val) => val,
-            None => u64::MAX,
-        }
+        Ok(ProjectId::new(s))
     }
 }
 
@@ -114,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::<ProjectId>(),
-            Err(ParseProjectIdError::InvalidValue)
-        );
+        assert_eq!(id, ProjectId::new("42"));
+        assert_eq!("42xxx".parse::<ProjectId>().unwrap().value(), "42xxx");
         assert_eq!(
             "".parse::<ProjectId>(),
             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::<ProjectId>("42").unwrap(),
-            ProjectId::new(42)
+            serde_json::to_string(&ProjectId::new("42")).unwrap(),
+            "\"42\""
+        );
+        assert_eq!(
+            serde_json::from_str::<ProjectId>("\"42\"").unwrap(),
+            ProjectId::new("42")
         );
     }
 }

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(), Some(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(), Some(42));
+        assert_eq!(dsn.project_id().value(), "42%21");
         assert_eq!(&c.options().release.as_ref().unwrap(), &"foo@1.0");
     }