api: Rename request macro attribute query_map to query_all

Remove its IntoIterator bound to allow to represent
the query fields as a single struct or enum.
This commit is contained in:
Kévin Commaille 2024-06-21 18:35:47 +02:00 committed by Kévin Commaille
parent 05c12bf3ba
commit 968c52b117
12 changed files with 80 additions and 52 deletions

View File

@ -33,7 +33,7 @@ pub mod v1 {
/// One or more custom fields to help identify the third party location. /// One or more custom fields to help identify the third party location.
// The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560). // The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560).
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub fields: BTreeMap<String, String>, pub fields: BTreeMap<String, String>,
} }

View File

@ -34,7 +34,7 @@ pub mod v1 {
/// One or more custom fields that are passed to the AS to help identify the user. /// One or more custom fields that are passed to the AS to help identify the user.
// The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560). // The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560).
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub fields: BTreeMap<String, String>, pub fields: BTreeMap<String, String>,
} }

View File

@ -34,7 +34,7 @@ pub mod v3 {
/// One or more custom fields to help identify the third party location. /// One or more custom fields to help identify the third party location.
// The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560). // The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560).
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub fields: BTreeMap<String, String>, pub fields: BTreeMap<String, String>,
} }

View File

@ -34,7 +34,7 @@ pub mod v3 {
/// One or more custom fields that are passed to the AS to help identify the user. /// One or more custom fields that are passed to the AS to help identify the user.
// The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560). // The specification is incorrect for this parameter. See [matrix-spec#560](https://github.com/matrix-org/matrix-spec/issues/560).
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub fields: BTreeMap<String, String>, pub fields: BTreeMap<String, String>,
} }

View File

@ -6,6 +6,14 @@ Bug fixes:
`Option<String>` for `ProtocolInstance`. It made the `unstable-unspecified` `Option<String>` for `ProtocolInstance`. It made the `unstable-unspecified`
feature non-additive. feature non-additive.
Breaking changes:
- Rename the `query_map` attribute of the `request` macro to `query_all`, and
remove the required bound to implement `IntoIterator<Item = (String, String)>`.
This allows to use a struct or enum as well as a map to represent the list of
query parameters. Note that the (de)serialization of the type used must work
with `serde_html_form`.
Improvements: Improvements:
- Add the `InvalidHeaderValue` variant to the `DeserializationError` struct, for - Add the `InvalidHeaderValue` variant to the `DeserializationError` struct, for

View File

@ -122,9 +122,11 @@ macro_rules! metadata {
/// they are declared must match the order in which they occur in the request path. /// they are declared must match the order in which they occur in the request path.
/// * `#[ruma_api(query)]`: Fields with this attribute will be inserting into the URL's query /// * `#[ruma_api(query)]`: Fields with this attribute will be inserting into the URL's query
/// string. /// string.
/// * `#[ruma_api(query_map)]`: Instead of individual query fields, one query_map field, of any /// * `#[ruma_api(query_all)]`: Instead of individual query fields, one query_all field, of any
/// type that implements `IntoIterator<Item = (String, String)>` (e.g. `HashMap<String, /// type that can be (de)serialized by [serde_html_form], can be used for cases where
/// String>`, can be used for cases where an endpoint supports arbitrary query parameters. /// multiple endpoints should share a query fields type, the query fields are better
/// expressed as an `enum` rather than a `struct`, or the endpoint supports arbitrary query
/// parameters.
/// * No attribute: Fields without an attribute are part of the body. They can use `#[serde]` /// * No attribute: Fields without an attribute are part of the body. They can use `#[serde]`
/// attributes to customize (de)serialization. /// attributes to customize (de)serialization.
/// * `#[ruma_api(body)]`: Use this if multiple endpoints should share a request body type, or /// * `#[ruma_api(body)]`: Use this if multiple endpoints should share a request body type, or
@ -209,6 +211,8 @@ macro_rules! metadata {
/// # pub struct Response {} /// # pub struct Response {}
/// } /// }
/// ``` /// ```
///
/// [serde_html_form]: https://crates.io/crates/serde_html_form
pub use ruma_macros::request; pub use ruma_macros::request;
/// Generates [`OutgoingResponse`] and [`IncomingResponse`] implementations. /// Generates [`OutgoingResponse`] and [`IncomingResponse`] implementations.
/// ///

View File

@ -133,7 +133,41 @@ pub mod raw_body_endpoint {
} }
} }
pub mod query_map_endpoint { pub mod query_all_enum_endpoint {
use ruma_common::{
api::{request, response, Metadata},
metadata,
};
#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)]
#[serde(untagged)]
pub enum MyCustomQueryEnum {
VariantA { field_a: String },
VariantB { field_b: String },
}
const METADATA: Metadata = metadata! {
method: GET,
rate_limited: false,
authentication: None,
history: {
unstable => "/_matrix/some/query/map/endpoint",
}
};
/// Request type for the `query_all_enum_endpoint` endpoint.
#[request]
pub struct Request {
#[ruma_api(query_all)]
pub query: MyCustomQueryEnum,
}
/// Response type for the `query_all_enum_endpoint` endpoint.
#[response]
pub struct Response {}
}
pub mod query_all_vec_endpoint {
use ruma_common::{ use ruma_common::{
api::{request, response, Metadata}, api::{request, response, Metadata},
metadata, metadata,
@ -148,14 +182,14 @@ pub mod query_map_endpoint {
} }
}; };
/// Request type for the `newtype_body_endpoint` endpoint. /// Request type for the `query_all_vec_endpoint` endpoint.
#[request] #[request]
pub struct Request { pub struct Request {
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub fields: Vec<(String, String)>, pub fields: Vec<(String, String)>,
} }
/// Response type for the `newtype_body_endpoint` endpoint. /// Response type for the `query_all_vec_endpoint` endpoint.
#[response] #[response]
pub struct Response {} pub struct Response {}
} }

