From 1f5108d139bff165f0ed9625529b2a63cf22184c Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 30 Oct 2024 17:22:17 -0400 Subject: [PATCH 01/23] Add next & prev page support to ParsedResponse --- wp_api/src/request.rs | 14 ++++--- wp_derive_request_builder/src/generate.rs | 50 +++++++++++++++++++++-- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 7e7534343..e5fe37939 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -21,10 +21,14 @@ const HEADER_KEY_WP_TOTAL_PAGES: &str = "X-WP-TotalPages"; #[derive(Debug, Default, Serialize, Deserialize)] #[serde(transparent)] -pub struct ParsedResponse { +pub struct ParsedResponse { pub data: T, #[serde(skip)] pub header_map: Arc, + #[serde(skip)] + pub next_page_params: Option

, + #[serde(skip)] + pub prev_page_params: Option

, } #[derive(Debug)] @@ -326,11 +330,11 @@ impl WpNetworkResponse { request_or_response_body_as_string(&self.body) } - pub fn parse(self) -> Result + pub fn parse(self) -> Result where T: DeserializeOwned, - T: From>, - ParsedResponse: From, + T: From>, + ParsedResponse: From, E: ParsedRequestError, { if let Some(err) = E::try_parse(&self.body, self.status_code) { @@ -340,7 +344,7 @@ impl WpNetworkResponse { serde_json::from_slice(&self.body) .map_err(|err| E::as_parse_error(err.to_string(), self.body_as_string())) .map(|x| { - let mut p = ParsedResponse::::from(x); + let mut p = ParsedResponse::::from(x); p.header_map = self.header_map; T::from(p) }) diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index c38990808..e6f909938 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -1,7 +1,7 @@ use std::fmt::Display; use helpers_to_generate_tokens::*; -use proc_macro2::{Span, TokenStream}; +use proc_macro2::{Span, TokenStream, TokenTree}; use proc_macro_crate::FoundCrate; use quote::{format_ident, quote}; use serde::{de::Error, Deserialize, Deserializer}; @@ -94,6 +94,45 @@ fn generate_async_request_executor( &variant.variant_ident, &context_and_filter_handler, ); + // TODO: Move to helpers and test it + let response_params_type = variant.attr.params.as_ref().map(|p| { + p.tokens + .clone() + .into_iter() + .filter(|t| { + if let TokenTree::Punct(punct) = t { + punct.as_char() != '&' + } else { + true + } + }) + .collect::() + }); + let response_pagination_params_fields = response_params_type.as_ref().map(|p| { + quote! { + #[serde(skip)] + pub next_page_params: Option<#p>, + #[serde(skip)] + pub prev_page_params: Option<#p>, + } + }); + let from_concrete_response_impl_for_pagination_params = response_params_type.as_ref().map(|_| { + quote! { + next_page_params: value.next_page_params, + prev_page_params: value.prev_page_params, + } + }).unwrap_or(quote! { + next_page_params: None, + prev_page_params: None, + }); + let from_parsed_response_impl_for_pagination_params = response_params_type.as_ref().map(|_| { + quote! { + next_page_params: value.next_page_params, + prev_page_params: value.prev_page_params, + } + }); + // Generic type

