diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cc6483372..d00e987a54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Fixed a bug that prevented project IDs from being used with the `sentry-cli releases new` command for users with self-hosted Sentry instances on versions older than 25.12.1 ([#3068](https://github.com/getsentry/sentry-cli/issues/3068)). + ## 3.0.3 ### Fixes diff --git a/src/api/mod.rs b/src/api/mod.rs index 5219c6991f..380d13ab86 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -10,6 +10,7 @@ mod data_types; mod encoding; mod errors; mod pagination; +mod serialization; use std::borrow::Cow; use std::cell::RefCell; @@ -1530,6 +1531,7 @@ pub struct AuthInfo { #[derive(Debug, Serialize, Default)] pub struct NewRelease { pub version: String, + #[serde(serialize_with = "serialization::serialize_id_slug_list")] pub projects: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub url: Option, diff --git a/src/api/serialization.rs b/src/api/serialization.rs new file mode 100644 index 0000000000..5d0f77391b --- /dev/null +++ b/src/api/serialization.rs @@ -0,0 +1,128 @@ +//! This module contains some custom serialization logic for the API. +use std::sync::LazyLock; + +use regex::Regex; +use serde::ser::SerializeSeq as _; +use serde::{Serialize, Serializer}; + +/// A container for either a numeric ID or an alphanumeric slug. +/// +/// IDs are serialized as integers, while slugs are serialized as strings. +#[derive(Serialize)] +#[serde(untagged)] +enum IdSlug<'s> { + Id(i64), + Slug(&'s str), +} + +/// Serializes a sequence of strings, which may contain either numeric IDs or alphanumeric slugs. +/// +/// We check each element in the sequence. If the element only contains digits and can be parsed as a 64-bit signed integer, +/// we consider the value to be an ID. Otherwise, we consider the value to be a slug. +/// +/// IDs are serialized as integers, while slugs are serialized as strings. +pub fn serialize_id_slug_list(list: I, serializer: S) -> Result +where + I: IntoIterator, + I::Item: AsRef, + S: Serializer, +{ + let mut seq = serializer.serialize_seq(None)?; + for item in list { + let item = item.as_ref(); + let id_slug = IdSlug::from(&item); + seq.serialize_element(&id_slug)?; + } + seq.end() +} + +impl<'a, S> From<&'a S> for IdSlug<'a> +where + S: AsRef, +{ + /// Convert from a string reference to an IdSlug. + /// + /// If the string contains only digits and can be parsed as a 64-bit signed integer, + /// we consider the value to be an ID. Otherwise, we consider the value to be a slug. + fn from(value: &'a S) -> Self { + /// Project ID regex + /// + /// Project IDs always contain only digits. + static PROJECT_ID_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"^\d+$").expect("regex is valid")); + + let value = value.as_ref(); + + PROJECT_ID_REGEX + .is_match(value) + .then(|| value.parse().ok().map(IdSlug::Id)) + .flatten() + .unwrap_or(IdSlug::Slug(value)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A test struct which serializes with serialize_id_slug_list + #[derive(Serialize)] + struct IdSlugListSerializerTest { + #[serde(serialize_with = "serialize_id_slug_list")] + value: [&'static str; N], + } + + #[test] + fn test_serialize_id_slug_list_empty() { + let to_serialize = IdSlugListSerializerTest { value: [] }; + + let serialized = serde_json::to_string(&to_serialize).unwrap(); + let expected = serde_json::json!({ "value": [] }).to_string(); + + assert_eq!(serialized, expected) + } + + #[test] + fn test_serialize_id_slug_list_single_id() { + let to_serialize = IdSlugListSerializerTest { value: ["123"] }; + + let serialized = serde_json::to_string(&to_serialize).unwrap(); + let expected = serde_json::json!({ "value": [123] }).to_string(); + + assert_eq!(serialized, expected) + } + + #[test] + fn test_serialize_id_slug_list_single_slug() { + let to_serialize = IdSlugListSerializerTest { value: ["abc"] }; + + let serialized = serde_json::to_string(&to_serialize).unwrap(); + let expected = serde_json::json!({ "value": ["abc"] }).to_string(); + + assert_eq!(serialized, expected) + } + + #[test] + fn test_serialize_id_slug_list_multiple_ids_and_slugs() { + let to_serialize = IdSlugListSerializerTest { + value: ["123", "abc", "456", "whatever"], + }; + + let serialized = serde_json::to_string(&to_serialize).unwrap(); + let expected = serde_json::json!({ "value": [123, "abc", 456, "whatever"] }).to_string(); + + assert_eq!(serialized, expected) + } + + /// Slugs of "-0" are possible. This test ensures that we serialize "-0" as a slug, + /// rather than as an ID 0. + #[test] + fn test_serialize_id_slug_minus_zero_edge_case() { + let to_serialize = IdSlugListSerializerTest { value: ["-0"] }; + + let serialized = serde_json::to_string(&to_serialize).unwrap(); + let expected = serde_json::json!({ "value": ["-0"] }).to_string(); + + assert_eq!(serialized, expected) + } +} diff --git a/tests/integration/_cases/releases/releases-new-mixed-projects.trycmd b/tests/integration/_cases/releases/releases-new-mixed-projects.trycmd new file mode 100644 index 0000000000..501ea29b22 --- /dev/null +++ b/tests/integration/_cases/releases/releases-new-mixed-projects.trycmd @@ -0,0 +1,6 @@ +``` +$ sentry-cli releases new -p 123 -p my-project -p 456 test-release +? success +Created release test-release + +``` diff --git a/tests/integration/_cases/releases/releases-new-multiple-numeric-projects.trycmd b/tests/integration/_cases/releases/releases-new-multiple-numeric-projects.trycmd new file mode 100644 index 0000000000..40fef3c8ea --- /dev/null +++ b/tests/integration/_cases/releases/releases-new-multiple-numeric-projects.trycmd @@ -0,0 +1,6 @@ +``` +$ sentry-cli releases new -p 123 -p 456 test-release +? success +Created release test-release + +``` diff --git a/tests/integration/_cases/releases/releases-new-numeric-project.trycmd b/tests/integration/_cases/releases/releases-new-numeric-project.trycmd new file mode 100644 index 0000000000..3bc36bdfa8 --- /dev/null +++ b/tests/integration/_cases/releases/releases-new-numeric-project.trycmd @@ -0,0 +1,6 @@ +``` +$ sentry-cli releases new -p 123 test-release +? success +Created release test-release + +``` diff --git a/tests/integration/releases/new.rs b/tests/integration/releases/new.rs index 1ac24b1cbd..f63a83e678 100644 --- a/tests/integration/releases/new.rs +++ b/tests/integration/releases/new.rs @@ -123,3 +123,51 @@ fn creates_release_which_is_instantly_finalized() { .register_trycmd_test("releases/releases-new-finalize.trycmd") .with_default_token(); } + +#[test] +fn creates_release_with_numeric_project_id() { + TestManager::new() + .mock_endpoint( + MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/releases/") + .with_status(201) + .with_response_file("releases/get-release.json") + .with_matcher(Matcher::PartialJson(json!({ + "version": "test-release", + "projects": [123], + }))), + ) + .register_trycmd_test("releases/releases-new-numeric-project.trycmd") + .with_default_token(); +} + +#[test] +fn creates_release_with_multiple_numeric_project_ids() { + TestManager::new() + .mock_endpoint( + MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/releases/") + .with_status(201) + .with_response_file("releases/get-release.json") + .with_matcher(Matcher::PartialJson(json!({ + "version": "test-release", + "projects": [123, 456], + }))), + ) + .register_trycmd_test("releases/releases-new-multiple-numeric-projects.trycmd") + .with_default_token(); +} + +#[test] +fn creates_release_with_mixed_project_ids() { + TestManager::new() + .mock_endpoint( + MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/releases/") + .with_status(201) + .with_response_file("releases/get-release.json") + .with_matcher(Matcher::PartialJson(json!({ + "version": "test-release", + "projects": [123, "my-project", 456], + }))), + ) + .register_trycmd_test("releases/releases-new-mixed-projects.trycmd") + .with_default_token(); +}