api: Make query_string parameter of make_endpoint_url non-optional
When Some(_), the argument would often be an empty string. By always using the empty string to mean "no query", we avoid trailing a `?` on URLs without having two parameter values that mean the same thing.
This commit is contained in:
parent
d25e40764b
commit
2500e8f8fa
@ -91,7 +91,7 @@ pub mod v3 {
|
||||
considering_versions,
|
||||
base_url,
|
||||
&[&self.scope, &self.rule.kind(), &self.rule.rule_id()],
|
||||
Some(&query_string),
|
||||
&query_string,
|
||||
)?;
|
||||
|
||||
let body: RequestBody = self.rule.into();
|
||||
|
@ -88,7 +88,7 @@ pub mod v3 {
|
||||
considering_versions,
|
||||
base_url,
|
||||
&[&self.room_id, &self.event_type, &self.state_key],
|
||||
None,
|
||||
"",
|
||||
)?)
|
||||
.header(header::CONTENT_TYPE, "application/json")
|
||||
.header(
|
||||
|
@ -134,7 +134,7 @@ pub mod v3 {
|
||||
considering_versions,
|
||||
base_url,
|
||||
&[&self.room_id, &self.event_type, &self.state_key],
|
||||
Some(&query_string),
|
||||
&query_string,
|
||||
)?)
|
||||
.header(header::CONTENT_TYPE, "application/json")
|
||||
.header(
|
||||
@ -220,7 +220,7 @@ pub mod v3 {
|
||||
};
|
||||
|
||||
// This used to panic in make_endpoint_url because of a mismatch in the path parameter count
|
||||
Request::new(
|
||||
let req = Request::new(
|
||||
room_id!("!room:server.tld"),
|
||||
&EmptyStateKey,
|
||||
&RoomNameEventContent::new(Some("Test room".to_owned())),
|
||||
@ -232,5 +232,10 @@ pub mod v3 {
|
||||
&[MatrixVersion::V1_1],
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
req.uri(),
|
||||
"https://server.tld/_matrix/client/v3/rooms/%21room%3Aserver%2Etld/state/m%2Eroom%2Ename/"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
@ -90,7 +90,7 @@ impl Metadata {
|
||||
versions: &[MatrixVersion],
|
||||
base_url: &str,
|
||||
path_args: &[&dyn Display],
|
||||
query_string: Option<&str>,
|
||||
query_string: &str,
|
||||
) -> Result<String, IntoHttpError> {
|
||||
let path_with_placeholders = self.history.select_path(versions, self.name)?;
|
||||
|
||||
@ -117,9 +117,9 @@ impl Metadata {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(query) = query_string {
|
||||
if !query_string.is_empty() {
|
||||
res.push('?');
|
||||
res.push_str(query);
|
||||
res.push_str(query_string);
|
||||
}
|
||||
|
||||
Ok(res)
|
||||
@ -699,22 +699,21 @@ mod tests {
|
||||
#[test]
|
||||
fn make_simple_endpoint_url() {
|
||||
let meta = stable_only_metadata(&[(V1_0, "/s")]);
|
||||
let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], None).unwrap();
|
||||
let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], "").unwrap();
|
||||
assert_eq!(url, "https://example.org/s");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn make_endpoint_url_with_path_args() {
|
||||
let meta = stable_only_metadata(&[(V1_0, "/s/:x")]);
|
||||
let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[&"123"], None).unwrap();
|
||||
let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[&"123"], "").unwrap();
|
||||
assert_eq!(url, "https://example.org/s/123");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn make_endpoint_url_with_query() {
|
||||
let meta = stable_only_metadata(&[(V1_0, "/s/")]);
|
||||
let url =
|
||||
meta.make_endpoint_url(&[V1_0], "https://example.org", &[], Some("foo=bar")).unwrap();
|
||||
let url = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], "foo=bar").unwrap();
|
||||
assert_eq!(url, "https://example.org/s/?foo=bar");
|
||||
}
|
||||
|
||||
@ -722,7 +721,7 @@ mod tests {
|
||||
#[should_panic]
|
||||
fn make_endpoint_url_wrong_num_path_args() {
|
||||
let meta = stable_only_metadata(&[(V1_0, "/s/:x")]);
|
||||
_ = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], None);
|
||||
_ = meta.make_endpoint_url(&[V1_0], "https://example.org", &[], "");
|
||||
}
|
||||
|
||||
const EMPTY: VersionHistory =
|
||||
|
@ -51,12 +51,8 @@ impl OutgoingRequest for Request {
|
||||
_access_token: SendAccessToken<'_>,
|
||||
considering_versions: &'_ [MatrixVersion],
|
||||
) -> Result<http::Request<T>, IntoHttpError> {
|
||||
let url = METADATA.make_endpoint_url(
|
||||
considering_versions,
|
||||
base_url,
|
||||
&[&self.room_alias],
|
||||
None,
|
||||
)?;
|
||||
let url =
|
||||
METADATA.make_endpoint_url(considering_versions, base_url, &[&self.room_alias], "")?;
|
||||
|
||||
let request_body = RequestBody { room_id: self.room_id };
|
||||
|
||||
|
@ -38,9 +38,7 @@ impl Request {
|
||||
let request_query = RequestQuery(self.#field_name);
|
||||
assert_trait_impl(&request_query.0);
|
||||
|
||||
::std::option::Option::Some(
|
||||
&#ruma_common::serde::urlencoded::to_string(request_query)?
|
||||
)
|
||||
&#ruma_common::serde::urlencoded::to_string(request_query)?
|
||||
}}
|
||||
} else if self.has_query_fields() {
|
||||
let request_query_init_fields = struct_init_fields(
|
||||
@ -53,12 +51,10 @@ impl Request {
|
||||
#request_query_init_fields
|
||||
};
|
||||
|
||||
::std::option::Option::Some(
|
||||
&#ruma_common::serde::urlencoded::to_string(request_query)?
|
||||
)
|
||||
&#ruma_common::serde::urlencoded::to_string(request_query)?
|
||||
}}
|
||||
} else {
|
||||
quote! { ::std::option::Option::None }
|
||||
quote! { "" }
|
||||
};
|
||||
|
||||
// If there are no body fields, the request body will be empty (not `{}`), so the
|
||||
|
Loading…
x
Reference in New Issue
Block a user