can't be `None` in `ParsedResponse` + let parsed_response_params_type = response_params_type.unwrap_or(quote! { () }); quote! { #[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)] #[serde(transparent)] @@ -101,20 +140,23 @@ fn generate_async_request_executor( pub data: #output_type, #[serde(skip)] pub header_map: std::sync::Arc, + #response_pagination_params_fields } - impl From<#response_type_ident> for crate::request::ParsedResponse<#output_type> { + impl From<#response_type_ident> for crate::request::ParsedResponse<#output_type, #parsed_response_params_type> { fn from(value: #response_type_ident) -> Self { Self { data: value.data, header_map: value.header_map, + #from_concrete_response_impl_for_pagination_params } } } - impl From> for #response_type_ident { - fn from(value: crate::request::ParsedResponse<#output_type>) -> Self { + impl From> for #response_type_ident { + fn from(value: crate::request::ParsedResponse<#output_type, #parsed_response_params_type>) -> Self { Self { data: value.data, header_map: value.header_map, + #from_parsed_response_impl_for_pagination_params } } } From 4da50afed5ad09a023f97a28f24ab70c93a9dc33 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Thu, 31 Oct 2024 18:35:18 -0400 Subject: [PATCH 02/23] Add FromUrlQueryPairs trait --- wp_api/src/lib.rs | 48 +++++++++++++++++++++++++++++++++++++++++++ wp_api/src/request.rs | 12 ++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 157e54cb4..8bf0ada05 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -2,8 +2,11 @@ pub use api_client::{WpApiClient, WpApiRequestBuilder}; pub use api_error::{ParsedRequestError, RequestExecutionError, WpApiError, WpError, WpErrorCode}; +use application_passwords::{ApplicationPasswordCreateParams, ApplicationPasswordUpdateParams}; pub use parsed_url::{ParseUrlError, ParsedUrl}; use plugins::*; +use posts::{PostCreateParams, PostListParams, PostRetrieveParams, PostUpdateParams}; +use site_settings::SiteSettingsUpdateParams; use url_query::AsQueryValue; use users::*; pub use uuid::{WpUuid, WpUuidParseError}; @@ -90,6 +93,51 @@ trait SparseField { fn as_str(&self) -> &str; } +pub trait FromUrlQueryPairs +where + Self: Sized, +{ + // TODO: We shouldn't have a default implementation + fn from_url_query_pairs(url: &url::Url) -> Option { + None + } +} + +impl FromUrlQueryPairs for () { + fn from_url_query_pairs(url: &url::Url) -> Option { + None + } +} + +// Temporary empty implementations for `FromUrlQueryPairs` +temp_from_url_query_pairs_impl!( + ApplicationPasswordCreateParams, + ApplicationPasswordUpdateParams, + PluginCreateParams, + PluginListParams, + PluginUpdateParams, + PostCreateParams, + PostListParams, + PostRetrieveParams, + PostUpdateParams, + SiteSettingsUpdateParams, + UserCreateParams, + UserDeleteParams, + UserListParams, + UserUpdateParams +); + +#[macro_export] +macro_rules! temp_from_url_query_pairs_impl { + ( $( $type_name:ident ), *) => { + $(impl FromUrlQueryPairs for $type_name { + fn from_url_query_pairs(url: &url::Url) -> Option { + None + } + })* + }; +} + #[macro_export] macro_rules! generate { ($type_name:ident) => { diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index e5fe37939..72ed47705 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -7,7 +7,7 @@ use url::Url; use crate::{ api_error::{ParsedRequestError, RequestExecutionError, WpError}, - WpApiError, WpAuthentication, + FromUrlQueryPairs, WpApiError, WpAuthentication, }; use self::endpoint::WpEndpointUrl; @@ -335,6 +335,7 @@ impl WpNetworkResponse { T: DeserializeOwned, T: From>, ParsedResponse: From, + P: FromUrlQueryPairs, E: ParsedRequestError, { if let Some(err) = E::try_parse(&self.body, self.status_code) { @@ -345,6 +346,15 @@ impl WpNetworkResponse { .map_err(|err| E::as_parse_error(err.to_string(), self.body_as_string())) .map(|x| { let mut p = ParsedResponse::::from(x); + // TODO: Use constants for "next" & "prev" + p.next_page_params = self + .get_link_header("next") + .first() + .and_then(|u| P::from_url_query_pairs(u)); + p.prev_page_params = self + .get_link_header("prev") + .first() + .and_then(|u| P::from_url_query_pairs(u)); p.header_map = self.header_map; T::from(p) }) From 5ceab3e3c4890869a0188fcbb860c5cbc843727c Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Thu, 31 Oct 2024 18:36:51 -0400 Subject: [PATCH 03/23] Fix clippy warning to use a single char instead of string --- wp_api/src/request/endpoint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wp_api/src/request/endpoint.rs b/wp_api/src/request/endpoint.rs index 49b6587ea..2bc6d7ba5 100644 --- a/wp_api/src/request/endpoint.rs +++ b/wp_api/src/request/endpoint.rs @@ -276,7 +276,7 @@ mod tests { fn wp_json_endpoint(base_url: &str) -> String { let mut url = base_url.to_string(); - if !url.ends_with("/") { + if !url.ends_with('/') { url.push('/') } url.push_str(WP_JSON_PATH_SEGMENTS.join("/").as_str()); From 93fb65ad7e665c9ae81f8f28f5023a3863cbe326 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 00:25:45 -0400 Subject: [PATCH 04/23] Mockup of FromUrlQueryPairs for UserListParams --- wp_api/src/lib.rs | 16 ++- wp_api/src/users.rs | 128 +++++++++++++++++- .../tests/test_users_immut.rs | 44 ++++++ 3 files changed, 184 insertions(+), 4 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 8bf0ada05..24ec065d3 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -1,5 +1,7 @@ #![allow(dead_code, unused_variables)] +use std::str::FromStr; + pub use api_client::{WpApiClient, WpApiRequestBuilder}; pub use api_error::{ParsedRequestError, RequestExecutionError, WpApiError, WpError, WpErrorCode}; use application_passwords::{ApplicationPasswordCreateParams, ApplicationPasswordUpdateParams}; @@ -89,6 +91,19 @@ impl WpApiParamOrder { } } +// TODO: Improve error handling +impl FromStr for WpApiParamOrder { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + "asc" => Ok(Self::Asc), + "desc" => Ok(Self::Desc), + _ => Err(()), + } + } +} + trait SparseField { fn as_str(&self) -> &str; } @@ -123,7 +138,6 @@ temp_from_url_query_pairs_impl!( SiteSettingsUpdateParams, UserCreateParams, UserDeleteParams, - UserListParams, UserUpdateParams ); diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index d9659fdb2..f95313966 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, fmt::Display}; +use std::{borrow::Cow, collections::HashMap, fmt::Display, num::ParseIntError, str::FromStr}; use serde::{Deserialize, Serialize}; use wp_contextual::WpContextual; @@ -7,7 +7,7 @@ use crate::{ impl_as_query_value_for_new_type, impl_as_query_value_from_as_str, impl_as_query_value_from_to_string, url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - WpApiParamOrder, + FromUrlQueryPairs, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] @@ -40,6 +40,25 @@ impl WpApiParamUsersOrderBy { } } +// TODO: Improve error handling +impl FromStr for WpApiParamUsersOrderBy { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + "id" => Ok(Self::Id), + "include" => Ok(Self::Include), + "name" => Ok(Self::Name), + "registered_date" => Ok(Self::RegisteredDate), + "slug" => Ok(Self::Slug), + "include_slugs" => Ok(Self::IncludeSlugs), + "email" => Ok(Self::Email), + "url" => Ok(Self::Url), + _ => Err(()), + } + } +} + #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] pub enum WpApiParamUsersWho { #[default] @@ -57,6 +76,20 @@ impl WpApiParamUsersWho { } } +// TODO: Improve error handling +impl FromStr for WpApiParamUsersWho { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + // TODO: Check if this is how it's returned from server + "" => Ok(Self::All), + "authors" => Ok(Self::Authors), + _ => Err(()), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, uniffi::Enum)] pub enum WpApiParamUsersHasPublishedPosts { True, @@ -64,6 +97,20 @@ pub enum WpApiParamUsersHasPublishedPosts { PostTypes(Vec), } +// TODO: Improve error handling +impl FromStr for WpApiParamUsersHasPublishedPosts { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + "true" => Ok(Self::True), + "false" => Ok(Self::False), + // TODO: How is Self::PostTypes handled? + _ => Err(()), + } + } +} + impl_as_query_value_from_to_string!(WpApiParamUsersHasPublishedPosts); impl Display for WpApiParamUsersHasPublishedPosts { @@ -80,7 +127,7 @@ impl Display for WpApiParamUsersHasPublishedPosts { } } -#[derive(Debug, Default, uniffi::Record)] +#[derive(Debug, Default, PartialEq, Eq, uniffi::Record)] pub struct UserListParams { /// Current page of the collection. /// Default: `1` @@ -155,6 +202,42 @@ impl AppendUrlQueryPairs for UserListParams { } } +fn get_parsed(h: &HashMap, Cow<'_, str>>, key: &str) -> Option { + h.get(key).and_then(|v| v.parse().ok()) +} + +fn get_csv(h: &HashMap, Cow<'_, str>>, key: &str) -> Vec { + h.get(key) + .and_then(|v| { + v.split(',') + .map(|s| T::from_str(s).ok()) + .collect::>>() + }) + .unwrap_or(Vec::default()) +} + +impl FromUrlQueryPairs for UserListParams { + fn from_url_query_pairs(url: &url::Url) -> Option { + let h: HashMap, Cow<'_, str>> = url.query_pairs().into_iter().collect(); + let get_string = |s: &str| h.get(s).map(|v| v.to_string()); + Some(UserListParams { + page: get_parsed(&h, "page"), + per_page: get_parsed(&h, "per_page"), + search: get_parsed(&h, "search"), + exclude: get_csv(&h, "exclude"), + include: get_csv(&h, "include"), + offset: get_parsed(&h, "offset"), + order: get_parsed(&h, "order"), + orderby: get_parsed(&h, "orderby"), + slug: get_csv(&h, "slug"), + roles: get_csv(&h, "roles"), + capabilities: get_csv(&h, "capabilities"), + who: get_parsed(&h, "who"), + has_published_posts: get_parsed(&h, "has_published_posts"), + }) + } +} + #[derive(Debug, Serialize, uniffi::Record)] pub struct UserCreateParams { /// Login name for the user. @@ -314,6 +397,14 @@ uniffi::custom_newtype!(UserId, i32); #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct UserId(pub i32); +impl FromStr for UserId { + type Err = ParseIntError; + + fn from_str(s: &str) -> Result { + s.parse().map(|user_id| UserId(user_id)) + } +} + impl std::fmt::Display for UserId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.0) @@ -367,6 +458,7 @@ mod tests { use super::*; use crate::{generate, unit_test_common::assert_expected_query_pairs}; use rstest::*; + use url::Url; #[rstest] #[case(UserListParams::default(), "")] @@ -397,4 +489,34 @@ mod tests { let params = UserDeleteParams::new(UserId(987)); assert_expected_query_pairs(params, "force=true&reassign=987"); } + + #[test] + #[ignore] + fn test_user_list_params_from_query_pairs() { + let mut url = Url::parse("https://example.com").unwrap(); + let original_params = UserListParams { + page: Some(2), + per_page: Some(3), + search: Some("ss".to_string()), + exclude: vec![UserId(1), UserId(7)], + include: vec![UserId(1), UserId(7)], + offset: Some(10), + order: Some(WpApiParamOrder::Asc), + orderby: Some(WpApiParamUsersOrderBy::Email), + slug: vec!["s1".to_string(), "s2".to_string()], + roles: vec!["r1".to_string(), "r2".to_string()], + capabilities: vec!["c1".to_string(), "c2".to_string()], + who: Some(WpApiParamUsersWho::Authors), + has_published_posts: Some(WpApiParamUsersHasPublishedPosts::True), + }; + original_params.append_query_pairs(&mut url.query_pairs_mut()); + println!("original: {:#?}", original_params); + println!("url: {}", url.to_string()); + let p = UserListParams::from_url_query_pairs(&url); + assert!(p.is_some()); + let p = p.unwrap(); + + println!("params: {:#?}", p); + assert_eq!(original_params, p); + } } diff --git a/wp_api_integration_tests/tests/test_users_immut.rs b/wp_api_integration_tests/tests/test_users_immut.rs index 92cf1a207..91152bf93 100644 --- a/wp_api_integration_tests/tests/test_users_immut.rs +++ b/wp_api_integration_tests/tests/test_users_immut.rs @@ -26,6 +26,50 @@ async fn list_users_with_edit_context(#[case] params: UserListParams) { .assert_response(); } +#[tokio::test] +#[parallel] +#[ignore] +async fn paginate_list_users_with_edit_context() { + let first_page_response = api_client() + .users() + .list_with_edit_context(&UserListParams { + per_page: Some(1), + ..Default::default() + }) + .await + .assert_response(); + let second_page_params = first_page_response.next_page_params.unwrap(); + println!( + "first_page_data: {:#?}", + first_page_response + .data + .into_iter() + .map(|u| u.id) + .collect::>() + ); + println!("second_page_params: {:#?}", second_page_params); + let second_page_response = api_client() + .users() + .list_with_edit_context(&second_page_params) + .await + .assert_response(); + let first_page_params_from_second_response = second_page_response.prev_page_params.unwrap(); + let third_page_params = second_page_response.next_page_params.unwrap(); + println!( + "second_page_data: {:#?}", + second_page_response + .data + .into_iter() + .map(|u| u.id) + .collect::>() + ); + println!("third_page_params: {:#?}", third_page_params); + println!( + "first_page_params_from_second_response: {:#?}", + first_page_params_from_second_response + ); +} + #[apply(list_users_cases)] #[tokio::test] #[parallel] From dbd38898d8680ed0c4db5d0f5f7634b2665ef33f Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 14:45:30 -0400 Subject: [PATCH 05/23] Add Kotlin integration test for user list pagination --- .../src/integrationTest/kotlin/UsersEndpointTest.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt index 6134cfed1..bcf3a7c24 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt @@ -91,4 +91,16 @@ class UsersEndpointTest { client.request { requestBuilder -> requestBuilder.users().listWithEditContext(params) } assert(result.wpErrorCode() is WpErrorCode.InvalidParam) } + + @Test + fun testUserListPagination() = runTest { + val firstPageResponse = client.request { requestBuilder -> + requestBuilder.users().listWithEditContext(params = UserListParams(perPage = 1u)) + }.assertSuccessAndRetrieveData() + assert(firstPageResponse.data.isNotEmpty()) + val secondPageResponse = client.request { requestBuilder -> + requestBuilder.users().listWithEditContext(firstPageResponse.nextPageParams!!) + }.assertSuccessAndRetrieveData() + assert(secondPageResponse.data.isNotEmpty()) + } } From 49fecbe0bddf02f2ee98c44faa1c83225dfd59f4 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 14:53:00 -0400 Subject: [PATCH 06/23] Address clippy warnings in user list pagination implementation --- wp_api/src/users.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index f95313966..97ea62625 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -213,7 +213,7 @@ fn get_csv(h: &HashMap, Cow<'_, str>>, key: &str) -> Ve .map(|s| T::from_str(s).ok()) .collect::>>() }) - .unwrap_or(Vec::default()) + .unwrap_or_default() } impl FromUrlQueryPairs for UserListParams { @@ -401,7 +401,7 @@ impl FromStr for UserId { type Err = ParseIntError; fn from_str(s: &str) -> Result { - s.parse().map(|user_id| UserId(user_id)) + s.parse().map(Self) } } @@ -511,7 +511,7 @@ mod tests { }; original_params.append_query_pairs(&mut url.query_pairs_mut()); println!("original: {:#?}", original_params); - println!("url: {}", url.to_string()); + println!("url: {}", url); let p = UserListParams::from_url_query_pairs(&url); assert!(p.is_some()); let p = p.unwrap(); From 1661e6c7e4873942a933e55869a4dac93c623a3d Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 16:34:24 -0400 Subject: [PATCH 07/23] Introduce `contextual_get` attribute for WpDerivedRequest --- wp_api/src/lib.rs | 31 ------------------ wp_api/src/request/endpoint/users_endpoint.rs | 2 +- wp_derive_request_builder/src/generate.rs | 32 +++++++++++-------- .../generate/helpers_to_generate_tokens.rs | 25 +++++++++------ wp_derive_request_builder/src/lib.rs | 5 ++- wp_derive_request_builder/src/parse.rs | 3 +- wp_derive_request_builder/src/variant_attr.rs | 1 + 7 files changed, 42 insertions(+), 57 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 24ec065d3..d9593b49b 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -4,11 +4,8 @@ use std::str::FromStr; pub use api_client::{WpApiClient, WpApiRequestBuilder}; pub use api_error::{ParsedRequestError, RequestExecutionError, WpApiError, WpError, WpErrorCode}; -use application_passwords::{ApplicationPasswordCreateParams, ApplicationPasswordUpdateParams}; pub use parsed_url::{ParseUrlError, ParsedUrl}; use plugins::*; -use posts::{PostCreateParams, PostListParams, PostRetrieveParams, PostUpdateParams}; -use site_settings::SiteSettingsUpdateParams; use url_query::AsQueryValue; use users::*; pub use uuid::{WpUuid, WpUuidParseError}; @@ -124,34 +121,6 @@ impl FromUrlQueryPairs for () { } } -// Temporary empty implementations for `FromUrlQueryPairs` -temp_from_url_query_pairs_impl!( - ApplicationPasswordCreateParams, - ApplicationPasswordUpdateParams, - PluginCreateParams, - PluginListParams, - PluginUpdateParams, - PostCreateParams, - PostListParams, - PostRetrieveParams, - PostUpdateParams, - SiteSettingsUpdateParams, - UserCreateParams, - UserDeleteParams, - UserUpdateParams -); - -#[macro_export] -macro_rules! temp_from_url_query_pairs_impl { - ( $( $type_name:ident ), *) => { - $(impl FromUrlQueryPairs for $type_name { - fn from_url_query_pairs(url: &url::Url) -> Option { - None - } - })* - }; -} - #[macro_export] macro_rules! generate { ($type_name:ident) => { diff --git a/wp_api/src/request/endpoint/users_endpoint.rs b/wp_api/src/request/endpoint/users_endpoint.rs index e04a24f2b..ff4a3a118 100644 --- a/wp_api/src/request/endpoint/users_endpoint.rs +++ b/wp_api/src/request/endpoint/users_endpoint.rs @@ -9,7 +9,7 @@ use super::{AsNamespace, DerivedRequest, WpNamespace}; #[derive(WpDerivedRequest)] enum UsersRequest { - #[contextual_get(url = "/users", params = &UserListParams, output = Vec, filter_by = crate::SparseUserField)] + #[contextual_paged(url = "/users", params = &UserListParams, output = Vec, filter_by = crate::SparseUserField)] List, #[post(url = "/users", params = &UserCreateParams, output = UserWithEditContext)] Create, diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index e6f909938..da69d6f4d 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -95,19 +95,24 @@ fn generate_async_request_executor( &context_and_filter_handler, ); // TODO: Move to helpers and test it - let response_params_type = variant.attr.params.as_ref().map(|p| { - p.tokens - .clone() - .into_iter() - .filter(|t| { - if let TokenTree::Punct(punct) = t { - punct.as_char() != '&' - } else { - true - } + // TODO: If `ContextualPaged` doesn't have params, fail during parsing + let response_params_type = if variant.attr.request_type == RequestType::ContextualPaged { + variant.attr.params.as_ref().map(|p| { + p.tokens + .clone() + .into_iter() + .filter(|t| { + if let TokenTree::Punct(punct) = t { + punct.as_char() != '&' + } else { + true + } + }) + .collect::() }) - .collect::() - }); + } else { + None + }; let response_pagination_params_fields = response_params_type.as_ref().map(|p| { quote! { #[serde(skip)] @@ -343,7 +348,8 @@ impl ContextAndFilterHandler { } v } - crate::parse::RequestType::ContextualGet => { + crate::parse::RequestType::ContextualGet + | crate::parse::RequestType::ContextualPaged => { let mut v = vec![]; WpContext::iter().for_each(|context| { v.push(Self::NoFilterTakeContextAsFunctionName(context)); diff --git a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs index cff4fc3fe..e3608c5b5 100644 --- a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs +++ b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs @@ -111,10 +111,11 @@ pub fn fn_provided_param( // Endpoints don't need the params type if it's a Post request because params will // be part of the body. PartOf::Endpoint => match request_type { - crate::parse::RequestType::ContextualGet - | crate::parse::RequestType::Delete - | crate::parse::RequestType::Get => tokens, - crate::parse::RequestType::Post => TokenStream::new(), + RequestType::ContextualGet + | RequestType::ContextualPaged + | RequestType::Delete + | RequestType::Get => tokens, + RequestType::Post => TokenStream::new(), }, PartOf::RequestBuilder | PartOf::RequestExecutor => tokens, } @@ -204,10 +205,11 @@ fn fn_arg_provided_params( // Endpoints don't need the params type if it's a Post request because params will // be part of the body. PartOf::Endpoint => match request_type { - crate::parse::RequestType::ContextualGet - | crate::parse::RequestType::Delete - | crate::parse::RequestType::Get => tokens, - crate::parse::RequestType::Post => TokenStream::new(), + RequestType::ContextualGet + | RequestType::ContextualPaged + | RequestType::Delete + | RequestType::Get => tokens, + RequestType::Post => TokenStream::new(), }, PartOf::RequestBuilder | PartOf::RequestExecutor => tokens, } @@ -266,7 +268,10 @@ pub fn fn_body_query_pairs( request_type: RequestType, ) -> TokenStream { match request_type { - RequestType::ContextualGet | RequestType::Delete | RequestType::Get => { + RequestType::ContextualGet + | RequestType::ContextualPaged + | RequestType::Delete + | RequestType::Get => { if let Some(params_type) = params_type { let is_option = if let Some(TokenTree::Ident(ref ident)) = params_type.tokens.clone().into_iter().next() @@ -354,7 +359,7 @@ pub fn fn_body_build_request_from_url( request_type: RequestType, ) -> TokenStream { match request_type { - RequestType::ContextualGet | RequestType::Get => quote! { + RequestType::ContextualGet | RequestType::ContextualPaged | RequestType::Get => quote! { self.inner.get(url) }, RequestType::Delete => quote! { diff --git a/wp_derive_request_builder/src/lib.rs b/wp_derive_request_builder/src/lib.rs index 56f43ce86..eeb54c4f2 100644 --- a/wp_derive_request_builder/src/lib.rs +++ b/wp_derive_request_builder/src/lib.rs @@ -8,7 +8,10 @@ mod generate; mod parse; mod variant_attr; -#[proc_macro_derive(WpDerivedRequest, attributes(contextual_get, delete, get, post))] +#[proc_macro_derive( + WpDerivedRequest, + attributes(contextual_get, contextual_paged, delete, get, post) +)] pub fn derive(input: TokenStream) -> TokenStream { let parsed_enum = parse_macro_input!(input as parse::ParsedEnum); diff --git a/wp_derive_request_builder/src/parse.rs b/wp_derive_request_builder/src/parse.rs index 7fc270f3f..56d695553 100644 --- a/wp_derive_request_builder/src/parse.rs +++ b/wp_derive_request_builder/src/parse.rs @@ -44,9 +44,10 @@ impl Parse for ParsedVariant { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub(crate) enum RequestType { ContextualGet, + ContextualPaged, Delete, Get, Post, diff --git a/wp_derive_request_builder/src/variant_attr.rs b/wp_derive_request_builder/src/variant_attr.rs index 9027d9b14..585985066 100644 --- a/wp_derive_request_builder/src/variant_attr.rs +++ b/wp_derive_request_builder/src/variant_attr.rs @@ -111,6 +111,7 @@ impl ParsedVariantAttribute { match path_segment.ident.to_string().as_str() { "contextual_get" => Ok(RequestType::ContextualGet), + "contextual_paged" => Ok(RequestType::ContextualPaged), "delete" => Ok(RequestType::Delete), "get" => Ok(RequestType::Get), "post" => Ok(RequestType::Post), From a28d07fbf3b0dfc2bf696af6d95b59aef28f14b7 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 17:15:48 -0400 Subject: [PATCH 08/23] Move WpDerivedRequest `response_params_type` to helper --- wp_derive_request_builder/src/generate.rs | 22 ++-------------- .../generate/helpers_to_generate_tokens.rs | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index da69d6f4d..876e427b9 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -94,25 +94,7 @@ fn generate_async_request_executor( &variant.variant_ident, &context_and_filter_handler, ); - // TODO: Move to helpers and test it - // TODO: If `ContextualPaged` doesn't have params, fail during parsing - let response_params_type = if variant.attr.request_type == RequestType::ContextualPaged { - variant.attr.params.as_ref().map(|p| { - p.tokens - .clone() - .into_iter() - .filter(|t| { - if let TokenTree::Punct(punct) = t { - punct.as_char() != '&' - } else { - true - } - }) - .collect::() - }) - } else { - None - }; + let response_params_type = response_params_type(variant.attr.params.as_ref(), variant.attr.request_type); let response_pagination_params_fields = response_params_type.as_ref().map(|p| { quote! { #[serde(skip)] @@ -136,7 +118,7 @@ fn generate_async_request_executor( prev_page_params: value.prev_page_params, } }); - // Generic type

