diff --git a/crates/ruma-api-macros/src/request.rs b/crates/ruma-api-macros/src/request.rs index 823f37ea..ed1df507 100644 --- a/crates/ruma-api-macros/src/request.rs +++ b/crates/ruma-api-macros/src/request.rs @@ -1,5 +1,5 @@ use std::{ - collections::BTreeSet, + collections::{BTreeMap, BTreeSet}, convert::{TryFrom, TryInto}, mem, }; @@ -164,8 +164,27 @@ impl Request { self.fields.iter().filter(|f| matches!(f, RequestField::Header(..))) } - fn path_fields(&self) -> impl Iterator { - self.fields.iter().filter_map(RequestField::as_path_field) + fn path_fields_ordered(&self) -> impl Iterator { + let map: BTreeMap = self + .fields + .iter() + .filter_map(RequestField::as_path_field) + .map(|f| (f.ident.as_ref().unwrap().to_string(), f)) + .collect(); + + self.stable_path + .as_ref() + .or(self.r0_path.as_ref()) + .or(self.unstable_path.as_ref()) + .expect("one of the paths to be defined") + .value() + .split('/') + .filter_map(|s| { + s.strip_prefix(':') + .map(|s| *map.get(s).expect("path args have already been checked")) + }) + .collect::>() + .into_iter() } fn raw_body_field(&self) -> Option<&Field> { @@ -249,6 +268,13 @@ impl Request { pub(super) fn check(&self) -> syn::Result<()> { // TODO: highlight problematic fields + let path_fields: Vec<_> = + self.fields.iter().filter_map(RequestField::as_path_field).collect(); + + self.check_path(&path_fields, self.unstable_path.as_ref())?; + self.check_path(&path_fields, self.r0_path.as_ref())?; + self.check_path(&path_fields, self.stable_path.as_ref())?; + let newtype_body_fields = self.fields.iter().filter(|field| { matches!(field, RequestField::NewtypeBody(_) | RequestField::RawBody(_)) }); @@ -311,6 +337,48 @@ impl Request { Ok(()) } + + fn check_path(&self, fields: &[&Field], path: Option<&LitStr>) -> syn::Result<()> { + let path = if let Some(lit) = path { lit } else { return Ok(()) }; + + let path_args: Vec<_> = path + .value() + .split('/') + .filter_map(|s| s.strip_prefix(':').map(&str::to_string)) + .collect(); + + let field_map: BTreeMap<_, _> = + fields.iter().map(|&f| (f.ident.as_ref().unwrap().to_string(), f)).collect(); + + // test if all macro fields exist in the path + for (name, field) in field_map.iter() { + if !path_args.contains(name) { + return Err({ + let mut err = syn::Error::new_spanned( + field, + "this path argument field is not defined in...", + ); + err.combine(syn::Error::new_spanned(path, "...this path.")); + err + }); + } + } + + // test if all path fields exists in macro fields + for arg in &path_args { + if !field_map.contains_key(arg) { + return Err(syn::Error::new_spanned( + path, + format!( + "a corresponding request path argument field for \"{}\" does not exist", + arg + ), + )); + } + } + + Ok(()) + } } /// The types of fields that a request can have. diff --git a/crates/ruma-api-macros/src/request/incoming.rs b/crates/ruma-api-macros/src/request/incoming.rs index dc4be593..f424263f 100644 --- a/crates/ruma-api-macros/src/request/incoming.rs +++ b/crates/ruma-api-macros/src/request/incoming.rs @@ -25,7 +25,8 @@ impl Request { // except this one. If we get errors about missing fields in IncomingRequest for // a path field look here. let (parse_request_path, path_vars) = if self.has_path_fields() { - let path_vars: Vec<_> = self.path_fields().filter_map(|f| f.ident.as_ref()).collect(); + let path_vars: Vec<_> = + self.path_fields_ordered().filter_map(|f| f.ident.as_ref()).collect(); let parse_request_path = quote! { let (#(#path_vars,)*) = #serde::Deserialize::deserialize( diff --git a/crates/ruma-api/tests/path_arg_ordering.rs b/crates/ruma-api/tests/path_arg_ordering.rs new file mode 100644 index 00000000..dc6c817b --- /dev/null +++ b/crates/ruma-api/tests/path_arg_ordering.rs @@ -0,0 +1,42 @@ +use ruma_api::{ruma_api, IncomingRequest}; + +ruma_api! { + metadata: { + description: "Does something.", + method: GET, + name: "some_path_args", + path: "/_matrix/:one/a/:two/b/:three/c", + rate_limited: false, + authentication: None, + } + + request: { + #[ruma_api(path)] + pub three: String, + + #[ruma_api(path)] + pub one: String, + + #[ruma_api(path)] + pub two: String, + } + + response: {} +} + +#[test] +fn path_ordering_is_correct() { + let request = http::Request::builder() + .method("GET") + // This explicitly puts wrong values in the URI, as now we rely on the side-supplied + // path_args slice, so this is just to ensure it *is* using that slice. + .uri("https://www.rust-lang.org/_matrix/non/a/non/b/non/c") + .body("") + .unwrap(); + + let resp = Request::try_from_http_request(request, &["1", "2", "3"]).unwrap(); + + assert_eq!(resp.one, "1"); + assert_eq!(resp.two, "2"); + assert_eq!(resp.three, "3"); +} diff --git a/crates/ruma-api/tests/ruma_api_lifetime.rs b/crates/ruma-api/tests/ruma_api_lifetime.rs index d8347b66..38790607 100644 --- a/crates/ruma-api/tests/ruma_api_lifetime.rs +++ b/crates/ruma-api/tests/ruma_api_lifetime.rs @@ -42,7 +42,7 @@ mod nested_types { description: "Add an alias to a room.", method: PUT, name: "create_alias", - path: "/_matrix/client/r0/directory/room/:room_alias", + path: "/_matrix/client/r0/directory/room", rate_limited: false, authentication: AccessToken, } diff --git a/crates/ruma-api/tests/ui/03-move-value.rs b/crates/ruma-api/tests/ui/03-move-value.rs index aec212c1..da6a790e 100644 --- a/crates/ruma-api/tests/ui/03-move-value.rs +++ b/crates/ruma-api/tests/ui/03-move-value.rs @@ -100,6 +100,7 @@ mod plain { request: { pub q2: Foo, + #[ruma_api(path)] pub bar: String, #[ruma_api(query)] diff --git a/crates/ruma-api/tests/ui/05-request-only.rs b/crates/ruma-api/tests/ui/05-request-only.rs index a60fc1aa..79e6e940 100644 --- a/crates/ruma-api/tests/ui/05-request-only.rs +++ b/crates/ruma-api/tests/ui/05-request-only.rs @@ -10,7 +10,7 @@ ruma_api! { description: "Does something.", method: POST, // An `http::Method` constant. No imports required. name: "some_endpoint", - path: "/_matrix/some/endpoint/:baz", + path: "/_matrix/some/endpoint/:foo", rate_limited: false, authentication: None, } @@ -18,6 +18,7 @@ ruma_api! { #[derive(PartialEq)] // Make sure attributes work request: { // With no attribute on the field, it will be put into the body of the request. + #[ruma_api(path)] pub foo: String, } }