From 52a21c495e58c7060f30cb838e225f88213f23b3 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Wed, 22 May 2024 19:09:04 -0400 Subject: [PATCH 01/11] refactor(http): Use query parameter builders --- twilight-http/Cargo.toml | 1 + twilight-http/src/lib.rs | 1 + twilight-http/src/query_array.rs | 49 ++++ twilight-http/src/routing.rs | 400 +++++++++++-------------------- 4 files changed, 187 insertions(+), 264 deletions(-) create mode 100644 twilight-http/src/query_array.rs diff --git a/twilight-http/Cargo.toml b/twilight-http/Cargo.toml index d2f6240d9ed..11f46f48cd5 100644 --- a/twilight-http/Cargo.toml +++ b/twilight-http/Cargo.toml @@ -34,6 +34,7 @@ twilight-validate = { default-features = false, path = "../twilight-validate", v # Optional dependencies. brotli-decompressor = { default-features = false, features = ["std"], optional = true, version = "4" } simd-json = { default-features = false, features = ["serde_impl", "swar-number-parsing"], optional = true, version = ">=0.4, <0.14" } +query-string-builder = "0.4.1" [features] default = ["decompression", "rustls-native-roots"] diff --git a/twilight-http/src/lib.rs b/twilight-http/src/lib.rs index 7562599b991..e7b823395b7 100644 --- a/twilight-http/src/lib.rs +++ b/twilight-http/src/lib.rs @@ -9,6 +9,7 @@ pub mod api_error; pub mod client; pub mod error; +pub mod query_array; pub mod request; pub mod response; pub mod routing; diff --git a/twilight-http/src/query_array.rs b/twilight-http/src/query_array.rs new file mode 100644 index 00000000000..4b1adfd16ed --- /dev/null +++ b/twilight-http/src/query_array.rs @@ -0,0 +1,49 @@ +use std::fmt::Display; + +/// Provides a display implementation for serializing arrays-like objects into +/// query params. +pub struct QueryArray(pub T); + +impl Display for QueryArray +where + T: IntoIterator + Copy, + U: Display, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut iter = self.0.into_iter().peekable(); + + while let Some(item) = iter.next() { + item.fmt(f)?; + if iter.peek().is_some() { + f.write_str(",")?; + } + } + + Ok(()) + } +} + +impl From> for String +where + T: IntoIterator + Copy, + U: Display, +{ + fn from(val: QueryArray) -> Self { + val.to_string() + } +} + +#[cfg(test)] +mod tests { + use super::QueryArray; + + #[test] + fn test_query_array() { + let query_array = QueryArray([1, 2, 3]); + assert_eq!(query_array.to_string(), "1,2,3"); + + let params = vec!["a", "b", "c"]; + let query_array = QueryArray(¶ms); + assert_eq!(query_array.to_string(), "a,b,c"); + } +} diff --git a/twilight-http/src/routing.rs b/twilight-http/src/routing.rs index 90fdaf2bf90..012fcb8cb20 100644 --- a/twilight-http/src/routing.rs +++ b/twilight-http/src/routing.rs @@ -1,7 +1,11 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; +use query_string_builder::QueryString; pub use twilight_http_ratelimiting::request::{Path, PathParseError, PathParseErrorType}; -use crate::request::{channel::reaction::RequestReactionType, Method}; +use crate::{ + query_array::QueryArray, + request::{channel::reaction::RequestReactionType, Method}, +}; use std::fmt::{Display, Formatter, Result as FmtResult}; use twilight_model::id::{marker::RoleMarker, Id}; @@ -1756,12 +1760,12 @@ impl Display for Route<'_> { Display::fmt(application_id, f)?; f.write_str("/commands")?; - if let Some(with_localizations) = with_localizations { - f.write_str("?with_localizations=")?; - Display::fmt(with_localizations, f)?; - } + let query = QueryString::new().with_opt_value( + "with_localizations", + with_localizations.map(|x| x.to_string()), + ); - Ok(()) + Display::fmt(&query, f) } Route::CreateGuild => f.write_str("guilds"), Route::CreateGuildCommand { @@ -1790,12 +1794,12 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/commands")?; - if let Some(with_localizations) = with_localizations { - f.write_str("?with_localizations=")?; - Display::fmt(with_localizations, f)?; - } + let query = QueryString::new().with_opt_value( + "with_localizations", + with_localizations.map(|x| x.to_string()), + ); - Ok(()) + Display::fmt(&query, f) } Route::CreateGuildFromTemplate { template_code } | Route::GetTemplate { template_code } => { @@ -1818,33 +1822,20 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/prune?")?; + f.write_str("/prune")?; - if let Some(compute_prune_count) = compute_prune_count { - f.write_str("compute_prune_count=")?; - Display::fmt(compute_prune_count, f)?; - } - - if let Some(days) = days { - f.write_str("&days=")?; - Display::fmt(days, f)?; - } + let mut query = QueryString::new() + .with_opt_value( + "compute_prune_count", + compute_prune_count.map(|x| x.to_string()), + ) + .with_opt_value("days", days.map(|x| x.to_string())); if !include_roles.is_empty() { - let role_count = include_roles.len() - 1; - - f.write_str("&include_roles=")?; - - for (idx, role_id) in include_roles.iter().enumerate() { - Display::fmt(role_id, f)?; - - if idx < role_count { - f.write_str(",")?; - } - } + query.push("include_roles", QueryArray(*include_roles)); } - Ok(()) + Display::fmt(&query, f) } Route::CreateGuildScheduledEvent { guild_id } => { f.write_str("guilds/")?; @@ -2198,12 +2189,10 @@ impl Display for Route<'_> { f.write_str("/messages/")?; Display::fmt(message_id, f)?; - if let Some(thread_id) = thread_id { - f.write_str("?thread_id=")?; - Display::fmt(thread_id, f)?; - } + let query = QueryString::new() + .with_opt_value("thread_id", thread_id.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::DeleteWebhook { token, webhook_id } | Route::GetWebhook { token, webhook_id } @@ -2228,20 +2217,12 @@ impl Display for Route<'_> { Display::fmt(webhook_id, f)?; f.write_str("/")?; f.write_str(token)?; - f.write_str("?")?; - if let Some(thread_id) = thread_id { - f.write_str("thread_id=")?; - Display::fmt(thread_id, f)?; - f.write_str("&")?; - } - - if let Some(wait) = wait { - f.write_str("wait=")?; - f.write_str(if *wait { "true" } else { "false" })?; - } + let query = QueryString::new() + .with_opt_value("thread_id", thread_id.map(|x| x.to_string())) + .with_opt_value("wait", wait.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::FollowNewsChannel { channel_id } => { f.write_str("channels/")?; @@ -2265,34 +2246,16 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/audit-logs?")?; - - if let Some(action_type) = action_type { - f.write_str("action_type=")?; - Display::fmt(action_type, f)?; - } - - if let Some(after) = after { - f.write_str("&after=")?; - Display::fmt(after, f)?; - } - - if let Some(before) = before { - f.write_str("&before=")?; - Display::fmt(before, f)?; - } - - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + f.write_str("/audit-logs")?; - if let Some(user_id) = user_id { - f.write_str("&user_id=")?; - Display::fmt(user_id, f)?; - } + let query = QueryString::new() + .with_opt_value("action_type", action_type.map(|x| x.to_string())) + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())) + .with_opt_value("user_id", user_id.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetBans { guild_id } => { f.write_str("guilds/")?; @@ -2308,24 +2271,14 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/bans?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } + f.write_str("/bans")?; - if let Some(before) = before { - f.write_str("&before=")?; - Display::fmt(before, f)?; - } - - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetGatewayBot => f.write_str("gateway/bot"), Route::GetCommandPermissions { @@ -2396,19 +2349,13 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/members?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } + f.write_str("/members")?; - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetGuildOnboarding { guild_id } | Route::UpdateGuildOnboarding { guild_id } => { f.write_str("guilds/")?; @@ -2429,25 +2376,13 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/prune?")?; + f.write_str("/prune")?; - if let Some(days) = days { - f.write_str("days=")?; - Display::fmt(days, f)?; - } + let mut query = + QueryString::new().with_opt_value("days", days.map(|x| x.to_string())); if !include_roles.is_empty() { - f.write_str("&include_roles=")?; - - let role_count = include_roles.len() - 1; - - for (idx, role_id) in include_roles.iter().enumerate() { - Display::fmt(role_id, f)?; - - if idx < role_count { - f.write_str(",")?; - } - } + query.push("include_roles", QueryArray(*include_roles)); } Ok(()) @@ -2462,11 +2397,13 @@ impl Display for Route<'_> { f.write_str("/scheduled-events/")?; Display::fmt(scheduled_event_id, f)?; + let mut query = QueryString::new(); + if *with_user_count { - f.write_str("?with_user_count=true")?; + query.push("with_user_count", with_user_count.to_string()); } - Ok(()) + Display::fmt(&query, f) } Route::GetGuildScheduledEventUsers { after, @@ -2480,28 +2417,18 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/scheduled-events/")?; Display::fmt(scheduled_event_id, f)?; - f.write_str("/users?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } - - if let Some(before) = before { - f.write_str("&before=")?; - Display::fmt(before, f)?; - } + f.write_str("/users")?; - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let mut query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); if *with_member { - f.write_str("&with_member=true")?; + query.push("with_member", with_member.to_string()); } - Ok(()) + Display::fmt(&query, f) } Route::GetGuildScheduledEvents { guild_id, @@ -2509,13 +2436,15 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/scheduled-events?")?; + f.write_str("/scheduled-events")?; + + let mut query = QueryString::new(); if *with_user_count { - f.write_str("with_user_count=true")?; + query.push("with_user_count", with_user_count.to_string()); } - Ok(()) + Display::fmt(&query, f) } Route::GetGuildSticker { guild_id, @@ -2581,34 +2510,26 @@ impl Display for Route<'_> { before, limit, } => { - f.write_str("users/@me/guilds?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } + f.write_str("users/@me/guilds")?; - if let Some(before) = before { - f.write_str("&before=")?; - Display::fmt(before, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } - - Ok(()) + Display::fmt(&query, f) } Route::GetInvite { code, with_counts } => { f.write_str("invites/")?; f.write_str(code)?; + let mut query = QueryString::new(); + if *with_counts { - f.write_str("?with_counts=true")?; + query.push("with_counts", with_counts.to_string()); } - Ok(()) + Display::fmt(&query, f) } Route::GetInviteWithExpiration { code, @@ -2617,17 +2538,18 @@ impl Display for Route<'_> { } => { f.write_str("invites/")?; f.write_str(code)?; - f.write_str("?")?; + + let mut query = QueryString::new(); if *with_counts { - f.write_str("with_counts=true")?; + query.push("with_counts", with_counts.to_string()); } if *with_expiration { - f.write_str("&with_expiration=true")?; + query.push("with_expiration", with_expiration.to_string()); } - Ok(()) + Display::fmt(&query, f) } Route::GetMessages { channel_id, @@ -2638,29 +2560,15 @@ impl Display for Route<'_> { } => { f.write_str("channels/")?; Display::fmt(channel_id, f)?; - f.write_str("/messages?")?; + f.write_str("/messages")?; - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("around", around.map(|x| x.to_string())) + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - if let Some(around) = around { - f.write_str("&around=")?; - Display::fmt(around, f)?; - } - - if let Some(before) = before { - f.write_str("&before=")?; - Display::fmt(before, f)?; - } - - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } - - Ok(()) + Display::fmt(&query, f) } Route::GetNitroStickerPacks { .. } => f.write_str("sticker-packs"), Route::GetPins { channel_id } => { @@ -2676,19 +2584,13 @@ impl Display for Route<'_> { } => { f.write_str("channels/")?; Display::fmt(channel_id, f)?; - f.write_str("/users/@me/threads/archived/private?")?; - - if let Some(before) = before { - f.write_str("before=")?; - Display::fmt(before, f)?; - } + f.write_str("/users/@me/threads/archived/private")?; - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_opt_value("before", before.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetPrivateArchivedThreads { before, @@ -2697,19 +2599,13 @@ impl Display for Route<'_> { } => { f.write_str("channels/")?; Display::fmt(channel_id, f)?; - f.write_str("/threads/archived/private?")?; + f.write_str("/threads/archived/private")?; - if let Some(before) = before { - f.write_str("before=")?; - Display::fmt(before, f)?; - } + let query = QueryString::new() + .with_opt_value("before", *before) + .with_opt_value("limit", limit.map(|x| x.to_string())); - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } - - Ok(()) + Display::fmt(&query, f) } Route::GetPublicArchivedThreads { before, @@ -2718,19 +2614,13 @@ impl Display for Route<'_> { } => { f.write_str("channels/")?; Display::fmt(channel_id, f)?; - f.write_str("/threads/archived/public?")?; + f.write_str("/threads/archived/public")?; - if let Some(before) = before { - f.write_str("before=")?; - Display::fmt(before, f)?; - } - - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_opt_value("before", *before) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetReactionUsers { after, @@ -2745,19 +2635,12 @@ impl Display for Route<'_> { Display::fmt(message_id, f)?; f.write_str("/reactions/")?; Display::fmt(&emoji, f)?; - f.write_str("?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetSticker { sticker_id } => { f.write_str("stickers/")?; @@ -2773,24 +2656,13 @@ impl Display for Route<'_> { f.write_str("channels/")?; Display::fmt(channel_id, f)?; f.write_str("/thread-members")?; - f.write_str("?")?; - - if let Some(after) = after { - f.write_str("after=")?; - Display::fmt(after, f)?; - } - - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } - if let Some(with_member) = with_member { - f.write_str("&with_member=")?; - Display::fmt(with_member, f)?; - } + let query = QueryString::new() + .with_opt_value("after", after.map(|x| x.to_string())) + .with_opt_value("limit", limit.map(|x| x.to_string())) + .with_opt_value("with_member", with_member.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::GetUserConnections => f.write_str("users/@me/connections"), Route::GetUser { user_id } => { @@ -2842,15 +2714,18 @@ impl Display for Route<'_> { } => { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; - f.write_str("/members/search?query=")?; - Display::fmt(&utf8_percent_encode(query, NON_ALPHANUMERIC), f)?; + f.write_str("/members/search")?; - if let Some(limit) = limit { - f.write_str("&limit=")?; - Display::fmt(limit, f)?; - } + let query = QueryString::new() + .with_value( + "query", + // The builder is supposed to encode strings into UTF-8 format, however for some reason, + // it doesn't handle slashes properly. Unclear if this is a bug in the library. + utf8_percent_encode(query, NON_ALPHANUMERIC).to_string(), + ) + .with_opt_value("limit", limit.map(|x| x.to_string())); - Ok(()) + Display::fmt(&query, f) } Route::SyncGuildIntegration { guild_id, @@ -3918,7 +3793,7 @@ mod tests { guild_id: GUILD_ID, limit: None, }; - assert_eq!(route.to_string(), format!("guilds/{GUILD_ID}/bans?")); + assert_eq!(route.to_string(), format!("guilds/{GUILD_ID}/bans")); let route = Route::GetBansWithParameters { after: Some(USER_ID), @@ -3939,7 +3814,7 @@ mod tests { }; assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/bans?&before={USER_ID}") + format!("guilds/{GUILD_ID}/bans?before={USER_ID}") ); let route = Route::GetBansWithParameters { @@ -3950,7 +3825,7 @@ mod tests { }; assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/bans?&limit={limit}", limit = 100) + format!("guilds/{GUILD_ID}/bans?limit={limit}", limit = 100) ); let route = Route::GetBansWithParameters { @@ -4197,7 +4072,7 @@ mod tests { }; assert_eq!( route.to_string(), - format!("channels/{CHANNEL_ID}/thread-members?") + format!("channels/{CHANNEL_ID}/thread-members") ); let route = Route::GetThreadMembers { @@ -4374,7 +4249,7 @@ mod tests { guild_id: GUILD_ID, include_roles: &[], }; - assert_eq!(route.to_string(), format!("guilds/{GUILD_ID}/prune?")); + assert_eq!(route.to_string(), format!("guilds/{GUILD_ID}/prune")); } #[test] @@ -4413,10 +4288,7 @@ mod tests { guild_id: GUILD_ID, include_roles: &[], }; - assert_eq!( - route.to_string(), - format!("guilds/{GUILD_ID}/prune?&days=4") - ); + assert_eq!(route.to_string(), format!("guilds/{GUILD_ID}/prune?days=4")); } #[test] @@ -4431,7 +4303,7 @@ mod tests { }; assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/prune?&include_roles=1") + format!("guilds/{GUILD_ID}/prune?include_roles=1") ); } @@ -4447,7 +4319,7 @@ mod tests { }; assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/prune?&include_roles=1,2") + format!("guilds/{GUILD_ID}/prune?include_roles=1,2") ); } @@ -4476,7 +4348,7 @@ mod tests { assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/scheduled-events?") + format!("guilds/{GUILD_ID}/scheduled-events") ); let route = Route::GetGuildScheduledEvents { @@ -4565,7 +4437,7 @@ mod tests { assert_eq!( route.to_string(), format!( - "guilds/{GUILD_ID}/scheduled-events/{SCHEDULED_EVENT_ID}/users?&before={USER_ID}&with_member=true" + "guilds/{GUILD_ID}/scheduled-events/{SCHEDULED_EVENT_ID}/users?before={USER_ID}&with_member=true" ) ); From 78619a873cd64c3ccbc9c7079ac5537ac0879ae2 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Wed, 22 May 2024 22:07:19 -0400 Subject: [PATCH 02/11] add missing fmt call --- twilight-http/src/routing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/twilight-http/src/routing.rs b/twilight-http/src/routing.rs index 012fcb8cb20..aebc214d297 100644 --- a/twilight-http/src/routing.rs +++ b/twilight-http/src/routing.rs @@ -2385,7 +2385,7 @@ impl Display for Route<'_> { query.push("include_roles", QueryArray(*include_roles)); } - Ok(()) + Display::fmt(&query, f) } Route::GetGuildScheduledEvent { guild_id, From 26ff7b96ab2783cb4e159ea0afd1cc0edf51f378 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Sun, 26 May 2024 16:55:53 -0500 Subject: [PATCH 03/11] remove builder and use custom struct --- twilight-http/Cargo.toml | 1 - twilight-http/src/lib.rs | 1 + twilight-http/src/query_array.rs | 14 +- twilight-http/src/query_str_writer.rs | 67 ++++++++ twilight-http/src/routing.rs | 224 +++++++++++++------------- 5 files changed, 184 insertions(+), 123 deletions(-) create mode 100644 twilight-http/src/query_str_writer.rs diff --git a/twilight-http/Cargo.toml b/twilight-http/Cargo.toml index 11f46f48cd5..d2f6240d9ed 100644 --- a/twilight-http/Cargo.toml +++ b/twilight-http/Cargo.toml @@ -34,7 +34,6 @@ twilight-validate = { default-features = false, path = "../twilight-validate", v # Optional dependencies. brotli-decompressor = { default-features = false, features = ["std"], optional = true, version = "4" } simd-json = { default-features = false, features = ["serde_impl", "swar-number-parsing"], optional = true, version = ">=0.4, <0.14" } -query-string-builder = "0.4.1" [features] default = ["decompression", "rustls-native-roots"] diff --git a/twilight-http/src/lib.rs b/twilight-http/src/lib.rs index e7b823395b7..dccc529e357 100644 --- a/twilight-http/src/lib.rs +++ b/twilight-http/src/lib.rs @@ -10,6 +10,7 @@ pub mod api_error; pub mod client; pub mod error; pub mod query_array; +pub mod query_str_writer; pub mod request; pub mod response; pub mod routing; diff --git a/twilight-http/src/query_array.rs b/twilight-http/src/query_array.rs index 4b1adfd16ed..b80d73a2a1c 100644 --- a/twilight-http/src/query_array.rs +++ b/twilight-http/src/query_array.rs @@ -6,11 +6,11 @@ pub struct QueryArray(pub T); impl Display for QueryArray where - T: IntoIterator + Copy, + T: IntoIterator + Clone, U: Display, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut iter = self.0.into_iter().peekable(); + let mut iter = self.0.clone().into_iter().peekable(); while let Some(item) = iter.next() { item.fmt(f)?; @@ -23,16 +23,6 @@ where } } -impl From> for String -where - T: IntoIterator + Copy, - U: Display, -{ - fn from(val: QueryArray) -> Self { - val.to_string() - } -} - #[cfg(test)] mod tests { use super::QueryArray; diff --git a/twilight-http/src/query_str_writer.rs b/twilight-http/src/query_str_writer.rs new file mode 100644 index 00000000000..3f02dc1324e --- /dev/null +++ b/twilight-http/src/query_str_writer.rs @@ -0,0 +1,67 @@ +use std::fmt::{Display, Formatter, Write}; + +/// A helper struct to write query paramseters to a formatter. +pub struct QueryStringFormatter<'w1, 'w2> { + formatter: &'w1 mut Formatter<'w2>, + is_first: bool, +} + +impl<'w1, 'w2> QueryStringFormatter<'w1, 'w2> { + pub fn new(formatter: &'w1 mut Formatter<'w2>) -> Self { + Self { + formatter, + is_first: true, + } + } + + pub fn write_param(&mut self, key: &str, value: &impl Display) -> std::fmt::Result { + if self.is_first { + self.formatter.write_char('?')?; + self.is_first = false; + } else { + self.formatter.write_char('&')?; + } + + self.formatter.write_str(key)?; + self.formatter.write_char('=')?; + Display::fmt(value, self.formatter) + } + + pub fn write_opt_param(&mut self, key: &str, value: Option<&impl Display>) -> std::fmt::Result { + if let Some(value) = value { + self.write_param(key, value) + } else { + Ok(()) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + struct Test { + a: u32, + b: String, + } + + impl Display for Test { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut writer = QueryStringFormatter::new(f); + writer.write_param("a", &self.a)?; + writer.write_param("b", &self.b) + } + } + + #[test] + fn test_query_string_writer() { + let test = Test { + a: 1, + b: "hello".to_string(), + }; + + let mut output = String::new(); + write!(output, "{}", test).unwrap(); + assert_eq!(output, "?a=1&b=hello"); + } +} diff --git a/twilight-http/src/routing.rs b/twilight-http/src/routing.rs index aebc214d297..da8e083b087 100644 --- a/twilight-http/src/routing.rs +++ b/twilight-http/src/routing.rs @@ -1,9 +1,9 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; -use query_string_builder::QueryString; pub use twilight_http_ratelimiting::request::{Path, PathParseError, PathParseErrorType}; use crate::{ query_array::QueryArray, + query_str_writer::QueryStringFormatter, request::{channel::reaction::RequestReactionType, Method}, }; use std::fmt::{Display, Formatter, Result as FmtResult}; @@ -1760,12 +1760,9 @@ impl Display for Route<'_> { Display::fmt(application_id, f)?; f.write_str("/commands")?; - let query = QueryString::new().with_opt_value( - "with_localizations", - with_localizations.map(|x| x.to_string()), - ); + let mut writer = QueryStringFormatter::new(f); - Display::fmt(&query, f) + writer.write_opt_param("with_localizations", with_localizations.as_ref()) } Route::CreateGuild => f.write_str("guilds"), Route::CreateGuildCommand { @@ -1794,12 +1791,8 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/commands")?; - let query = QueryString::new().with_opt_value( - "with_localizations", - with_localizations.map(|x| x.to_string()), - ); - - Display::fmt(&query, f) + let mut writer = QueryStringFormatter::new(f); + writer.write_opt_param("with_localizations", with_localizations.as_ref()) } Route::CreateGuildFromTemplate { template_code } | Route::GetTemplate { template_code } => { @@ -1824,18 +1817,16 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/prune")?; - let mut query = QueryString::new() - .with_opt_value( - "compute_prune_count", - compute_prune_count.map(|x| x.to_string()), - ) - .with_opt_value("days", days.map(|x| x.to_string())); + let mut writer = QueryStringFormatter::new(f); + + writer.write_opt_param("compute_prune_count", compute_prune_count.as_ref())?; + writer.write_opt_param("days", days.as_ref())?; if !include_roles.is_empty() { - query.push("include_roles", QueryArray(*include_roles)); + writer.write_param("include_roles", &QueryArray(*include_roles))?; } - Display::fmt(&query, f) + Ok(()) } Route::CreateGuildScheduledEvent { guild_id } => { f.write_str("guilds/")?; @@ -2189,10 +2180,13 @@ impl Display for Route<'_> { f.write_str("/messages/")?; Display::fmt(message_id, f)?; - let query = QueryString::new() - .with_opt_value("thread_id", thread_id.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); + + if let Some(thread_id) = thread_id { + query_formatter.write_param("thread_id", thread_id)?; + } - Display::fmt(&query, f) + Ok(()) } Route::DeleteWebhook { token, webhook_id } | Route::GetWebhook { token, webhook_id } @@ -2218,11 +2212,17 @@ impl Display for Route<'_> { f.write_str("/")?; f.write_str(token)?; - let query = QueryString::new() - .with_opt_value("thread_id", thread_id.map(|x| x.to_string())) - .with_opt_value("wait", wait.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); + + if let Some(thread_id) = thread_id { + query_formatter.write_param("thread_id", thread_id)?; + } + + if let Some(wait) = wait { + query_formatter.write_param("wait", wait)?; + } - Display::fmt(&query, f) + Ok(()) } Route::FollowNewsChannel { channel_id } => { f.write_str("channels/")?; @@ -2248,14 +2248,29 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/audit-logs")?; - let query = QueryString::new() - .with_opt_value("action_type", action_type.map(|x| x.to_string())) - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())) - .with_opt_value("user_id", user_id.map(|x| x.to_string())); + let mut writer = QueryStringFormatter::new(f); + + if let Some(action_type) = action_type { + writer.write_param("action_type", action_type)?; + } + + if let Some(after) = after { + writer.write_param("after", after)?; + } - Display::fmt(&query, f) + if let Some(before) = before { + writer.write_param("before", before)?; + } + + if let Some(limit) = limit { + writer.write_param("limit", limit)?; + } + + if let Some(user_id) = user_id { + writer.write_param("user_id", user_id)?; + } + + Ok(()) } Route::GetBans { guild_id } => { f.write_str("guilds/")?; @@ -2273,12 +2288,11 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/bans")?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetGatewayBot => f.write_str("gateway/bot"), Route::GetCommandPermissions { @@ -2319,8 +2333,10 @@ impl Display for Route<'_> { f.write_str("guilds/")?; Display::fmt(guild_id, f)?; + let mut query_formatter = QueryStringFormatter::new(f); + if *with_counts { - f.write_str("?with_counts=true")?; + query_formatter.write_param("with_counts", with_counts)?; } Ok(()) @@ -2351,11 +2367,10 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/members")?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetGuildOnboarding { guild_id } | Route::UpdateGuildOnboarding { guild_id } => { f.write_str("guilds/")?; @@ -2378,14 +2393,15 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/prune")?; - let mut query = - QueryString::new().with_opt_value("days", days.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); + + query_formatter.write_opt_param("days", days.as_ref())?; if !include_roles.is_empty() { - query.push("include_roles", QueryArray(*include_roles)); + query_formatter.write_param("include_roles", &QueryArray(*include_roles))?; } - Display::fmt(&query, f) + Ok(()) } Route::GetGuildScheduledEvent { guild_id, @@ -2397,13 +2413,13 @@ impl Display for Route<'_> { f.write_str("/scheduled-events/")?; Display::fmt(scheduled_event_id, f)?; - let mut query = QueryString::new(); + let mut query_formatter = QueryStringFormatter::new(f); if *with_user_count { - query.push("with_user_count", with_user_count.to_string()); + query_formatter.write_param("with_user_count", with_user_count)?; } - Display::fmt(&query, f) + Ok(()) } Route::GetGuildScheduledEventUsers { after, @@ -2419,16 +2435,17 @@ impl Display for Route<'_> { Display::fmt(scheduled_event_id, f)?; f.write_str("/users")?; - let mut query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); + + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref())?; if *with_member { - query.push("with_member", with_member.to_string()); + query_formatter.write_param("with_member", with_member)?; } - Display::fmt(&query, f) + Ok(()) } Route::GetGuildScheduledEvents { guild_id, @@ -2438,13 +2455,13 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/scheduled-events")?; - let mut query = QueryString::new(); + let mut query_formatter = QueryStringFormatter::new(f); if *with_user_count { - query.push("with_user_count", with_user_count.to_string()); + query_formatter.write_param("with_user_count", with_user_count)?; } - Display::fmt(&query, f) + Ok(()) } Route::GetGuildSticker { guild_id, @@ -2512,24 +2529,23 @@ impl Display for Route<'_> { } => { f.write_str("users/@me/guilds")?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetInvite { code, with_counts } => { f.write_str("invites/")?; f.write_str(code)?; - let mut query = QueryString::new(); + let mut query_formatter = QueryStringFormatter::new(f); if *with_counts { - query.push("with_counts", with_counts.to_string()); + query_formatter.write_param("with_counts", with_counts)?; } - Display::fmt(&query, f) + Ok(()) } Route::GetInviteWithExpiration { code, @@ -2539,17 +2555,17 @@ impl Display for Route<'_> { f.write_str("invites/")?; f.write_str(code)?; - let mut query = QueryString::new(); + let mut query_formatter = QueryStringFormatter::new(f); if *with_counts { - query.push("with_counts", with_counts.to_string()); + query_formatter.write_param("with_counts", with_counts)?; } if *with_expiration { - query.push("with_expiration", with_expiration.to_string()); + query_formatter.write_param("with_expiration", with_expiration)?; } - Display::fmt(&query, f) + Ok(()) } Route::GetMessages { channel_id, @@ -2562,13 +2578,12 @@ impl Display for Route<'_> { Display::fmt(channel_id, f)?; f.write_str("/messages")?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("around", around.map(|x| x.to_string())) - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("around", around.as_ref())?; + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetNitroStickerPacks { .. } => f.write_str("sticker-packs"), Route::GetPins { channel_id } => { @@ -2586,11 +2601,10 @@ impl Display for Route<'_> { Display::fmt(channel_id, f)?; f.write_str("/users/@me/threads/archived/private")?; - let query = QueryString::new() - .with_opt_value("before", before.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetPrivateArchivedThreads { before, @@ -2601,11 +2615,10 @@ impl Display for Route<'_> { Display::fmt(channel_id, f)?; f.write_str("/threads/archived/private")?; - let query = QueryString::new() - .with_opt_value("before", *before) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetPublicArchivedThreads { before, @@ -2616,11 +2629,10 @@ impl Display for Route<'_> { Display::fmt(channel_id, f)?; f.write_str("/threads/archived/public")?; - let query = QueryString::new() - .with_opt_value("before", *before) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetReactionUsers { after, @@ -2636,11 +2648,10 @@ impl Display for Route<'_> { f.write_str("/reactions/")?; Display::fmt(&emoji, f)?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::GetSticker { sticker_id } => { f.write_str("stickers/")?; @@ -2657,12 +2668,11 @@ impl Display for Route<'_> { Display::fmt(channel_id, f)?; f.write_str("/thread-members")?; - let query = QueryString::new() - .with_opt_value("after", after.map(|x| x.to_string())) - .with_opt_value("limit", limit.map(|x| x.to_string())) - .with_opt_value("with_member", with_member.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref())?; + query_formatter.write_opt_param("with_member", with_member.as_ref()) } Route::GetUserConnections => f.write_str("users/@me/connections"), Route::GetUser { user_id } => { @@ -2716,16 +2726,10 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/members/search")?; - let query = QueryString::new() - .with_value( - "query", - // The builder is supposed to encode strings into UTF-8 format, however for some reason, - // it doesn't handle slashes properly. Unclear if this is a bug in the library. - utf8_percent_encode(query, NON_ALPHANUMERIC).to_string(), - ) - .with_opt_value("limit", limit.map(|x| x.to_string())); + let mut query_formatter = QueryStringFormatter::new(f); - Display::fmt(&query, f) + query_formatter.write_opt_param("limit", limit.as_ref())?; + query_formatter.write_param("query", &utf8_percent_encode(query, NON_ALPHANUMERIC)) } Route::SyncGuildIntegration { guild_id, @@ -4484,7 +4488,7 @@ mod tests { assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/members/search?query=foo%20bar&limit=99") + format!("guilds/5/members/search?limit=99&query=foo%20bar") ); let route = Route::SearchGuildMembers { @@ -4495,7 +4499,7 @@ mod tests { assert_eq!( route.to_string(), - format!("guilds/{GUILD_ID}/members/search?query=foo%2Fbar&limit=99") + format!("guilds/5/members/search?limit=99&query=foo%2Fbar") ); } From d3ba2f070b97eaa460d363b29b5b6667fd4c6488 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Sun, 26 May 2024 16:58:09 -0500 Subject: [PATCH 04/11] use display::fmt --- twilight-http/src/query_array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/twilight-http/src/query_array.rs b/twilight-http/src/query_array.rs index b80d73a2a1c..ca818f6e315 100644 --- a/twilight-http/src/query_array.rs +++ b/twilight-http/src/query_array.rs @@ -13,7 +13,7 @@ where let mut iter = self.0.clone().into_iter().peekable(); while let Some(item) = iter.next() { - item.fmt(f)?; + Display::fmt(&item, f)?; if iter.peek().is_some() { f.write_str(",")?; } From 2114da4b649874811b438be9c9cfb451453442fb Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:30:08 -0400 Subject: [PATCH 05/11] fix clippy lints --- CODE_OF_CONDUCT.md | 2 +- twilight-http/src/lib.rs | 2 +- ...y_str_writer.rs => query_str_formatter.rs} | 43 +++++++++++++++++-- twilight-http/src/routing.rs | 2 +- 4 files changed, 43 insertions(+), 6 deletions(-) rename twilight-http/src/{query_str_writer.rs => query_str_formatter.rs} (60%) diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 2f696d4a58f..1e9defb48ff 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -3,7 +3,7 @@ We as members, contributors, and leaders pledge to make participation in our community a harassment-free experience for everyone, regardless of age, body size, visible or invisible disability, ethnicity, sex characteristics, gender -identity and expression, level of experience, education, socio-economic status, +identity and expression, level of experience, education, socioeconomic status, nationality, personal appearance, race, caste, color, religion, or sexual identity and orientation. diff --git a/twilight-http/src/lib.rs b/twilight-http/src/lib.rs index dccc529e357..26baa71e00c 100644 --- a/twilight-http/src/lib.rs +++ b/twilight-http/src/lib.rs @@ -10,7 +10,7 @@ pub mod api_error; pub mod client; pub mod error; pub mod query_array; -pub mod query_str_writer; +pub mod query_str_formatter; pub mod request; pub mod response; pub mod routing; diff --git a/twilight-http/src/query_str_writer.rs b/twilight-http/src/query_str_formatter.rs similarity index 60% rename from twilight-http/src/query_str_writer.rs rename to twilight-http/src/query_str_formatter.rs index 3f02dc1324e..4cc0e39969b 100644 --- a/twilight-http/src/query_str_writer.rs +++ b/twilight-http/src/query_str_formatter.rs @@ -1,6 +1,35 @@ use std::fmt::{Display, Formatter, Write}; /// A helper struct to write query paramseters to a formatter. +/// +/// # Example +/// +/// ``` +/// use twilight_http::query_str_formatter::QueryStringFormatter; +/// +/// struct Params { +/// foo: String, +/// bar: u64, +/// } +/// +/// impl Display for Params { +/// fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +/// let mut formatter = QueryStringFormatter::new(f); +/// formatter.write_param("foo", &self.foo)?; +/// formatter.write_param("bar", &self.bar)?; +/// Ok(()) +/// } +/// +/// fn main() { +/// let params = Params { +/// foo: "hello".to_string(), +/// bar: 123, +/// }; +/// +/// assert_eq!(params.to_string(), "foo=hello&bar=123"); +/// } +/// +/// pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, is_first: bool, @@ -14,6 +43,11 @@ impl<'w1, 'w2> QueryStringFormatter<'w1, 'w2> { } } + /// Writes a query parameter to the formatter. + /// + /// # Errors + /// + /// This returns a [`std::fmt::Error`] if the formatter returns an error. pub fn write_param(&mut self, key: &str, value: &impl Display) -> std::fmt::Result { if self.is_first { self.formatter.write_char('?')?; @@ -27,6 +61,11 @@ impl<'w1, 'w2> QueryStringFormatter<'w1, 'w2> { Display::fmt(value, self.formatter) } + /// Writes a query parameter to the formatter. + /// + /// # Errors + /// + /// This returns a [`std::fmt::Error`] if the formatter returns an error. pub fn write_opt_param(&mut self, key: &str, value: Option<&impl Display>) -> std::fmt::Result { if let Some(value) = value { self.write_param(key, value) @@ -60,8 +99,6 @@ mod tests { b: "hello".to_string(), }; - let mut output = String::new(); - write!(output, "{}", test).unwrap(); - assert_eq!(output, "?a=1&b=hello"); + assert_eq!(test.to_string(), "?a=1&b=hello"); } } diff --git a/twilight-http/src/routing.rs b/twilight-http/src/routing.rs index da8e083b087..f4aecb6f404 100644 --- a/twilight-http/src/routing.rs +++ b/twilight-http/src/routing.rs @@ -3,7 +3,7 @@ pub use twilight_http_ratelimiting::request::{Path, PathParseError, PathParseErr use crate::{ query_array::QueryArray, - query_str_writer::QueryStringFormatter, + query_str_formatter::QueryStringFormatter, request::{channel::reaction::RequestReactionType, Method}, }; use std::fmt::{Display, Formatter, Result as FmtResult}; From a5c765655f55ec9582f3690b51412ef3ee20cf27 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:31:05 -0400 Subject: [PATCH 06/11] fix typo --- twilight-http/src/query_array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/twilight-http/src/query_array.rs b/twilight-http/src/query_array.rs index ca818f6e315..302645484d7 100644 --- a/twilight-http/src/query_array.rs +++ b/twilight-http/src/query_array.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -/// Provides a display implementation for serializing arrays-like objects into +/// Provides a display implementation for serializing iterable objects into /// query params. pub struct QueryArray(pub T); From 91154e0000c3d445735ffaf932cb29f5c0821e28 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:33:53 -0400 Subject: [PATCH 07/11] fix doc comment --- twilight-http/src/query_str_formatter.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/twilight-http/src/query_str_formatter.rs b/twilight-http/src/query_str_formatter.rs index 4cc0e39969b..a80d723e079 100644 --- a/twilight-http/src/query_str_formatter.rs +++ b/twilight-http/src/query_str_formatter.rs @@ -28,8 +28,7 @@ use std::fmt::{Display, Formatter, Write}; /// /// assert_eq!(params.to_string(), "foo=hello&bar=123"); /// } -/// -/// +/// ``` pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, is_first: bool, From 137b50ba728091a124c35e5168064c9b86b91d36 Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:40:15 -0400 Subject: [PATCH 08/11] fix doctest --- twilight-http/src/query_str_formatter.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/twilight-http/src/query_str_formatter.rs b/twilight-http/src/query_str_formatter.rs index a80d723e079..96bd7d7b736 100644 --- a/twilight-http/src/query_str_formatter.rs +++ b/twilight-http/src/query_str_formatter.rs @@ -14,20 +14,20 @@ use std::fmt::{Display, Formatter, Write}; /// /// impl Display for Params { /// fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { -/// let mut formatter = QueryStringFormatter::new(f); -/// formatter.write_param("foo", &self.foo)?; -/// formatter.write_param("bar", &self.bar)?; -/// Ok(()) +/// let mut formatter = QueryStringFormatter::new(f); +/// formatter.write_param("foo", &self.foo)?; +/// formatter.write_param("bar", &self.bar)?; +/// Ok(()) +/// } /// } /// -/// fn main() { -/// let params = Params { -/// foo: "hello".to_string(), -/// bar: 123, -/// }; +/// let params = Params { +/// foo: "hello".to_string(), +/// bar: 123, +/// }; +/// +/// assert_eq!(params.to_string(), "foo=hello&bar=123"); /// -/// assert_eq!(params.to_string(), "foo=hello&bar=123"); -/// } /// ``` pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, From b79c6fb999cdc731d7a7d832335f089a451fd10a Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:43:56 -0400 Subject: [PATCH 09/11] fix doc formatting --- twilight-http/src/query_str_formatter.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/twilight-http/src/query_str_formatter.rs b/twilight-http/src/query_str_formatter.rs index 96bd7d7b736..87f89f60508 100644 --- a/twilight-http/src/query_str_formatter.rs +++ b/twilight-http/src/query_str_formatter.rs @@ -8,26 +8,25 @@ use std::fmt::{Display, Formatter, Write}; /// use twilight_http::query_str_formatter::QueryStringFormatter; /// /// struct Params { -/// foo: String, -/// bar: u64, +/// foo: String, +/// bar: u64, /// } /// /// impl Display for Params { -/// fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { -/// let mut formatter = QueryStringFormatter::new(f); -/// formatter.write_param("foo", &self.foo)?; -/// formatter.write_param("bar", &self.bar)?; -/// Ok(()) -/// } +/// fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +/// let mut formatter = QueryStringFormatter::new(f); +/// formatter.write_param("foo", &self.foo)?; +/// formatter.write_param("bar", &self.bar)?; +/// Ok(()) +/// } /// } /// /// let params = Params { -/// foo: "hello".to_string(), -/// bar: 123, +/// foo: "hello".to_string(), +/// bar: 123, /// }; /// /// assert_eq!(params.to_string(), "foo=hello&bar=123"); -/// /// ``` pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, From 955c8f717c3498eaebe1b86fd2d9b14a4023f48b Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 00:51:31 -0400 Subject: [PATCH 10/11] fix doc test part 2 --- twilight-http/src/query_str_formatter.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/twilight-http/src/query_str_formatter.rs b/twilight-http/src/query_str_formatter.rs index 87f89f60508..2e1ef8f47f4 100644 --- a/twilight-http/src/query_str_formatter.rs +++ b/twilight-http/src/query_str_formatter.rs @@ -5,6 +5,7 @@ use std::fmt::{Display, Formatter, Write}; /// # Example /// /// ``` +/// use std::fmt::{Display, Formatter, Write}; /// use twilight_http::query_str_formatter::QueryStringFormatter; /// /// struct Params { @@ -26,7 +27,7 @@ use std::fmt::{Display, Formatter, Write}; /// bar: 123, /// }; /// -/// assert_eq!(params.to_string(), "foo=hello&bar=123"); +/// assert_eq!(params.to_string(), "?foo=hello&bar=123"); /// ``` pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, From 2b8c341543983ea20515a2e0f5c9da5c365ab91d Mon Sep 17 00:00:00 2001 From: suneettipirneni Date: Mon, 27 May 2024 16:18:13 -0400 Subject: [PATCH 11/11] add requested changes --- twilight-http/src/lib.rs | 3 +- twilight-http/src/query_array.rs | 39 -------- ...ry_str_formatter.rs => query_formatter.rs} | 93 ++++++++++++------- twilight-http/src/routing.rs | 71 +++++--------- 4 files changed, 81 insertions(+), 125 deletions(-) delete mode 100644 twilight-http/src/query_array.rs rename twilight-http/src/{query_str_formatter.rs => query_formatter.rs} (53%) diff --git a/twilight-http/src/lib.rs b/twilight-http/src/lib.rs index 26baa71e00c..7fb0cf867d2 100644 --- a/twilight-http/src/lib.rs +++ b/twilight-http/src/lib.rs @@ -9,13 +9,12 @@ pub mod api_error; pub mod client; pub mod error; -pub mod query_array; -pub mod query_str_formatter; pub mod request; pub mod response; pub mod routing; mod json; +mod query_formatter; /// Discord API version used by this crate. pub const API_VERSION: u8 = 10; diff --git a/twilight-http/src/query_array.rs b/twilight-http/src/query_array.rs deleted file mode 100644 index 302645484d7..00000000000 --- a/twilight-http/src/query_array.rs +++ /dev/null @@ -1,39 +0,0 @@ -use std::fmt::Display; - -/// Provides a display implementation for serializing iterable objects into -/// query params. -pub struct QueryArray(pub T); - -impl Display for QueryArray -where - T: IntoIterator + Clone, - U: Display, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut iter = self.0.clone().into_iter().peekable(); - - while let Some(item) = iter.next() { - Display::fmt(&item, f)?; - if iter.peek().is_some() { - f.write_str(",")?; - } - } - - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::QueryArray; - - #[test] - fn test_query_array() { - let query_array = QueryArray([1, 2, 3]); - assert_eq!(query_array.to_string(), "1,2,3"); - - let params = vec!["a", "b", "c"]; - let query_array = QueryArray(¶ms); - assert_eq!(query_array.to_string(), "a,b,c"); - } -} diff --git a/twilight-http/src/query_str_formatter.rs b/twilight-http/src/query_formatter.rs similarity index 53% rename from twilight-http/src/query_str_formatter.rs rename to twilight-http/src/query_formatter.rs index 2e1ef8f47f4..4b42d9a51de 100644 --- a/twilight-http/src/query_str_formatter.rs +++ b/twilight-http/src/query_formatter.rs @@ -1,34 +1,6 @@ use std::fmt::{Display, Formatter, Write}; /// A helper struct to write query paramseters to a formatter. -/// -/// # Example -/// -/// ``` -/// use std::fmt::{Display, Formatter, Write}; -/// use twilight_http::query_str_formatter::QueryStringFormatter; -/// -/// struct Params { -/// foo: String, -/// bar: u64, -/// } -/// -/// impl Display for Params { -/// fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { -/// let mut formatter = QueryStringFormatter::new(f); -/// formatter.write_param("foo", &self.foo)?; -/// formatter.write_param("bar", &self.bar)?; -/// Ok(()) -/// } -/// } -/// -/// let params = Params { -/// foo: "hello".to_string(), -/// bar: 123, -/// }; -/// -/// assert_eq!(params.to_string(), "?foo=hello&bar=123"); -/// ``` pub struct QueryStringFormatter<'w1, 'w2> { formatter: &'w1 mut Formatter<'w2>, is_first: bool, @@ -74,30 +46,81 @@ impl<'w1, 'w2> QueryStringFormatter<'w1, 'w2> { } } +/// Provides a display implementation for serializing iterable objects into +/// query params. +#[derive(Debug)] +pub struct QueryArray(pub T); + +impl Display for QueryArray +where + T: IntoIterator + Clone, + U: Display, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut iter = self.0.clone().into_iter().peekable(); + + while let Some(item) = iter.next() { + Display::fmt(&item, f)?; + if iter.peek().is_some() { + f.write_str(",")?; + } + } + + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; struct Test { - a: u32, - b: String, + a: Option, + b: Option, } impl Display for Test { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let mut writer = QueryStringFormatter::new(f); - writer.write_param("a", &self.a)?; - writer.write_param("b", &self.b) + writer.write_opt_param("a", self.a.as_ref())?; + writer.write_opt_param("b", self.b.as_ref()) } } #[test] - fn test_query_string_writer() { + fn test_query_string_formatter_filled() { let test = Test { - a: 1, - b: "hello".to_string(), + a: Some(1), + b: Some("hello".to_string()), }; assert_eq!(test.to_string(), "?a=1&b=hello"); } + + #[test] + fn test_query_string_formatter_empty() { + let test = Test { a: None, b: None }; + + assert_eq!(test.to_string(), ""); + } + + #[test] + fn test_query_string_formatter_single() { + let test = Test { + a: Some(1), + b: None, + }; + + assert_eq!(test.to_string(), "?a=1"); + } + + #[test] + fn test_query_array() { + let query_array = QueryArray([1, 2, 3]); + assert_eq!(query_array.to_string(), "1,2,3"); + + let params = vec!["a", "b", "c"]; + let query_array = QueryArray(¶ms); + assert_eq!(query_array.to_string(), "a,b,c"); + } } diff --git a/twilight-http/src/routing.rs b/twilight-http/src/routing.rs index f4aecb6f404..83c47886419 100644 --- a/twilight-http/src/routing.rs +++ b/twilight-http/src/routing.rs @@ -2,8 +2,7 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; pub use twilight_http_ratelimiting::request::{Path, PathParseError, PathParseErrorType}; use crate::{ - query_array::QueryArray, - query_str_formatter::QueryStringFormatter, + query_formatter::{QueryArray, QueryStringFormatter}, request::{channel::reaction::RequestReactionType, Method}, }; use std::fmt::{Display, Formatter, Result as FmtResult}; @@ -2182,11 +2181,7 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); - if let Some(thread_id) = thread_id { - query_formatter.write_param("thread_id", thread_id)?; - } - - Ok(()) + query_formatter.write_opt_param("thread_id", thread_id.as_ref()) } Route::DeleteWebhook { token, webhook_id } | Route::GetWebhook { token, webhook_id } @@ -2214,15 +2209,8 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); - if let Some(thread_id) = thread_id { - query_formatter.write_param("thread_id", thread_id)?; - } - - if let Some(wait) = wait { - query_formatter.write_param("wait", wait)?; - } - - Ok(()) + query_formatter.write_opt_param("thread_id", thread_id.as_ref())?; + query_formatter.write_opt_param("wait", wait.as_ref()) } Route::FollowNewsChannel { channel_id } => { f.write_str("channels/")?; @@ -2248,29 +2236,13 @@ impl Display for Route<'_> { Display::fmt(guild_id, f)?; f.write_str("/audit-logs")?; - let mut writer = QueryStringFormatter::new(f); - - if let Some(action_type) = action_type { - writer.write_param("action_type", action_type)?; - } - - if let Some(after) = after { - writer.write_param("after", after)?; - } - - if let Some(before) = before { - writer.write_param("before", before)?; - } - - if let Some(limit) = limit { - writer.write_param("limit", limit)?; - } - - if let Some(user_id) = user_id { - writer.write_param("user_id", user_id)?; - } + let mut query_formatter = QueryStringFormatter::new(f); - Ok(()) + query_formatter.write_opt_param("action_type", action_type.as_ref())?; + query_formatter.write_opt_param("after", after.as_ref())?; + query_formatter.write_opt_param("before", before.as_ref())?; + query_formatter.write_opt_param("limit", limit.as_ref())?; + query_formatter.write_opt_param("user_id", user_id.as_ref()) } Route::GetBans { guild_id } => { f.write_str("guilds/")?; @@ -2336,7 +2308,7 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); if *with_counts { - query_formatter.write_param("with_counts", with_counts)?; + query_formatter.write_param("with_counts", &true)?; } Ok(()) @@ -2416,7 +2388,7 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); if *with_user_count { - query_formatter.write_param("with_user_count", with_user_count)?; + query_formatter.write_param("with_user_count", &true)?; } Ok(()) @@ -2442,7 +2414,7 @@ impl Display for Route<'_> { query_formatter.write_opt_param("limit", limit.as_ref())?; if *with_member { - query_formatter.write_param("with_member", with_member)?; + query_formatter.write_param("with_member", &true)?; } Ok(()) @@ -2458,7 +2430,7 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); if *with_user_count { - query_formatter.write_param("with_user_count", with_user_count)?; + query_formatter.write_param("with_user_count", &true)?; } Ok(()) @@ -2542,7 +2514,7 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); if *with_counts { - query_formatter.write_param("with_counts", with_counts)?; + query_formatter.write_param("with_counts", &true)?; } Ok(()) @@ -2558,11 +2530,11 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); if *with_counts { - query_formatter.write_param("with_counts", with_counts)?; + query_formatter.write_param("with_counts", &true)?; } if *with_expiration { - query_formatter.write_param("with_expiration", with_expiration)?; + query_formatter.write_param("with_expiration", &true)?; } Ok(()) @@ -2728,8 +2700,9 @@ impl Display for Route<'_> { let mut query_formatter = QueryStringFormatter::new(f); - query_formatter.write_opt_param("limit", limit.as_ref())?; - query_formatter.write_param("query", &utf8_percent_encode(query, NON_ALPHANUMERIC)) + query_formatter + .write_param("query", &utf8_percent_encode(query, NON_ALPHANUMERIC))?; + query_formatter.write_opt_param("limit", limit.as_ref()) } Route::SyncGuildIntegration { guild_id, @@ -4488,7 +4461,7 @@ mod tests { assert_eq!( route.to_string(), - format!("guilds/5/members/search?limit=99&query=foo%20bar") + format!("guilds/{GUILD_ID}/members/search?query=foo%20bar&limit=99") ); let route = Route::SearchGuildMembers { @@ -4499,7 +4472,7 @@ mod tests { assert_eq!( route.to_string(), - format!("guilds/5/members/search?limit=99&query=foo%2Fbar") + format!("guilds/{GUILD_ID}/members/search?query=foo%2Fbar&limit=99") ); }