can't be `None` in `ParsedResponse` + // Generic type

of `ParsedResponse` can't be `None` let parsed_response_params_type = response_params_type.unwrap_or(quote! { () }); quote! { #[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)] diff --git a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs index e3608c5b5..c0345e52c 100644 --- a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs +++ b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs @@ -410,6 +410,31 @@ pub fn ident_response_type( ) } +// TODO: Test it +// TODO: If `ContextualPaged` doesn't have params, fail during parsing +pub fn response_params_type( + params_type: Option<&ParamsType>, + request_type: RequestType, +) -> Option { + if request_type == RequestType::ContextualPaged { + params_type.map(|p| { + p.tokens + .clone() + .into_iter() + .filter(|t| { + if let TokenTree::Punct(punct) = t { + punct.as_char() != '&' + } else { + true + } + }) + .collect::() + }) + } else { + None + } +} + #[cfg(test)] mod tests { #![allow(clippy::too_many_arguments)] From c164a3f23dbd2af387f5bebc027e1492ba086906 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 18:47:10 -0400 Subject: [PATCH 09/23] Update FromUrlQueryPairs function signature to accept HashMap --- wp_api/src/lib.rs | 9 ++---- wp_api/src/request.rs | 4 +-- wp_api/src/users.rs | 37 +++++++++++------------ wp_derive_request_builder/src/generate.rs | 2 +- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index d9593b49b..320bd1006 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -1,6 +1,6 @@ #![allow(dead_code, unused_variables)] -use std::str::FromStr; +use std::{borrow::Cow, collections::HashMap, str::FromStr}; pub use api_client::{WpApiClient, WpApiRequestBuilder}; pub use api_error::{ParsedRequestError, RequestExecutionError, WpApiError, WpError, WpErrorCode}; @@ -109,14 +109,11 @@ pub trait FromUrlQueryPairs where Self: Sized, { - // TODO: We shouldn't have a default implementation - fn from_url_query_pairs(url: &url::Url) -> Option { - None - } + fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option; } impl FromUrlQueryPairs for () { - fn from_url_query_pairs(url: &url::Url) -> Option { + fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option { None } } diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 72ed47705..5ee801c07 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -350,11 +350,11 @@ impl WpNetworkResponse { p.next_page_params = self .get_link_header("next") .first() - .and_then(|u| P::from_url_query_pairs(u)); + .and_then(|u| P::from_url_query_pairs(u.query_pairs().into_iter().collect())); p.prev_page_params = self .get_link_header("prev") .first() - .and_then(|u| P::from_url_query_pairs(u)); + .and_then(|u| P::from_url_query_pairs(u.query_pairs().into_iter().collect())); p.header_map = self.header_map; T::from(p) }) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 97ea62625..cb3c03572 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -202,11 +202,11 @@ impl AppendUrlQueryPairs for UserListParams { } } -fn get_parsed(h: &HashMap, Cow<'_, str>>, key: &str) -> Option { +fn get_parsed(h: &HashMap, Cow>, key: &str) -> Option { h.get(key).and_then(|v| v.parse().ok()) } -fn get_csv(h: &HashMap, Cow<'_, str>>, key: &str) -> Vec { +fn get_csv(h: &HashMap, Cow>, key: &str) -> Vec { h.get(key) .and_then(|v| { v.split(',') @@ -217,23 +217,22 @@ fn get_csv(h: &HashMap, Cow<'_, str>>, key: &str) -> Ve } impl FromUrlQueryPairs for UserListParams { - fn from_url_query_pairs(url: &url::Url) -> Option { - let h: HashMap, Cow<'_, str>> = url.query_pairs().into_iter().collect(); - let get_string = |s: &str| h.get(s).map(|v| v.to_string()); + fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option { + let get_string = |s: &str| query_pairs.get(s).map(|v| v.to_string()); Some(UserListParams { - page: get_parsed(&h, "page"), - per_page: get_parsed(&h, "per_page"), - search: get_parsed(&h, "search"), - exclude: get_csv(&h, "exclude"), - include: get_csv(&h, "include"), - offset: get_parsed(&h, "offset"), - order: get_parsed(&h, "order"), - orderby: get_parsed(&h, "orderby"), - slug: get_csv(&h, "slug"), - roles: get_csv(&h, "roles"), - capabilities: get_csv(&h, "capabilities"), - who: get_parsed(&h, "who"), - has_published_posts: get_parsed(&h, "has_published_posts"), + page: get_parsed(&query_pairs, "page"), + per_page: get_parsed(&query_pairs, "per_page"), + search: get_parsed(&query_pairs, "search"), + exclude: get_csv(&query_pairs, "exclude"), + include: get_csv(&query_pairs, "include"), + offset: get_parsed(&query_pairs, "offset"), + order: get_parsed(&query_pairs, "order"), + orderby: get_parsed(&query_pairs, "orderby"), + slug: get_csv(&query_pairs, "slug"), + roles: get_csv(&query_pairs, "roles"), + capabilities: get_csv(&query_pairs, "capabilities"), + who: get_parsed(&query_pairs, "who"), + has_published_posts: get_parsed(&query_pairs, "has_published_posts"), }) } } @@ -512,7 +511,7 @@ mod tests { original_params.append_query_pairs(&mut url.query_pairs_mut()); println!("original: {:#?}", original_params); println!("url: {}", url); - let p = UserListParams::from_url_query_pairs(&url); + let p = UserListParams::from_url_query_pairs(url.query_pairs().into_iter().collect()); assert!(p.is_some()); let p = p.unwrap(); diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 876e427b9..7a16adeb4 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -1,7 +1,7 @@ use std::fmt::Display; use helpers_to_generate_tokens::*; -use proc_macro2::{Span, TokenStream, TokenTree}; +use proc_macro2::{Span, TokenStream}; use proc_macro_crate::FoundCrate; use quote::{format_ident, quote}; use serde::{de::Error, Deserialize, Deserializer}; From 8b220f6f3ab6523cbef526a230aebf4d87b5e963 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 21:27:33 -0400 Subject: [PATCH 10/23] Introduce UrlQueryPairsMap --- wp_api/src/lib.rs | 30 +++++++++++++++++++++++-- wp_api/src/request.rs | 20 +++++++++-------- wp_api/src/users.rs | 51 ++++++++++++++++--------------------------- 3 files changed, 58 insertions(+), 43 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 320bd1006..acb949e28 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -105,15 +105,41 @@ trait SparseField { fn as_str(&self) -> &str; } +#[derive(Debug)] +pub struct UrlQueryPairsMap<'a> { + inner: HashMap, Cow<'a, str>>, +} + +impl<'a> UrlQueryPairsMap<'a> { + fn new(query_pairs: HashMap, Cow<'a, str>>) -> Self { + Self { inner: query_pairs } + } + + fn get(&self, key: &str) -> Option { + self.inner.get(key).and_then(|v| v.parse().ok()) + } + + fn get_csv(&self, key: &str) -> Vec { + self.inner + .get(key) + .and_then(|v| { + v.split(',') + .map(|s| T::from_str(s).ok()) + .collect::>>() + }) + .unwrap_or_default() + } +} + pub trait FromUrlQueryPairs where Self: Sized, { - fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option; + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option; } impl FromUrlQueryPairs for () { - fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option { + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { None } } diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 5ee801c07..4ad1f41d1 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -7,7 +7,7 @@ use url::Url; use crate::{ api_error::{ParsedRequestError, RequestExecutionError, WpError}, - FromUrlQueryPairs, WpApiError, WpAuthentication, + FromUrlQueryPairs, UrlQueryPairsMap, WpApiError, WpAuthentication, }; use self::endpoint::WpEndpointUrl; @@ -347,14 +347,16 @@ impl WpNetworkResponse { .map(|x| { let mut p = ParsedResponse::::from(x); // TODO: Use constants for "next" & "prev" - p.next_page_params = self - .get_link_header("next") - .first() - .and_then(|u| P::from_url_query_pairs(u.query_pairs().into_iter().collect())); - p.prev_page_params = self - .get_link_header("prev") - .first() - .and_then(|u| P::from_url_query_pairs(u.query_pairs().into_iter().collect())); + p.next_page_params = self.get_link_header("next").first().and_then(|u| { + P::from_url_query_pairs(UrlQueryPairsMap::new( + u.query_pairs().into_iter().collect(), + )) + }); + p.prev_page_params = self.get_link_header("prev").first().and_then(|u| { + P::from_url_query_pairs(UrlQueryPairsMap::new( + u.query_pairs().into_iter().collect(), + )) + }); p.header_map = self.header_map; T::from(p) }) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index cb3c03572..3ca2d966e 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashMap, fmt::Display, num::ParseIntError, str::FromStr}; +use std::{collections::HashMap, fmt::Display, num::ParseIntError, str::FromStr}; use serde::{Deserialize, Serialize}; use wp_contextual::WpContextual; @@ -7,7 +7,7 @@ use crate::{ impl_as_query_value_for_new_type, impl_as_query_value_from_as_str, impl_as_query_value_from_to_string, url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - FromUrlQueryPairs, WpApiParamOrder, + FromUrlQueryPairs, UrlQueryPairsMap, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] @@ -202,37 +202,22 @@ impl AppendUrlQueryPairs for UserListParams { } } -fn get_parsed(h: &HashMap, Cow>, key: &str) -> Option { - h.get(key).and_then(|v| v.parse().ok()) -} - -fn get_csv(h: &HashMap, Cow>, key: &str) -> Vec { - h.get(key) - .and_then(|v| { - v.split(',') - .map(|s| T::from_str(s).ok()) - .collect::>>() - }) - .unwrap_or_default() -} - impl FromUrlQueryPairs for UserListParams { - fn from_url_query_pairs(query_pairs: HashMap, Cow>) -> Option { - let get_string = |s: &str| query_pairs.get(s).map(|v| v.to_string()); + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { Some(UserListParams { - page: get_parsed(&query_pairs, "page"), - per_page: get_parsed(&query_pairs, "per_page"), - search: get_parsed(&query_pairs, "search"), - exclude: get_csv(&query_pairs, "exclude"), - include: get_csv(&query_pairs, "include"), - offset: get_parsed(&query_pairs, "offset"), - order: get_parsed(&query_pairs, "order"), - orderby: get_parsed(&query_pairs, "orderby"), - slug: get_csv(&query_pairs, "slug"), - roles: get_csv(&query_pairs, "roles"), - capabilities: get_csv(&query_pairs, "capabilities"), - who: get_parsed(&query_pairs, "who"), - has_published_posts: get_parsed(&query_pairs, "has_published_posts"), + page: query_pairs.get("page"), + per_page: query_pairs.get("per_page"), + search: query_pairs.get("search"), + exclude: query_pairs.get_csv("exclude"), + include: query_pairs.get_csv("include"), + offset: query_pairs.get("offset"), + order: query_pairs.get("order"), + orderby: query_pairs.get("orderby"), + slug: query_pairs.get_csv("slug"), + roles: query_pairs.get_csv("roles"), + capabilities: query_pairs.get_csv("capabilities"), + who: query_pairs.get("who"), + has_published_posts: query_pairs.get("has_published_posts"), }) } } @@ -511,7 +496,9 @@ mod tests { original_params.append_query_pairs(&mut url.query_pairs_mut()); println!("original: {:#?}", original_params); println!("url: {}", url); - let p = UserListParams::from_url_query_pairs(url.query_pairs().into_iter().collect()); + let p = UserListParams::from_url_query_pairs(UrlQueryPairsMap::new( + url.query_pairs().into_iter().collect(), + )); assert!(p.is_some()); let p = p.unwrap(); From 1a5a71b0a3d579cb633818d3512efea3d0fe7fa9 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 21:53:12 -0400 Subject: [PATCH 11/23] Implement UserListParamsField enum --- Cargo.lock | 1 + wp_api/Cargo.toml | 1 + wp_api/src/lib.rs | 8 ++-- wp_api/src/users.rs | 98 +++++++++++++++++++++++++++++++++------------ 4 files changed, 78 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a19bca28c..4f6637ef6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3046,6 +3046,7 @@ dependencies = [ "rstest_reuse", "serde", "serde_json", + "strum_macros", "thiserror", "uniffi", "url", diff --git a/wp_api/Cargo.toml b/wp_api/Cargo.toml index 53ffbc9bb..070e1f12f 100644 --- a/wp_api/Cargo.toml +++ b/wp_api/Cargo.toml @@ -22,6 +22,7 @@ paste = { workspace = true } regex = { workspace = true } serde = { workspace = true, features = [ "derive" ] } serde_json = { workspace = true } +strum_macros = { workspace = true } thiserror = { workspace = true } uniffi = { workspace = true } uuid = { workspace = true, features = [ "v4" ] } diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index acb949e28..003b7e170 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -115,13 +115,13 @@ impl<'a> UrlQueryPairsMap<'a> { Self { inner: query_pairs } } - fn get(&self, key: &str) -> Option { - self.inner.get(key).and_then(|v| v.parse().ok()) + fn get<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Option { + self.inner.get(key.into()).and_then(|v| v.parse().ok()) } - fn get_csv(&self, key: &str) -> Vec { + fn get_csv<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Vec { self.inner - .get(key) + .get(key.into()) .and_then(|v| { v.split(',') .map(|s| T::from_str(s).ok()) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 3ca2d966e..3c05a06f2 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -1,6 +1,7 @@ use std::{collections::HashMap, fmt::Display, num::ParseIntError, str::FromStr}; use serde::{Deserialize, Serialize}; +use strum_macros::IntoStaticStr; use wp_contextual::WpContextual; use crate::{ @@ -177,26 +178,71 @@ pub struct UserListParams { pub has_published_posts: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, IntoStaticStr)] +enum UserListParamsField { + #[strum(serialize = "page")] + Page, + #[strum(serialize = "per_page")] + PerPage, + #[strum(serialize = "search")] + Search, + #[strum(serialize = "exclude")] + Exclude, + #[strum(serialize = "include")] + Include, + #[strum(serialize = "offset")] + Offset, + #[strum(serialize = "order")] + Order, + #[strum(serialize = "orderby")] + Orderby, + #[strum(serialize = "slug")] + Slug, + #[strum(serialize = "roles")] + Roles, + #[strum(serialize = "capabilities")] + Capabilities, + #[strum(serialize = "who")] + Who, + #[strum(serialize = "has_published_posts")] + HasPublishedPosts, +} + impl AppendUrlQueryPairs for UserListParams { 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_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("slug", &self.slug) - .append_vec_query_value_pair("roles", &self.roles) - .append_vec_query_value_pair("capabilities", &self.capabilities) + .append_option_query_value_pair(UserListParamsField::Page.into(), self.page.as_ref()) + .append_option_query_value_pair( + UserListParamsField::PerPage.into(), + self.per_page.as_ref(), + ) + .append_option_query_value_pair( + UserListParamsField::Search.into(), + self.search.as_ref(), + ) + .append_vec_query_value_pair(UserListParamsField::Exclude.into(), &self.exclude) + .append_vec_query_value_pair(UserListParamsField::Include.into(), &self.include) + .append_option_query_value_pair( + UserListParamsField::Offset.into(), + self.offset.as_ref(), + ) + .append_option_query_value_pair(UserListParamsField::Order.into(), self.order.as_ref()) + .append_option_query_value_pair( + UserListParamsField::Orderby.into(), + self.orderby.as_ref(), + ) + .append_vec_query_value_pair(UserListParamsField::Slug.into(), &self.slug) + .append_vec_query_value_pair(UserListParamsField::Roles.into(), &self.roles) + .append_vec_query_value_pair( + UserListParamsField::Capabilities.into(), + &self.capabilities, + ) .append_option_query_value_pair( - "who", + UserListParamsField::Who.into(), self.who.as_ref().and_then(|w| w.as_str()).as_ref(), ) .append_option_query_value_pair( - "has_published_posts", + UserListParamsField::HasPublishedPosts.into(), self.has_published_posts.as_ref(), ); } @@ -205,19 +251,19 @@ impl AppendUrlQueryPairs for UserListParams { impl FromUrlQueryPairs for UserListParams { fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { Some(UserListParams { - page: query_pairs.get("page"), - per_page: query_pairs.get("per_page"), - search: query_pairs.get("search"), - exclude: query_pairs.get_csv("exclude"), - include: query_pairs.get_csv("include"), - offset: query_pairs.get("offset"), - order: query_pairs.get("order"), - orderby: query_pairs.get("orderby"), - slug: query_pairs.get_csv("slug"), - roles: query_pairs.get_csv("roles"), - capabilities: query_pairs.get_csv("capabilities"), - who: query_pairs.get("who"), - has_published_posts: query_pairs.get("has_published_posts"), + page: query_pairs.get(UserListParamsField::Page), + per_page: query_pairs.get(UserListParamsField::PerPage), + search: query_pairs.get(UserListParamsField::Search), + exclude: query_pairs.get_csv(UserListParamsField::Exclude), + include: query_pairs.get_csv(UserListParamsField::Include), + offset: query_pairs.get(UserListParamsField::Offset), + order: query_pairs.get(UserListParamsField::Order), + orderby: query_pairs.get(UserListParamsField::Orderby), + slug: query_pairs.get_csv(UserListParamsField::Slug), + roles: query_pairs.get_csv(UserListParamsField::Roles), + capabilities: query_pairs.get_csv(UserListParamsField::Capabilities), + who: query_pairs.get(UserListParamsField::Who), + has_published_posts: query_pairs.get(UserListParamsField::HasPublishedPosts), }) } } From 04f76721d2752ae446e949a01d866d0b21ba9884 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 1 Nov 2024 22:34:46 -0400 Subject: [PATCH 12/23] Update QueryPairsExtension functions to take Into<&'a str> --- wp_api/src/url_query.rs | 32 ++++++++++++++++++++++++-------- wp_api/src/users.rs | 41 +++++++++++++---------------------------- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/wp_api/src/url_query.rs b/wp_api/src/url_query.rs index 78f6f3244..18b9ded47 100644 --- a/wp_api/src/url_query.rs +++ b/wp_api/src/url_query.rs @@ -9,27 +9,39 @@ pub(crate) trait AppendUrlQueryPairs { } pub(crate) trait QueryPairsExtension { - fn append_query_value_pair(&mut self, key: &str, value: &T) -> &mut Self + fn append_query_value_pair<'a, T>(&mut self, key: impl Into<&'a str>, value: &T) -> &mut Self where T: AsQueryValue; - fn append_option_query_value_pair(&mut self, key: &str, value: Option<&T>) -> &mut Self + fn append_option_query_value_pair<'a, T>( + &mut self, + key: impl Into<&'a str>, + value: Option<&T>, + ) -> &mut Self where T: AsQueryValue; - fn append_vec_query_value_pair(&mut self, key: &str, value: &[T]) -> &mut Self + fn append_vec_query_value_pair<'a, T>( + &mut self, + key: impl Into<&'a str>, + value: &[T], + ) -> &mut Self where T: AsQueryValue; } impl QueryPairsExtension for QueryPairs<'_> { - fn append_query_value_pair(&mut self, key: &str, value: &T) -> &mut Self + fn append_query_value_pair<'a, T>(&mut self, key: impl Into<&'a str>, value: &T) -> &mut Self where T: AsQueryValue, { - self.append_pair(key, value.as_query_value().as_ref()); + self.append_pair(key.into(), value.as_query_value().as_ref()); self } - fn append_option_query_value_pair(&mut self, key: &str, value: Option<&T>) -> &mut Self + fn append_option_query_value_pair<'a, T>( + &mut self, + key: impl Into<&'a str>, + value: Option<&T>, + ) -> &mut Self where T: AsQueryValue, { @@ -39,7 +51,11 @@ impl QueryPairsExtension for QueryPairs<'_> { self } - fn append_vec_query_value_pair(&mut self, key: &str, value: &[T]) -> &mut Self + fn append_vec_query_value_pair<'a, T>( + &mut self, + key: impl Into<&'a str>, + value: &[T], + ) -> &mut Self where T: AsQueryValue, { @@ -50,7 +66,7 @@ impl QueryPairsExtension for QueryPairs<'_> { acc }); csv.pop(); // remove the last ',' - self.append_pair(key, &csv); + self.append_pair(key.into(), &csv); } self } diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 3c05a06f2..b3b1115fd 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -211,38 +211,23 @@ enum UserListParamsField { impl AppendUrlQueryPairs for UserListParams { fn append_query_pairs(&self, query_pairs_mut: &mut QueryPairs) { query_pairs_mut - .append_option_query_value_pair(UserListParamsField::Page.into(), self.page.as_ref()) + .append_option_query_value_pair(UserListParamsField::Page, self.page.as_ref()) + .append_option_query_value_pair(UserListParamsField::PerPage, self.per_page.as_ref()) + .append_option_query_value_pair(UserListParamsField::Search, self.search.as_ref()) + .append_vec_query_value_pair(UserListParamsField::Exclude, &self.exclude) + .append_vec_query_value_pair(UserListParamsField::Include, &self.include) + .append_option_query_value_pair(UserListParamsField::Offset, self.offset.as_ref()) + .append_option_query_value_pair(UserListParamsField::Order, self.order.as_ref()) + .append_option_query_value_pair(UserListParamsField::Orderby, self.orderby.as_ref()) + .append_vec_query_value_pair(UserListParamsField::Slug, &self.slug) + .append_vec_query_value_pair(UserListParamsField::Roles, &self.roles) + .append_vec_query_value_pair(UserListParamsField::Capabilities, &self.capabilities) .append_option_query_value_pair( - UserListParamsField::PerPage.into(), - self.per_page.as_ref(), - ) - .append_option_query_value_pair( - UserListParamsField::Search.into(), - self.search.as_ref(), - ) - .append_vec_query_value_pair(UserListParamsField::Exclude.into(), &self.exclude) - .append_vec_query_value_pair(UserListParamsField::Include.into(), &self.include) - .append_option_query_value_pair( - UserListParamsField::Offset.into(), - self.offset.as_ref(), - ) - .append_option_query_value_pair(UserListParamsField::Order.into(), self.order.as_ref()) - .append_option_query_value_pair( - UserListParamsField::Orderby.into(), - self.orderby.as_ref(), - ) - .append_vec_query_value_pair(UserListParamsField::Slug.into(), &self.slug) - .append_vec_query_value_pair(UserListParamsField::Roles.into(), &self.roles) - .append_vec_query_value_pair( - UserListParamsField::Capabilities.into(), - &self.capabilities, - ) - .append_option_query_value_pair( - UserListParamsField::Who.into(), + UserListParamsField::Who, self.who.as_ref().and_then(|w| w.as_str()).as_ref(), ) .append_option_query_value_pair( - UserListParamsField::HasPublishedPosts.into(), + UserListParamsField::HasPublishedPosts, self.has_published_posts.as_ref(), ); } From ff807cd6d52f5ba0625877f4a58ae23b9bd19e8b Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Sat, 2 Nov 2024 20:26:58 -0400 Subject: [PATCH 13/23] Implement FromUrlQueryPairs::supports_pagination and improve pagination parsing --- wp_api/src/lib.rs | 13 -------- wp_api/src/request.rs | 78 ++++++++++++++++++++++++++++++++++--------- wp_api/src/users.rs | 7 +++- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 003b7e170..c84a4045b 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -131,19 +131,6 @@ impl<'a> UrlQueryPairsMap<'a> { } } -pub trait FromUrlQueryPairs -where - Self: Sized, -{ - fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option; -} - -impl FromUrlQueryPairs for () { - fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { - None - } -} - #[macro_export] macro_rules! generate { ($type_name:ident) => { diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 4ad1f41d1..bf4acb1e7 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -7,7 +7,7 @@ use url::Url; use crate::{ api_error::{ParsedRequestError, RequestExecutionError, WpError}, - FromUrlQueryPairs, UrlQueryPairsMap, WpApiError, WpAuthentication, + UrlQueryPairsMap, WpApiError, WpAuthentication, }; use self::endpoint::WpEndpointUrl; @@ -31,6 +31,46 @@ pub struct ParsedResponse { pub prev_page_params: Option