View File

@ -33,7 +33,7 @@ pub mod v1 {
pub query_type: String, pub query_type: String,
/// The query parameters. /// The query parameters.
#[ruma_api(query_map)] #[ruma_api(query_all)]
pub params: BTreeMap<String, String>, pub params: BTreeMap<String, String>,
} }

View File

@ -10,7 +10,7 @@ mod kw {
syn::custom_keyword!(raw_body); syn::custom_keyword!(raw_body);
syn::custom_keyword!(path); syn::custom_keyword!(path);
syn::custom_keyword!(query); syn::custom_keyword!(query);
syn::custom_keyword!(query_map); syn::custom_keyword!(query_all);
syn::custom_keyword!(header); syn::custom_keyword!(header);
syn::custom_keyword!(error); syn::custom_keyword!(error);
syn::custom_keyword!(manual_body_serde); syn::custom_keyword!(manual_body_serde);
@ -22,7 +22,7 @@ pub enum RequestMeta {
RawBody, RawBody,
Path, Path,
Query, Query,
QueryMap, QueryAll,
Header(Ident), Header(Ident),
} }
@ -41,9 +41,9 @@ impl Parse for RequestMeta {
} else if lookahead.peek(kw::query) { } else if lookahead.peek(kw::query) {
let _: kw::query = input.parse()?; let _: kw::query = input.parse()?;
Ok(Self::Query) Ok(Self::Query)
} else if lookahead.peek(kw::query_map) { } else if lookahead.peek(kw::query_all) {
let _: kw::query_map = input.parse()?; let _: kw::query_all = input.parse()?;
Ok(Self::QueryMap) Ok(Self::QueryAll)
} else if lookahead.peek(kw::header) { } else if lookahead.peek(kw::header) {
let _: kw::header = input.parse()?; let _: kw::header = input.parse()?;
let _: Token![=] = input.parse()?; let _: Token![=] = input.parse()?;

View File

@ -137,8 +137,8 @@ impl Request {
self.fields.iter().find_map(RequestField::as_raw_body_field) self.fields.iter().find_map(RequestField::as_raw_body_field)
} }
fn query_map_field(&self) -> Option<&Field> { fn query_all_field(&self) -> Option<&Field> {
self.fields.iter().find_map(RequestField::as_query_map_field) self.fields.iter().find_map(RequestField::as_query_all_field)
} }
fn expand_all(&self, ruma_common: &TokenStream) -> TokenStream { fn expand_all(&self, ruma_common: &TokenStream) -> TokenStream {
@ -161,7 +161,7 @@ impl Request {
} }
}); });
let request_query_def = if let Some(f) = self.query_map_field() { let request_query_def = if let Some(f) = self.query_all_field() {
let field = Field { ident: None, colon_token: None, ..f.clone() }; let field = Field { ident: None, colon_token: None, ..f.clone() };
let field = PrivateField(&field); let field = PrivateField(&field);
Some(quote! { (#field); }) Some(quote! { (#field); })
@ -220,15 +220,15 @@ impl Request {
} }
}; };
let query_map_fields = let query_all_fields =
self.fields.iter().filter(|f| matches!(&f.kind, RequestFieldKind::QueryMap)); self.fields.iter().filter(|f| matches!(&f.kind, RequestFieldKind::QueryAll));
let has_query_map_field = match query_map_fields.count() { let has_query_all_field = match query_all_fields.count() {
0 => false, 0 => false,
1 => true, 1 => true,
_ => { _ => {
return Err(syn::Error::new_spanned( return Err(syn::Error::new_spanned(
&self.ident, &self.ident,
"Can't have more than one query_map field", "Can't have more than one query_all field",
)) ))
} }
}; };
@ -244,10 +244,10 @@ impl Request {
)); ));
} }
if has_query_map_field && has_query_fields { if has_query_all_field && has_query_fields {
return Err(syn::Error::new_spanned( return Err(syn::Error::new_spanned(
&self.ident, &self.ident,
"Can't have both a query map field and regular query fields", "Can't have both a query_all field and regular query fields",
)); ));
} }
@ -307,8 +307,8 @@ pub(super) enum RequestFieldKind {
/// Data that appears in the query string. /// Data that appears in the query string.
Query, Query,
/// Data that appears in the query string as dynamic key-value pairs. /// Data that represents all the query string as a single type.
QueryMap, QueryAll,
} }
impl RequestField { impl RequestField {
@ -319,7 +319,7 @@ impl RequestField {
Some(RequestMeta::RawBody) => RequestFieldKind::RawBody, Some(RequestMeta::RawBody) => RequestFieldKind::RawBody,
Some(RequestMeta::Path) => RequestFieldKind::Path, Some(RequestMeta::Path) => RequestFieldKind::Path,
Some(RequestMeta::Query) => RequestFieldKind::Query, Some(RequestMeta::Query) => RequestFieldKind::Query,
Some(RequestMeta::QueryMap) => RequestFieldKind::QueryMap, Some(RequestMeta::QueryAll) => RequestFieldKind::QueryAll,
Some(RequestMeta::Header(header)) => RequestFieldKind::Header(header), Some(RequestMeta::Header(header)) => RequestFieldKind::Header(header),
None => RequestFieldKind::Body, None => RequestFieldKind::Body,
}; };
@ -359,10 +359,10 @@ impl RequestField {
} }
} }
/// Return the contained field if this request field is a query map kind. /// Return the contained field if this request field is a query all kind.
pub fn as_query_map_field(&self) -> Option<&Field> { pub fn as_query_all_field(&self) -> Option<&Field> {
match &self.kind { match &self.kind {
RequestFieldKind::QueryMap => Some(&self.inner), RequestFieldKind::QueryAll => Some(&self.inner),
_ => None, _ => None,
} }
} }

