Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppendUrlQueryPairs & QueryPairsExtension traits #266

Merged
merged 5 commits into from
Aug 18, 2024
Merged

AppendUrlQueryPairs & QueryPairsExtension traits #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd01532
Introduce url_query module to help with adding url query parameters
oguzkocer Aug 16, 2024
0dd1576
Implement AppendUrlQueryPairs for PostListParams
oguzkocer Aug 16, 2024
eed0627
Add helper macros that implement AsQueryValue for given type
oguzkocer Aug 16, 2024
6a36bca
Implement AsQueryValue for post param types
oguzkocer Aug 16, 2024
252d932
Implement unit tests for QueryPairsExtension
oguzkocer Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions wp_api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub use api_client::{WpApiClient, WpApiRequestBuilder};
pub use api_error::{RequestExecutionError, WpApiError, WpErrorCode};
pub use parsed_url::{ParseUrlError, ParsedUrl};
use plugins::*;
use url_query::AsQueryValue;
use users::*;
pub use uuid::{WpUuid, WpUuidParseError};

Expand All @@ -19,6 +20,7 @@ pub mod post_types;
pub mod posts;
pub mod request;
pub mod site_settings;
pub mod url_query;
pub mod users;
pub mod wp_site_health_tests;

Expand Down Expand Up @@ -73,6 +75,8 @@ pub enum WpApiParamOrder {
Desc,
}

impl_as_query_value_from_as_str!(WpApiParamOrder);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to implement a #[derive(AsQueryValue)] to do the same? This is not a request for changes though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's, but the implementation would be more complicated because if we are going to have a single derive such as AsQueryValue, then we'd need to have options for it, such as from=as_str, from=to_string etc. Handling that is more complicated, so it add maintenance burden. We could add different derives such as AsQueryValueFromAsStr, AsQueryValueFromToString, but that's not any better unfortunately.

We are trying to implement derive macros when there is significant benefit to them compared to other solutions. This is not such a case, because the simple macro_rules! does a good job and it is easier to understand.

Fwiw, for a personal project I'd probably go with the derive macro as I am used to working with them. However, I don't think it's the right choice for this project - at least not at this time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation!


