Skip to content

Commit c54ee37

Browse files
oguzkoceroguzkocer
authored
Combine is_wordpress_site with api discovery (#537)
* Make is_wordpress_site check a part of api discovery * Combine api link header check for is_wordpress_site & api discovery
1 parent 784a3dc commit c54ee37

File tree

4 files changed

+67
-146
lines changed

4 files changed

+67
-146
lines changed

‎wp_api/src/login/login_client.rs

+14-28
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use super::{
44
AutoDiscoveryAttemptSuccess, AutoDiscoveryResult, AutoDiscoveryUniffiResult,
55
FetchWpJsonFailure, FetchWpJsonSuccess, FindApiRootLinkHeaderFailure,
66
FindApiRootLinkHeaderSuccess, IsWordPressSiteAttemptResult, IsWordPressSiteParseHtmlResult,
7-
IsWordPressSiteResult, IsWordPressSiteUniffiResult, ParseApiRootUrlError, ParseHtmlFailure,
7+
ParseApiRootUrlError, ParseHtmlFailure,
88
},
99
WpApiDetails,
1010
};
@@ -38,13 +38,6 @@ impl UniffiWpLoginClient {
3838
async fn api_discovery(&self, site_url: String) -> AutoDiscoveryUniffiResult {
3939
self.inner.api_discovery(site_url).await.into()
4040
}
41-
42-
async fn is_wordpress_site_discovery(&self, site_url: String) -> IsWordPressSiteUniffiResult {
43-
self.inner
44-
.is_wordpress_site_discovery(site_url)
45-
.await
46-
.into()
47-
}
4841
}
4942

5043
#[derive(Debug)]
@@ -69,37 +62,32 @@ impl WpLoginClient {
6962
}
7063
}
7164

72-
pub async fn is_wordpress_site_discovery(&self, site_url: String) -> IsWordPressSiteResult {
73-
let attempts = futures::future::join_all(
74-
url_discovery::construct_attempts(site_url)
75-
.into_iter()
76-
.map(|attempt| async { self.attempt_is_wordpress_site(attempt).await }),
77-
)
78-
.await;
79-
IsWordPressSiteResult {
80-
attempts: attempts.into_iter().map(|r| (r.attempt_type, r)).collect(),
81-
}
82-
}
83-
8465
async fn attempt_api_discovery(
8566
&self,
8667
attempt: AutoDiscoveryAttempt,
8768
) -> AutoDiscoveryAttemptResult {
88-
let result = self
89-
.inner_attempt_api_discovery(attempt.attempt_site_url.as_str())
69+
let attempt_site_url = attempt.attempt_site_url.as_str();
70+
let api_link_header_result = self.find_api_root_url(attempt_site_url).await;
71+
let discovery_result = self
72+
.inner_attempt_api_discovery(attempt_site_url, api_link_header_result.clone())
73+
.await;
74+
let is_wordpress_site = self
75+
.attempt_is_wordpress_site(attempt_site_url, api_link_header_result)
9076
.await;
9177
AutoDiscoveryAttemptResult {
9278
attempt_type: attempt.attempt_type,
9379
attempt_site_url: attempt.attempt_site_url,
94-
result,
80+
api_discovery_result: discovery_result,
81+
is_wordpress_site,
9582
}
9683
}
9784