View File

@ -31,7 +31,7 @@ impl Request {
(TokenStream::new(), TokenStream::new()) (TokenStream::new(), TokenStream::new())
}; };
let (parse_query, query_vars) = if let Some(field) = self.query_map_field() { let (parse_query, query_vars) = if let Some(field) = self.query_all_field() {
let cfg_attrs = let cfg_attrs =
field.attrs.iter().filter(|a| a.path().is_ident("cfg")).collect::<Vec<_>>(); field.attrs.iter().filter(|a| a.path().is_ident("cfg")).collect::<Vec<_>>();
let field_name = field.ident.as_ref().expect("expected field to have an identifier"); let field_name = field.ident.as_ref().expect("expected field to have an identifier");

View File

@ -15,29 +15,11 @@ impl Request {
let path_fields = let path_fields =
self.path_fields().map(|f| f.ident.as_ref().expect("path fields have a name")); self.path_fields().map(|f| f.ident.as_ref().expect("path fields have a name"));
let request_query_string = if let Some(field) = self.query_map_field() { let request_query_string = if let Some(field) = self.query_all_field() {
let field_name = field.ident.as_ref().expect("expected field to have identifier"); let field_name = field.ident.as_ref().expect("expected field to have identifier");
quote! {{ quote! {{
// This function exists so that the compiler will throw an error when the type of
// the field with the query_map attribute doesn't implement
// `IntoIterator<Item = (String, String)>`.
//
// This is necessary because the `serde_html_form::to_string` call will result in a
// runtime error when the type cannot be encoded as a list key-value pairs
// (?key1=value1&key2=value2).
//
// By asserting that it implements the iterator trait, we can ensure that it won't
// fail.
fn assert_trait_impl<T>(_: &T)
where
T: ::std::iter::IntoIterator<
Item = (::std::string::String, ::std::string::String),
>,
{}
let request_query = RequestQuery(self.#field_name); let request_query = RequestQuery(self.#field_name);
assert_trait_impl(&request_query.0);
&#serde_html_form::to_string(request_query)? &#serde_html_form::to_string(request_query)?
}} }}