impl WpApiParamOrder {
fn as_str(&self) -> &str {
match self {
Expand Down
36 changes: 18 additions & 18 deletions wp_api/src/plugins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ use std::fmt::Display;
use serde::{Deserialize, Serialize};
use wp_contextual::WpContextual;

use crate::{
impl_as_query_value_from_as_str,
url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension},
};

#[derive(Debug, Default, uniffi::Record)]
pub struct PluginListParams {
/// Limit results to those matching a string.
Expand All @@ -13,15 +18,11 @@ pub struct PluginListParams {
pub status: Option<PluginStatus>,
}

impl PluginListParams {
pub fn query_pairs(&self) -> impl IntoIterator<Item = (&str, String)> {
[
("search", self.search.clone()),
("status", self.status.map(|x| x.as_str().to_string())),
]
.into_iter()
// Remove `None` values
.filter_map(|(k, opt_v)| opt_v.map(|v| (k, v)))
impl AppendUrlQueryPairs for PluginListParams {
fn append_query_pairs(&self, query_pairs_mut: &mut QueryPairs) {
query_pairs_mut
.append_option_query_value_pair("search", self.search.as_ref())
.append_option_query_value_pair("status", self.status.as_ref());
}
}

Expand Down Expand Up @@ -127,6 +128,8 @@ pub enum PluginStatus {
NetworkActive,
}

impl_as_query_value_from_as_str!(PluginStatus);

impl PluginStatus {
fn as_str(&self) -> &str {
match self {
Expand All @@ -150,15 +153,12 @@ mod tests {
use rstest::*;

#[rstest]
#[case(PluginListParams::default(), &[])]
#[case(generate!(PluginListParams, (search, Some("foo".to_string()))), &[("search", "foo")])]
#[case(generate!(PluginListParams, (status, Some(PluginStatus::Active))), &[("status", "active")])]
#[case(generate!(PluginListParams, (search, Some("foo".to_string())), (status, Some(PluginStatus::Inactive))), &[("search", "foo"), ("status", "inactive")])]
#[case(PluginListParams::default(), "")]
#[case(generate!(PluginListParams, (search, Some("foo".to_string()))), "search=foo")]
#[case(generate!(PluginListParams, (status, Some(PluginStatus::Active))), "status=active")]
#[case(generate!(PluginListParams, (search, Some("foo".to_string())), (status, Some(PluginStatus::Inactive))), "search=foo&status=inactive")]
#[trace]
fn test_plugin_list_params(
#[case] params: PluginListParams,
#[case] expected_pairs: &[(&str, &str)],
) {
assert_expected_query_pairs(params.query_pairs(), expected_pairs);
fn test_plugin_list_params(#[case] params: PluginListParams, #[case] expected_query: &str) {
assert_expected_query_pairs(params, expected_query);
}
}
178 changes: 42 additions & 136 deletions wp_api/src/posts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use serde::{Deserialize, Serialize};
use wp_contextual::WpContextual;

use crate::{UserId, WpApiParamOrder};
use crate::{
impl_as_query_value_for_new_type, impl_as_query_value_from_as_str,
url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension},
UserId, WpApiParamOrder,
};

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)]
pub enum WpApiParamPostsOrderBy {
Expand All @@ -18,6 +22,8 @@ pub enum WpApiParamPostsOrderBy {
Title,
}

impl_as_query_value_from_as_str!(WpApiParamPostsOrderBy);

impl WpApiParamPostsOrderBy {
fn as_str(&self) -> &str {
match self {
Expand All @@ -41,6 +47,8 @@ pub enum WpApiParamPostsTaxRelation {
Or,
}

impl_as_query_value_from_as_str!(WpApiParamPostsTaxRelation);

impl WpApiParamPostsTaxRelation {
fn as_str(&self) -> &str {
match self {
Expand All @@ -57,6 +65,8 @@ pub enum WpApiParamPostsSearchColumn {
PostTitle,
}

impl_as_query_value_from_as_str!(WpApiParamPostsSearchColumn);

impl WpApiParamPostsSearchColumn {
fn as_str(&self) -> &str {
match self {
Expand Down Expand Up @@ -148,152 +158,46 @@ pub struct PostListParams {
pub sticky: Option<bool>,
}

impl PostListParams {
pub fn query_pairs(&self) -> impl IntoIterator<Item = (&str, String)> {
[
("page", self.page.map(|x| x.to_string())),
("per_page", self.per_page.map(|x| x.to_string())),
("search", self.search.clone()),
("after", self.after.clone()),
("modified_after", self.modified_after.clone()),
(
"author",
(!self.author.is_empty()).then_some(
self.author
.iter()
.map(|x| x.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"author_exclude",
(!self.author_exclude.is_empty()).then_some(
self.author_exclude
.iter()
.map(|x| x.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
("before", self.before.clone()),
("modified_before", self.modified_before.clone()),
(
"exclude",
(!self.exclude.is_empty()).then_some(
self.exclude
.iter()
.map(|x| x.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"include",
(!self.include.is_empty()).then_some(
self.include
.iter()
.map(|x| x.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
("offset", self.offset.map(|x| x.to_string())),
("order", self.order.as_ref().map(|x| x.as_str().to_string())),
(
"orderby",
self.orderby.as_ref().map(|x| x.as_str().to_string()),
),
(
"search_columns",
(!self.search_columns.is_empty()).then_some(
self.search_columns
.iter()
.map(|x| x.as_str().to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"slug",
(!self.slug.is_empty()).then_some(
self.slug
.iter()
.map(|x| x.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"status",
(!self.status.is_empty()).then_some(
self.status
.iter()
.map(|x| x.as_str().to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"tax_relation",
self.tax_relation.as_ref().map(|x| x.as_str().to_string()),
),
(
"categories",
(!self.categories.is_empty()).then_some(
self.categories
.iter()
.map(|x| x.0.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"categories_exclude",
(!self.categories_exclude.is_empty()).then_some(
self.categories_exclude
.iter()
.map(|x| x.0.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"tags",
(!self.tags.is_empty()).then_some(
self.tags
.iter()
.map(|x| x.0.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
(
"tags_exclude",
(!self.tags_exclude.is_empty()).then_some(
self.tags_exclude
.iter()
.map(|x| x.0.to_string())
.collect::<Vec<String>>()
.join(","),
),
),
("sticky", self.sticky.map(|x| x.to_string())),
]
.into_iter()
// Remove `None` values
.filter_map(|(k, opt_v)| opt_v.map(|v| (k, v)))
impl AppendUrlQueryPairs for PostListParams {
fn append_query_pairs(&self, query_pairs_mut: &mut QueryPairs) {
query_pairs_mut
.append_option_query_value_pair("page", self.page.as_ref())
.append_option_query_value_pair("per_page", self.per_page.as_ref())
.append_option_query_value_pair("search", self.search.as_ref())
.append_option_query_value_pair("after", self.after.as_ref())
.append_option_query_value_pair("modified_after", self.modified_after.as_ref())
.append_vec_query_value_pair("author", &self.author)
.append_vec_query_value_pair("author_exclude", &self.author_exclude)
.append_option_query_value_pair("before", self.before.as_ref())
.append_option_query_value_pair("modified_before", self.modified_before.as_ref())
.append_vec_query_value_pair("exclude", &self.exclude)
.append_vec_query_value_pair("include", &self.include)
.append_option_query_value_pair("offset", self.offset.as_ref())
.append_option_query_value_pair("order", self.order.as_ref())
.append_option_query_value_pair("orderby", self.orderby.as_ref())
.append_vec_query_value_pair("search_columns", &self.search_columns)
.append_vec_query_value_pair("slug", &self.slug)
.append_vec_query_value_pair("status", &self.status)
.append_option_query_value_pair("tax_relation", self.tax_relation.as_ref())
.append_vec_query_value_pair("categories", &self.categories)
.append_vec_query_value_pair("categories_exclude", &self.categories_exclude)
.append_vec_query_value_pair("tags", &self.tags)
.append_vec_query_value_pair("tags_exclude", &self.tags_exclude)
.append_option_query_value_pair("sticky", self.sticky.as_ref());
}
}

impl_as_query_value_for_new_type!(PostId);
uniffi::custom_newtype!(PostId, i32);
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct PostId(pub i32);

impl_as_query_value_for_new_type!(TagId);
uniffi::custom_newtype!(TagId, i32);
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct TagId(pub i32);

impl_as_query_value_for_new_type!(CategoryId);
uniffi::custom_newtype!(CategoryId, i32);
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct CategoryId(pub i32);
Expand Down Expand Up @@ -413,6 +317,8 @@ pub enum PostStatus {
Custom(String),
}

impl_as_query_value_from_as_str!(PostStatus);

impl PostStatus {
fn as_str(&self) -> &str {
match self {
Expand Down
4 changes: 2 additions & 2 deletions wp_api/src/request/endpoint/users_endpoint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ mod tests {
reassign: UserId(98),
},
),
"/users/54?reassign=98&force=true",
"/users/54?force=true&reassign=98",
);
}

Expand All @@ -76,7 +76,7 @@ mod tests {
endpoint.delete_me(&UserDeleteParams {
reassign: UserId(98),
}),
"/users/me?reassign=98&force=true",
"/users/me?force=true&reassign=98",
);
}

Expand Down
20 changes: 7 additions & 13 deletions wp_api/src/unit_test_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
use crate::url_query::AppendUrlQueryPairs;
use url::Url;

#[cfg(test)]
pub fn assert_expected_query_pairs<'a>(
query_pairs: impl IntoIterator<Item = (&'a str, String)>,
expected_pairs: &[(&'a str, &str)],
) {
let mut query_pairs = query_pairs.into_iter().collect::<Vec<_>>();
let mut expected_pairs: Vec<(&str, String)> = expected_pairs
.iter()
.map(|(k, v)| (*k, v.to_string()))
.collect();
// The order of query pairs doesn't matter
query_pairs.sort();
expected_pairs.sort();
assert_eq!(query_pairs, expected_pairs);
pub fn assert_expected_query_pairs(params: impl AppendUrlQueryPairs, expected_query: &str) {
let mut url = Url::parse("https://example.com").unwrap();
params.append_query_pairs(&mut url.query_pairs_mut());
assert_eq!(url.query(), Some(expected_query));
}
Loading
Loading