diff --git a/crates/ruma-client-api/src/state/get_state_events_for_key.rs b/crates/ruma-client-api/src/state/get_state_events_for_key.rs index 1ca51e87..74ee6026 100644 --- a/crates/ruma-client-api/src/state/get_state_events_for_key.rs +++ b/crates/ruma-client-api/src/state/get_state_events_for_key.rs @@ -85,8 +85,7 @@ pub mod v3 { use http::header; use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; - let mut url = ruma_common::api::make_endpoint_url( - &METADATA, + let mut url = METADATA.make_endpoint_url( considering_versions, base_url, &[&self.room_id, &self.event_type], diff --git a/crates/ruma-client-api/src/state/send_state_event.rs b/crates/ruma-client-api/src/state/send_state_event.rs index e93e445b..0234a3e6 100644 --- a/crates/ruma-client-api/src/state/send_state_event.rs +++ b/crates/ruma-client-api/src/state/send_state_event.rs @@ -127,8 +127,7 @@ pub mod v3 { use http::header::{self, HeaderValue}; use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; - let mut url = ruma_common::api::make_endpoint_url( - &METADATA, + let mut url = METADATA.make_endpoint_url( considering_versions, base_url, &[&self.room_id, &self.event_type], diff --git a/crates/ruma-common/src/api.rs b/crates/ruma-common/src/api.rs index 730ca6b6..806c7521 100644 --- a/crates/ruma-common/src/api.rs +++ b/crates/ruma-common/src/api.rs @@ -12,15 +12,9 @@ //! //! [apis]: https://spec.matrix.org/v1.2/#matrix-apis -use std::{ - convert::TryInto as _, - error::Error as StdError, - fmt::{Display, Write}, -}; +use std::{convert::TryInto as _, error::Error as StdError}; use bytes::BufMut; -use percent_encoding::utf8_percent_encode; -use tracing::warn; use crate::UserId; @@ -403,206 +397,3 @@ pub enum AuthScheme { /// Authentication is performed by setting the `access_token` query parameter. QueryOnlyAccessToken, } - -// This function needs to be public, yet hidden, as it is used the code generated by `ruma_api!`. -#[doc(hidden)] -pub fn make_endpoint_url( - metadata: &Metadata, - versions: &[MatrixVersion], - base_url: &str, - path_args: &[&dyn Display], - query_string: Option<&str>, -) -> Result { - let path_with_placeholders = select_path(metadata, versions)?; - - let mut res = base_url.strip_suffix('/').unwrap_or(base_url).to_owned(); - let mut segments = path_with_placeholders.split('/'); - let mut path_args = path_args.iter(); - - let first_segment = segments.next().expect("split iterator is never empty"); - assert!(first_segment.is_empty(), "endpoint paths must start with '/'"); - - for segment in segments { - if segment.starts_with(':') { - let arg = path_args - .next() - .expect("number of placeholders must match number of arguments") - .to_string(); - let arg = utf8_percent_encode(&arg, percent_encoding::NON_ALPHANUMERIC); - - write!(res, "/{arg}").expect("writing to a String using fmt::Write can't fail"); - } else { - res.reserve(segment.len() + 1); - res.push('/'); - res.push_str(segment); - } - } - - if let Some(query) = query_string { - res.push('?'); - res.push_str(query); - } - - Ok(res) -} - -// This function helps picks the right path (or an error) from a set of matrix versions. -fn select_path<'a>( - metadata: &'a Metadata, - versions: &[MatrixVersion], -) -> Result<&'a str, IntoHttpError> { - match metadata.versioning_decision_for(versions) { - VersioningDecision::Removed => Err(IntoHttpError::EndpointRemoved( - metadata.removed.expect("VersioningDecision::Removed implies metadata.removed"), - )), - VersioningDecision::Stable { any_deprecated, all_deprecated, any_removed } => { - if any_removed { - if all_deprecated { - warn!( - "endpoint {} is removed in some (and deprecated in ALL) of the following versions: {:?}", - metadata.name, - versions - ); - } else if any_deprecated { - warn!( - "endpoint {} is removed (and deprecated) in some of the following versions: {:?}", - metadata.name, - versions - ); - } else { - unreachable!("any_removed implies *_deprecated"); - } - } else if all_deprecated { - warn!( - "endpoint {} is deprecated in ALL of the following versions: {:?}", - metadata.name, versions - ); - } else if any_deprecated { - warn!( - "endpoint {} is deprecated in some of the following versions: {:?}", - metadata.name, versions - ); - } - - if let Some(r0) = metadata.r0_path { - if versions.iter().all(|&v| v == MatrixVersion::V1_0) { - // Endpoint was added in 1.0, we return the r0 variant. - return Ok(r0); - } - } - - Ok(metadata.stable_path.expect("metadata.added enforces the stable path to exist")) - } - VersioningDecision::Unstable => metadata.unstable_path.ok_or(IntoHttpError::NoUnstablePath), - } -} - -#[cfg(test)] -mod tests { - use super::{ - error::IntoHttpError, - make_endpoint_url, select_path, AuthScheme, - MatrixVersion::{V1_0, V1_1, V1_2}, - Metadata, - }; - use assert_matches::assert_matches; - use http::Method; - - const BASE: Metadata = Metadata { - description: "", - method: Method::GET, - name: "test_endpoint", - unstable_path: None, - r0_path: None, - stable_path: None, - rate_limited: false, - authentication: AuthScheme::None, - added: None, - deprecated: None, - removed: None, - }; - - // TODO add test that can hook into tracing and verify the deprecation warning is emitted - - #[test] - fn make_simple_endpoint_url() { - let meta = Metadata { added: Some(V1_0), stable_path: Some("/s"), ..BASE }; - let url = make_endpoint_url(&meta, &[V1_0], "https://example.org", &[], None).unwrap(); - assert_eq!(url, "https://example.org/s"); - } - - #[test] - fn make_endpoint_url_with_path_args() { - let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/:x"), ..BASE }; - let url = - make_endpoint_url(&meta, &[V1_0], "https://example.org", &[&"123"], None).unwrap(); - assert_eq!(url, "https://example.org/s/123"); - } - - #[test] - fn make_endpoint_url_with_query() { - let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/"), ..BASE }; - let url = - make_endpoint_url(&meta, &[V1_0], "https://example.org", &[], Some("foo=bar")).unwrap(); - assert_eq!(url, "https://example.org/s/?foo=bar"); - } - - #[test] - #[should_panic] - fn make_endpoint_url_wrong_num_path_args() { - let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/:x"), ..BASE }; - _ = make_endpoint_url(&meta, &[V1_0], "https://example.org", &[], None); - } - - #[test] - fn select_stable() { - let meta = Metadata { added: Some(V1_1), stable_path: Some("s"), ..BASE }; - assert_matches!(select_path(&meta, &[V1_0, V1_1]), Ok("s")); - } - - #[test] - fn select_unstable() { - let meta = Metadata { unstable_path: Some("u"), ..BASE }; - assert_matches!(select_path(&meta, &[V1_0]), Ok("u")); - } - - #[test] - fn select_r0() { - let meta = Metadata { added: Some(V1_0), r0_path: Some("r"), ..BASE }; - assert_matches!(select_path(&meta, &[V1_0]), Ok("r")); - } - - #[test] - fn select_removed_err() { - let meta = Metadata { - added: Some(V1_0), - deprecated: Some(V1_1), - removed: Some(V1_2), - unstable_path: Some("u"), - r0_path: Some("r"), - stable_path: Some("s"), - ..BASE - }; - assert_matches!(select_path(&meta, &[V1_2]), Err(IntoHttpError::EndpointRemoved(V1_2))); - } - - #[test] - fn partially_removed_but_stable() { - let meta = Metadata { - added: Some(V1_0), - deprecated: Some(V1_1), - removed: Some(V1_2), - r0_path: Some("r"), - stable_path: Some("s"), - ..BASE - }; - assert_matches!(select_path(&meta, &[V1_1]), Ok("s")); - } - - #[test] - fn no_unstable() { - let meta = - Metadata { added: Some(V1_1), r0_path: Some("r"), stable_path: Some("s"), ..BASE }; - assert_matches!(select_path(&meta, &[V1_0]), Err(IntoHttpError::NoUnstablePath)); - } -} diff --git a/crates/ruma-common/src/api/metadata.rs b/crates/ruma-common/src/api/metadata.rs index c7f233a8..082c824d 100644 --- a/crates/ruma-common/src/api/metadata.rs +++ b/crates/ruma-common/src/api/metadata.rs @@ -1,11 +1,16 @@ use std::{ - fmt::{self, Display}, + fmt::{self, Display, Write}, str::FromStr, }; use http::Method; +use percent_encoding::utf8_percent_encode; +use tracing::warn; -use super::{error::UnknownVersionError, AuthScheme}; +use super::{ + error::{IntoHttpError, UnknownVersionError}, + AuthScheme, +}; use crate::RoomVersionId; /// Metadata about an API endpoint. @@ -96,6 +101,95 @@ impl Metadata { VersioningDecision::Unstable } + + /// Generate the endpoint URL for this endpoint. + pub fn make_endpoint_url( + &self, + versions: &[MatrixVersion], + base_url: &str, + path_args: &[&dyn Display], + query_string: Option<&str>, + ) -> Result { + let path_with_placeholders = self.select_path(versions)?; + + let mut res = base_url.strip_suffix('/').unwrap_or(base_url).to_owned(); + let mut segments = path_with_placeholders.split('/'); + let mut path_args = path_args.iter(); + + let first_segment = segments.next().expect("split iterator is never empty"); + assert!(first_segment.is_empty(), "endpoint paths must start with '/'"); + + for segment in segments { + if segment.starts_with(':') { + let arg = path_args + .next() + .expect("number of placeholders must match number of arguments") + .to_string(); + let arg = utf8_percent_encode(&arg, percent_encoding::NON_ALPHANUMERIC); + + write!(res, "/{arg}").expect("writing to a String using fmt::Write can't fail"); + } else { + res.reserve(segment.len() + 1); + res.push('/'); + res.push_str(segment); + } + } + + if let Some(query) = query_string { + res.push('?'); + res.push_str(query); + } + + Ok(res) + } + + // This function helps picks the right path (or an error) from a set of matrix versions. + fn select_path(&self, versions: &[MatrixVersion]) -> Result<&str, IntoHttpError> { + match self.versioning_decision_for(versions) { + VersioningDecision::Removed => Err(IntoHttpError::EndpointRemoved( + self.removed.expect("VersioningDecision::Removed implies metadata.removed"), + )), + VersioningDecision::Stable { any_deprecated, all_deprecated, any_removed } => { + if any_removed { + if all_deprecated { + warn!( + "endpoint {} is removed in some (and deprecated in ALL) \ + of the following versions: {:?}", + self.name, versions + ); + } else if any_deprecated { + warn!( + "endpoint {} is removed (and deprecated) in some of the \ + following versions: {:?}", + self.name, versions + ); + } else { + unreachable!("any_removed implies *_deprecated"); + } + } else if all_deprecated { + warn!( + "endpoint {} is deprecated in ALL of the following versions: {:?}", + self.name, versions + ); + } else if any_deprecated { + warn!( + "endpoint {} is deprecated in some of the following versions: {:?}", + self.name, versions + ); + } + + if let Some(r0) = self.r0_path { + if versions.iter().all(|&v| v == MatrixVersion::V1_0) { + // Endpoint was added in 1.0, we return the r0 variant. + return Ok(r0); + } + } + + Ok(self.stable_path.expect("metadata.added enforces the stable path to exist")) + } + VersioningDecision::Unstable => self.unstable_path.ok_or(IntoHttpError::NoUnstablePath), + } + } } /// A versioning "decision" derived from a set of matrix versions. @@ -251,3 +345,113 @@ impl Display for MatrixVersion { f.write_str(&format!("v{major}.{minor}")) } } + +#[cfg(test)] +mod tests { + use assert_matches::assert_matches; + use http::Method; + + use super::{ + AuthScheme, + MatrixVersion::{V1_0, V1_1, V1_2}, + Metadata, + }; + use crate::api::error::IntoHttpError; + + const BASE: Metadata = Metadata { + description: "", + method: Method::GET, + name: "test_endpoint", + unstable_path: None, + r0_path: None, + stable_path: None, + rate_limited: false, + authentication: AuthScheme::None, + added: None, + deprecated: None, + removed: None, + }; + + // TODO add test that can hook into tracing and verify the deprecation warning is emitted + + #[test] + fn make_simple_endpoint_url() { + let meta = Metadata { added: Some(V1_0), stable_path: Some("/s"), ..BASE }; + let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], None).unwrap(); + assert_eq!(url, "https://example.org/s"); + } + + #[test] + fn make_endpoint_url_with_path_args() { + let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/:x"), ..BASE }; + let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[&"123"], None).unwrap(); + assert_eq!(url, "https://example.org/s/123"); + } + + #[test] + fn make_endpoint_url_with_query() { + let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/"), ..BASE }; + let url = + meta.make_endpoint_url(&[V1_0], "https://example.org", &[], Some("foo=bar")).unwrap(); + assert_eq!(url, "https://example.org/s/?foo=bar"); + } + + #[test] + #[should_panic] + fn make_endpoint_url_wrong_num_path_args() { + let meta = Metadata { added: Some(V1_0), stable_path: Some("/s/:x"), ..BASE }; + _ = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], None); + } + + #[test] + fn select_stable() { + let meta = Metadata { added: Some(V1_1), stable_path: Some("s"), ..BASE }; + assert_matches!(meta.select_path(&[V1_0, V1_1]), Ok("s")); + } + + #[test] + fn select_unstable() { + let meta = Metadata { unstable_path: Some("u"), ..BASE }; + assert_matches!(meta.select_path(&[V1_0]), Ok("u")); + } + + #[test] + fn select_r0() { + let meta = Metadata { added: Some(V1_0), r0_path: Some("r"), ..BASE }; + assert_matches!(meta.select_path(&[V1_0]), Ok("r")); + } + + #[test] + fn select_removed_err() { + let meta = Metadata { + added: Some(V1_0), + deprecated: Some(V1_1), + removed: Some(V1_2), + unstable_path: Some("u"), + r0_path: Some("r"), + stable_path: Some("s"), + ..BASE + }; + assert_matches!(meta.select_path(&[V1_2]), Err(IntoHttpError::EndpointRemoved(V1_2))); + } + + #[test] + fn partially_removed_but_stable() { + let meta = Metadata { + added: Some(V1_0), + deprecated: Some(V1_1), + removed: Some(V1_2), + r0_path: Some("r"), + stable_path: Some("s"), + ..BASE + }; + assert_matches!(meta.select_path(&[V1_1]), Ok("s")); + } + + #[test] + fn no_unstable() { + let meta = + Metadata { added: Some(V1_1), r0_path: Some("r"), stable_path: Some("s"), ..BASE }; + assert_matches!(meta.select_path(&[V1_0]), Err(IntoHttpError::NoUnstablePath)); + } +} diff --git a/crates/ruma-common/tests/api/manual_endpoint_impl.rs b/crates/ruma-common/tests/api/manual_endpoint_impl.rs index 5c1965ef..00acd641 100644 --- a/crates/ruma-common/tests/api/manual_endpoint_impl.rs +++ b/crates/ruma-common/tests/api/manual_endpoint_impl.rs @@ -49,8 +49,7 @@ impl OutgoingRequest for Request { _access_token: SendAccessToken<'_>, considering_versions: &'_ [MatrixVersion], ) -> Result, IntoHttpError> { - let url = ruma_common::api::make_endpoint_url( - &METADATA, + let url = METADATA.make_endpoint_url( considering_versions, base_url, &[&self.room_alias], diff --git a/crates/ruma-macros/src/api/request/outgoing.rs b/crates/ruma-macros/src/api/request/outgoing.rs index 2d7884fc..71250bb6 100644 --- a/crates/ruma-macros/src/api/request/outgoing.rs +++ b/crates/ruma-macros/src/api/request/outgoing.rs @@ -173,8 +173,7 @@ impl Request { let mut req_builder = #http::Request::builder() .method(#http::Method::#method) - .uri(#ruma_common::api::make_endpoint_url( - &metadata, + .uri(metadata.make_endpoint_url( considering_versions, base_url, &[ #( &self.#path_fields ),* ],