9885
async fn inner_attempt_api_discovery(
9986
&self,
10087
attempt_site_url: &str,
88+
api_link_header_result: Result<FindApiRootLinkHeaderSuccess, FindApiRootLinkHeaderFailure>,
10189
) -> Result<AutoDiscoveryAttemptSuccess, AutoDiscoveryAttemptFailure> {
102-
let api_root_url_success = self.find_api_root_url(attempt_site_url).await?;
90+
let api_root_url_success = api_link_header_result?;
10391
let fetch_api_details_response = match self
10492
.fetch_wp_api_details(&api_root_url_success.api_root_url)
10593
.await
@@ -134,17 +122,15 @@ impl WpLoginClient {
134122

135123
async fn attempt_is_wordpress_site(
136124
&self,
137-
attempt: AutoDiscoveryAttempt,
125+
attempt_site_url: &str,
126+
api_link_header_result: Result<FindApiRootLinkHeaderSuccess, FindApiRootLinkHeaderFailure>,
138127
) -> IsWordPressSiteAttemptResult {
139-
let attempt_site_url = attempt.attempt_site_url.as_str();
140-
let api_link_header_result = self.find_api_root_url(attempt_site_url).await;
141128
let fetch_wp_json_result = self.fetch_wp_json(attempt_site_url).await;
142129
let parse_html_result = self
143130
.fetch_site(attempt_site_url)
144131
.await
145132
.map(|r| IsWordPressSiteParseHtmlResult::parse_response(&r.body_as_string()));
146133
IsWordPressSiteAttemptResult {
147-
attempt_type: attempt.attempt_type,
148134
api_link_header_result,
149135
fetch_wp_json_result,
150136
parse_html_result,

‎wp_api/src/login/url_discovery.rs

+18-90
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,20 @@ impl AutoDiscoveryResult {
8787
) -> Option<&AutoDiscoveryAttemptResult> {
8888
self.attempts.get(attempt_type)
8989
}
90+
91+
pub fn is_wordpress_site(&self) -> bool {
92+
self.attempts
93+
.iter()
94+
.any(|(_, result)| result.is_wordpress_site.is_successful())
95+
}
9096
}
9197

9298
#[derive(Debug, Clone, uniffi::Object)]
9399
pub struct AutoDiscoveryAttemptResult {
94100
pub attempt_type: AutoDiscoveryAttemptType,
95101
pub attempt_site_url: String,
96-
pub result: Result<AutoDiscoveryAttemptSuccess, AutoDiscoveryAttemptFailure>,
102+
pub api_discovery_result: Result<AutoDiscoveryAttemptSuccess, AutoDiscoveryAttemptFailure>,
103+
pub is_wordpress_site: IsWordPressSiteAttemptResult,
97104
}
98105

99106
#[uniffi::export]
@@ -103,18 +110,18 @@ impl AutoDiscoveryAttemptResult {
103110
}
104111

105112
fn error_message(&self) -> Option<String> {
106-
match &self.result {
113+
match &self.api_discovery_result {
107114
Ok(_) => None,
108115
Err(error) => Some(error.error_message()),
109116
}
110117
}
111118

112119
fn is_successful(&self) -> bool {
113-
self.result.is_ok()
120+
self.api_discovery_result.is_ok()
114121
}
115122

116123
fn is_network_error(&self) -> bool {
117-
match &self.result {
124+
match &self.api_discovery_result {
118125
Ok(_) => false,
119126
Err(error) => error.is_network_error(),
120127
}
@@ -125,124 +132,57 @@ impl AutoDiscoveryAttemptResult {
125132
}
126133

127134
fn has_failed_to_parse_site_url(&self) -> bool {
128-
match &self.result {
135+
match &self.api_discovery_result {
129136
Ok(success) => false,
130137
Err(error) => error.parsed_site_url().is_none(),
131138
}
132139
}
133140

134141
fn has_failed_to_parse_api_root_url(&self) -> Option<bool> {
135-
match &self.result {
142+
match &self.api_discovery_result {
136143
Ok(success) => Some(false),
137144
Err(error) => error.has_failed_to_parse_api_root_url(),
138145
}
139146
}
140147

141148
fn has_failed_to_parse_api_details(&self) -> Option<bool> {
142-
match &self.result {
149+
match &self.api_discovery_result {
143150
Ok(success) => Some(false),
144151
Err(error) => error.has_failed_to_parse_api_details(),
145152
}
146153
}
147154

148155
fn parsed_site_url(&self) -> Option<Arc<ParsedUrl>> {
149-
match &self.result {
156+
match &self.api_discovery_result {
150157
Ok(success) => Some(Arc::new(success.parsed_site_url.clone())),
151158
Err(error) => error.parsed_site_url().map(|p| Arc::new(p.clone())),
152159
}
153160
}
154161

155162
fn api_root_url(&self) -> Option<Arc<ParsedUrl>> {
156-
match &self.result {
163+
match &self.api_discovery_result {
157164
Ok(success) => Some(Arc::new(success.api_root_url.clone())),
158165
Err(error) => error.api_root_url().map(|p| Arc::new(p.clone())),
159166
}
160167
}
161168

162169
fn api_details(&self) -> Option<Arc<WpApiDetails>> {
163-
match &self.result {
170+
match &self.api_discovery_result {
164171
Ok(success) => Some(Arc::new(success.api_details.clone())),
165172
Err(_) => None,
166173
}
167174
}
168175
}
169176

170-
#[derive(Debug, uniffi::Record)]
171-
pub struct IsWordPressSiteUniffiResult {
172-
pub user_input_attempt: Arc<IsWordPressSiteAttemptResult>,
173-
pub successful_attempt: Option<Arc<IsWordPressSiteAttemptResult>>,
174-
pub auto_https_attempt: Option<Arc<IsWordPressSiteAttemptResult>>,
175-
pub auto_dot_php_extension_for_wp_admin_attempt: Option<Arc<IsWordPressSiteAttemptResult>>,
176-
}
177-
178-
#[derive(Debug)]
179-
pub struct IsWordPressSiteResult {
180-
pub attempts: HashMap<AutoDiscoveryAttemptType, IsWordPressSiteAttemptResult>,
181-
}
182-
183-
impl From<IsWordPressSiteResult> for IsWordPressSiteUniffiResult {
184-
fn from(value: IsWordPressSiteResult) -> Self {
185-
let get_attempt_result = |attempt_type| {
186-
value
187-
.get_attempt(&attempt_type)
188-
.map(|a| Arc::new(a.clone()))
189-
};
190-
Self {
191-
user_input_attempt: Arc::new(value.user_input_attempt().clone()),
192-
successful_attempt: value.find_successful().map(|a| Arc::new(a.clone())),
193-
auto_https_attempt: get_attempt_result(AutoDiscoveryAttemptType::AutoHttps),
194-
auto_dot_php_extension_for_wp_admin_attempt: get_attempt_result(
195-
AutoDiscoveryAttemptType::AutoDotPhpExtensionForWpAdmin,
196-
),
197-
}
198-
}
199-
}
200-
201-
impl IsWordPressSiteResult {
202-
pub fn is_successful(&self) -> bool {
203-
self.attempts
204-
.iter()
205-
.any(|(_, result)| result.is_successful())
206-
}
207-
208-
pub fn user_input_attempt(&self) -> &IsWordPressSiteAttemptResult {
209-
self.get_attempt(&AutoDiscoveryAttemptType::UserInput)
210-
.expect("User input url is always attempted")
211-
}
212-
213-
pub fn get_attempt(
214-
&self,
215-
attempt_type: &AutoDiscoveryAttemptType,
216-
) -> Option<&IsWordPressSiteAttemptResult> {
217-
self.attempts.get(attempt_type)
218-
}
219-
220-
pub fn find_successful(&self) -> Option<&IsWordPressSiteAttemptResult> {
221-
// If the user attempt is successful, prefer it over other attempts
222-
let user_input_attempt = self.user_input_attempt();
223-
if user_input_attempt.is_successful() {
224-
return Some(user_input_attempt);
225-
}
226-
self.attempts.iter().find_map(|(_, result)| {
227-
if result.is_successful() {
228-
Some(result)
229-
} else {
230-
None
231-
}
232-
})
233-
}
234-
}
235-
236177
#[derive(Debug, Clone, uniffi::Object)]
237178
pub struct IsWordPressSiteAttemptResult {
238-
pub attempt_type: AutoDiscoveryAttemptType,
239179
pub api_link_header_result: Result<FindApiRootLinkHeaderSuccess, FindApiRootLinkHeaderFailure>,
240180
pub fetch_wp_json_result: Result<FetchWpJsonSuccess, FetchWpJsonFailure>,
241181
pub parse_html_result: Result<IsWordPressSiteParseHtmlResult, ParseHtmlFailure>,
242182
}
243183

244184
impl IsWordPressSiteAttemptResult {
245-
fn is_successful(&self) -> bool {
185+
pub fn is_successful(&self) -> bool {
246186
self.api_link_header_result.is_ok()
247187
|| self.fetch_wp_json_result.is_ok()
248188
|| self
@@ -411,18 +351,6 @@ pub enum AutoDiscoveryAttemptFailure {
411351
}
412352

413353
impl AutoDiscoveryAttemptFailure {
414-
pub fn into_attempt_result(
415-
self,
416-
attempt_type: AutoDiscoveryAttemptType,
417-
attempt_site_url: String,
418-
) -> AutoDiscoveryAttemptResult {
419-
AutoDiscoveryAttemptResult {
420-
attempt_type,
421-
attempt_site_url,
422-
result: Err(self),
423-
}
424-
}
425-
426354
pub fn error_message(&self) -> String {
427355
match self {
428356
AutoDiscoveryAttemptFailure::ParseSiteUrl { error } => error.to_string(),

‎wp_api_integration_tests/tests/test_login_err.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ async fn test_login_flow_err_parse_api_details(#[case] site_url: &str) {
1616
.attempts
1717
.remove(&AutoDiscoveryAttemptType::UserInput)
1818
.unwrap()
19-
.result
19+
.api_discovery_result
2020
.unwrap_err();
2121
assert_eq!(
2222
original_attempt_error.has_failed_to_parse_api_details(),
@@ -37,7 +37,7 @@ async fn test_login_flow_err_network_error(#[case] site_url: &str) {
3737
.attempts
3838
.remove(&AutoDiscoveryAttemptType::UserInput)
3939
.unwrap()
40-
.result
40+
.api_discovery_result
4141
.unwrap_err();
4242
assert!(
4343
original_attempt_error.is_network_error(),

‎wp_api_integration_tests/tests/test_login_immut.rs

+33-26
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rstest::rstest;
22
use serial_test::parallel;
33
use std::sync::Arc;
4-
use wp_api::login::{login_client::WpLoginClient, url_discovery::IsWordPressSiteParseHtmlResult};
4+
use wp_api::login::login_client::WpLoginClient;
55
use wp_api_integration_tests::AsyncWpNetworking;
66

77
const LOCALHOST_AUTH_URL: &str = "http://localhost/wp-admin/authorize-application.php";
@@ -64,38 +64,45 @@ async fn test_login_flow(#[case] site_url: &str, #[case] expected_auth_url: &str
6464
"Auto discovery failed: {:#?}",
6565
result
6666
);
67+
68+
let successful_attempt = result
69+
.find_successful()
70+
.expect("Already verified that auto discovery is successful");
6771
assert_eq!(
68-
result
69-
.find_successful()
70-
.expect("Already verified that auto discovery is successful")
71-
.result
72+
successful_attempt
73+
.api_discovery_result
7274
.clone()
7375
.expect("Already verified that auto discovery is successful")
7476
.api_details
7577
.find_application_passwords_authentication_url(),
7678
Some(expected_auth_url.to_string())
7779
);
78-
}
7980

80-
#[rstest]
81-
#[case("http://localhost")]
82-
#[tokio::test]
83-
#[parallel]
84-
async fn test_is_wordpress_site(#[case] site_url: &str) {
85-
let client = WpLoginClient::new(Arc::new(AsyncWpNetworking::default()));
86-
let result = client
87-
.is_wordpress_site_discovery(site_url.to_string())
88-
.await;
89-
let successful_attempt = result.find_successful().unwrap();
90-
assert!(successful_attempt.api_link_header_result.is_ok());
91-
assert!(successful_attempt.fetch_wp_json_result.is_ok());
92-
assert_eq!(
93-
successful_attempt.parse_html_result,
94-
Ok(IsWordPressSiteParseHtmlResult {
95-
has_wordpress_generator_meta_tag: true,
96-
// `wp-content` is not mentioned in relevant `http://localhost` HTML source tags
97-
mentions_wp_content: false,
98-
mentions_wp_includes: true
99-
})
81+
let is_wordpress_site = &successful_attempt.is_wordpress_site;
82+
assert!(
83+
is_wordpress_site.is_successful(),
84+
"'{site_url}' is incorrectly marked as non-WordPress site: {:#?}",
85+
result
10086
);
87+
assert!(is_wordpress_site.api_link_header_result.is_ok());
88+
89+
// We can't do a reasonable assertion of `is_wordpress_site.fetch_wp_json_result` because not
90+
// all of the test cases will return a parseable JSON from `/wp-json`.
91+
//
92+
// assert!(is_wordpress_site.fetch_wp_json_result.is_ok());
93+
//
94+
// ---
95+
//
96+
// We can't do a reasonable assertion of `is_wordpress_site.parse_html_result` because each
97+
// test site has a different configuration, so we have a mixed results of
98+
// `has_wordpress_generator_meta_tag`, `mentions_wp_content` & `mentions_wp_includes` fields.
99+
//
100+
// assert_eq!(
101+
// is_wordpress_site.parse_html_result,
102+
// Ok(IsWordPressSiteParseHtmlResult {
103+
// has_wordpress_generator_meta_tag: true,
104+
// mentions_wp_content: true,
105+
// mentions_wp_includes: true
106+
// })
107+
// );
101108
}

0 commit comments

Comments
 (0)