, } +pub trait FromUrlQueryPairs +where + Self: Sized, +{ + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option; + + // Used to avoid unnecessary parsing of the `next` & `prev` headers for params types that don't + // support pagination. + // + // All manually implemented types should return `true` and the implementation for `()` should + // return `false` since `#[derive(WpDerivedRequest)]` will use `()` for parameter `P` of + // `ParsedRequest`. + fn supports_pagination() -> bool; +} + +impl FromUrlQueryPairs for () { + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { + None + } + + fn supports_pagination() -> bool { + false + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum PaginationHeaderKey { + Next, + Prev, +} + +impl PaginationHeaderKey { + fn as_str(&self) -> &str { + match self { + Self::Next => "next", + Self::Prev => "prev", + } + } +} + #[derive(Debug)] pub struct InnerRequestBuilder { authentication: WpAuthentication, @@ -326,6 +366,19 @@ impl WpNetworkResponse { .collect() } + pub fn get_pagination_header( + &self, + header_key: PaginationHeaderKey, + ) -> Option

{ + self.get_link_header(header_key.as_str()) + .first() + .and_then(|u| { + P::from_url_query_pairs(UrlQueryPairsMap::new( + u.query_pairs().into_iter().collect(), + )) + }) + } + pub fn body_as_string(&self) -> String { request_or_response_body_as_string(&self.body) } @@ -345,20 +398,15 @@ impl WpNetworkResponse { serde_json::from_slice(&self.body) .map_err(|err| E::as_parse_error(err.to_string(), self.body_as_string())) .map(|x| { - let mut p = ParsedResponse::::from(x); - // TODO: Use constants for "next" & "prev" - p.next_page_params = self.get_link_header("next").first().and_then(|u| { - P::from_url_query_pairs(UrlQueryPairsMap::new( - u.query_pairs().into_iter().collect(), - )) - }); - p.prev_page_params = self.get_link_header("prev").first().and_then(|u| { - P::from_url_query_pairs(UrlQueryPairsMap::new( - u.query_pairs().into_iter().collect(), - )) - }); - p.header_map = self.header_map; - T::from(p) + let mut parsed_response = ParsedResponse::::from(x); + if P::supports_pagination() { + parsed_response.next_page_params = + self.get_pagination_header(PaginationHeaderKey::Next); + parsed_response.prev_page_params = + self.get_pagination_header(PaginationHeaderKey::Prev); + } + parsed_response.header_map = self.header_map; + T::from(parsed_response) }) } diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index b3b1115fd..fccfd6b7f 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -7,8 +7,9 @@ use wp_contextual::WpContextual; use crate::{ impl_as_query_value_for_new_type, impl_as_query_value_from_as_str, impl_as_query_value_from_to_string, + request::FromUrlQueryPairs, url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - FromUrlQueryPairs, UrlQueryPairsMap, WpApiParamOrder, + UrlQueryPairsMap, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] @@ -251,6 +252,10 @@ impl FromUrlQueryPairs for UserListParams { has_published_posts: query_pairs.get(UserListParamsField::HasPublishedPosts), }) } + + fn supports_pagination() -> bool { + true + } } #[derive(Debug, Serialize, uniffi::Record)] From cf72843a54d5ae3b9380f4617676c027ea08b9ff Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Sat, 2 Nov 2024 20:34:49 -0400 Subject: [PATCH 14/23] Use descriptive names for fn parse() & ParsedResponse generic types --- wp_api/src/request.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index bf4acb1e7..1e57dd4dc 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -21,14 +21,14 @@ const HEADER_KEY_WP_TOTAL_PAGES: &str = "X-WP-TotalPages"; #[derive(Debug, Default, Serialize, Deserialize)] #[serde(transparent)] -pub struct ParsedResponse { - pub data: T, +pub struct ParsedResponse { + pub data: DataType, #[serde(skip)] pub header_map: Arc, #[serde(skip)] - pub next_page_params: Option

, + pub next_page_params: Option, #[serde(skip)] - pub prev_page_params: Option

, + pub prev_page_params: Option, } pub trait FromUrlQueryPairs @@ -383,12 +383,12 @@ impl WpNetworkResponse { request_or_response_body_as_string(&self.body) } - pub fn parse(self) -> Result + pub fn parse(self) -> Result where - T: DeserializeOwned, - T: From>, - ParsedResponse: From, - P: FromUrlQueryPairs, + ResponseType: DeserializeOwned, + ResponseType: From>, + ParsedResponse: From, + ParamsType: FromUrlQueryPairs, E: ParsedRequestError, { if let Some(err) = E::try_parse(&self.body, self.status_code) { @@ -398,15 +398,15 @@ impl WpNetworkResponse { serde_json::from_slice(&self.body) .map_err(|err| E::as_parse_error(err.to_string(), self.body_as_string())) .map(|x| { - let mut parsed_response = ParsedResponse::::from(x); - if P::supports_pagination() { + let mut parsed_response = ParsedResponse::::from(x); + if ParamsType::supports_pagination() { parsed_response.next_page_params = self.get_pagination_header(PaginationHeaderKey::Next); parsed_response.prev_page_params = self.get_pagination_header(PaginationHeaderKey::Prev); } parsed_response.header_map = self.header_map; - T::from(parsed_response) + ResponseType::from(parsed_response) }) } From 0257692c291979c6d45e8eea404891727419c59f Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 15:09:50 -0500 Subject: [PATCH 15/23] Implement EnumFromStrParsingError --- wp_api/src/lib.rs | 13 ++++++++++--- wp_api/src/users.rs | 25 ++++++++++++++----------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index c84a4045b..bcf2e630d 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -88,15 +88,16 @@ impl WpApiParamOrder { } } -// TODO: Improve error handling impl FromStr for WpApiParamOrder { - type Err = (); + type Err = EnumFromStrParsingError; fn from_str(s: &str) -> Result { match s { "asc" => Ok(Self::Asc), "desc" => Ok(Self::Desc), - _ => Err(()), + value => Err(EnumFromStrParsingError::UnknownVariant { + value: value.to_string(), + }), } } } @@ -131,6 +132,12 @@ impl<'a> UrlQueryPairsMap<'a> { } } +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, thiserror::Error)] +pub enum EnumFromStrParsingError { + #[error("'{}' is not a valid variant for this enum", value)] + UnknownVariant { value: String }, +} + #[macro_export] macro_rules! generate { ($type_name:ident) => { diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index fccfd6b7f..626e510eb 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -9,7 +9,7 @@ use crate::{ impl_as_query_value_from_to_string, request::FromUrlQueryPairs, url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - UrlQueryPairsMap, WpApiParamOrder, + EnumFromStrParsingError, UrlQueryPairsMap, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] @@ -42,9 +42,8 @@ impl WpApiParamUsersOrderBy { } } -// TODO: Improve error handling impl FromStr for WpApiParamUsersOrderBy { - type Err = (); + type Err = EnumFromStrParsingError; fn from_str(s: &str) -> Result { match s { @@ -56,7 +55,9 @@ impl FromStr for WpApiParamUsersOrderBy { "include_slugs" => Ok(Self::IncludeSlugs), "email" => Ok(Self::Email), "url" => Ok(Self::Url), - _ => Err(()), + value => Err(EnumFromStrParsingError::UnknownVariant { + value: value.to_string(), + }), } } } @@ -78,16 +79,17 @@ impl WpApiParamUsersWho { } } -// TODO: Improve error handling impl FromStr for WpApiParamUsersWho { - type Err = (); + type Err = EnumFromStrParsingError; fn from_str(s: &str) -> Result { match s { // TODO: Check if this is how it's returned from server "" => Ok(Self::All), "authors" => Ok(Self::Authors), - _ => Err(()), + value => Err(EnumFromStrParsingError::UnknownVariant { + value: value.to_string(), + }), } } } @@ -99,16 +101,17 @@ pub enum WpApiParamUsersHasPublishedPosts { PostTypes(Vec), } -// TODO: Improve error handling impl FromStr for WpApiParamUsersHasPublishedPosts { - type Err = (); + type Err = EnumFromStrParsingError; fn from_str(s: &str) -> Result { + // TODO: How is Self::PostTypes handled? match s { "true" => Ok(Self::True), "false" => Ok(Self::False), - // TODO: How is Self::PostTypes handled? - _ => Err(()), + value => Err(EnumFromStrParsingError::UnknownVariant { + value: value.to_string(), + }), } } } From e3e785988e9009742d3af130a3f457304644a7af Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 16:21:12 -0500 Subject: [PATCH 16/23] Unit test wp_derive_request_builder's response_params_type --- .../generate/helpers_to_generate_tokens.rs | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs index c0345e52c..546f62216 100644 --- a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs +++ b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs @@ -410,14 +410,14 @@ pub fn ident_response_type( ) } -// TODO: Test it // TODO: If `ContextualPaged` doesn't have params, fail during parsing pub fn response_params_type( params_type: Option<&ParamsType>, request_type: RequestType, ) -> Option { if request_type == RequestType::ContextualPaged { - params_type.map(|p| { + let p = params_type.expect("`contextual_paged` should only be used when the request has a params type. This is validated during parsing, so this panic should never occur."); + Some( p.tokens .clone() .into_iter() @@ -428,8 +428,8 @@ pub fn response_params_type( true } }) - .collect::() - }) + .collect::(), + ) } else { None } @@ -1161,6 +1161,37 @@ mod tests { ); } + #[rstest] + #[case(Some(ParamsType { tokens: quote!{ UserListParams } }), RequestType::ContextualPaged, Some("UserListParams"))] + #[case(Some(ParamsType { tokens: quote!{ &UserListParams } }), RequestType::ContextualPaged, Some("UserListParams"))] + #[case(Some(ParamsType { tokens: quote!{ wp_api::users::UserListParams } }), RequestType::ContextualPaged, Some("wp_api :: users :: UserListParams"))] + #[case(Some(ParamsType { tokens: quote!{ &wp_api::users::UserListParams } }), RequestType::ContextualPaged, Some("wp_api :: users :: UserListParams"))] + #[case(None, RequestType::ContextualGet, None)] + #[case(Some(ParamsType { tokens: quote!{ UserListParams } }), RequestType::ContextualGet, None)] + #[case(None, RequestType::Delete, None)] + #[case(Some(ParamsType { tokens: quote!{ UserListParams } }), RequestType::Delete, None)] + #[case(None, RequestType::Get, None)] + #[case(Some(ParamsType { tokens: quote!{ UserListParams } }), RequestType::Get, None)] + #[case(None, RequestType::Post, None)] + #[case(Some(ParamsType { tokens: quote!{ UserListParams } }), RequestType::Post, None)] + fn test_response_params_type( + #[case] params_type: Option, + #[case] request_type: RequestType, + #[case] expected_str: Option<&str>, + ) { + assert_eq!( + response_params_type(params_type.as_ref(), request_type).map(|t| t.to_string()), + expected_str.map(|e| e.to_string()) + ); + } + + #[test] + fn test_response_params_type_panic() { + let result = + std::panic::catch_unwind(|| response_params_type(None, RequestType::ContextualPaged)); + assert!(result.is_err()); + } + fn referenced_params_type(str: &str) -> Option { let ident = format_ident!("{}", str); Some(ParamsType { From 00e1c1cf185427874f69132982a82c958066134c Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 18:27:09 -0500 Subject: [PATCH 17/23] Throw parsing error if #[contextual_paged] is missing params type --- .../generate/helpers_to_generate_tokens.rs | 1 - wp_derive_request_builder/src/variant_attr.rs | 26 ++++++++++++------- .../contextual_paged_missing_params_type.rs | 7 +++++ ...ontextual_paged_missing_params_type.stderr | 5 ++++ 4 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.rs create mode 100644 wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.stderr diff --git a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs index 546f62216..50c385eb3 100644 --- a/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs +++ b/wp_derive_request_builder/src/generate/helpers_to_generate_tokens.rs @@ -410,7 +410,6 @@ pub fn ident_response_type( ) } -// TODO: If `ContextualPaged` doesn't have params, fail during parsing pub fn response_params_type( params_type: Option<&ParamsType>, request_type: RequestType, diff --git a/wp_derive_request_builder/src/variant_attr.rs b/wp_derive_request_builder/src/variant_attr.rs index 585985066..17a987214 100644 --- a/wp_derive_request_builder/src/variant_attr.rs +++ b/wp_derive_request_builder/src/variant_attr.rs @@ -33,7 +33,7 @@ impl ParsedVariantAttribute { params: Option>, output: Vec, filter_by: Option>, - ) -> Self { + ) -> Result { let non_empty_token_tree_or_none = |tokens: Option>| -> Option> { tokens.and_then(|tokens| { @@ -44,18 +44,23 @@ impl ParsedVariantAttribute { } }) }; + let params_type = non_empty_token_tree_or_none(params).map(|tokens| ParamsType { + tokens: TokenStream::from_iter(tokens), + }); - Self { + if request_type == RequestType::ContextualPaged && params_type.is_none() { + return Err(ItemVariantAttributeParseError::ContextualPagedRequiresParamsType); + } + + Ok(Self { request_type, url_parts, - params: non_empty_token_tree_or_none(params).map(|tokens| ParamsType { - tokens: TokenStream::from_iter(tokens), - }), + params: params_type, output, filter_by: non_empty_token_tree_or_none(filter_by).map(|tokens| FilterByType { tokens: TokenStream::from_iter(tokens), }), - } + }) } // Parses the attribute and finds the [syn::MetaList] @@ -279,18 +284,21 @@ impl Parse for ParsedVariantAttribute { let url_parts = UrlPart::split(url_str.to_string(), &meta_list_span)?; - Ok(ParsedVariantAttribute::new( + ParsedVariantAttribute::new( request_type, url_parts, params_tokens, output, filter_by_tokens, - )) + ) + .map_err(|e| e.into_syn_error(meta_list_span)) } } #[derive(Debug, thiserror::Error)] enum ItemVariantAttributeParseError { + #[error("#[contextual_paged(params = ?)] is missing `params` type")] + ContextualPagedRequiresParamsType, #[error("Missing variant attribute")] MissingAttr, #[error("Only a single attribute is supported")] @@ -307,7 +315,7 @@ enum ItemVariantAttributeParseError { UrlShouldBeLiteral, #[error("Missing (output = crate::Foo)")] MissingOutput, - #[error("Only 'contextual_get', 'get', 'post' & 'delete' are supported")] + #[error("Only 'contextual_get', 'contextual_paged', 'get', 'post' & 'delete' are supported")] UnsupportedRequestType, } diff --git a/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.rs b/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.rs new file mode 100644 index 000000000..b543bcc10 --- /dev/null +++ b/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.rs @@ -0,0 +1,7 @@ +#[derive(wp_derive_request_builder::WpDerivedRequest)] +enum UsersRequest { + #[contextual_paged(url = "/users", output = Vec)] + List, +} + +fn main() {} diff --git a/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.stderr b/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.stderr new file mode 100644 index 000000000..0db11c16d --- /dev/null +++ b/wp_derive_request_builder/tests/fail/contextual_paged_missing_params_type.stderr @@ -0,0 +1,5 @@ +error: #[contextual_paged(params = ?)] is missing `params` type + --> tests/fail/contextual_paged_missing_params_type.rs:3:7 + | +3 | #[contextual_paged(url = "/users", output = Vec)] + | ^^^^^^^^^^^^^^^^ From adfe198a45d97e6c21cc5b09ded2c7da7d265d20 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 20:00:23 -0500 Subject: [PATCH 18/23] Unit test UsersListParams::from_query_pairs --- wp_api/src/unit_test_common.rs | 17 ++++++++++ wp_api/src/users.rs | 61 +++++++++++++--------------------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/wp_api/src/unit_test_common.rs b/wp_api/src/unit_test_common.rs index 965a5be86..565f209b3 100644 --- a/wp_api/src/unit_test_common.rs +++ b/wp_api/src/unit_test_common.rs @@ -1,4 +1,6 @@ +use crate::request::FromUrlQueryPairs; use crate::url_query::AppendUrlQueryPairs; +use crate::UrlQueryPairsMap; use url::Url; #[cfg(test)] @@ -7,3 +9,18 @@ pub fn assert_expected_query_pairs(params: impl AppendUrlQueryPairs, expected_qu params.append_query_pairs(&mut url.query_pairs_mut()); assert_eq!(url.query(), Some(expected_query)); } + +#[cfg(test)] +pub fn assert_expected_and_from_query_pairs

(params: P, expected_query: &str) +where + P: AppendUrlQueryPairs + FromUrlQueryPairs + std::fmt::Debug + PartialEq, +{ + 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)); + + let parsed_params = P::from_url_query_pairs(UrlQueryPairsMap::new( + url.query_pairs().into_iter().collect(), + )); + assert_eq!(Some(params), parsed_params); +} diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 626e510eb..59eddcafb 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -479,9 +479,11 @@ pub struct SparseUser { #[cfg(test)] mod tests { use super::*; - use crate::{generate, unit_test_common::assert_expected_query_pairs}; + use crate::{ + generate, + unit_test_common::{assert_expected_and_from_query_pairs, assert_expected_query_pairs}, + }; use rstest::*; - use url::Url; #[rstest] #[case(UserListParams::default(), "")] @@ -498,32 +500,17 @@ mod tests { #[case(generate!(UserListParams, (roles, vec!["author".to_string(), "editor".to_string()])), "roles=author%2Ceditor")] #[case(generate!(UserListParams, (slug, vec!["foo".to_string(), "bar".to_string()]), (roles, vec!["author".to_string(), "editor".to_string()])), "slug=foo%2Cbar&roles=author%2Ceditor")] #[case(generate!(UserListParams, (capabilities, vec!["edit_themes".to_string(), "delete_pages".to_string()])), "capabilities=edit_themes%2Cdelete_pages")] - #[case::who_all_param_should_be_empty(generate!(UserListParams, (who, Some(WpApiParamUsersWho::All))), "")] + //#[case::who_all_param_should_be_empty(generate!(UserListParams, (who, Some(WpApiParamUsersWho::All))), "")] #[case(generate!(UserListParams, (who, Some(WpApiParamUsersWho::Authors))), "who=authors")] #[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::True))), "has_published_posts=true")] - #[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::PostTypes(vec!["post".to_string(), "page".to_string()])))), "has_published_posts=post%2Cpage")] - #[trace] - fn test_user_list_params2(#[case] params: UserListParams, #[case] expected_query: &str) { - assert_expected_query_pairs(params, expected_query); - } - - #[test] - fn test_user_delete_params() { - let params = UserDeleteParams::new(UserId(987)); - assert_expected_query_pairs(params, "force=true&reassign=987"); - } - - #[test] - #[ignore] - fn test_user_list_params_from_query_pairs() { - let mut url = Url::parse("https://example.com").unwrap(); - let original_params = UserListParams { - page: Some(2), - per_page: Some(3), - search: Some("ss".to_string()), - exclude: vec![UserId(1), UserId(7)], - include: vec![UserId(1), UserId(7)], - offset: Some(10), + //#[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::PostTypes(vec!["post".to_string(), "page".to_string()])))), "has_published_posts=post%2Cpage")] + #[case(UserListParams { + page: Some(11), + per_page: Some(22), + search: Some("s_q".to_string()), + exclude: vec![UserId(111), UserId(112)], + include: vec![UserId(211), UserId(212)], + offset: Some(311), order: Some(WpApiParamOrder::Asc), orderby: Some(WpApiParamUsersOrderBy::Email), slug: vec!["s1".to_string(), "s2".to_string()], @@ -531,17 +518,15 @@ mod tests { capabilities: vec!["c1".to_string(), "c2".to_string()], who: Some(WpApiParamUsersWho::Authors), has_published_posts: Some(WpApiParamUsersHasPublishedPosts::True), - }; - original_params.append_query_pairs(&mut url.query_pairs_mut()); - println!("original: {:#?}", original_params); - println!("url: {}", url); - let p = UserListParams::from_url_query_pairs(UrlQueryPairsMap::new( - url.query_pairs().into_iter().collect(), - )); - assert!(p.is_some()); - let p = p.unwrap(); - - println!("params: {:#?}", p); - assert_eq!(original_params, p); + }, "page=11&per_page=22&search=s_q&exclude=111%2C112&include=211%2C212&offset=311&order=asc&orderby=email&slug=s1%2Cs2&roles=r1%2Cr2&capabilities=c1%2Cc2&who=authors&has_published_posts=true")] + #[trace] + fn test_user_list_query_pairs(#[case] params: UserListParams, #[case] expected_query: &str) { + assert_expected_and_from_query_pairs(params, expected_query); + } + + #[test] + fn test_user_delete_params() { + let params = UserDeleteParams::new(UserId(987)); + assert_expected_query_pairs(params, "force=true&reassign=987"); } } From 2ec7bfd8f0554053e6531fe0c9c493020f3a1f9b Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 20:23:06 -0500 Subject: [PATCH 19/23] Add OptionFromStr and implement for WpApiParamUsersWho --- wp_api/src/lib.rs | 14 ++++++++++++++ wp_api/src/users.rs | 27 +++++++++++++++++++-------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index bcf2e630d..233675957 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -106,6 +106,14 @@ trait SparseField { fn as_str(&self) -> &str; } +trait OptionFromStr { + type Err; + + fn option_from_str(s: &str) -> Result, Self::Err> + where + Self: Sized; +} + #[derive(Debug)] pub struct UrlQueryPairsMap<'a> { inner: HashMap, Cow<'a, str>>, @@ -120,6 +128,12 @@ impl<'a> UrlQueryPairsMap<'a> { self.inner.get(key.into()).and_then(|v| v.parse().ok()) } + fn get_option<'b, T: OptionFromStr>(&self, key: impl Into<&'b str>) -> Option { + self.inner + .get(key.into()) + .and_then(|v| T::option_from_str(v).ok().flatten()) + } + fn get_csv<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Vec { self.inner .get(key.into()) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 59eddcafb..ed927fd3d 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -9,7 +9,7 @@ use crate::{ impl_as_query_value_from_to_string, request::FromUrlQueryPairs, url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - EnumFromStrParsingError, UrlQueryPairsMap, WpApiParamOrder, + EnumFromStrParsingError, OptionFromStr, UrlQueryPairsMap, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] @@ -79,14 +79,12 @@ impl WpApiParamUsersWho { } } -impl FromStr for WpApiParamUsersWho { +impl OptionFromStr for WpApiParamUsersWho { type Err = EnumFromStrParsingError; - fn from_str(s: &str) -> Result { + fn option_from_str(s: &str) -> Result, Self::Err> { match s { - // TODO: Check if this is how it's returned from server - "" => Ok(Self::All), - "authors" => Ok(Self::Authors), + "authors" => Ok(Some(Self::Authors)), value => Err(EnumFromStrParsingError::UnknownVariant { value: value.to_string(), }), @@ -251,7 +249,7 @@ impl FromUrlQueryPairs for UserListParams { slug: query_pairs.get_csv(UserListParamsField::Slug), roles: query_pairs.get_csv(UserListParamsField::Roles), capabilities: query_pairs.get_csv(UserListParamsField::Capabilities), - who: query_pairs.get(UserListParamsField::Who), + who: query_pairs.get_option(UserListParamsField::Who), has_published_posts: query_pairs.get(UserListParamsField::HasPublishedPosts), }) } @@ -500,7 +498,6 @@ mod tests { #[case(generate!(UserListParams, (roles, vec!["author".to_string(), "editor".to_string()])), "roles=author%2Ceditor")] #[case(generate!(UserListParams, (slug, vec!["foo".to_string(), "bar".to_string()]), (roles, vec!["author".to_string(), "editor".to_string()])), "slug=foo%2Cbar&roles=author%2Ceditor")] #[case(generate!(UserListParams, (capabilities, vec!["edit_themes".to_string(), "delete_pages".to_string()])), "capabilities=edit_themes%2Cdelete_pages")] - //#[case::who_all_param_should_be_empty(generate!(UserListParams, (who, Some(WpApiParamUsersWho::All))), "")] #[case(generate!(UserListParams, (who, Some(WpApiParamUsersWho::Authors))), "who=authors")] #[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::True))), "has_published_posts=true")] //#[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::PostTypes(vec!["post".to_string(), "page".to_string()])))), "has_published_posts=post%2Cpage")] @@ -524,6 +521,20 @@ mod tests { assert_expected_and_from_query_pairs(params, expected_query); } + #[test] + fn test_user_list_params_who_all_should_be_empty() { + // This test is separate from `test_user_list_query_pairs` because converting the empty + // query back to `UserListParams` will result in `who: None` instead of + // `who: WpApiParamUsersWho::All`. + assert_expected_query_pairs( + UserListParams { + who: Some(WpApiParamUsersWho::All), + ..Default::default() + }, + "", + ); + } + #[test] fn test_user_delete_params() { let params = UserDeleteParams::new(UserId(987)); From 078f738ccc9295bbfaede06d16031120216e9909 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 20:28:17 -0500 Subject: [PATCH 20/23] Move url query related traits and implementations to url_query mod --- wp_api/src/lib.rs | 35 +------------------- wp_api/src/request.rs | 28 ++-------------- wp_api/src/unit_test_common.rs | 4 +-- wp_api/src/url_query.rs | 60 +++++++++++++++++++++++++++++++++- wp_api/src/users.rs | 8 +++-- 5 files changed, 68 insertions(+), 67 deletions(-) diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 233675957..02314f2a1 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -1,11 +1,10 @@ #![allow(dead_code, unused_variables)] -use std::{borrow::Cow, collections::HashMap, str::FromStr}; - pub use api_client::{WpApiClient, WpApiRequestBuilder}; pub use api_error::{ParsedRequestError, RequestExecutionError, WpApiError, WpError, WpErrorCode}; pub use parsed_url::{ParseUrlError, ParsedUrl}; use plugins::*; +use std::str::FromStr; use url_query::AsQueryValue; use users::*; pub use uuid::{WpUuid, WpUuidParseError}; @@ -114,38 +113,6 @@ trait OptionFromStr { Self: Sized; } -#[derive(Debug)] -pub struct UrlQueryPairsMap<'a> { - inner: HashMap, Cow<'a, str>>, -} - -impl<'a> UrlQueryPairsMap<'a> { - fn new(query_pairs: HashMap, Cow<'a, str>>) -> Self { - Self { inner: query_pairs } - } - - fn get<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Option { - self.inner.get(key.into()).and_then(|v| v.parse().ok()) - } - - fn get_option<'b, T: OptionFromStr>(&self, key: impl Into<&'b str>) -> Option { - self.inner - .get(key.into()) - .and_then(|v| T::option_from_str(v).ok().flatten()) - } - - fn get_csv<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Vec { - self.inner - .get(key.into()) - .and_then(|v| { - v.split(',') - .map(|s| T::from_str(s).ok()) - .collect::>>() - }) - .unwrap_or_default() - } -} - #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, thiserror::Error)] pub enum EnumFromStrParsingError { #[error("'{}' is not a valid variant for this enum", value)] diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 1e57dd4dc..7eee4bf42 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -7,7 +7,8 @@ use url::Url; use crate::{ api_error::{ParsedRequestError, RequestExecutionError, WpError}, - UrlQueryPairsMap, WpApiError, WpAuthentication, + url_query::{FromUrlQueryPairs, UrlQueryPairsMap}, + WpApiError, WpAuthentication, }; use self::endpoint::WpEndpointUrl; @@ -31,31 +32,6 @@ pub struct ParsedResponse { pub prev_page_params: Option, } -pub trait FromUrlQueryPairs -where - Self: Sized, -{ - fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option; - - // Used to avoid unnecessary parsing of the `next` & `prev` headers for params types that don't - // support pagination. - // - // All manually implemented types should return `true` and the implementation for `()` should - // return `false` since `#[derive(WpDerivedRequest)]` will use `()` for parameter `P` of - // `ParsedRequest`. - fn supports_pagination() -> bool; -} - -impl FromUrlQueryPairs for () { - fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { - None - } - - fn supports_pagination() -> bool { - false - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum PaginationHeaderKey { Next, diff --git a/wp_api/src/unit_test_common.rs b/wp_api/src/unit_test_common.rs index 565f209b3..305fdd601 100644 --- a/wp_api/src/unit_test_common.rs +++ b/wp_api/src/unit_test_common.rs @@ -1,6 +1,4 @@ -use crate::request::FromUrlQueryPairs; -use crate::url_query::AppendUrlQueryPairs; -use crate::UrlQueryPairsMap; +use crate::url_query::{AppendUrlQueryPairs, FromUrlQueryPairs, UrlQueryPairsMap}; use url::Url; #[cfg(test)] diff --git a/wp_api/src/url_query.rs b/wp_api/src/url_query.rs index 18b9ded47..e6537a995 100644 --- a/wp_api/src/url_query.rs +++ b/wp_api/src/url_query.rs @@ -1,6 +1,7 @@ +use std::{borrow::Cow, collections::HashMap, str::FromStr}; use url::{form_urlencoded, UrlQuery}; -use crate::impl_as_query_value_from_to_string; +use crate::{impl_as_query_value_from_to_string, OptionFromStr}; pub(crate) type QueryPairs<'a> = form_urlencoded::Serializer<'a, UrlQuery<'a>>; @@ -92,6 +93,63 @@ impl AsQueryValue for String { } } +#[derive(Debug)] +pub struct UrlQueryPairsMap<'a> { + inner: HashMap, Cow<'a, str>>, +} + +impl<'a> UrlQueryPairsMap<'a> { + pub(crate) fn new(query_pairs: HashMap, Cow<'a, str>>) -> Self { + Self { inner: query_pairs } + } + + pub(crate) fn get<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Option { + self.inner.get(key.into()).and_then(|v| v.parse().ok()) + } + + pub(crate) fn get_option<'b, T: OptionFromStr>(&self, key: impl Into<&'b str>) -> Option { + self.inner + .get(key.into()) + .and_then(|v| T::option_from_str(v).ok().flatten()) + } + + pub(crate) fn get_csv<'b, T: FromStr>(&self, key: impl Into<&'b str>) -> Vec { + self.inner + .get(key.into()) + .and_then(|v| { + v.split(',') + .map(|s| T::from_str(s).ok()) + .collect::>>() + }) + .unwrap_or_default() + } +} + +pub trait FromUrlQueryPairs +where + Self: Sized, +{ + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option; + + // Used to avoid unnecessary parsing of the `next` & `prev` headers for params types that don't + // support pagination. + // + // All manually implemented types should return `true` and the implementation for `()` should + // return `false` since `#[derive(WpDerivedRequest)]` will use `()` for parameter `P` of + // `ParsedRequest`. + fn supports_pagination() -> bool; +} + +impl FromUrlQueryPairs for () { + fn from_url_query_pairs(query_pairs: UrlQueryPairsMap) -> Option { + None + } + + fn supports_pagination() -> bool { + false + } +} + mod macro_helper { #[macro_export] macro_rules! impl_as_query_value_from_as_str { diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index ed927fd3d..e8e9f4a56 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -7,9 +7,11 @@ use wp_contextual::WpContextual; use crate::{ impl_as_query_value_for_new_type, impl_as_query_value_from_as_str, impl_as_query_value_from_to_string, - request::FromUrlQueryPairs, - url_query::{AppendUrlQueryPairs, AsQueryValue, QueryPairs, QueryPairsExtension}, - EnumFromStrParsingError, OptionFromStr, UrlQueryPairsMap, WpApiParamOrder, + url_query::{ + AppendUrlQueryPairs, AsQueryValue, FromUrlQueryPairs, QueryPairs, QueryPairsExtension, + UrlQueryPairsMap, + }, + EnumFromStrParsingError, OptionFromStr, WpApiParamOrder, }; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, uniffi::Enum)] From 8ee93116c945b0012fdb0ae0d4b04033c932bcb1 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 20:30:39 -0500 Subject: [PATCH 21/23] Rename UrlQueryPairsMap::get_option to get_using_option_from_str --- wp_api/src/url_query.rs | 5 ++++- wp_api/src/users.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/wp_api/src/url_query.rs b/wp_api/src/url_query.rs index e6537a995..3341c0dd8 100644 --- a/wp_api/src/url_query.rs +++ b/wp_api/src/url_query.rs @@ -107,7 +107,10 @@ impl<'a> UrlQueryPairsMap<'a> { self.inner.get(key.into()).and_then(|v| v.parse().ok()) } - pub(crate) fn get_option<'b, T: OptionFromStr>(&self, key: impl Into<&'b str>) -> Option { + pub(crate) fn get_using_option_from_str<'b, T: OptionFromStr>( + &self, + key: impl Into<&'b str>, + ) -> Option { self.inner .get(key.into()) .and_then(|v| T::option_from_str(v).ok().flatten()) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index e8e9f4a56..673f60026 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -251,7 +251,7 @@ impl FromUrlQueryPairs for UserListParams { slug: query_pairs.get_csv(UserListParamsField::Slug), roles: query_pairs.get_csv(UserListParamsField::Roles), capabilities: query_pairs.get_csv(UserListParamsField::Capabilities), - who: query_pairs.get_option(UserListParamsField::Who), + who: query_pairs.get_using_option_from_str(UserListParamsField::Who), has_published_posts: query_pairs.get(UserListParamsField::HasPublishedPosts), }) } From 60652b1159895e6169f60beefa2c5407c0cb54a4 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 20:34:47 -0500 Subject: [PATCH 22/23] Implement from_str for WpApiParamUsersHasPublishedPosts::PostTypes --- wp_api/src/users.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index 673f60026..82c0482ec 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -105,13 +105,12 @@ impl FromStr for WpApiParamUsersHasPublishedPosts { type Err = EnumFromStrParsingError; fn from_str(s: &str) -> Result { - // TODO: How is Self::PostTypes handled? match s { "true" => Ok(Self::True), "false" => Ok(Self::False), - value => Err(EnumFromStrParsingError::UnknownVariant { - value: value.to_string(), - }), + value => Ok(Self::PostTypes( + value.split(',').map(|s| s.to_string()).collect(), + )), } } } @@ -502,7 +501,7 @@ mod tests { #[case(generate!(UserListParams, (capabilities, vec!["edit_themes".to_string(), "delete_pages".to_string()])), "capabilities=edit_themes%2Cdelete_pages")] #[case(generate!(UserListParams, (who, Some(WpApiParamUsersWho::Authors))), "who=authors")] #[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::True))), "has_published_posts=true")] - //#[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::PostTypes(vec!["post".to_string(), "page".to_string()])))), "has_published_posts=post%2Cpage")] + #[case(generate!(UserListParams, (has_published_posts, Some(WpApiParamUsersHasPublishedPosts::PostTypes(vec!["post".to_string(), "page".to_string()])))), "has_published_posts=post%2Cpage")] #[case(UserListParams { page: Some(11), per_page: Some(22), From 4f58cd92540766b5354c6f1a704ca3772f2a210a Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Mon, 4 Nov 2024 21:53:42 -0500 Subject: [PATCH 23/23] Improve user list pagination integration tests --- .../kotlin/UsersEndpointTest.kt | 8 +- .../tests/test_users_immut.rs | 73 ++++++++----------- 2 files changed, 35 insertions(+), 46 deletions(-) diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt index bcf3a7c24..aa352851d 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/UsersEndpointTest.kt @@ -98,9 +98,13 @@ class UsersEndpointTest { requestBuilder.users().listWithEditContext(params = UserListParams(perPage = 1u)) }.assertSuccessAndRetrieveData() assert(firstPageResponse.data.isNotEmpty()) - val secondPageResponse = client.request { requestBuilder -> + val nextPageResponse = client.request { requestBuilder -> requestBuilder.users().listWithEditContext(firstPageResponse.nextPageParams!!) }.assertSuccessAndRetrieveData() - assert(secondPageResponse.data.isNotEmpty()) + assert(nextPageResponse.data.isNotEmpty()) + val prevPageResponse = client.request { requestBuilder -> + requestBuilder.users().listWithEditContext(nextPageResponse.prevPageParams!!) + }.assertSuccessAndRetrieveData() + assert(prevPageResponse.data.isNotEmpty()) } } diff --git a/wp_api_integration_tests/tests/test_users_immut.rs b/wp_api_integration_tests/tests/test_users_immut.rs index 91152bf93..ab44a01af 100644 --- a/wp_api_integration_tests/tests/test_users_immut.rs +++ b/wp_api_integration_tests/tests/test_users_immut.rs @@ -26,50 +26,6 @@ async fn list_users_with_edit_context(#[case] params: UserListParams) { .assert_response(); } -#[tokio::test] -#[parallel] -#[ignore] -async fn paginate_list_users_with_edit_context() { - let first_page_response = api_client() - .users() - .list_with_edit_context(&UserListParams { - per_page: Some(1), - ..Default::default() - }) - .await - .assert_response(); - let second_page_params = first_page_response.next_page_params.unwrap(); - println!( - "first_page_data: {:#?}", - first_page_response - .data - .into_iter() - .map(|u| u.id) - .collect::>() - ); - println!("second_page_params: {:#?}", second_page_params); - let second_page_response = api_client() - .users() - .list_with_edit_context(&second_page_params) - .await - .assert_response(); - let first_page_params_from_second_response = second_page_response.prev_page_params.unwrap(); - let third_page_params = second_page_response.next_page_params.unwrap(); - println!( - "second_page_data: {:#?}", - second_page_response - .data - .into_iter() - .map(|u| u.id) - .collect::>() - ); - println!("third_page_params: {:#?}", third_page_params); - println!( - "first_page_params_from_second_response: {:#?}", - first_page_params_from_second_response - ); -} - #[apply(list_users_cases)] #[tokio::test] #[parallel] @@ -226,6 +182,35 @@ async fn retrieve_me_with_view_context() { assert_eq!(FIRST_USER_ID, user.id); } +#[tokio::test] +#[rstest] +#[parallel] +#[case(UserListParams { per_page: Some(1), ..Default::default() })] +#[case(UserListParams { per_page: Some(1), order: Some(WpApiParamOrder::Desc), ..Default::default() })] +#[case(UserListParams { per_page: Some(1), orderby: Some(WpApiParamUsersOrderBy::Email), ..Default::default() })] +async fn paginate_list_users_with_edit_context(#[case] params: UserListParams) { + let first_page_response = api_client() + .users() + .list_with_edit_context(¶ms) + .await + .assert_response(); + assert!(!first_page_response.data.is_empty()); + let next_page_params = first_page_response.next_page_params.unwrap(); + let next_page_response = api_client() + .users() + .list_with_edit_context(&next_page_params) + .await + .assert_response(); + assert!(!next_page_response.data.is_empty()); + let prev_page_params = next_page_response.prev_page_params.unwrap(); + let prev_page_response = api_client() + .users() + .list_with_edit_context(&prev_page_params) + .await + .assert_response(); + assert!(!prev_page_response.data.is_empty()); +} + #[template] #[rstest] #[case(None)]