From 01780bfd3a5dc4387777ff530354c2743d3f2d14 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 12:33:56 +1200 Subject: [PATCH 01/22] Add CancellationTests --- .../integration-tests/CancellationTests.swift | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 native/swift/Tests/integration-tests/CancellationTests.swift diff --git a/native/swift/Tests/integration-tests/CancellationTests.swift b/native/swift/Tests/integration-tests/CancellationTests.swift new file mode 100644 index 000000000..af3961be6 --- /dev/null +++ b/native/swift/Tests/integration-tests/CancellationTests.swift @@ -0,0 +1,33 @@ +import Foundation +import Testing + +@testable import WordPressAPI +@testable import WordPressAPIInternal + +struct CancellationTests { + let api = WordPressAPI.admin() + + @Test + func cancelUploadingLongPost() async throws { + let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) + let content = try String(data: Data(contentsOf: file).base64EncodedData(), encoding: .utf8)! + + let title = UUID().uuidString + await #expect( + throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), + performing: { + let task = Task { + _ = try await api.posts.create(params: .init(title: title, content: content, meta: nil)) + Issue.record("The creating post function should throw") + } + + try await Task.sleep(for: .milliseconds(10)) + task.cancel() + + try await task.value + } + ) + + try await restoreTestServer() + } +} From c467c685f2962b7c1d1c70fe04914c13a2eb9d95 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 16 Sep 2025 15:44:38 +1200 Subject: [PATCH 02/22] Proof of concept --- .../xcshareddata/xcschemes/Example.xcscheme | 78 +++++ .../wordpress-api/SafeRequestExecutor.swift | 81 +++++- .../Sources/wordpress-api/WordPressAPI.swift | 19 +- .../Tests/integration-tests/KnownIssues.swift | 38 +++ .../Tests/integration-tests/MediaTests.swift | 53 +++- .../wordpress-api/Support/HTTPStubs.swift | 3 +- wp_api/src/api_client.rs | 31 +- wp_api/src/cancellation.rs | 272 ++++++++++++++++++ wp_api/src/lib.rs | 1 + wp_api/src/middleware.rs | 18 +- wp_api/src/request.rs | 2 + wp_api/src/request/endpoint/media_endpoint.rs | 4 +- wp_api/src/reqwest_request_executor.rs | 10 +- 13 files changed, 582 insertions(+), 28 deletions(-) create mode 100644 native/swift/Example/Example.xcodeproj/xcshareddata/xcschemes/Example.xcscheme create mode 100644 native/swift/Tests/integration-tests/KnownIssues.swift create mode 100644 wp_api/src/cancellation.rs diff --git a/native/swift/Example/Example.xcodeproj/xcshareddata/xcschemes/Example.xcscheme b/native/swift/Example/Example.xcodeproj/xcshareddata/xcschemes/Example.xcscheme new file mode 100644 index 000000000..81cf8a708 --- /dev/null +++ b/native/swift/Example/Example.xcodeproj/xcshareddata/xcschemes/Example.xcscheme @@ -0,0 +1,78 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index ee03b9a2f..acfef5eba 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -12,7 +12,8 @@ import Combine public protocol SafeRequestExecutor: RequestExecutor, Sendable { func execute(_ request: WpNetworkRequest) async -> Result func uploadMedia( - mediaUploadRequest: MediaUploadRequest + mediaUploadRequest: MediaUploadRequest, + cancellationToken: CancellationToken? ) async -> Result #if PROGRESS_REPORTING_ENABLED @@ -28,8 +29,8 @@ extension SafeRequestExecutor { return try result.get() } - public func uploadMedia(mediaUploadRequest: MediaUploadRequest) async throws -> WpNetworkResponse { - let result = await uploadMedia(mediaUploadRequest: mediaUploadRequest) + public func uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?) async throws -> WpNetworkResponse { + let result = await uploadMedia(mediaUploadRequest: mediaUploadRequest, cancellationToken: cancellationToken) return try result.get() } } @@ -40,6 +41,8 @@ public final class WpRequestExecutor: SafeRequestExecutor { private let additionalHttpHeadersForAllRequests: [String: String] + private let cancellationHandlers = CancellationHandlers() + public init( urlSession: URLSession, additionalHttpHeadersForAllRequests: [String: String] = [:], @@ -60,9 +63,19 @@ public final class WpRequestExecutor: SafeRequestExecutor { } public func uploadMedia( - mediaUploadRequest: MediaUploadRequest + mediaUploadRequest: MediaUploadRequest, + cancellationToken: CancellationToken? ) async -> Result { - (await perform(mediaUploadRequest)) + if let cancellationToken { + let requestId = mediaUploadRequest.requestId() + await cancellationHandlers.whenCancelling(cancellationToken) { + Task { [weak self] in + await self?.cancelRequest(withId: requestId) + } + } + } + + let result = (await perform(mediaUploadRequest)) .mapError { error in switch error { case let .RequestExecutionFailed(statusCode, redirects, reason): @@ -73,6 +86,12 @@ public final class WpRequestExecutor: SafeRequestExecutor { ) } } + + if let cancellationToken { + await cancellationHandlers.remove(cancellationToken) + } + + return result } func perform(_ request: NetworkRequestContent) async -> Result { @@ -131,6 +150,24 @@ public final class WpRequestExecutor: SafeRequestExecutor { } #endif + private func cancelRequest(withId requestId: String) async { + var task = (await self.session.allTasks).first { + $0.originalRequest?.requestId == requestId + } + + if task == nil { + task = await NotificationCenter.default + .publisher(for: RequestExecutorDelegate.didCreateTaskNotification) + .compactMap { $0.object as? URLSessionTask } + .first { $0.originalRequest?.requestId == requestId } + .timeout(.seconds(1), scheduler: DispatchQueue.global()) + .values + .first { _ in true } + } + + task?.cancel() + } + private func handleHttpsError( _ error: Error, for request: NetworkRequestContent @@ -393,3 +430,37 @@ extension MediaUploadRequest: NetworkRequestContent { } } + +private actor CancellationHandlers { + private var handlers: [String /* CancellationToken.uuid */: [RequestCancellationHandler]] = [:] + + func whenCancelling(_ token: CancellationToken, closure: @escaping @Sendable () -> Void) { + let handler = RequestCancellationHandler(closure: closure) + handlers[token.uuid(), default: []].append(handler) + try? token.registerHandler(handler: handler) + } + + func remove(_ token: CancellationToken) { + handlers.removeValue(forKey: token.uuid()) + } +} + +private final class RequestCancellationHandler: CancellationHandler, Hashable { + let closure: @Sendable () -> Void + + init(closure: @escaping @Sendable () -> Void) { + self.closure = closure + } + + func cancelled() { + self.closure() + } + + func hash(into hasher: inout Hasher) { + hasher.combine(ObjectIdentifier(self)) + } + + static func == (lhs: RequestCancellationHandler, rhs: RequestCancellationHandler) -> Bool { + ObjectIdentifier(lhs) == ObjectIdentifier(rhs) + } +} diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index e47477383..620e8b2ad 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -20,7 +20,7 @@ public actor WordPressAPI { } private let apiUrlResolver: ApiUrlResolver - private let requestExecutor: SafeRequestExecutor + let requestExecutor: SafeRequestExecutor private let apiClientDelegate: WpApiClientDelegate package let requestBuilder: UniffiWpApiClient @@ -177,7 +177,8 @@ public actor WordPressAPI { params: params, filePath: localFileURL.path, fileContentType: fileContentType, - requestId: requestId + requestId: requestId, + cancellationToken: nil ) } @@ -200,6 +201,20 @@ public actor WordPressAPI { } #endif + public func createMedia(params: MediaCreateParams, filePath: String, fileContentType: String, requestId: WpUuid?) async throws -> MediaRequestCreateResponse { + let cancellation = CancellationToken() + return try await withTaskCancellationHandler { + try await media.create(params: params, filePath: filePath, fileContentType: fileContentType, requestId: requestId, cancellationToken: cancellation) + } onCancel: { + do { + try cancellation.cancel() + } catch { + NSLog("Failed to cancel \(#function): \(error)") + } + } + + } + enum ParseError: Error { case invalidUrl case invalidHtml diff --git a/native/swift/Tests/integration-tests/KnownIssues.swift b/native/swift/Tests/integration-tests/KnownIssues.swift new file mode 100644 index 000000000..9ac4efcfa --- /dev/null +++ b/native/swift/Tests/integration-tests/KnownIssues.swift @@ -0,0 +1,38 @@ +import Foundation +import Testing + +@testable import WordPressAPI +@testable import WordPressAPIInternal + +@Suite(.disabled("The tests fails due to issues in uniffi-rs")) +struct KnownIssues { + let api = WordPressAPI.admin() + + @Test + func uniffiAsyncFunctionsAreNotCancellable() async throws { + // See https://mozilla.github.io/uniffi-rs/0.29/futures.html#cancelling-async-code + + let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) + let task = Task { + _ = try await api.media.create( + params: .init(title: "Image", altText: "This is a test image"), + filePath: file.path, + fileContentType: "image/jpeg", + requestId: nil, + cancellationToken: nil + ) + } + + try await Task.sleep(nanoseconds: 50_000_000) + task.cancel() + + await #expect( + throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), + performing: { + try await task.value + } + ) + + try await restoreTestServer() + } +} diff --git a/native/swift/Tests/integration-tests/MediaTests.swift b/native/swift/Tests/integration-tests/MediaTests.swift index a21de5a04..473151ab2 100644 --- a/native/swift/Tests/integration-tests/MediaTests.swift +++ b/native/swift/Tests/integration-tests/MediaTests.swift @@ -1,5 +1,5 @@ import Foundation -import WordPressAPI +@testable import WordPressAPI import Testing @Suite @@ -13,7 +13,8 @@ struct MediaTests { params: .init(title: "Image", altText: "This is a test image"), filePath: file.path, fileContentType: "image/jpeg", - requestId: nil + requestId: nil, + cancellationToken: nil ) #expect(response.data.mimeType == "image/jpeg") #expect(response.data.title.raw == "Image") @@ -96,5 +97,53 @@ struct MediaTests { try await restoreTestServer() } + + @Test + func cancelCreateMediaTask() async throws { + let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) + await #expect( + throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), + performing: { + let requestId = WpUuid() + let task = Task { + _ = try await api.media.create(params: .init(), filePath: file.path(), fileContentType: "image/jpg", requestId: requestId, cancellationToken: nil) + } + + let progress = try await api.requestExecutor.progress(forRequestWithId: requestId.uuidString()).values.first { _ in true } + let cancellable = progress!.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in + task.cancel() + } + defer { cancellable.cancel() } + + try await task.value + } + ) + + try await restoreTestServer() + } + + @Test + func cancelNewCreateMediaTask() async throws { + let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) + await #expect( + throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), + performing: { + let requestId = WpUuid() + let task = Task { + _ = try await api.createMedia(params: .init(), filePath: file.path(), fileContentType: "image/jpg", requestId: requestId) + } + + let progress = try await api.requestExecutor.progress(forRequestWithId: requestId.uuidString()).values.first { _ in true } + let cancellable = progress!.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in + task.cancel() + } + defer { cancellable.cancel() } + + try await task.value + } + ) + + try await restoreTestServer() + } #endif } diff --git a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift index bac4e3697..24806f4ed 100644 --- a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift +++ b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift @@ -47,7 +47,8 @@ final class HTTPStubs: SafeRequestExecutor { } func uploadMedia( - mediaUploadRequest: MediaUploadRequest + mediaUploadRequest: MediaUploadRequest, + cancellationToken: CancellationToken? ) async -> Result { preconditionFailure("This method is not yet implemented") } diff --git a/wp_api/src/api_client.rs b/wp_api/src/api_client.rs index 804a6d538..3ad8f64a6 100644 --- a/wp_api/src/api_client.rs +++ b/wp_api/src/api_client.rs @@ -2,7 +2,10 @@ use crate::{ WpAppNotifier, api_client_generate_api_client, api_client_generate_endpoint_impl, api_client_generate_request_builder, auth::WpAuthenticationProvider, + cancellation::CancellationToken, + media::MediaCreateParams, middleware::WpApiMiddlewarePipeline, + prelude::WpApiError, request::{ RequestExecutor, endpoint::{ @@ -12,7 +15,9 @@ use crate::{ }, categories_endpoint::{CategoriesRequestBuilder, CategoriesRequestExecutor}, comments_endpoint::{CommentsRequestBuilder, CommentsRequestExecutor}, - media_endpoint::{MediaRequestBuilder, MediaRequestExecutor}, + media_endpoint::{ + MediaRequestBuilder, MediaRequestCreateResponse, MediaRequestExecutor, + }, plugins_endpoint::{PluginsRequestBuilder, PluginsRequestExecutor}, post_autosaves_endpoint::{AutosavesRequestBuilder, AutosavesRequestExecutor}, post_revisions_endpoint::{PostRevisionsRequestBuilder, PostRevisionsRequestExecutor}, @@ -32,6 +37,7 @@ use crate::{ }, }, }, + uuid::WpUuid, }; use std::sync::Arc; @@ -121,6 +127,29 @@ impl UniffiWpApiClient { } } +#[uniffi::export] +impl UniffiWpApiClient { + async fn create_media( + &self, + params: MediaCreateParams, + file_path: String, + file_content_type: String, + request_id: Option>, + cancellation_token: Option>, + ) -> Result { + self.inner + .media + .create( + params, + file_path, + file_content_type, + request_id, + cancellation_token, + ) + .await + } +} + pub struct WpApiClient { application_passwords: Arc, autosaves: Arc, diff --git a/wp_api/src/cancellation.rs b/wp_api/src/cancellation.rs new file mode 100644 index 000000000..bd383651f --- /dev/null +++ b/wp_api/src/cancellation.rs @@ -0,0 +1,272 @@ +use std::{ + collections::VecDeque, + sync::{Arc, Mutex, PoisonError}, +}; + +use futures::StreamExt; +use futures::channel::mpsc::{TryRecvError, UnboundedSender, unbounded}; + +use crate::uuid::WpUuid; + +#[uniffi::export(with_foreign)] +pub trait CancellationHandler: Send + Sync + std::fmt::Debug { + fn cancelled(&self); +} + +#[derive(Debug, Default, uniffi::Object)] +pub struct CancellationToken { + uuid: WpUuid, + cancelled: Mutex, + waiters: Mutex>>, + handler: Mutex>>, +} + +#[uniffi::export] +impl CancellationToken { + #[uniffi::constructor] + pub fn new() -> Self { + Self { + uuid: WpUuid::default(), + cancelled: Mutex::new(false), + waiters: Mutex::new(VecDeque::new()), + handler: Mutex::new(VecDeque::new()), + } + } + + pub fn uuid(&self) -> String { + self.uuid.uuid_string() + } + + pub fn register_handler(&self, handler: Arc) -> Result<(), CancellationTokenError> { + let mut handlers = self.handler.lock()?; + handlers.push_back(handler); + Ok(()) + } + + pub async fn wait_for_cancellation(&self) -> Result<(), CancellationTokenError> { + if *self.cancelled.lock()? { + return Ok(()); + } + + let (sender, mut receiver) = unbounded(); + + { + let mut waiters = self.waiters.lock()?; + waiters.push_back(sender); + } + + let _ = receiver.next().await; + Ok(()) + } + + pub fn cancel(&self) -> Result<(), CancellationTokenError> { + let mut cancelled = self.cancelled.lock()?; + if *cancelled { + return Ok(()); + } + + *cancelled = true; + + let mut handlers = self.handler.lock()?; + while let Some(handler) = handlers.pop_front() { + handler.cancelled(); + } + + let mut waiters = self.waiters.lock()?; + waiters.retain_mut(|sender| sender.unbounded_send(()).is_ok()); + + Ok(()) + } +} + +#[derive(Debug, uniffi::Error, thiserror::Error)] +pub enum CancellationTokenError { + #[error("Error acquiring lock")] + Locking, +} + +impl From> for CancellationTokenError { + fn from(_: PoisonError) -> Self { + CancellationTokenError::Locking + } +} + +impl From for CancellationTokenError { + fn from(_: TryRecvError) -> Self { + CancellationTokenError::Locking + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::Duration; + use tokio::time::{sleep, timeout}; + + #[tokio::test] + async fn test_wait_for_cancellation_returns_immediately_when_already_cancelled() { + let token = CancellationToken::new(); + token.cancel().unwrap(); + + let result = token.wait_for_cancellation().await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_wait_for_cancellation_waits_until_cancelled() { + let token = CancellationToken::new(); + let token_clone = std::sync::Arc::new(token); + let token_for_task = token_clone.clone(); + + let wait_task = tokio::spawn(async move { token_for_task.wait_for_cancellation().await }); + + sleep(Duration::from_millis(10)).await; + assert!(!wait_task.is_finished()); + + token_clone.cancel().unwrap(); + + let result = wait_task.await.unwrap(); + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_wait_for_cancellation_multiple_waiters() { + let token = std::sync::Arc::new(CancellationToken::new()); + + let mut tasks = Vec::new(); + for _ in 0..5 { + let token_clone = token.clone(); + let task = tokio::spawn(async move { token_clone.wait_for_cancellation().await }); + tasks.push(task); + } + + sleep(Duration::from_millis(10)).await; + for task in &tasks { + assert!(!task.is_finished()); + } + + token.cancel().unwrap(); + + for task in tasks { + let result = task.await.unwrap(); + assert!(result.is_ok()); + } + } + + #[tokio::test] + async fn test_wait_for_cancellation_timeout_when_not_cancelled() { + let token = CancellationToken::new(); + + let result = timeout(Duration::from_millis(50), token.wait_for_cancellation()).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_wait_for_cancellation_with_concurrent_cancel() { + let token = std::sync::Arc::new(CancellationToken::new()); + let token_for_wait = token.clone(); + let token_for_cancel = token.clone(); + + let wait_task = tokio::spawn(async move { token_for_wait.wait_for_cancellation().await }); + + let cancel_task = tokio::spawn(async move { + sleep(Duration::from_millis(20)).await; + token_for_cancel.cancel() + }); + + let wait_result = wait_task.await.unwrap(); + let cancel_result = cancel_task.await.unwrap(); + + assert!(wait_result.is_ok()); + assert!(cancel_result.is_ok()); + } + + #[derive(Debug)] + struct TestHandler { + called: std::sync::Arc, + } + + impl TestHandler { + fn new() -> (Self, std::sync::Arc) { + let called = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + (Self { called: called.clone() }, called) + } + } + + impl CancellationHandler for TestHandler { + fn cancelled(&self) { + self.called.store(true, std::sync::atomic::Ordering::SeqCst); + } + } + + #[tokio::test] + async fn test_register_handler_single_handler() { + let token = CancellationToken::new(); + let (handler, called_flag) = TestHandler::new(); + + token.register_handler(Arc::new(handler)).unwrap(); + + assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); + + token.cancel().unwrap(); + + assert!(called_flag.load(std::sync::atomic::Ordering::SeqCst)); + } + + #[tokio::test] + async fn test_register_handler_multiple_handlers() { + let token = CancellationToken::new(); + let (handler1, called_flag1) = TestHandler::new(); + let (handler2, called_flag2) = TestHandler::new(); + let (handler3, called_flag3) = TestHandler::new(); + + token.register_handler(Arc::new(handler1)).unwrap(); + token.register_handler(Arc::new(handler2)).unwrap(); + token.register_handler(Arc::new(handler3)).unwrap(); + + assert!(!called_flag1.load(std::sync::atomic::Ordering::SeqCst)); + assert!(!called_flag2.load(std::sync::atomic::Ordering::SeqCst)); + assert!(!called_flag3.load(std::sync::atomic::Ordering::SeqCst)); + + token.cancel().unwrap(); + + assert!(called_flag1.load(std::sync::atomic::Ordering::SeqCst)); + assert!(called_flag2.load(std::sync::atomic::Ordering::SeqCst)); + assert!(called_flag3.load(std::sync::atomic::Ordering::SeqCst)); + } + + #[tokio::test] + async fn test_register_handler_after_cancellation() { + let token = CancellationToken::new(); + let (handler, called_flag) = TestHandler::new(); + + token.cancel().unwrap(); + + token.register_handler(Arc::new(handler)).unwrap(); + + assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); + } + + #[tokio::test] + async fn test_register_handler_with_wait_for_cancellation() { + let token = std::sync::Arc::new(CancellationToken::new()); + let (handler, called_flag) = TestHandler::new(); + + token.register_handler(Arc::new(handler)).unwrap(); + + let token_for_wait = token.clone(); + let wait_task = tokio::spawn(async move { + token_for_wait.wait_for_cancellation().await + }); + + sleep(Duration::from_millis(10)).await; + assert!(!wait_task.is_finished()); + assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); + + token.cancel().unwrap(); + + let result = wait_task.await.unwrap(); + assert!(result.is_ok()); + assert!(called_flag.load(std::sync::atomic::Ordering::SeqCst)); + } +} diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 50756d6a8..245eb3690 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -13,6 +13,7 @@ pub mod api_client; pub mod api_error; pub mod application_passwords; pub mod auth; +pub mod cancellation; pub mod categories; pub mod comments; pub mod date; diff --git a/wp_api/src/middleware.rs b/wp_api/src/middleware.rs index 82080bd60..b1d70f856 100644 --- a/wp_api/src/middleware.rs +++ b/wp_api/src/middleware.rs @@ -236,11 +236,9 @@ mod tests { mod api_discovery_authentication_middleware { use crate::{ - api_error::MediaUploadRequestExecutionError, - request::{ - WpNetworkHeaderMap, - endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, - }, + api_error::MediaUploadRequestExecutionError, cancellation::CancellationToken, request::{ + endpoint::{media_endpoint::MediaUploadRequest, WpEndpointUrl}, WpNetworkHeaderMap + } }; use super::*; @@ -265,6 +263,7 @@ mod tests { async fn upload_media( &self, _: Arc, + _cancellation_token: Option>, ) -> Result { Err(MediaUploadRequestExecutionError::RequestExecutionFailed { status_code: None, @@ -372,11 +371,9 @@ mod tests { mod retry_after_middleware { use super::*; use crate::{ - api_error::MediaUploadRequestExecutionError, - request::{ - WpNetworkHeaderMap, - endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, - }, + api_error::MediaUploadRequestExecutionError, cancellation::CancellationToken, request::{ + endpoint::{media_endpoint::MediaUploadRequest, WpEndpointUrl}, WpNetworkHeaderMap + } }; use async_trait::async_trait; use http::HeaderMap; @@ -413,6 +410,7 @@ mod tests { async fn upload_media( &self, _: Arc, + _cancellation_token: Option>, ) -> Result { Err(MediaUploadRequestExecutionError::RequestExecutionFailed { status_code: None, diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 0a435db1f..ddc4e428c 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -5,6 +5,7 @@ use crate::{ RequestExecutionErrorReason, WpApiError, WpErrorCode, }, auth::WpAuthenticationProvider, + cancellation::CancellationToken, url_query::{FromUrlQueryPairs, UrlQueryPairsMap}, }; use base64::Engine; @@ -148,6 +149,7 @@ pub trait RequestExecutor: Send + Sync { async fn upload_media( &self, media_upload_request: Arc, + cancellation_token: Option>, ) -> Result; async fn sleep(&self, millis: u64); diff --git a/wp_api/src/request/endpoint/media_endpoint.rs b/wp_api/src/request/endpoint/media_endpoint.rs index 72844035b..381415c7d 100644 --- a/wp_api/src/request/endpoint/media_endpoint.rs +++ b/wp_api/src/request/endpoint/media_endpoint.rs @@ -1,6 +1,7 @@ use super::{AsNamespace, DerivedRequest, WpEndpointUrl, WpNamespace}; use crate::{ api_error::WpApiError, + cancellation::CancellationToken, media::{MediaCreateParams, MediaId, MediaListParams, MediaUpdateParams, MediaWithEditContext}, request::{ CONTENT_TYPE_MULTIPART, NetworkRequestAccessor, ParsedResponse, RequestMethod, @@ -188,13 +189,14 @@ impl MediaRequestExecutor { file_path: String, file_content_type: String, request_id: Option>, + cancellation_token: Option>, ) -> Result { let request = self .request_builder .create(params, file_path, file_content_type, request_id); self.delegate .request_executor - .upload_media(Arc::new(request)) + .upload_media(Arc::new(request), cancellation_token) .await? .parse() } diff --git a/wp_api/src/reqwest_request_executor.rs b/wp_api/src/reqwest_request_executor.rs index 00a23f4bb..62b704296 100644 --- a/wp_api/src/reqwest_request_executor.rs +++ b/wp_api/src/reqwest_request_executor.rs @@ -2,12 +2,9 @@ use crate::{ api_error::{ InvalidSslErrorReason, MediaUploadRequestExecutionError, RequestExecutionError, RequestExecutionErrorReason, - }, - request::{ - NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, - WpNetworkRequest, WpNetworkResponse, endpoint::media_endpoint::MediaUploadRequest, - user_agent, - }, + }, cancellation::CancellationToken, request::{ + endpoint::media_endpoint::MediaUploadRequest, user_agent, NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, WpNetworkRequest, WpNetworkResponse + } }; use async_trait::async_trait; use h2::Error as Http2Error; @@ -150,6 +147,7 @@ impl RequestExecutor for ReqwestRequestExecutor { async fn upload_media( &self, media_upload_request: Arc, + _cancellation_token: Option>, ) -> Result { self.upload_media_request(media_upload_request) .await From 703a4a639a6cb7c3e9110315f24ea54434c74326 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 16 Sep 2025 22:18:17 +1200 Subject: [PATCH 03/22] Generate cancellation variants --- .../Sources/wordpress-api/Middleware.swift | 3 +- .../wordpress-api/SafeRequestExecutor.swift | 34 +++++---- .../wordpress-api/Support/HTTPStubs.swift | 5 +- .../wordpress-api/WordPressAPITests.swift | 3 +- wp_api/src/cancellation.rs | 16 +++-- wp_api/src/login/login_client.rs | 5 ++ wp_api/src/middleware.rs | 49 ++++++++++--- wp_api/src/request.rs | 3 +- wp_api/src/reqwest_request_executor.rs | 11 ++- wp_api/src/wordpress_org/client.rs | 5 +- wp_api_integration_tests/src/mock.rs | 7 +- .../tests/test_app_notifier_immut.rs | 9 ++- .../tests/test_media_err.rs | 5 ++ .../tests/test_media_mut.rs | 1 + wp_derive_request_builder/src/generate.rs | 8 ++- .../generate/helpers_to_generate_tokens.rs | 69 ++++++++++++++++++- 16 files changed, 189 insertions(+), 44 deletions(-) diff --git a/native/swift/Sources/wordpress-api/Middleware.swift b/native/swift/Sources/wordpress-api/Middleware.swift index 1e4524e23..8302a394b 100644 --- a/native/swift/Sources/wordpress-api/Middleware.swift +++ b/native/swift/Sources/wordpress-api/Middleware.swift @@ -4,7 +4,8 @@ public final class DebugMiddleware: WpApiMiddleware { public func process( requestExecutor: any WordPressAPIInternal.RequestExecutor, response: WordPressAPIInternal.WpNetworkResponse, - request: WordPressAPIInternal.WpNetworkRequest + request: WordPressAPIInternal.WpNetworkRequest, + cancellationToken: CancellationToken? ) async throws -> WordPressAPIInternal.WpNetworkResponse { debugPrint("Performed request: \(String(describing: try? request.buildURLRequest(additionalHeaders: [:])))") debugPrint("Received response: \(response)") diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index acfef5eba..db471fc41 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -10,7 +10,7 @@ import Combine #endif public protocol SafeRequestExecutor: RequestExecutor, Sendable { - func execute(_ request: WpNetworkRequest) async -> Result + func execute(_ request: WpNetworkRequest, cancellationToken: CancellationToken?) async -> Result func uploadMedia( mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken? @@ -24,8 +24,8 @@ public protocol SafeRequestExecutor: RequestExecutor, Sendable { } extension SafeRequestExecutor { - public func execute(request: WpNetworkRequest) async throws -> WpNetworkResponse { - let result = await execute(request) + public func execute(request: WpNetworkRequest, cancellationToken: CancellationToken?) async throws -> WpNetworkResponse { + let result = await execute(request, cancellationToken: cancellationToken) return try result.get() } @@ -58,24 +58,15 @@ public final class WpRequestExecutor: SafeRequestExecutor { self.additionalHttpHeadersForAllRequests = headers } - public func execute(_ request: WpNetworkRequest) async -> Result { - await perform(request) + public func execute(_ request: WpNetworkRequest, cancellationToken: CancellationToken?) async -> Result { + await perform(request, cancellationToken: cancellationToken) } public func uploadMedia( mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken? ) async -> Result { - if let cancellationToken { - let requestId = mediaUploadRequest.requestId() - await cancellationHandlers.whenCancelling(cancellationToken) { - Task { [weak self] in - await self?.cancelRequest(withId: requestId) - } - } - } - - let result = (await perform(mediaUploadRequest)) + (await perform(mediaUploadRequest, cancellationToken: cancellationToken)) .mapError { error in switch error { case let .RequestExecutionFailed(statusCode, redirects, reason): @@ -86,6 +77,19 @@ public final class WpRequestExecutor: SafeRequestExecutor { ) } } + } + + func perform(_ request: NetworkRequestContent, cancellationToken: CancellationToken?) async -> Result { + if let cancellationToken { + let requestId = request.requestId() + await cancellationHandlers.whenCancelling(cancellationToken) { + Task { [weak self] in + await self?.cancelRequest(withId: requestId) + } + } + } + + let result = await perform(request) if let cancellationToken { await cancellationHandlers.remove(cancellationToken) diff --git a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift index 24806f4ed..03d20b0bf 100644 --- a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift +++ b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift @@ -25,7 +25,10 @@ final class HTTPStubs: SafeRequestExecutor { self } - public func execute(_ request: WpNetworkRequest) async -> Result { + public func execute( + _ request: WpNetworkRequest, + cancellationToken: CancellationToken? + ) async -> Result { if let response = stub(for: request) { return .success(response) } diff --git a/native/swift/Tests/wordpress-api/WordPressAPITests.swift b/native/swift/Tests/wordpress-api/WordPressAPITests.swift index 3143ad936..c3e8e627a 100644 --- a/native/swift/Tests/wordpress-api/WordPressAPITests.swift +++ b/native/swift/Tests/wordpress-api/WordPressAPITests.swift @@ -79,7 +79,8 @@ private actor CounterMiddleware: Middleware { func process( requestExecutor: RequestExecutor, response: WpNetworkResponse, - request: WpNetworkRequest + request: WpNetworkRequest, + cancellationToken: CancellationToken? ) async throws -> WpNetworkResponse { count += 1 return response diff --git a/wp_api/src/cancellation.rs b/wp_api/src/cancellation.rs index bd383651f..1e49cca42 100644 --- a/wp_api/src/cancellation.rs +++ b/wp_api/src/cancellation.rs @@ -37,7 +37,10 @@ impl CancellationToken { self.uuid.uuid_string() } - pub fn register_handler(&self, handler: Arc) -> Result<(), CancellationTokenError> { + pub fn register_handler( + &self, + handler: Arc, + ) -> Result<(), CancellationTokenError> { let mut handlers = self.handler.lock()?; handlers.push_back(handler); Ok(()) @@ -189,7 +192,12 @@ mod tests { impl TestHandler { fn new() -> (Self, std::sync::Arc) { let called = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); - (Self { called: called.clone() }, called) + ( + Self { + called: called.clone(), + }, + called, + ) } } @@ -255,9 +263,7 @@ mod tests { token.register_handler(Arc::new(handler)).unwrap(); let token_for_wait = token.clone(); - let wait_task = tokio::spawn(async move { - token_for_wait.wait_for_cancellation().await - }); + let wait_task = tokio::spawn(async move { token_for_wait.wait_for_cancellation().await }); sleep(Duration::from_millis(10)).await; assert!(!wait_task.is_finished()); diff --git a/wp_api/src/login/login_client.rs b/wp_api/src/login/login_client.rs index c95f66999..97295f2a9 100644 --- a/wp_api/src/login/login_client.rs +++ b/wp_api/src/login/login_client.rs @@ -299,6 +299,7 @@ impl WpLoginClient { body: None, } .into(), + None, ) .await } @@ -326,6 +327,7 @@ impl WpLoginClient { body: None, } .into(), + None, ) .await } @@ -411,6 +413,7 @@ impl WpLoginClient { body: Some(Arc::new(WpNetworkRequestBody::new(r#"system.listMethods"#.as_bytes().to_vec()))), } .into(), + None, ) .await // It's very likely xml-rpc is blocked by the hosting provider (the request has not reached to WordPress), @@ -449,6 +452,7 @@ impl WpLoginClient { body: None, } .into(), + None, ) .await .map_err(|error| XmlrpcDiscoveryError::FetchHomepage { error })?; @@ -467,6 +471,7 @@ impl WpLoginClient { body: None, } .into(), + None, ) .await .map_err(|_| XmlrpcDiscoveryError::Disabled { diff --git a/wp_api/src/middleware.rs b/wp_api/src/middleware.rs index b1d70f856..36aa7dc6e 100644 --- a/wp_api/src/middleware.rs +++ b/wp_api/src/middleware.rs @@ -1,6 +1,7 @@ use crate::{ api_client::IsWpApiClientDelegate, api_error::{RequestExecutionError, RequestExecutionErrorReason}, + cancellation::CancellationToken, request::{RequestExecutor, WpNetworkRequest, WpNetworkResponse}, }; use std::{fmt::Debug, sync::Arc, time::Duration}; @@ -22,12 +23,18 @@ impl WpApiMiddlewarePipeline { request_executor: Arc, response: WpNetworkResponse, request: Arc, + cancellation_token: Option>, ) -> Result { let mut response = response; for middleware in &self.middlewares { response = middleware - .process(request_executor.clone(), response, request.clone()) + .process( + request_executor.clone(), + response, + request.clone(), + cancellation_token.clone(), + ) .await?; } @@ -71,6 +78,7 @@ pub trait WpApiMiddleware: Send + Sync + Debug { request_executor: Arc, response: WpNetworkResponse, request: Arc, + cancellation_token: Option>, ) -> Result; } @@ -84,15 +92,20 @@ pub trait PerformsRequests { async fn perform( &self, request: Arc, + cancellation_token: Option>, ) -> Result { let pipeline = &self.get_middleware_pipeline(); - let response = self.get_request_executor().execute(request.clone()).await?; + let response = self + .get_request_executor() + .execute(request.clone(), cancellation_token.clone()) + .await?; let response = pipeline .process( self.get_request_executor().clone(), response, request.clone(), + cancellation_token.clone(), ) .await?; @@ -153,6 +166,7 @@ impl WpApiMiddleware for RetryAfterMiddleware { request_executor: Arc, response: WpNetworkResponse, request: Arc, + cancellation_token: Option>, ) -> Result { let mut response = response; @@ -176,8 +190,11 @@ impl WpApiMiddleware for RetryAfterMiddleware { ) .await; let new_request = Arc::new(request.clone_with_incremented_retry_count()); - response = request_executor.execute(new_request.clone()).await?; - self.process(request_executor, response, new_request).await + response = request_executor + .execute(new_request.clone(), cancellation_token.clone()) + .await?; + self.process(request_executor, response, new_request, cancellation_token) + .await } else { // We have no idea how long to wait so we shouldn't try Ok(response) @@ -210,6 +227,7 @@ impl WpApiMiddleware for ApiDiscoveryAuthenticationMiddleware { request_executor: Arc, response: WpNetworkResponse, request: Arc, + cancellation_token: Option>, ) -> Result { if response.request_header_map.has_http_authentication() { // Request was already authenticated @@ -225,6 +243,7 @@ impl WpApiMiddleware for ApiDiscoveryAuthenticationMiddleware { request .adding_http_authentication(&self.username, &self.password) .into(), + cancellation_token, ) .await } @@ -236,9 +255,12 @@ mod tests { mod api_discovery_authentication_middleware { use crate::{ - api_error::MediaUploadRequestExecutionError, cancellation::CancellationToken, request::{ - endpoint::{media_endpoint::MediaUploadRequest, WpEndpointUrl}, WpNetworkHeaderMap - } + api_error::MediaUploadRequestExecutionError, + cancellation::CancellationToken, + request::{ + WpNetworkHeaderMap, + endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, + }, }; use super::*; @@ -256,6 +278,7 @@ mod tests { async fn execute( &self, request: Arc, + _cancellation_token: Option>, ) -> Result { (self.execute_fn)(request) } @@ -363,6 +386,7 @@ mod tests { request_header_map: Arc::new(map.into()), }, WpNetworkRequest::get(WpEndpointUrl("unused".to_string())).into(), + None, ) .await } @@ -371,9 +395,12 @@ mod tests { mod retry_after_middleware { use super::*; use crate::{ - api_error::MediaUploadRequestExecutionError, cancellation::CancellationToken, request::{ - endpoint::{media_endpoint::MediaUploadRequest, WpEndpointUrl}, WpNetworkHeaderMap - } + api_error::MediaUploadRequestExecutionError, + cancellation::CancellationToken, + request::{ + WpNetworkHeaderMap, + endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, + }, }; use async_trait::async_trait; use http::HeaderMap; @@ -390,6 +417,7 @@ mod tests { async fn execute( &self, _: Arc, + _cancellation_token: Option>, ) -> Result { if self.first_request.load(Ordering::Relaxed) { println!("First mock request; returning 429.."); @@ -458,6 +486,7 @@ mod tests { Arc::new(foo_executor), rate_limit_exceeded_response(), WpNetworkRequest::get(WpEndpointUrl("unused".to_string())).into(), + None, ) .await } diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index ddc4e428c..bd64216ab 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -144,6 +144,7 @@ pub trait RequestExecutor: Send + Sync { async fn execute( &self, request: Arc, + cancellation_token: Option>, ) -> Result; async fn upload_media( @@ -789,7 +790,7 @@ pub async fn fetch_authentication_state( ApplicationPasswordsRequestBuilder::new(api_url_resolver, authentication_provider) .retrieve_current_with_edit_context() .into(); - let response = request_executor.execute(request).await?; + let response = request_executor.execute(request, None).await?; let parsed_res: Result< ApplicationPasswordsRequestRetrieveCurrentWithEditContextResponse, WpApiError, diff --git a/wp_api/src/reqwest_request_executor.rs b/wp_api/src/reqwest_request_executor.rs index 62b704296..057ad9a32 100644 --- a/wp_api/src/reqwest_request_executor.rs +++ b/wp_api/src/reqwest_request_executor.rs @@ -2,9 +2,13 @@ use crate::{ api_error::{ InvalidSslErrorReason, MediaUploadRequestExecutionError, RequestExecutionError, RequestExecutionErrorReason, - }, cancellation::CancellationToken, request::{ - endpoint::media_endpoint::MediaUploadRequest, user_agent, NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, WpNetworkRequest, WpNetworkResponse - } + }, + cancellation::CancellationToken, + request::{ + NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, + WpNetworkRequest, WpNetworkResponse, endpoint::media_endpoint::MediaUploadRequest, + user_agent, + }, }; use async_trait::async_trait; use h2::Error as Http2Error; @@ -140,6 +144,7 @@ impl RequestExecutor for ReqwestRequestExecutor { async fn execute( &self, request: Arc, + _cancellation_token: Option>, ) -> Result { self.async_request(request).await.map_err(|e| e.into()) } diff --git a/wp_api/src/wordpress_org/client.rs b/wp_api/src/wordpress_org/client.rs index b92a895f6..340ab8fc6 100644 --- a/wp_api/src/wordpress_org/client.rs +++ b/wp_api/src/wordpress_org/client.rs @@ -134,7 +134,10 @@ impl WordPressOrgApiClient { where T: DeserializeOwned, { - let response = self.request_executor.execute(Arc::new(request)).await?; + let response = self + .request_executor + .execute(Arc::new(request), None) + .await?; Self::parse(response) } diff --git a/wp_api_integration_tests/src/mock.rs b/wp_api_integration_tests/src/mock.rs index 32456d6c6..573666f56 100644 --- a/wp_api_integration_tests/src/mock.rs +++ b/wp_api_integration_tests/src/mock.rs @@ -1,6 +1,9 @@ use async_trait::async_trait; use std::sync::Arc; -use wp_api::{prelude::*, request::endpoint::media_endpoint::MediaUploadRequest}; +use wp_api::{ + cancellation::CancellationToken, prelude::*, + request::endpoint::media_endpoint::MediaUploadRequest, +}; #[derive(Debug)] pub struct MockExecutor { @@ -27,6 +30,7 @@ impl RequestExecutor for MockExecutor { async fn execute( &self, request: Arc, + _cancellation_token: Option>, ) -> Result { (self.execute_fn)(request) } @@ -34,6 +38,7 @@ impl RequestExecutor for MockExecutor { async fn upload_media( &self, media_upload_request: Arc, + _cancellation_token: Option>, ) -> Result { (self.upload_media_fn)(media_upload_request) } diff --git a/wp_api_integration_tests/tests/test_app_notifier_immut.rs b/wp_api_integration_tests/tests/test_app_notifier_immut.rs index c99a4244c..fe2832c54 100644 --- a/wp_api_integration_tests/tests/test_app_notifier_immut.rs +++ b/wp_api_integration_tests/tests/test_app_notifier_immut.rs @@ -2,7 +2,7 @@ use std::sync::{ Mutex, atomic::{AtomicBool, Ordering}, }; -use wp_api::users::UserListParams; +use wp_api::{cancellation::CancellationToken, users::UserListParams}; use wp_api_integration_tests::prelude::*; #[tokio::test] @@ -117,19 +117,22 @@ impl RequestExecutor for TrackedRequestExecutor { async fn execute( &self, request: Arc, + cancellation_token: Option>, ) -> Result { self.requested_urls .lock() .unwrap() .push(request.url().0.clone()); - self.executor.execute(request).await + self.executor.execute(request, cancellation_token).await } async fn upload_media( &self, media_upload_request: Arc, + cancellation_token: Option>, ) -> Result { - self.upload_media(media_upload_request).await + self.upload_media(media_upload_request, cancellation_token) + .await } async fn sleep(&self, _: u64) {} diff --git a/wp_api_integration_tests/tests/test_media_err.rs b/wp_api_integration_tests/tests/test_media_err.rs index 10d24c2ed..d21657f4d 100644 --- a/wp_api_integration_tests/tests/test_media_err.rs +++ b/wp_api_integration_tests/tests/test_media_err.rs @@ -1,5 +1,6 @@ use wp_api::{ auth::WpAuthenticationProvider, + cancellation::CancellationToken, media::{MediaCreateParams, MediaId, MediaListParams, MediaUpdateParams}, posts::WpApiParamPostsOrderBy, prelude::*, @@ -18,6 +19,7 @@ async fn create_media_err_cannot_create() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, + None, ) .await .assert_wp_error(WpErrorCode::CannotCreate) @@ -33,6 +35,7 @@ async fn create_media_err_upload_no_data() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, + None, ) .await .assert_wp_error(WpErrorCode::UploadNoData) @@ -199,6 +202,7 @@ impl RequestExecutor for MediaErrNetworking { async fn execute( &self, _request: Arc, + _cancellation_token: Option>, ) -> Result { Err(RequestExecutionError::RequestExecutionFailed { status_code: None, @@ -212,6 +216,7 @@ impl RequestExecutor for MediaErrNetworking { async fn upload_media( &self, media_upload_request: Arc, + _cancellation_token: Option>, ) -> Result { let mut request = self .client diff --git a/wp_api_integration_tests/tests/test_media_mut.rs b/wp_api_integration_tests/tests/test_media_mut.rs index fe6429c7a..2271644d9 100644 --- a/wp_api_integration_tests/tests/test_media_mut.rs +++ b/wp_api_integration_tests/tests/test_media_mut.rs @@ -19,6 +19,7 @@ async fn upload_media() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, + None, ) .await .assert_response(); diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 009fa0144..51ae0c2c8 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -65,6 +65,8 @@ fn generate_async_request_executor( variant.attr.request_type, &context_and_filter_handler, ); + let fn_signature_cancellable = append_cancellation_token_param(fn_signature.clone()); + let fn_signature_body = invoke_cancellation_variant(fn_signature.clone()); let response_type_ident = ident_response_type( &parsed_enum.enum_ident, &variant.variant_ident, @@ -72,13 +74,17 @@ fn generate_async_request_executor( ); quote! { pub async #fn_signature -> Result<#response_type_ident, #error_type> { + #fn_signature_body + } + + pub async #fn_signature_cancellable -> Result<#response_type_ident, #error_type> { use #crate_ident::api_error::MaybeWpError; use #crate_ident::middleware::PerformsRequests; use #crate_ident::request::NetworkRequestAccessor; let perform_request = async || { #request_from_request_builder let request_url: String = request.url().into(); - let response = self.perform(std::sync::Arc::new(request)).await?; + let response = self.perform(std::sync::Arc::new(request), cancellation_token.clone()).await?; let response_status_code = response.status_code; let parsed_response = response.parse(); let unauthorized = parsed_response.is_unauthorized_error().unwrap_or_default() || (response_status_code == 401 && self.fetch_authentication_state().await.map(|auth_state| auth_state.is_unauthorized()).unwrap_or_default()); 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 cce078704..8e3987e43 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 @@ -1,7 +1,7 @@ use convert_case::{Case, Casing}; use proc_macro2::{TokenStream, TokenTree}; use quote::{format_ident, quote}; -use syn::Ident; +use syn::{FnArg, Ident, Pat, parse_quote}; use super::{ContextAndFilterHandler, PartOf, WpContext}; use crate::{ @@ -84,6 +84,45 @@ pub fn fn_signature( quote! { fn #fn_name(&self, #url_params #provided_param #fields_param) } } +pub fn append_cancellation_token_param(input: TokenStream) -> TokenStream { + let mut signature: syn::Signature = syn::parse2(input).unwrap(); + + let original_name = signature.ident.to_string(); + signature.ident = format_ident!("{}_cancellation", original_name); + + let new_arg: FnArg = parse_quote! { cancellation_token: Option> }; + signature.inputs.push(new_arg); + + quote! { #signature } +} + +pub fn invoke_cancellation_variant(input: TokenStream) -> TokenStream { + let signature: syn::Signature = syn::parse2(input).unwrap(); + + let fn_name = &signature.ident; + let fn_name = format_ident!("{}_cancellation", fn_name); + + let param_names: Vec<_> = signature + .inputs + .iter() + .filter_map(|arg| { + match arg { + FnArg::Typed(pat_type) => match &*pat_type.pat { + Pat::Ident(ident) => Some(ident.ident.clone()), + _ => None, + }, + FnArg::Receiver(_) => None, // Skip 'self' parameter + } + }) + .collect(); + + if param_names.is_empty() { + quote! { self.#fn_name(None).await } + } else { + quote! { self.#fn_name(#(#param_names),*, None).await } + } +} + pub fn fn_url_params(url_parts: &[UrlPart]) -> TokenStream { let params = url_parts.iter().filter_map(|p| { if let UrlPart::Dynamic(p) = p { @@ -1243,4 +1282,32 @@ mod tests { tokens: quote! { crate::SparseUserField }, }) } + + #[rstest] + #[case( + quote! { fn list(&self, params: &UserListParams) }, + "fn list_cancellation (& self , params : & UserListParams , cancellation_token : Option < std :: sync :: Arc < crate :: cancellation :: CancellationToken > >)" + )] + #[case( + quote! { fn list(&self) }, + "fn list_cancellation (& self , cancellation_token : Option < std :: sync :: Arc < crate :: cancellation :: CancellationToken > >)" + )] + fn test_append_cancellation_token_param(#[case] input: TokenStream, #[case] expected: &str) { + let result = append_cancellation_token_param(input); + assert_eq!(result.to_string(), expected); + } + + #[rstest] + #[case( + quote! { fn list(&self, params: &UserListParams, fields: &[SparseUserField]) }, + "self . list_cancellation (params , fields , None) . await" + )] + #[case( + quote! { fn list(&self) }, + "self . list_cancellation (None) . await" + )] + fn test_invoke_cancellation_variant(#[case] input: TokenStream, #[case] expected: &str) { + let result = invoke_cancellation_variant(input); + assert_eq!(result.to_string(), expected); + } } From 3f8e4c01356739e1dfc6ffd17ae3f33babf91b5b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 12:34:31 +1200 Subject: [PATCH 04/22] Add feature flag to disable exporting uncancellable APIs --- wp_api/Cargo.toml | 1 + wp_derive_request_builder/Cargo.toml | 1 + wp_derive_request_builder/src/generate.rs | 33 +++++++++++++++++------ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/wp_api/Cargo.toml b/wp_api/Cargo.toml index 8b67c2536..7f27f46f8 100644 --- a/wp_api/Cargo.toml +++ b/wp_api/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" edition = "2024" [features] +without-uncancellable-endpoints = ["wp_derive_request_builder/without-uncancellable-endpoints"] integration-tests = [] reqwest-request-executor = ["dep:reqwest", "dep:tokio", "dep:hyper-util", "dep:rustls", "dep:hickory-resolver", "dep:hyper", "dep:h2"] diff --git a/wp_derive_request_builder/Cargo.toml b/wp_derive_request_builder/Cargo.toml index 564d333ad..72757b5ab 100644 --- a/wp_derive_request_builder/Cargo.toml +++ b/wp_derive_request_builder/Cargo.toml @@ -6,6 +6,7 @@ autotests = false [features] generate_request_builder = [] +without-uncancellable-endpoints = [] [lib] proc-macro = true diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 51ae0c2c8..9f37b87fc 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -39,17 +39,17 @@ fn generate_async_request_executor( let generated_request_builder_ident = &config.generated_idents.request_builder; let generated_request_executor_ident = &config.generated_idents.request_executor; - let functions = parsed_enum.variants.iter().map(|variant| { + let (exported_functions, unexported_functions) = parsed_enum.variants.iter().fold((vec![], vec![]), |(mut exported, mut unexported), variant| { let url_parts = variant.attr.url_parts.as_slice(); let params_type = &variant.attr.params; - ContextAndFilterHandler::from_request_type( + (exported, unexported) = ContextAndFilterHandler::from_request_type( variant.attr.request_type, variant.attr.filter_by.clone(), &variant.attr.available_contexts, ) .into_iter() - .map(|context_and_filter_handler| { + .fold((exported, unexported), |(mut exported, mut unexported), context_and_filter_handler| { let request_from_request_builder = fn_body_get_request_from_request_builder( &variant.variant_ident, url_parts, @@ -72,11 +72,13 @@ fn generate_async_request_executor( &variant.variant_ident, &context_and_filter_handler, ); - quote! { + + let uncancellable = quote! { pub async #fn_signature -> Result<#response_type_ident, #error_type> { #fn_signature_body } - + }; + let cancellable = quote! { pub async #fn_signature_cancellable -> Result<#response_type_ident, #error_type> { use #crate_ident::api_error::MaybeWpError; use #crate_ident::middleware::PerformsRequests; @@ -112,9 +114,20 @@ fn generate_async_request_executor( parsed_response } + }; + + exported.push(cancellable); + + if cfg!(feature = "without-uncancellable-endpoints") { + unexported.push(uncancellable); + } else { + exported.push(uncancellable); } - }) - .collect::() + + (exported, unexported) + }); + + (exported, unexported) }); let generated_return_types = parsed_enum.variants.iter().map(|variant| { @@ -220,12 +233,16 @@ fn generate_async_request_executor( } #[uniffi::export] impl #generated_request_executor_ident { - #(#functions)* + #(#exported_functions)* pub async fn fetch_authentication_state(&self) -> Result<#crate_ident::request::AuthenticationState, #error_type> { #crate_ident::request::fetch_authentication_state(self.delegate.request_executor.clone(), self.api_url_resolver.clone(), self.delegate.auth_provider.clone()).await } } + + impl #generated_request_executor_ident { + #(#unexported_functions)* + } } } From f7149ad5d2c827323a4c40f393b3bf3fb492a71d Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 12:42:26 +1200 Subject: [PATCH 05/22] Update swift binding generation to generate cancellable APIs --- Makefile | 6 +- native/swift/Tools/.gitignore | 8 + native/swift/Tools/Package.resolved | 33 ++ native/swift/Tools/Package.swift | 30 ++ .../Sources/GenerateCancellable/main.swift | 360 ++++++++++++++++++ scripts/swift-bindings.sh | 11 + 6 files changed, 445 insertions(+), 3 deletions(-) create mode 100644 native/swift/Tools/.gitignore create mode 100644 native/swift/Tools/Package.resolved create mode 100644 native/swift/Tools/Package.swift create mode 100644 native/swift/Tools/Sources/GenerateCancellable/main.swift diff --git a/Makefile b/Makefile index 729deb4fa..780c954af 100644 --- a/Makefile +++ b/Makefile @@ -102,7 +102,7 @@ _build-apple-%-tvos _build-apple-%-tvos-sim _build-apple-%-watchos _build-apple- # Build the library for a specific target _build-apple-%: - cargo $(CARGO_OPTS) $(cargo_config_library) build --target $* --package wp_api --profile $(CARGO_PROFILE) + cargo $(CARGO_OPTS) $(cargo_config_library) build --target $* --features without-uncancellable-endpoints --package wp_api --profile $(CARGO_PROFILE) ./scripts/swift-bindings.sh target/$*/$(CARGO_PROFILE_DIRNAME)/libwp_api.a # Build the library for one single platform, including real device and simulator. @@ -141,7 +141,7 @@ docker-image-web: docker build -t wordpress-rs-web -f wp_rs_web/Dockerfile . --progress=plain swift-linux-library: - cargo build --release --package wp_api + cargo build --release --features without-uncancellable-endpoints --package wp_api ./scripts/swift-bindings.sh target/release/libwp_api.a mkdir -p target/release/libwordpressFFI-linux cp target/release/swift-bindings/Headers/* target/release/libwordpressFFI-linux/ @@ -272,7 +272,7 @@ validate-localizations: @# Help: Validate localization files using `wp_localization_validation` crate $(rust_docker_run) /bin/bash -c "cargo run --bin wp_localization_validation -- --localization-folder ./wp_localization/localization/" -format-swift: +fmt-swift: @# Help: Format the Swift binding code xcrun swift format -i -r native/swift/Sources/wordpress-api-wrapper diff --git a/native/swift/Tools/.gitignore b/native/swift/Tools/.gitignore new file mode 100644 index 000000000..0023a5340 --- /dev/null +++ b/native/swift/Tools/.gitignore @@ -0,0 +1,8 @@ +.DS_Store +/.build +/Packages +xcuserdata/ +DerivedData/ +.swiftpm/configuration/registries.json +.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata +.netrc diff --git a/native/swift/Tools/Package.resolved b/native/swift/Tools/Package.resolved new file mode 100644 index 000000000..52f8e4db7 --- /dev/null +++ b/native/swift/Tools/Package.resolved @@ -0,0 +1,33 @@ +{ + "originHash" : "8dd1ad804e0fca931c3e5e129e82648c4c38fe782cdd7db4b112f0f922bd5225", + "pins" : [ + { + "identity" : "swift-argument-parser", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-argument-parser.git", + "state" : { + "revision" : "309a47b2b1d9b5e991f36961c983ecec72275be3", + "version" : "1.6.1" + } + }, + { + "identity" : "swift-log", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-log.git", + "state" : { + "revision" : "ce592ae52f982c847a4efc0dd881cc9eb32d29f2", + "version" : "1.6.4" + } + }, + { + "identity" : "swift-syntax", + "kind" : "remoteSourceControl", + "location" : "https://github.com/swiftlang/swift-syntax.git", + "state" : { + "revision" : "0687f71944021d616d34d922343dcef086855920", + "version" : "600.0.1" + } + } + ], + "version" : 3 +} diff --git a/native/swift/Tools/Package.swift b/native/swift/Tools/Package.swift new file mode 100644 index 000000000..8a6561ee8 --- /dev/null +++ b/native/swift/Tools/Package.swift @@ -0,0 +1,30 @@ +// swift-tools-version: 6.2 + +import PackageDescription + +let package = Package( + name: "Tools", + platforms: [ + .macOS(.v12) + ], + products: [ + .executable(name: "generate-cancellable", targets: ["GenerateCancellable"]), + ], + dependencies: [ + .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "600.0.0"), + .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.0.0"), + .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"), + ], + targets: [ + .executableTarget( + name: "GenerateCancellable", + dependencies: [ + .product(name: "SwiftSyntax", package: "swift-syntax"), + .product(name: "SwiftParser", package: "swift-syntax"), + .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), + .product(name: "ArgumentParser", package: "swift-argument-parser"), + .product(name: "Logging", package: "swift-log"), + ] + ), + ] +) diff --git a/native/swift/Tools/Sources/GenerateCancellable/main.swift b/native/swift/Tools/Sources/GenerateCancellable/main.swift new file mode 100644 index 000000000..91fa29dd2 --- /dev/null +++ b/native/swift/Tools/Sources/GenerateCancellable/main.swift @@ -0,0 +1,360 @@ +import ArgumentParser +import Foundation +import Logging +import SwiftSyntax +import SwiftParser +import SwiftSyntaxBuilder + +@main +struct GenerateCancellable: ParsableCommand { + static let configuration = CommandConfiguration( + commandName: "generate-cancellable", + abstract: "Generate cancellable APIs" + ) + + @Argument(help: "The Swift source file to analyze") + var inputFile: String + + @Argument(help: "Output file path for generated extensions") + var output: String + + @Option(name: .long, help: "Set log level (trace, debug, info, notice, warning, error, critical)") + var logLevel: String = "info" + + mutating func run() throws { + let logger = setupLogger() + + let inputURL = URL(fileURLWithPath: inputFile) + let outputURL = URL(fileURLWithPath: output) + + logger.info("Analyzing file: \(inputURL.path)") + logger.info("Generating extensions to: \(outputURL.path)") + + guard FileManager.default.fileExists(atPath: inputURL.path) else { + throw ValidationError("Input file does not exist: \(inputURL.path)") + } + + let sourceCode = try String(contentsOf: inputURL, encoding: .utf8) + let extensionsCode = try generateExtensions(sourceCode: sourceCode, logger: logger) + + try extensionsCode.write(to: outputURL, atomically: true, encoding: .utf8) + + logger.info("Successfully generated extensions to: \(outputURL.path)") + } + + private func setupLogger() -> Logger { + let logLevelValue = Logger.Level(rawValue: logLevel.lowercased()) ?? .info + + LoggingSystem.bootstrap { label in + var handler = StreamLogHandler.standardOutput(label: label) + handler.logLevel = logLevelValue + return handler + } + + return Logger(label: "generate-cancellable") + } + + private func generateExtensions(sourceCode: String, logger: Logger) throws -> String { + let syntaxTree = Parser.parse(source: sourceCode) + + logger.debug("Parsed syntax tree successfully") + + let analyzer = CancellationAnalyzer(logger: logger) + let analysis = analyzer.analyze(syntaxTree) + + logger.info("Found \(analysis.count) RequestExecutor classes") + + let generator = ExtensionGenerator(analysis: analysis, logger: logger) + return generator.generateExtensionsCode() + } +} + +struct FunctionInfo { + let declaration: FunctionDeclSyntax + let name: String + let parameters: FunctionParameterListSyntax + let returnType: TypeSyntax? + let modifiers: DeclModifierListSyntax? + let attributes: AttributeListSyntax? + let asyncKeyword: TokenSyntax? + let throwsClause: ThrowsClauseSyntax? +} + +struct ClassAnalysis { + let className: String + let cancellationFunctions: [FunctionInfo] + let existingFunctions: [String: FunctionInfo] +} + +class CancellationAnalyzer: SyntaxVisitor { + private let logger: Logger + private var analysis: [String: ClassAnalysis] = [:] + private var currentClass: String? + private var currentCancellationFunctions: [FunctionInfo] = [] + private var currentExistingFunctions: [String: FunctionInfo] = [:] + + init(logger: Logger) { + self.logger = logger + super.init(viewMode: .sourceAccurate) + } + + func analyze(_ tree: SourceFileSyntax) -> [String: ClassAnalysis] { + walk(tree) + return analysis + } + + override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind { + let className = node.name.text + + if className.hasSuffix("RequestExecutor") { + logger.debug("Analyzing RequestExecutor class: \(className)") + + currentClass = className + currentCancellationFunctions = [] + currentExistingFunctions = [:] + + return .visitChildren + } + + return .skipChildren + } + + override func visitPost(_ node: ClassDeclSyntax) { + if let className = currentClass { + analysis[className] = ClassAnalysis( + className: className, + cancellationFunctions: currentCancellationFunctions, + existingFunctions: currentExistingFunctions + ) + + logger.debug("Found \(currentCancellationFunctions.count) cancellation functions in \(className)") + logger.debug("Found \(currentExistingFunctions.count) existing functions in \(className)") + + currentClass = nil + } + } + + override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { + guard currentClass != nil else { return .skipChildren } + + let functionName = node.name.text + + let functionInfo = FunctionInfo( + declaration: node, + name: functionName, + parameters: node.signature.parameterClause.parameters, + returnType: node.signature.returnClause?.type, + modifiers: node.modifiers, + attributes: node.attributes, + asyncKeyword: node.signature.effectSpecifiers?.asyncSpecifier, + throwsClause: node.signature.effectSpecifiers?.throwsClause + ) + + if functionName.hasSuffix("Cancellation") && hasCancellationTokenParameter(node) { + currentCancellationFunctions.append(functionInfo) + logger.trace("Found cancellation function: \(functionName)") + } else { + currentExistingFunctions[functionName] = functionInfo + } + + return .skipChildren + } + + private func hasCancellationTokenParameter(_ function: FunctionDeclSyntax) -> Bool { + let parameters = function.signature.parameterClause.parameters + guard let lastParam = parameters.last else { return false } + + let paramName = lastParam.firstName.text + let paramType = lastParam.type.description.trimmingCharacters(in: .whitespacesAndNewlines) + + return paramName == "cancellationToken" && paramType == "CancellationToken?" + } +} + +class ExtensionGenerator { + private let analysis: [String: ClassAnalysis] + private let logger: Logger + + init(analysis: [String: ClassAnalysis], logger: Logger) { + self.analysis = analysis + self.logger = logger + } + + func generateExtensionsCode() -> String { + var extensions: [ExtensionDeclSyntax] = [] + + for (className, classAnalysis) in analysis { + logger.debug("Generating extension for class: \(className)") + + let extensionMembers = generateExtensionMembers(for: classAnalysis) + + if !extensionMembers.isEmpty { + let extensionDecl = ExtensionDeclSyntax( + extensionKeyword: .keyword(.extension, trailingTrivia: .space), + extendedType: IdentifierTypeSyntax(name: .identifier(className)), + memberBlock: MemberBlockSyntax( + leftBrace: .leftBraceToken(leadingTrivia: .space, trailingTrivia: .newlines(2)), + members: MemberBlockItemListSyntax(extensionMembers), + rightBrace: .rightBraceToken(leadingTrivia: .newline) + ), + trailingTrivia: .newlines(2) + ) + extensions.append(extensionDecl) + logger.info("Generated extension for \(className) with \(extensionMembers.count) functions") + } + } + + let sourceFile = SourceFileSyntax( + statements: CodeBlockItemListSyntax( + extensions.map { ext in + CodeBlockItemSyntax(item: .decl(DeclSyntax(ext))) + } + ) + ) + + return """ + // Do not modify. This file is automatically generated. + // swiftlint:disable all + + import Foundation + + \(sourceFile.description) + """ + } + + private func generateExtensionMembers(for classAnalysis: ClassAnalysis) -> [MemberBlockItemSyntax] { + var members: [MemberBlockItemSyntax] = [] + + for cancellationFunc in classAnalysis.cancellationFunctions { + let nonCancellationName = String(cancellationFunc.name.dropLast("Cancellation".count)) + + if classAnalysis.existingFunctions[nonCancellationName] != nil { + logger.warning("Skipping generation of '\(nonCancellationName)' - function already exists in \(classAnalysis.className)") + continue + } + + logger.debug("Generating new function: \(nonCancellationName)") + + let newFunction = createNonCancellationFunction(from: cancellationFunc, name: nonCancellationName) + let memberItem = MemberBlockItemSyntax( + decl: DeclSyntax(newFunction), + trailingTrivia: .newline + ) + members.append(memberItem) + } + + return members + } + + private func createNonCancellationFunction(from cancellationFunc: FunctionInfo, name: String) -> FunctionDeclSyntax { + var parametersWithoutCancellation: FunctionParameterListSyntax + + if cancellationFunc.parameters.isEmpty || cancellationFunc.parameters.count == 1 { + parametersWithoutCancellation = FunctionParameterListSyntax([]) + } else { + var paramsArray = Array(cancellationFunc.parameters.dropLast()) + + if !paramsArray.isEmpty { + let lastIndex = paramsArray.count - 1 + paramsArray[lastIndex] = paramsArray[lastIndex].with(\.trailingComma, nil) + } + + parametersWithoutCancellation = FunctionParameterListSyntax(paramsArray) + } + + let effectSpecifiers = FunctionEffectSpecifiersSyntax( + asyncSpecifier: cancellationFunc.asyncKeyword?.with(\.leadingTrivia, .space).with(\.trailingTrivia, .space), + throwsClause: cancellationFunc.throwsClause?.with(\.leadingTrivia, .init()).with(\.trailingTrivia, .init()) + ) + + let returnClause = cancellationFunc.returnType.map { type in + let cleanType = type.with(\.leadingTrivia, .init()).with(\.trailingTrivia, .init()) + return ReturnClauseSyntax( + arrow: .arrowToken(leadingTrivia: .space, trailingTrivia: .space), + type: cleanType + ) + } + + let signature = FunctionSignatureSyntax( + parameterClause: FunctionParameterClauseSyntax( + leftParen: .leftParenToken(), + parameters: parametersWithoutCancellation, + rightParen: .rightParenToken() + ), + effectSpecifiers: effectSpecifiers, + returnClause: returnClause + ) + + let functionCallBody = createFunctionCallBody( + cancellationFunctionName: cancellationFunc.name, + parameters: parametersWithoutCancellation, + hasReturnValue: cancellationFunc.returnType != nil + ) + + let cleanModifiers: DeclModifierListSyntax + if let originalModifiers = cancellationFunc.modifiers, !originalModifiers.isEmpty { + var modifiersArray: [DeclModifierSyntax] = [] + + for (index, modifier) in originalModifiers.enumerated() { + var cleanModifier = modifier + + if modifier.name.text == "open" { + cleanModifier = cleanModifier.with(\.name, .keyword(.public)) + } + + cleanModifier = cleanModifier.with(\.leadingTrivia, index == 0 ? .spaces(4) : .space) + .with(\.trailingTrivia, .space) + + modifiersArray.append(cleanModifier) + } + + cleanModifiers = DeclModifierListSyntax(modifiersArray) + } else { + cleanModifiers = DeclModifierListSyntax([]) + } + + let funcKeywordLeadingTrivia: Trivia = cleanModifiers.isEmpty ? .spaces(4) : .init() + + return FunctionDeclSyntax( + attributes: cancellationFunc.attributes ?? AttributeListSyntax([]), + modifiers: cleanModifiers, + funcKeyword: .keyword(.func, leadingTrivia: funcKeywordLeadingTrivia, trailingTrivia: .space), + name: .identifier(name), + signature: signature, + body: functionCallBody + ) + } + + private func createFunctionCallBody( + cancellationFunctionName: String, + parameters: FunctionParameterListSyntax, + hasReturnValue: Bool + ) -> CodeBlockSyntax { + // Build parameter arguments for the function call + let parameterArguments = parameters.map { parameter in + let paramName = parameter.firstName.text + return "\(paramName): \(paramName)" + }.joined(separator: ", ") + + let functionCallArgs = parameterArguments.isEmpty ? + "cancellationToken: token" : + "\(parameterArguments), cancellationToken: token" + + return CodeBlockSyntax { + DeclSyntax( + """ + let token = CancellationToken() + return try await withTaskCancellationHandler { + try await \(raw: cancellationFunctionName)(\(raw: functionCallArgs)) + } onCancel: { + do { + try token.cancel() + } catch { + NSLog("Failed to cancel \\(#function): \\(error)") + } + } + """ + ) + } + } +} diff --git a/scripts/swift-bindings.sh b/scripts/swift-bindings.sh index 79bfd9442..0e8b5c8e1 100755 --- a/scripts/swift-bindings.sh +++ b/scripts/swift-bindings.sh @@ -7,6 +7,8 @@ if [ $# -ne 1 ]; then exit 1 fi +echo "Generating Swift bindings... (This may take a while when running for the first time.)" + module_name="libwordpressFFI" library_path=$1 output_dir="$(dirname "$library_path")/swift-bindings" @@ -49,6 +51,15 @@ done rm -f native/swift/Sources/wordpress-api-wrapper/*.swift mv "$output_dir"/*.swift native/swift/Sources/wordpress-api-wrapper/ +for swift_file in native/swift/Sources/wordpress-api-wrapper/*.swift; do + basename=$(basename "$swift_file" .swift) + output_file="native/swift/Sources/wordpress-api-wrapper/${basename}_cancellable.swift" + + xcrun swift run -c release --quiet \ + --package-path native/swift/Tools \ + generate-cancellable --log-level warning "$swift_file" "$output_file" +done + header_dir="$output_dir/Headers" mkdir -p "$header_dir" From 29fffc3f1911eca0e30112b3e15175eee50c3573 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 12:42:34 +1200 Subject: [PATCH 06/22] Remove the `wait_for_cancellation` API --- wp_api/src/cancellation.rs | 131 ------------------------------------- 1 file changed, 131 deletions(-) diff --git a/wp_api/src/cancellation.rs b/wp_api/src/cancellation.rs index 1e49cca42..668503ae8 100644 --- a/wp_api/src/cancellation.rs +++ b/wp_api/src/cancellation.rs @@ -3,9 +3,6 @@ use std::{ sync::{Arc, Mutex, PoisonError}, }; -use futures::StreamExt; -use futures::channel::mpsc::{TryRecvError, UnboundedSender, unbounded}; - use crate::uuid::WpUuid; #[uniffi::export(with_foreign)] @@ -17,7 +14,6 @@ pub trait CancellationHandler: Send + Sync + std::fmt::Debug { pub struct CancellationToken { uuid: WpUuid, cancelled: Mutex, - waiters: Mutex>>, handler: Mutex>>, } @@ -28,7 +24,6 @@ impl CancellationToken { Self { uuid: WpUuid::default(), cancelled: Mutex::new(false), - waiters: Mutex::new(VecDeque::new()), handler: Mutex::new(VecDeque::new()), } } @@ -46,22 +41,6 @@ impl CancellationToken { Ok(()) } - pub async fn wait_for_cancellation(&self) -> Result<(), CancellationTokenError> { - if *self.cancelled.lock()? { - return Ok(()); - } - - let (sender, mut receiver) = unbounded(); - - { - let mut waiters = self.waiters.lock()?; - waiters.push_back(sender); - } - - let _ = receiver.next().await; - Ok(()) - } - pub fn cancel(&self) -> Result<(), CancellationTokenError> { let mut cancelled = self.cancelled.lock()?; if *cancelled { @@ -75,9 +54,6 @@ impl CancellationToken { handler.cancelled(); } - let mut waiters = self.waiters.lock()?; - waiters.retain_mut(|sender| sender.unbounded_send(()).is_ok()); - Ok(()) } } @@ -94,95 +70,9 @@ impl From> for CancellationTokenError { } } -impl From for CancellationTokenError { - fn from(_: TryRecvError) -> Self { - CancellationTokenError::Locking - } -} - #[cfg(test)] mod tests { use super::*; - use std::time::Duration; - use tokio::time::{sleep, timeout}; - - #[tokio::test] - async fn test_wait_for_cancellation_returns_immediately_when_already_cancelled() { - let token = CancellationToken::new(); - token.cancel().unwrap(); - - let result = token.wait_for_cancellation().await; - assert!(result.is_ok()); - } - - #[tokio::test] - async fn test_wait_for_cancellation_waits_until_cancelled() { - let token = CancellationToken::new(); - let token_clone = std::sync::Arc::new(token); - let token_for_task = token_clone.clone(); - - let wait_task = tokio::spawn(async move { token_for_task.wait_for_cancellation().await }); - - sleep(Duration::from_millis(10)).await; - assert!(!wait_task.is_finished()); - - token_clone.cancel().unwrap(); - - let result = wait_task.await.unwrap(); - assert!(result.is_ok()); - } - - #[tokio::test] - async fn test_wait_for_cancellation_multiple_waiters() { - let token = std::sync::Arc::new(CancellationToken::new()); - - let mut tasks = Vec::new(); - for _ in 0..5 { - let token_clone = token.clone(); - let task = tokio::spawn(async move { token_clone.wait_for_cancellation().await }); - tasks.push(task); - } - - sleep(Duration::from_millis(10)).await; - for task in &tasks { - assert!(!task.is_finished()); - } - - token.cancel().unwrap(); - - for task in tasks { - let result = task.await.unwrap(); - assert!(result.is_ok()); - } - } - - #[tokio::test] - async fn test_wait_for_cancellation_timeout_when_not_cancelled() { - let token = CancellationToken::new(); - - let result = timeout(Duration::from_millis(50), token.wait_for_cancellation()).await; - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_wait_for_cancellation_with_concurrent_cancel() { - let token = std::sync::Arc::new(CancellationToken::new()); - let token_for_wait = token.clone(); - let token_for_cancel = token.clone(); - - let wait_task = tokio::spawn(async move { token_for_wait.wait_for_cancellation().await }); - - let cancel_task = tokio::spawn(async move { - sleep(Duration::from_millis(20)).await; - token_for_cancel.cancel() - }); - - let wait_result = wait_task.await.unwrap(); - let cancel_result = cancel_task.await.unwrap(); - - assert!(wait_result.is_ok()); - assert!(cancel_result.is_ok()); - } #[derive(Debug)] struct TestHandler { @@ -254,25 +144,4 @@ mod tests { assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); } - - #[tokio::test] - async fn test_register_handler_with_wait_for_cancellation() { - let token = std::sync::Arc::new(CancellationToken::new()); - let (handler, called_flag) = TestHandler::new(); - - token.register_handler(Arc::new(handler)).unwrap(); - - let token_for_wait = token.clone(); - let wait_task = tokio::spawn(async move { token_for_wait.wait_for_cancellation().await }); - - sleep(Duration::from_millis(10)).await; - assert!(!wait_task.is_finished()); - assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); - - token.cancel().unwrap(); - - let result = wait_task.await.unwrap(); - assert!(result.is_ok()); - assert!(called_flag.load(std::sync::atomic::Ordering::SeqCst)); - } } From a6d846fd3c9b6dae6b88be41cd049987832fb4d4 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 13:47:24 +1200 Subject: [PATCH 07/22] Fix compilation issues in the Kotlin wrapper --- .../kotlin/src/integrationTest/kotlin/ManualParserTest.kt | 2 +- .../kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt | 1 + .../kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt | 5 +++-- .../main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt | 4 +++- .../main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt | 5 +++-- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt index 175bddb5c..be5554203 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt @@ -24,7 +24,7 @@ class ManualParserTest { authProvider ) val userListRequest = requestBuilder.users().listWithEditContext(UserListParams()) - val userListResponse = requestExecutor.execute(userListRequest) + val userListResponse = requestExecutor.execute(userListRequest, null) val userList = parseAsUsersRequestListWithEditContextResponse(userListResponse).data assertEquals(NUMBER_OF_USERS, userList.count()) } diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt index 8b77edf63..0d4d50ddc 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt @@ -70,6 +70,7 @@ class MediaEndpointTest { params = MediaCreateParams(title = title), "test_media.jpg", "image/jpeg", + null, null ) }.assertSuccessAndRetrieveData().data diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt index c1dabe837..f6c298d08 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt @@ -2,6 +2,7 @@ package rs.wordpress.api.kotlin import kotlinx.coroutines.delay import okio.FileNotFoundException +import uniffi.wp_api.CancellationToken import uniffi.wp_api.MediaUploadRequest import uniffi.wp_api.RequestExecutor import uniffi.wp_api.WpNetworkHeaderMap @@ -28,7 +29,7 @@ class NoStubFoundException(message: String) : Exception(message) // A class used for testing the request executor. class MockRequestExecutor(private var stubs: List = listOf()) : RequestExecutor { - override suspend fun execute(request: WpNetworkRequest): WpNetworkResponse { + override suspend fun execute(request: WpNetworkRequest, cancellationToken: CancellationToken?): WpNetworkResponse { val stub = stubs.firstOrNull { it.evaluator(request) } @@ -40,7 +41,7 @@ class MockRequestExecutor(private var stubs: List = listOf()) : RequestExe throw NoStubFoundException("No stub found for ${request.url()}") } - override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest): WpNetworkResponse { + override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?): WpNetworkResponse { TODO("Not yet implemented") } diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt index 8498787f1..82e51ff00 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt @@ -1,4 +1,5 @@ package rs.wordpress.api.kotlin +import uniffi.wp_api.CancellationToken import uniffi.wp_api.RequestExecutor import uniffi.wp_api.WpApiMiddleware import uniffi.wp_api.WpNetworkRequest @@ -9,7 +10,8 @@ class DebugMiddleware : WpApiMiddleware { override suspend fun process( requestExecutor: RequestExecutor, response: WpNetworkResponse, - request: WpNetworkRequest + request: WpNetworkRequest, + cancellationToken: CancellationToken? ): WpNetworkResponse { println("Request: ${request.url()}") println("Response:") diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 96b2cb7ed..74f8c469f 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -13,6 +13,7 @@ import okhttp3.Request import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody +import uniffi.wp_api.CancellationToken import uniffi.wp_api.InvalidSslErrorReason import uniffi.wp_api.MediaUploadRequest import uniffi.wp_api.MediaUploadRequestExecutionException @@ -38,7 +39,7 @@ class WpRequestExecutor( private val fileResolver: FileResolver = DefaultFileResolver(), private val uploadListener: UploadListener? = null ) : RequestExecutor { - override suspend fun execute(request: WpNetworkRequest): WpNetworkResponse = + override suspend fun execute(request: WpNetworkRequest, cancellationToken: CancellationToken?): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(request.url()) val wpNetworkRequestBody = request.body()?.contents()?.toRequestBody() @@ -84,7 +85,7 @@ class WpRequestExecutor( } } - override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest): WpNetworkResponse = + override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(mediaUploadRequest.url()) val multipartBodyBuilder = MultipartBody.Builder() From f6be30f97638f23fd78e52d376b356a0f04275a9 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 14:49:54 +1200 Subject: [PATCH 08/22] Update swift-tools-version --- native/swift/Tools/Package.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/native/swift/Tools/Package.swift b/native/swift/Tools/Package.swift index 8a6561ee8..b0ae8950c 100644 --- a/native/swift/Tools/Package.swift +++ b/native/swift/Tools/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version: 6.2 +// swift-tools-version: 6.1 import PackageDescription From 622475b336074529576a5af11eb0f49cdf28a1fc Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 14:50:40 +1200 Subject: [PATCH 09/22] Remove xcrun --- scripts/swift-bindings.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/swift-bindings.sh b/scripts/swift-bindings.sh index 0e8b5c8e1..df9f91012 100755 --- a/scripts/swift-bindings.sh +++ b/scripts/swift-bindings.sh @@ -55,7 +55,7 @@ for swift_file in native/swift/Sources/wordpress-api-wrapper/*.swift; do basename=$(basename "$swift_file" .swift) output_file="native/swift/Sources/wordpress-api-wrapper/${basename}_cancellable.swift" - xcrun swift run -c release --quiet \ + swift run -c release --quiet \ --package-path native/swift/Tools \ generate-cancellable --log-level warning "$swift_file" "$output_file" done From 9aa05cfefd44ce01e25cbec480ee1d30f7ad4813 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 15:27:36 +1200 Subject: [PATCH 10/22] All swift files in wordpress-api-wrapper are auto-generated --- .buildkite/download-xcframework.sh | 3 +-- .buildkite/pipeline.yml | 5 +---- .swiftlint.yml | 6 +----- fastlane/Fastfile | 3 +-- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/.buildkite/download-xcframework.sh b/.buildkite/download-xcframework.sh index b8b7f2abb..e7fb211c3 100755 --- a/.buildkite/download-xcframework.sh +++ b/.buildkite/download-xcframework.sh @@ -4,7 +4,6 @@ set -euo pipefail echo "--- :arrow_down: Downloading xcframework" buildkite-agent artifact download target/libwordpressFFI.xcframework.zip . --step "xcframework" -buildkite-agent artifact download native/swift/Sources/wordpress-api-wrapper/wp_api.swift . --step "xcframework" -buildkite-agent artifact download native/swift/Sources/wordpress-api-wrapper/wp_localization.swift . --step "xcframework" +buildkite-agent artifact download 'native/swift/Sources/wordpress-api-wrapper/*.swift' . --step "xcframework" unzip target/libwordpressFFI.xcframework.zip -d . rm target/libwordpressFFI.xcframework.zip diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 01f7bcca0..927ad6018 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -74,10 +74,7 @@ steps: zip -r target/libwordpressFFI.xcframework.zip target/libwordpressFFI.xcframework artifact_paths: - target/libwordpressFFI.xcframework.zip - - native/swift/Sources/wordpress-api-wrapper/wp_api.swift - - native/swift/Sources/wordpress-api-wrapper/wp_localization.swift - - native/swift/Sources/wordpress-api-wrapper/wp_com.swift - - native/swift/Sources/wordpress-api-wrapper/jetpack.swift + - native/swift/Sources/wordpress-api-wrapper/*.swift agents: queue: mac - label: ":swift: Build Docs" diff --git a/.swiftlint.yml b/.swiftlint.yml index d0a3be016..960495ece 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -3,10 +3,7 @@ strict: true included: - native/swift excluded: # paths to ignore during linting. Takes precedence over `included`. - - native/swift/Sources/wordpress-api-wrapper/wp_api.swift # auto-generated code - - native/swift/Sources/wordpress-api-wrapper/wp_localization.swift # auto-generated code - - native/swift/Sources/wordpress-api-wrapper/wp_com.swift # auto-generated code - - native/swift/Sources/wordpress-api-wrapper/jetpack.swift # auto-generated code + - native/swift/Sources/wordpress-api-wrapper # auto-generated code disabled_rules: # Don't think we should enable this rule. # See https://github.com/realm/SwiftLint/issues/5263 for context. @@ -19,4 +16,3 @@ disabled_rules: - file_length - function_body_length - type_body_length - diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 9f53aff15..ac7bd7377 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -412,8 +412,7 @@ end def xcframework_bindings_file_path dir = File.join(PROJECT_ROOT, 'native', 'swift', 'Sources', 'wordpress-api-wrapper') - %w[wp_api.swift wp_localization.swift] - .map { |file| File.join(dir, file) } + Dir.glob(File.join(dir, '*.swift')) end def remove_lane_context_values(names) From 8ea35e5938c5459c2412d5818461d2d7a6af87dd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 15:28:59 +1200 Subject: [PATCH 11/22] Fix Kotlin issues --- .../rs/wordpress/api/kotlin/WpRequestExecutor.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 74f8c469f..0b223021b 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -39,7 +39,10 @@ class WpRequestExecutor( private val fileResolver: FileResolver = DefaultFileResolver(), private val uploadListener: UploadListener? = null ) : RequestExecutor { - override suspend fun execute(request: WpNetworkRequest, cancellationToken: CancellationToken?): WpNetworkResponse = + override suspend fun execute( + request: WpNetworkRequest, + cancellationToken: CancellationToken? + ): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(request.url()) val wpNetworkRequestBody = request.body()?.contents()?.toRequestBody() @@ -85,7 +88,10 @@ class WpRequestExecutor( } } - override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?): WpNetworkResponse = + override suspend fun uploadMedia( + mediaUploadRequest: MediaUploadRequest, + cancellationToken: CancellationToken? + ): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(mediaUploadRequest.url()) val multipartBodyBuilder = MultipartBody.Builder() From 55eb06e2226b91c28460570bbc50925565f2560c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 16:12:20 +1200 Subject: [PATCH 12/22] Fix swiftlint issues --- .../wordpress-api/SafeRequestExecutor.swift | 25 ++++++++++++--- .../Sources/wordpress-api/WordPressAPI.swift | 16 ++++++++-- .../Tests/integration-tests/MediaTests.swift | 32 +++++++++++++++---- native/swift/Tools/Package.swift | 8 ++--- .../Sources/GenerateCancellable/main.swift | 27 ++++++++++------ 5 files changed, 80 insertions(+), 28 deletions(-) diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index db471fc41..4729d4ccb 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -10,7 +10,10 @@ import Combine #endif public protocol SafeRequestExecutor: RequestExecutor, Sendable { - func execute(_ request: WpNetworkRequest, cancellationToken: CancellationToken?) async -> Result + func execute( + _ request: WpNetworkRequest, + cancellationToken: CancellationToken? + ) async -> Result func uploadMedia( mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken? @@ -24,12 +27,18 @@ public protocol SafeRequestExecutor: RequestExecutor, Sendable { } extension SafeRequestExecutor { - public func execute(request: WpNetworkRequest, cancellationToken: CancellationToken?) async throws -> WpNetworkResponse { + public func execute( + request: WpNetworkRequest, + cancellationToken: CancellationToken? + ) async throws -> WpNetworkResponse { let result = await execute(request, cancellationToken: cancellationToken) return try result.get() } - public func uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?) async throws -> WpNetworkResponse { + public func uploadMedia( + mediaUploadRequest: MediaUploadRequest, + cancellationToken: CancellationToken? + ) async throws -> WpNetworkResponse { let result = await uploadMedia(mediaUploadRequest: mediaUploadRequest, cancellationToken: cancellationToken) return try result.get() } @@ -58,7 +67,10 @@ public final class WpRequestExecutor: SafeRequestExecutor { self.additionalHttpHeadersForAllRequests = headers } - public func execute(_ request: WpNetworkRequest, cancellationToken: CancellationToken?) async -> Result { + public func execute( + _ request: WpNetworkRequest, + cancellationToken: CancellationToken? + ) async -> Result { await perform(request, cancellationToken: cancellationToken) } @@ -79,7 +91,10 @@ public final class WpRequestExecutor: SafeRequestExecutor { } } - func perform(_ request: NetworkRequestContent, cancellationToken: CancellationToken?) async -> Result { + func perform( + _ request: NetworkRequestContent, + cancellationToken: CancellationToken? + ) async -> Result { if let cancellationToken { let requestId = request.requestId() await cancellationHandlers.whenCancelling(cancellationToken) { diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index 620e8b2ad..f4c2dff61 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -201,10 +201,21 @@ public actor WordPressAPI { } #endif - public func createMedia(params: MediaCreateParams, filePath: String, fileContentType: String, requestId: WpUuid?) async throws -> MediaRequestCreateResponse { + public func createMedia( + params: MediaCreateParams, + filePath: String, + fileContentType: String, + requestId: WpUuid? + ) async throws -> MediaRequestCreateResponse { let cancellation = CancellationToken() return try await withTaskCancellationHandler { - try await media.create(params: params, filePath: filePath, fileContentType: fileContentType, requestId: requestId, cancellationToken: cancellation) + try await media.create( + params: params, + filePath: filePath, + fileContentType: fileContentType, + requestId: requestId, + cancellationToken: cancellation + ) } onCancel: { do { try cancellation.cancel() @@ -212,7 +223,6 @@ public actor WordPressAPI { NSLog("Failed to cancel \(#function): \(error)") } } - } enum ParseError: Error { diff --git a/native/swift/Tests/integration-tests/MediaTests.swift b/native/swift/Tests/integration-tests/MediaTests.swift index 473151ab2..f1ffdbe54 100644 --- a/native/swift/Tests/integration-tests/MediaTests.swift +++ b/native/swift/Tests/integration-tests/MediaTests.swift @@ -106,13 +106,23 @@ struct MediaTests { performing: { let requestId = WpUuid() let task = Task { - _ = try await api.media.create(params: .init(), filePath: file.path(), fileContentType: "image/jpg", requestId: requestId, cancellationToken: nil) + _ = try await api.media.create( + params: .init(), + filePath: file.path(), + fileContentType: "image/jpg", + requestId: requestId, + cancellationToken: nil + ) } - let progress = try await api.requestExecutor.progress(forRequestWithId: requestId.uuidString()).values.first { _ in true } - let cancellable = progress!.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in - task.cancel() - } + let progress = try await api.requestExecutor + .progress(forRequestWithId: requestId.uuidString()) + .values + .first { _ in true } + let cancellable = progress! + .publisher(for: \.fractionCompleted) + .first { $0 > 0 } + .sink { _ in task.cancel() } defer { cancellable.cancel() } try await task.value @@ -130,10 +140,18 @@ struct MediaTests { performing: { let requestId = WpUuid() let task = Task { - _ = try await api.createMedia(params: .init(), filePath: file.path(), fileContentType: "image/jpg", requestId: requestId) + _ = try await api.createMedia( + params: .init(), + filePath: file.path(), + fileContentType: "image/jpg", + requestId: requestId + ) } - let progress = try await api.requestExecutor.progress(forRequestWithId: requestId.uuidString()).values.first { _ in true } + let progress = try await api.requestExecutor + .progress(forRequestWithId: requestId.uuidString()) + .values + .first { _ in true } let cancellable = progress!.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in task.cancel() } diff --git a/native/swift/Tools/Package.swift b/native/swift/Tools/Package.swift index b0ae8950c..b6c283095 100644 --- a/native/swift/Tools/Package.swift +++ b/native/swift/Tools/Package.swift @@ -8,12 +8,12 @@ let package = Package( .macOS(.v12) ], products: [ - .executable(name: "generate-cancellable", targets: ["GenerateCancellable"]), + .executable(name: "generate-cancellable", targets: ["GenerateCancellable"]) ], dependencies: [ .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "600.0.0"), .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.0.0"), - .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"), + .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0") ], targets: [ .executableTarget( @@ -23,8 +23,8 @@ let package = Package( .product(name: "SwiftParser", package: "swift-syntax"), .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), .product(name: "ArgumentParser", package: "swift-argument-parser"), - .product(name: "Logging", package: "swift-log"), + .product(name: "Logging", package: "swift-log") ] - ), + ) ] ) diff --git a/native/swift/Tools/Sources/GenerateCancellable/main.swift b/native/swift/Tools/Sources/GenerateCancellable/main.swift index 91fa29dd2..c715211a4 100644 --- a/native/swift/Tools/Sources/GenerateCancellable/main.swift +++ b/native/swift/Tools/Sources/GenerateCancellable/main.swift @@ -18,7 +18,7 @@ struct GenerateCancellable: ParsableCommand { @Argument(help: "Output file path for generated extensions") var output: String - @Option(name: .long, help: "Set log level (trace, debug, info, notice, warning, error, critical)") + @Option(name: .long, help: "Set log level (debug, info, notice, warning, error, critical)") var logLevel: String = "info" mutating func run() throws { @@ -127,8 +127,12 @@ class CancellationAnalyzer: SyntaxVisitor { existingFunctions: currentExistingFunctions ) - logger.debug("Found \(currentCancellationFunctions.count) cancellation functions in \(className)") - logger.debug("Found \(currentExistingFunctions.count) existing functions in \(className)") + logger.debug( + "Found \(currentCancellationFunctions.count) cancellation functions in \(className)" + ) + logger.debug( + "Found \(currentExistingFunctions.count) existing functions in \(className)" + ) currentClass = nil } @@ -226,16 +230,18 @@ class ExtensionGenerator { var members: [MemberBlockItemSyntax] = [] for cancellationFunc in classAnalysis.cancellationFunctions { - let nonCancellationName = String(cancellationFunc.name.dropLast("Cancellation".count)) + let funcName = String(cancellationFunc.name.dropLast("Cancellation".count)) - if classAnalysis.existingFunctions[nonCancellationName] != nil { - logger.warning("Skipping generation of '\(nonCancellationName)' - function already exists in \(classAnalysis.className)") + if classAnalysis.existingFunctions[funcName] != nil { + logger.warning( + "Skipping generation of '\(funcName)' - function already exists in \(classAnalysis.className)" + ) continue } - logger.debug("Generating new function: \(nonCancellationName)") + logger.debug("Generating new function: \(funcName)") - let newFunction = createNonCancellationFunction(from: cancellationFunc, name: nonCancellationName) + let newFunction = createNonCancellationFunction(from: cancellationFunc, name: funcName) let memberItem = MemberBlockItemSyntax( decl: DeclSyntax(newFunction), trailingTrivia: .newline @@ -246,7 +252,10 @@ class ExtensionGenerator { return members } - private func createNonCancellationFunction(from cancellationFunc: FunctionInfo, name: String) -> FunctionDeclSyntax { + private func createNonCancellationFunction( + from cancellationFunc: FunctionInfo, + name: String + ) -> FunctionDeclSyntax { var parametersWithoutCancellation: FunctionParameterListSyntax if cancellationFunc.parameters.isEmpty || cancellationFunc.parameters.count == 1 { From b8956e1aa2807dc5eeaf040e22bec8a3f43117d6 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 19:57:12 +1200 Subject: [PATCH 13/22] No cancellation on Linux --- native/swift/Sources/wordpress-api/SafeRequestExecutor.swift | 2 ++ native/swift/Tests/integration-tests/CancellationTests.swift | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index 4729d4ccb..c8cfc0842 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -170,6 +170,7 @@ public final class WpRequestExecutor: SafeRequestExecutor { #endif private func cancelRequest(withId requestId: String) async { +#if canImport(Combine) var task = (await self.session.allTasks).first { $0.originalRequest?.requestId == requestId } @@ -185,6 +186,7 @@ public final class WpRequestExecutor: SafeRequestExecutor { } task?.cancel() +#endif } private func handleHttpsError( diff --git a/native/swift/Tests/integration-tests/CancellationTests.swift b/native/swift/Tests/integration-tests/CancellationTests.swift index af3961be6..71a540e01 100644 --- a/native/swift/Tests/integration-tests/CancellationTests.swift +++ b/native/swift/Tests/integration-tests/CancellationTests.swift @@ -4,6 +4,8 @@ import Testing @testable import WordPressAPI @testable import WordPressAPIInternal +#if os(macOS) + struct CancellationTests { let api = WordPressAPI.admin() @@ -31,3 +33,5 @@ struct CancellationTests { try await restoreTestServer() } } + +#endif From e470b1005add800f8abecbdef28641925470ceb4 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 17 Sep 2025 21:20:27 +1200 Subject: [PATCH 14/22] Only export either the cancellable or the uncancellable endpoints API When exporting both, the native binding code would be so large that they can't be loaded by kotlin. --- Makefile | 4 ++-- wp_api/Cargo.toml | 2 +- wp_derive_request_builder/Cargo.toml | 2 +- wp_derive_request_builder/src/generate.rs | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 780c954af..19035d941 100644 --- a/Makefile +++ b/Makefile @@ -102,7 +102,7 @@ _build-apple-%-tvos _build-apple-%-tvos-sim _build-apple-%-watchos _build-apple- # Build the library for a specific target _build-apple-%: - cargo $(CARGO_OPTS) $(cargo_config_library) build --target $* --features without-uncancellable-endpoints --package wp_api --profile $(CARGO_PROFILE) + cargo $(CARGO_OPTS) $(cargo_config_library) build --target $* --features export-uncancellable-endpoints --package wp_api --profile $(CARGO_PROFILE) ./scripts/swift-bindings.sh target/$*/$(CARGO_PROFILE_DIRNAME)/libwp_api.a # Build the library for one single platform, including real device and simulator. @@ -141,7 +141,7 @@ docker-image-web: docker build -t wordpress-rs-web -f wp_rs_web/Dockerfile . --progress=plain swift-linux-library: - cargo build --release --features without-uncancellable-endpoints --package wp_api + cargo build --release --features export-uncancellable-endpoints --package wp_api ./scripts/swift-bindings.sh target/release/libwp_api.a mkdir -p target/release/libwordpressFFI-linux cp target/release/swift-bindings/Headers/* target/release/libwordpressFFI-linux/ diff --git a/wp_api/Cargo.toml b/wp_api/Cargo.toml index 7f27f46f8..314ff5c81 100644 --- a/wp_api/Cargo.toml +++ b/wp_api/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2024" [features] -without-uncancellable-endpoints = ["wp_derive_request_builder/without-uncancellable-endpoints"] +export-uncancellable-endpoints = ["wp_derive_request_builder/export-uncancellable-endpoints"] integration-tests = [] reqwest-request-executor = ["dep:reqwest", "dep:tokio", "dep:hyper-util", "dep:rustls", "dep:hickory-resolver", "dep:hyper", "dep:h2"] diff --git a/wp_derive_request_builder/Cargo.toml b/wp_derive_request_builder/Cargo.toml index 72757b5ab..3480f105a 100644 --- a/wp_derive_request_builder/Cargo.toml +++ b/wp_derive_request_builder/Cargo.toml @@ -6,7 +6,7 @@ autotests = false [features] generate_request_builder = [] -without-uncancellable-endpoints = [] +export-uncancellable-endpoints = [] [lib] proc-macro = true diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 9f37b87fc..f720cc6c2 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -116,11 +116,11 @@ fn generate_async_request_executor( } }; - exported.push(cancellable); - - if cfg!(feature = "without-uncancellable-endpoints") { + if cfg!(feature = "export-uncancellable-endpoints") { + exported.push(cancellable); unexported.push(uncancellable); } else { + unexported.push(cancellable); exported.push(uncancellable); } From b6aced61d45fcc73f71e521a4ec4f6ffdf3ef9f5 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 18 Sep 2025 09:59:37 +1200 Subject: [PATCH 15/22] Move WordPressAPI.createMedia to MediaRequestExecutor --- .../Sources/wordpress-api/Extensions.swift | 25 ++++++++++ .../Sources/wordpress-api/WordPressAPI.swift | 24 --------- .../Tests/integration-tests/MediaTests.swift | 49 ++----------------- wp_api/src/api_client.rs | 31 +----------- 4 files changed, 31 insertions(+), 98 deletions(-) diff --git a/native/swift/Sources/wordpress-api/Extensions.swift b/native/swift/Sources/wordpress-api/Extensions.swift index dc32ee574..00920b698 100644 --- a/native/swift/Sources/wordpress-api/Extensions.swift +++ b/native/swift/Sources/wordpress-api/Extensions.swift @@ -73,3 +73,28 @@ extension WpApiError { return false } } + +extension MediaRequestExecutor { + public func create( + params: MediaCreateParams, + filePath: String, + fileContentType: String, + ) async throws -> MediaRequestCreateResponse { + let cancellation = CancellationToken() + return try await withTaskCancellationHandler { + try await create( + params: params, + filePath: filePath, + fileContentType: fileContentType, + requestId: nil, + cancellationToken: cancellation + ) + } onCancel: { + do { + try cancellation.cancel() + } catch { + NSLog("Failed to cancel \(#function): \(error)") + } + } + } +} diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index f4c2dff61..f700cb94b 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -201,30 +201,6 @@ public actor WordPressAPI { } #endif - public func createMedia( - params: MediaCreateParams, - filePath: String, - fileContentType: String, - requestId: WpUuid? - ) async throws -> MediaRequestCreateResponse { - let cancellation = CancellationToken() - return try await withTaskCancellationHandler { - try await media.create( - params: params, - filePath: filePath, - fileContentType: fileContentType, - requestId: requestId, - cancellationToken: cancellation - ) - } onCancel: { - do { - try cancellation.cancel() - } catch { - NSLog("Failed to cancel \(#function): \(error)") - } - } - } - enum ParseError: Error { case invalidUrl case invalidHtml diff --git a/native/swift/Tests/integration-tests/MediaTests.swift b/native/swift/Tests/integration-tests/MediaTests.swift index f1ffdbe54..b0d93046e 100644 --- a/native/swift/Tests/integration-tests/MediaTests.swift +++ b/native/swift/Tests/integration-tests/MediaTests.swift @@ -56,6 +56,7 @@ struct MediaTests { fromLocalFileURL: file, fulfilling: progress ) + Issue.record("The creating post function should throw") } let cancellable = progress.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in @@ -84,6 +85,7 @@ struct MediaTests { fromLocalFileURL: file, fulfilling: progress ) + Issue.record("The creating post function should throw") } let cancellable = progress.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in @@ -104,58 +106,17 @@ struct MediaTests { await #expect( throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), performing: { - let requestId = WpUuid() let task = Task { _ = try await api.media.create( params: .init(), filePath: file.path(), fileContentType: "image/jpg", - requestId: requestId, - cancellationToken: nil ) + Issue.record("The creating post function should throw") } - let progress = try await api.requestExecutor - .progress(forRequestWithId: requestId.uuidString()) - .values - .first { _ in true } - let cancellable = progress! - .publisher(for: \.fractionCompleted) - .first { $0 > 0 } - .sink { _ in task.cancel() } - defer { cancellable.cancel() } - - try await task.value - } - ) - - try await restoreTestServer() - } - - @Test - func cancelNewCreateMediaTask() async throws { - let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) - await #expect( - throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), - performing: { - let requestId = WpUuid() - let task = Task { - _ = try await api.createMedia( - params: .init(), - filePath: file.path(), - fileContentType: "image/jpg", - requestId: requestId - ) - } - - let progress = try await api.requestExecutor - .progress(forRequestWithId: requestId.uuidString()) - .values - .first { _ in true } - let cancellable = progress!.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in - task.cancel() - } - defer { cancellable.cancel() } + try await Task.sleep(for: .milliseconds(10)) + task.cancel() try await task.value } diff --git a/wp_api/src/api_client.rs b/wp_api/src/api_client.rs index 3ad8f64a6..804a6d538 100644 --- a/wp_api/src/api_client.rs +++ b/wp_api/src/api_client.rs @@ -2,10 +2,7 @@ use crate::{ WpAppNotifier, api_client_generate_api_client, api_client_generate_endpoint_impl, api_client_generate_request_builder, auth::WpAuthenticationProvider, - cancellation::CancellationToken, - media::MediaCreateParams, middleware::WpApiMiddlewarePipeline, - prelude::WpApiError, request::{ RequestExecutor, endpoint::{ @@ -15,9 +12,7 @@ use crate::{ }, categories_endpoint::{CategoriesRequestBuilder, CategoriesRequestExecutor}, comments_endpoint::{CommentsRequestBuilder, CommentsRequestExecutor}, - media_endpoint::{ - MediaRequestBuilder, MediaRequestCreateResponse, MediaRequestExecutor, - }, + media_endpoint::{MediaRequestBuilder, MediaRequestExecutor}, plugins_endpoint::{PluginsRequestBuilder, PluginsRequestExecutor}, post_autosaves_endpoint::{AutosavesRequestBuilder, AutosavesRequestExecutor}, post_revisions_endpoint::{PostRevisionsRequestBuilder, PostRevisionsRequestExecutor}, @@ -37,7 +32,6 @@ use crate::{ }, }, }, - uuid::WpUuid, }; use std::sync::Arc; @@ -127,29 +121,6 @@ impl UniffiWpApiClient { } } -#[uniffi::export] -impl UniffiWpApiClient { - async fn create_media( - &self, - params: MediaCreateParams, - file_path: String, - file_content_type: String, - request_id: Option>, - cancellation_token: Option>, - ) -> Result { - self.inner - .media - .create( - params, - file_path, - file_content_type, - request_id, - cancellation_token, - ) - .await - } -} - pub struct WpApiClient { application_passwords: Arc, autosaves: Arc, From 5ccc5df4ed78c40fe918036768ef1e54336da261 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 23 Sep 2025 21:14:00 +1200 Subject: [PATCH 16/22] Change CancellationToken to RequestContext --- .../kotlin/ManualParserTest.kt | 2 +- .../kotlin/MediaEndpointTest.kt | 1 - .../kotlin/MockRequestExecutor.kt | 10 +- .../wordpress/api/kotlin/DebugMiddleware.kt | 4 +- .../wordpress/api/kotlin/WpRequestExecutor.kt | 16 +- .../main/kotlin/rs/wordpress/api/kotlin/pr.md | 86 +++++++++++ .../Sources/wordpress-api/Extensions.swift | 25 ---- .../Sources/wordpress-api/Middleware.swift | 2 +- .../wordpress-api/SafeRequestExecutor.swift | 92 ++---------- .../Sources/wordpress-api/WordPressAPI.swift | 3 +- .../Tests/integration-tests/KnownIssues.swift | 1 - .../Tests/integration-tests/MediaTests.swift | 28 +--- .../wordpress-api/Support/HTTPStubs.swift | 10 +- .../wordpress-api/WordPressAPITests.swift | 2 +- .../Sources/GenerateCancellable/main.swift | 14 +- wp_api/src/cancellation.rs | 141 ++---------------- wp_api/src/middleware.rs | 49 +++--- wp_api/src/request.rs | 8 +- wp_api/src/request/endpoint/media_endpoint.rs | 4 +- wp_api/src/reqwest_request_executor.rs | 8 +- wp_api/src/wordpress_org/client.rs | 5 +- wp_api_integration_tests/src/mock.rs | 7 +- .../tests/test_app_notifier_immut.rs | 11 +- .../tests/test_media_err.rs | 8 +- .../tests/test_media_mut.rs | 1 - wp_derive_request_builder/src/generate.rs | 8 +- .../generate/helpers_to_generate_tokens.rs | 13 +- 27 files changed, 205 insertions(+), 354 deletions(-) create mode 100644 native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt index be5554203..175bddb5c 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/ManualParserTest.kt @@ -24,7 +24,7 @@ class ManualParserTest { authProvider ) val userListRequest = requestBuilder.users().listWithEditContext(UserListParams()) - val userListResponse = requestExecutor.execute(userListRequest, null) + val userListResponse = requestExecutor.execute(userListRequest) val userList = parseAsUsersRequestListWithEditContextResponse(userListResponse).data assertEquals(NUMBER_OF_USERS, userList.count()) } diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt index 0d4d50ddc..8b77edf63 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt @@ -70,7 +70,6 @@ class MediaEndpointTest { params = MediaCreateParams(title = title), "test_media.jpg", "image/jpeg", - null, null ) }.assertSuccessAndRetrieveData().data diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt index f6c298d08..2db1f170a 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MockRequestExecutor.kt @@ -2,8 +2,8 @@ package rs.wordpress.api.kotlin import kotlinx.coroutines.delay import okio.FileNotFoundException -import uniffi.wp_api.CancellationToken import uniffi.wp_api.MediaUploadRequest +import uniffi.wp_api.RequestContext import uniffi.wp_api.RequestExecutor import uniffi.wp_api.WpNetworkHeaderMap import uniffi.wp_api.WpNetworkRequest @@ -29,7 +29,7 @@ class NoStubFoundException(message: String) : Exception(message) // A class used for testing the request executor. class MockRequestExecutor(private var stubs: List = listOf()) : RequestExecutor { - override suspend fun execute(request: WpNetworkRequest, cancellationToken: CancellationToken?): WpNetworkResponse { + override suspend fun execute(request: WpNetworkRequest): WpNetworkResponse { val stub = stubs.firstOrNull { it.evaluator(request) } @@ -41,13 +41,17 @@ class MockRequestExecutor(private var stubs: List = listOf()) : RequestExe throw NoStubFoundException("No stub found for ${request.url()}") } - override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest, cancellationToken: CancellationToken?): WpNetworkResponse { + override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest): WpNetworkResponse { TODO("Not yet implemented") } override suspend fun sleep(millis: ULong) { delay(millis.toLong()) } + + override fun cancel(context: RequestContext) { + // No-op + } } val WpNetworkResponse.Companion.empty: WpNetworkResponse diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt index 82e51ff00..172f4d949 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/DebugMiddleware.kt @@ -1,5 +1,5 @@ package rs.wordpress.api.kotlin -import uniffi.wp_api.CancellationToken +import uniffi.wp_api.RequestContext import uniffi.wp_api.RequestExecutor import uniffi.wp_api.WpApiMiddleware import uniffi.wp_api.WpNetworkRequest @@ -11,7 +11,7 @@ class DebugMiddleware : WpApiMiddleware { requestExecutor: RequestExecutor, response: WpNetworkResponse, request: WpNetworkRequest, - cancellationToken: CancellationToken? + context: RequestContext? ): WpNetworkResponse { println("Request: ${request.url()}") println("Response:") diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 0b223021b..c15826bfd 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -13,10 +13,10 @@ import okhttp3.Request import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody -import uniffi.wp_api.CancellationToken import uniffi.wp_api.InvalidSslErrorReason import uniffi.wp_api.MediaUploadRequest import uniffi.wp_api.MediaUploadRequestExecutionException +import uniffi.wp_api.RequestContext import uniffi.wp_api.RequestExecutionErrorReason import uniffi.wp_api.RequestExecutionException import uniffi.wp_api.RequestExecutor @@ -39,10 +39,7 @@ class WpRequestExecutor( private val fileResolver: FileResolver = DefaultFileResolver(), private val uploadListener: UploadListener? = null ) : RequestExecutor { - override suspend fun execute( - request: WpNetworkRequest, - cancellationToken: CancellationToken? - ): WpNetworkResponse = + override suspend fun execute(request: WpNetworkRequest): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(request.url()) val wpNetworkRequestBody = request.body()?.contents()?.toRequestBody() @@ -88,10 +85,7 @@ class WpRequestExecutor( } } - override suspend fun uploadMedia( - mediaUploadRequest: MediaUploadRequest, - cancellationToken: CancellationToken? - ): WpNetworkResponse = + override suspend fun uploadMedia(mediaUploadRequest: MediaUploadRequest): WpNetworkResponse = withContext(dispatcher) { val requestBuilder = Request.Builder().url(mediaUploadRequest.url()) val multipartBodyBuilder = MultipartBody.Builder() @@ -157,6 +151,10 @@ class WpRequestExecutor( delay(millis.toLong()) } + override fun cancel(context: RequestContext) { + // No-op + } + private fun File.canBeUploaded() = exists() && isFile && canRead() /** diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md new file mode 100644 index 000000000..12d2da2a5 --- /dev/null +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md @@ -0,0 +1,86 @@ +Relates to https://github.com/Automattic/wordpress-rs/issues/824. + +This PR adds cancellation support to the Swift wrapper. The same API (for example, `api.posts.create(...)`), which was not cancellable, can now be cancelled. + +To verify this change, you can checkout the first commit (Add CancellationTests), run `make xcframework-only-macos`, then run the `CancellationTests` from Xcode. The test should fail. Checkout the last commit, repeat the same steps, the same tests should pass. + +### Goal + +Use this Swift code as an example: + +```swift +let task = Task { + let newPost = try await api.posts.create(...) + viewPostDetails(newPost) +} + +// a short moment later +task.cancel() +``` + +When you cancel the task that is creating a new post and the POST HTTP request is still going, we expect the HTTP request to be cancelled, and the following `viewPostDetails` call should not be called. + +Putting this scenario into the mobile app's context, you can think of it as cancelling the publishing of a new post, replying to a comment, uploading an image, etc. When user taps a cancel button to cancel those pending jobs, we should try our best to abort the underlying HTTP request to prevent the content reaching the server. Obviously, we can't 100% guarantee that, because the request may have already reached the server when user taps the cancel button, and we can't do much from client side about that. + +### Difficulties + +There are several difficulties involved in supporting the native Swift task cancellation, when the task calls Rust's async functions underneath. Because both Swift's Task and Rust's Future API are involved, and they obviously work quite differently. + +As I mentioned in https://github.com/Automattic/wordpress-rs/issues/824, uniffi-rs does not have built-in cancellation support. We'd need to implement the mechanism ourselves. + +### Approach + +First, Rust API needs to expose a `cancellation_token: CancellationToken`. This PR modified the derive macro to add the parameter to all endpoint functions. + +The generated code now looks like this: + +```rust +impl UsersRequestExecutor { + // This is a new function, which supports cancellation when the `cancellation_token` parameter value is not None. + fn list_with_view_context_cancellation(params: UserListParams, cancellation_token: Option>) { + let request = ... + let response = self.request_executor.execute(request, cancellation_token).await; + return response.parse() + } + + // This is the existing function, which does not support cancellation, since it passes `cancellation_token: None` to the function above. + fn list_with_view_context(params: UserListParams) { + list_with_view_context_cancellation(params, None) + } +} +``` + +This change does not break the endpoint APIs. Their function signature stays the same, and they cannot be cancelled as before. The Rust code acts as a facilitator, which accepts a `CancellationToken` instance and passes it to the native implementation of `RequestExecutor`, so that the request executor can cancel HTTP requests when cancelled. + +The `RequestExecutor` API is changed to support that. Both existing functions now accept a `cancellation_token: CancellationToken?` parameter. The native implementation can use `CancellationToken.register_handler` function to get a callback when `CancellationToken` gets cancelled. The implementation here is isolated within the native `RequestExecutor`, which makes the actual HTTP request cancellation relatively straightforward. + +Finally, in order to provide a cancellable version of `list_with_view_context`, and keep the source code backward compatible, I have introduced a feature to prevent the `list_with_view_context` from being exported as a uniffi function. The native wrapper can then generate the `list_with_view_context` function according to the cancellable version: `list_with_view_context_cancellation`. It looks like this in Swift: + +```swift +extension UsersRequestExecutor { + func listWithViewContext(params: UserListParam) async throws { + let token = CancellationToken() + return try await withTaskCancellationHandler { + try await self.listWithViewContextCancellation(params, cancellationToken: token) + } onCancel: { + token.cancel() + } + } +} +``` + +### Limitations + +The cancellation support is opt-in. As in, the Rust functions need to accept a `CancellationToken` parameter, and make sure to pass the instance down to its call chain. + +```rust +fn upload_post(cancellation_token: CancellationToken) { + for image in images { + media.upload_image(image, cancellation_token).await + } + + post.create(title, content, cancellation_token).await +} +``` + +The macro change should cover the majority of the HTTP requests sent to `RequestExecutor`. But there is still code that does not support cancellation. For example, the API discovery, which we can look into later if needed. diff --git a/native/swift/Sources/wordpress-api/Extensions.swift b/native/swift/Sources/wordpress-api/Extensions.swift index 00920b698..dc32ee574 100644 --- a/native/swift/Sources/wordpress-api/Extensions.swift +++ b/native/swift/Sources/wordpress-api/Extensions.swift @@ -73,28 +73,3 @@ extension WpApiError { return false } } - -extension MediaRequestExecutor { - public func create( - params: MediaCreateParams, - filePath: String, - fileContentType: String, - ) async throws -> MediaRequestCreateResponse { - let cancellation = CancellationToken() - return try await withTaskCancellationHandler { - try await create( - params: params, - filePath: filePath, - fileContentType: fileContentType, - requestId: nil, - cancellationToken: cancellation - ) - } onCancel: { - do { - try cancellation.cancel() - } catch { - NSLog("Failed to cancel \(#function): \(error)") - } - } - } -} diff --git a/native/swift/Sources/wordpress-api/Middleware.swift b/native/swift/Sources/wordpress-api/Middleware.swift index 8302a394b..6f4c0c7eb 100644 --- a/native/swift/Sources/wordpress-api/Middleware.swift +++ b/native/swift/Sources/wordpress-api/Middleware.swift @@ -5,7 +5,7 @@ public final class DebugMiddleware: WpApiMiddleware { requestExecutor: any WordPressAPIInternal.RequestExecutor, response: WordPressAPIInternal.WpNetworkResponse, request: WordPressAPIInternal.WpNetworkRequest, - cancellationToken: CancellationToken? + context: RequestContext? ) async throws -> WordPressAPIInternal.WpNetworkResponse { debugPrint("Performed request: \(String(describing: try? request.buildURLRequest(additionalHeaders: [:])))") debugPrint("Received response: \(response)") diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index c8cfc0842..545fdb854 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -10,13 +10,9 @@ import Combine #endif public protocol SafeRequestExecutor: RequestExecutor, Sendable { - func execute( - _ request: WpNetworkRequest, - cancellationToken: CancellationToken? - ) async -> Result + func execute(_ request: WpNetworkRequest) async -> Result func uploadMedia( - mediaUploadRequest: MediaUploadRequest, - cancellationToken: CancellationToken? + mediaUploadRequest: MediaUploadRequest ) async -> Result #if PROGRESS_REPORTING_ENABLED @@ -27,19 +23,13 @@ public protocol SafeRequestExecutor: RequestExecutor, Sendable { } extension SafeRequestExecutor { - public func execute( - request: WpNetworkRequest, - cancellationToken: CancellationToken? - ) async throws -> WpNetworkResponse { - let result = await execute(request, cancellationToken: cancellationToken) + public func execute(request: WpNetworkRequest) async throws -> WpNetworkResponse { + let result = await execute(request) return try result.get() } - public func uploadMedia( - mediaUploadRequest: MediaUploadRequest, - cancellationToken: CancellationToken? - ) async throws -> WpNetworkResponse { - let result = await uploadMedia(mediaUploadRequest: mediaUploadRequest, cancellationToken: cancellationToken) + public func uploadMedia(mediaUploadRequest: MediaUploadRequest) async throws -> WpNetworkResponse { + let result = await uploadMedia(mediaUploadRequest: mediaUploadRequest) return try result.get() } } @@ -50,8 +40,6 @@ public final class WpRequestExecutor: SafeRequestExecutor { private let additionalHttpHeadersForAllRequests: [String: String] - private let cancellationHandlers = CancellationHandlers() - public init( urlSession: URLSession, additionalHttpHeadersForAllRequests: [String: String] = [:], @@ -67,18 +55,14 @@ public final class WpRequestExecutor: SafeRequestExecutor { self.additionalHttpHeadersForAllRequests = headers } - public func execute( - _ request: WpNetworkRequest, - cancellationToken: CancellationToken? - ) async -> Result { - await perform(request, cancellationToken: cancellationToken) + public func execute(_ request: WpNetworkRequest,) async -> Result { + await perform(request) } public func uploadMedia( - mediaUploadRequest: MediaUploadRequest, - cancellationToken: CancellationToken? + mediaUploadRequest: MediaUploadRequest ) async -> Result { - (await perform(mediaUploadRequest, cancellationToken: cancellationToken)) + (await perform(mediaUploadRequest)) .mapError { error in switch error { case let .RequestExecutionFailed(statusCode, redirects, reason): @@ -91,26 +75,12 @@ public final class WpRequestExecutor: SafeRequestExecutor { } } - func perform( - _ request: NetworkRequestContent, - cancellationToken: CancellationToken? - ) async -> Result { - if let cancellationToken { - let requestId = request.requestId() - await cancellationHandlers.whenCancelling(cancellationToken) { - Task { [weak self] in - await self?.cancelRequest(withId: requestId) - } + public func cancel(context: RequestContext) { + for requestId in context.requestIds() { + Task { + await self.cancelRequest(withId: requestId) } } - - let result = await perform(request) - - if let cancellationToken { - await cancellationHandlers.remove(cancellationToken) - } - - return result } func perform(_ request: NetworkRequestContent) async -> Result { @@ -451,37 +421,3 @@ extension MediaUploadRequest: NetworkRequestContent { } } - -private actor CancellationHandlers { - private var handlers: [String /* CancellationToken.uuid */: [RequestCancellationHandler]] = [:] - - func whenCancelling(_ token: CancellationToken, closure: @escaping @Sendable () -> Void) { - let handler = RequestCancellationHandler(closure: closure) - handlers[token.uuid(), default: []].append(handler) - try? token.registerHandler(handler: handler) - } - - func remove(_ token: CancellationToken) { - handlers.removeValue(forKey: token.uuid()) - } -} - -private final class RequestCancellationHandler: CancellationHandler, Hashable { - let closure: @Sendable () -> Void - - init(closure: @escaping @Sendable () -> Void) { - self.closure = closure - } - - func cancelled() { - self.closure() - } - - func hash(into hasher: inout Hasher) { - hasher.combine(ObjectIdentifier(self)) - } - - static func == (lhs: RequestCancellationHandler, rhs: RequestCancellationHandler) -> Bool { - ObjectIdentifier(lhs) == ObjectIdentifier(rhs) - } -} diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index f700cb94b..47294eea4 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -177,8 +177,7 @@ public actor WordPressAPI { params: params, filePath: localFileURL.path, fileContentType: fileContentType, - requestId: requestId, - cancellationToken: nil + requestId: requestId ) } diff --git a/native/swift/Tests/integration-tests/KnownIssues.swift b/native/swift/Tests/integration-tests/KnownIssues.swift index 9ac4efcfa..59eee69e8 100644 --- a/native/swift/Tests/integration-tests/KnownIssues.swift +++ b/native/swift/Tests/integration-tests/KnownIssues.swift @@ -19,7 +19,6 @@ struct KnownIssues { filePath: file.path, fileContentType: "image/jpeg", requestId: nil, - cancellationToken: nil ) } diff --git a/native/swift/Tests/integration-tests/MediaTests.swift b/native/swift/Tests/integration-tests/MediaTests.swift index b0d93046e..86e12a61c 100644 --- a/native/swift/Tests/integration-tests/MediaTests.swift +++ b/native/swift/Tests/integration-tests/MediaTests.swift @@ -13,8 +13,7 @@ struct MediaTests { params: .init(title: "Image", altText: "This is a test image"), filePath: file.path, fileContentType: "image/jpeg", - requestId: nil, - cancellationToken: nil + requestId: nil ) #expect(response.data.mimeType == "image/jpeg") #expect(response.data.title.raw == "Image") @@ -99,30 +98,5 @@ struct MediaTests { try await restoreTestServer() } - - @Test - func cancelCreateMediaTask() async throws { - let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) - await #expect( - throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), - performing: { - let task = Task { - _ = try await api.media.create( - params: .init(), - filePath: file.path(), - fileContentType: "image/jpg", - ) - Issue.record("The creating post function should throw") - } - - try await Task.sleep(for: .milliseconds(10)) - task.cancel() - - try await task.value - } - ) - - try await restoreTestServer() - } #endif } diff --git a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift index 03d20b0bf..33d0f5e98 100644 --- a/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift +++ b/native/swift/Tests/wordpress-api/Support/HTTPStubs.swift @@ -26,8 +26,7 @@ final class HTTPStubs: SafeRequestExecutor { } public func execute( - _ request: WpNetworkRequest, - cancellationToken: CancellationToken? + _ request: WpNetworkRequest ) async -> Result { if let response = stub(for: request) { return .success(response) @@ -50,8 +49,7 @@ final class HTTPStubs: SafeRequestExecutor { } func uploadMedia( - mediaUploadRequest: MediaUploadRequest, - cancellationToken: CancellationToken? + mediaUploadRequest: MediaUploadRequest ) async -> Result { preconditionFailure("This method is not yet implemented") } @@ -92,6 +90,10 @@ final class HTTPStubs: SafeRequestExecutor { // swiftlint:disable:next force_try try! await Task.sleep(nanoseconds: millis * 1000) } + + func cancel(context: RequestContext) { + // No-op + } } extension WpNetworkResponse { diff --git a/native/swift/Tests/wordpress-api/WordPressAPITests.swift b/native/swift/Tests/wordpress-api/WordPressAPITests.swift index c3e8e627a..28f3f6db8 100644 --- a/native/swift/Tests/wordpress-api/WordPressAPITests.swift +++ b/native/swift/Tests/wordpress-api/WordPressAPITests.swift @@ -80,7 +80,7 @@ private actor CounterMiddleware: Middleware { requestExecutor: RequestExecutor, response: WpNetworkResponse, request: WpNetworkRequest, - cancellationToken: CancellationToken? + context: RequestContext? ) async throws -> WpNetworkResponse { count += 1 return response diff --git a/native/swift/Tools/Sources/GenerateCancellable/main.swift b/native/swift/Tools/Sources/GenerateCancellable/main.swift index c715211a4..049b4c9f7 100644 --- a/native/swift/Tools/Sources/GenerateCancellable/main.swift +++ b/native/swift/Tools/Sources/GenerateCancellable/main.swift @@ -171,7 +171,7 @@ class CancellationAnalyzer: SyntaxVisitor { let paramName = lastParam.firstName.text let paramType = lastParam.type.description.trimmingCharacters(in: .whitespacesAndNewlines) - return paramName == "cancellationToken" && paramType == "CancellationToken?" + return paramName == "context" && paramType == "RequestContext?" } } @@ -346,21 +346,17 @@ class ExtensionGenerator { }.joined(separator: ", ") let functionCallArgs = parameterArguments.isEmpty ? - "cancellationToken: token" : - "\(parameterArguments), cancellationToken: token" + "context: context" : + "\(parameterArguments), context: context" return CodeBlockSyntax { DeclSyntax( """ - let token = CancellationToken() + let context = RequestContext() return try await withTaskCancellationHandler { try await \(raw: cancellationFunctionName)(\(raw: functionCallArgs)) } onCancel: { - do { - try token.cancel() - } catch { - NSLog("Failed to cancel \\(#function): \\(error)") - } + self.requestExecutor().cancel(context: context) } """ ) diff --git a/wp_api/src/cancellation.rs b/wp_api/src/cancellation.rs index 668503ae8..f6ecea450 100644 --- a/wp_api/src/cancellation.rs +++ b/wp_api/src/cancellation.rs @@ -1,147 +1,30 @@ -use std::{ - collections::VecDeque, - sync::{Arc, Mutex, PoisonError}, -}; - -use crate::uuid::WpUuid; - -#[uniffi::export(with_foreign)] -pub trait CancellationHandler: Send + Sync + std::fmt::Debug { - fn cancelled(&self); -} +use std::sync::Mutex; #[derive(Debug, Default, uniffi::Object)] -pub struct CancellationToken { - uuid: WpUuid, - cancelled: Mutex, - handler: Mutex>>, +pub struct RequestContext { + request_ids: Mutex>, } #[uniffi::export] -impl CancellationToken { +impl RequestContext { #[uniffi::constructor] pub fn new() -> Self { Self { - uuid: WpUuid::default(), - cancelled: Mutex::new(false), - handler: Mutex::new(VecDeque::new()), + request_ids: Mutex::new(Vec::new()), } } - pub fn uuid(&self) -> String { - self.uuid.uuid_string() - } - - pub fn register_handler( - &self, - handler: Arc, - ) -> Result<(), CancellationTokenError> { - let mut handlers = self.handler.lock()?; - handlers.push_back(handler); - Ok(()) - } - - pub fn cancel(&self) -> Result<(), CancellationTokenError> { - let mut cancelled = self.cancelled.lock()?; - if *cancelled { - return Ok(()); - } - - *cancelled = true; - - let mut handlers = self.handler.lock()?; - while let Some(handler) = handlers.pop_front() { - handler.cancelled(); + pub fn add_request_id(&self, request_id: String) { + if let Ok(mut ids) = self.request_ids.lock() { + ids.push(request_id); } - - Ok(()) - } -} - -#[derive(Debug, uniffi::Error, thiserror::Error)] -pub enum CancellationTokenError { - #[error("Error acquiring lock")] - Locking, -} - -impl From> for CancellationTokenError { - fn from(_: PoisonError) -> Self { - CancellationTokenError::Locking - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[derive(Debug)] - struct TestHandler { - called: std::sync::Arc, } - impl TestHandler { - fn new() -> (Self, std::sync::Arc) { - let called = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); - ( - Self { - called: called.clone(), - }, - called, - ) + pub fn request_ids(&self) -> Vec { + if let Ok(ids) = self.request_ids.lock() { + return (*ids).clone(); } - } - - impl CancellationHandler for TestHandler { - fn cancelled(&self) { - self.called.store(true, std::sync::atomic::Ordering::SeqCst); - } - } - - #[tokio::test] - async fn test_register_handler_single_handler() { - let token = CancellationToken::new(); - let (handler, called_flag) = TestHandler::new(); - - token.register_handler(Arc::new(handler)).unwrap(); - - assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); - - token.cancel().unwrap(); - - assert!(called_flag.load(std::sync::atomic::Ordering::SeqCst)); - } - - #[tokio::test] - async fn test_register_handler_multiple_handlers() { - let token = CancellationToken::new(); - let (handler1, called_flag1) = TestHandler::new(); - let (handler2, called_flag2) = TestHandler::new(); - let (handler3, called_flag3) = TestHandler::new(); - - token.register_handler(Arc::new(handler1)).unwrap(); - token.register_handler(Arc::new(handler2)).unwrap(); - token.register_handler(Arc::new(handler3)).unwrap(); - - assert!(!called_flag1.load(std::sync::atomic::Ordering::SeqCst)); - assert!(!called_flag2.load(std::sync::atomic::Ordering::SeqCst)); - assert!(!called_flag3.load(std::sync::atomic::Ordering::SeqCst)); - - token.cancel().unwrap(); - - assert!(called_flag1.load(std::sync::atomic::Ordering::SeqCst)); - assert!(called_flag2.load(std::sync::atomic::Ordering::SeqCst)); - assert!(called_flag3.load(std::sync::atomic::Ordering::SeqCst)); - } - - #[tokio::test] - async fn test_register_handler_after_cancellation() { - let token = CancellationToken::new(); - let (handler, called_flag) = TestHandler::new(); - - token.cancel().unwrap(); - - token.register_handler(Arc::new(handler)).unwrap(); - assert!(!called_flag.load(std::sync::atomic::Ordering::SeqCst)); + vec![] } } diff --git a/wp_api/src/middleware.rs b/wp_api/src/middleware.rs index 36aa7dc6e..c5847ab63 100644 --- a/wp_api/src/middleware.rs +++ b/wp_api/src/middleware.rs @@ -1,7 +1,7 @@ use crate::{ api_client::IsWpApiClientDelegate, api_error::{RequestExecutionError, RequestExecutionErrorReason}, - cancellation::CancellationToken, + cancellation::RequestContext, request::{RequestExecutor, WpNetworkRequest, WpNetworkResponse}, }; use std::{fmt::Debug, sync::Arc, time::Duration}; @@ -23,7 +23,7 @@ impl WpApiMiddlewarePipeline { request_executor: Arc, response: WpNetworkResponse, request: Arc, - cancellation_token: Option>, + context: Option>, ) -> Result { let mut response = response; @@ -33,7 +33,7 @@ impl WpApiMiddlewarePipeline { request_executor.clone(), response, request.clone(), - cancellation_token.clone(), + context.clone(), ) .await?; } @@ -78,7 +78,7 @@ pub trait WpApiMiddleware: Send + Sync + Debug { request_executor: Arc, response: WpNetworkResponse, request: Arc, - cancellation_token: Option>, + context: Option>, ) -> Result; } @@ -92,20 +92,21 @@ pub trait PerformsRequests { async fn perform( &self, request: Arc, - cancellation_token: Option>, + context: Option>, ) -> Result { + if let Some(context) = &context { + context.add_request_id(request.uuid.clone()); + } + let pipeline = &self.get_middleware_pipeline(); - let response = self - .get_request_executor() - .execute(request.clone(), cancellation_token.clone()) - .await?; + let response = self.get_request_executor().execute(request.clone()).await?; let response = pipeline .process( self.get_request_executor().clone(), response, request.clone(), - cancellation_token.clone(), + context.clone(), ) .await?; @@ -166,7 +167,7 @@ impl WpApiMiddleware for RetryAfterMiddleware { request_executor: Arc, response: WpNetworkResponse, request: Arc, - cancellation_token: Option>, + context: Option>, ) -> Result { let mut response = response; @@ -190,10 +191,11 @@ impl WpApiMiddleware for RetryAfterMiddleware { ) .await; let new_request = Arc::new(request.clone_with_incremented_retry_count()); - response = request_executor - .execute(new_request.clone(), cancellation_token.clone()) - .await?; - self.process(request_executor, response, new_request, cancellation_token) + if let Some(context) = &context { + context.add_request_id(new_request.uuid.clone()); + } + response = request_executor.execute(new_request.clone()).await?; + self.process(request_executor, response, new_request, context) .await } else { // We have no idea how long to wait so we shouldn't try @@ -227,7 +229,7 @@ impl WpApiMiddleware for ApiDiscoveryAuthenticationMiddleware { request_executor: Arc, response: WpNetworkResponse, request: Arc, - cancellation_token: Option>, + context: Option>, ) -> Result { if response.request_header_map.has_http_authentication() { // Request was already authenticated @@ -238,12 +240,15 @@ impl WpApiMiddleware for ApiDiscoveryAuthenticationMiddleware { return Ok(response); } + if let Some(context) = &context { + context.add_request_id(request.uuid.clone()); + } + request_executor .execute( request .adding_http_authentication(&self.username, &self.password) .into(), - cancellation_token, ) .await } @@ -256,7 +261,6 @@ mod tests { mod api_discovery_authentication_middleware { use crate::{ api_error::MediaUploadRequestExecutionError, - cancellation::CancellationToken, request::{ WpNetworkHeaderMap, endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, @@ -278,7 +282,6 @@ mod tests { async fn execute( &self, request: Arc, - _cancellation_token: Option>, ) -> Result { (self.execute_fn)(request) } @@ -286,7 +289,6 @@ mod tests { async fn upload_media( &self, _: Arc, - _cancellation_token: Option>, ) -> Result { Err(MediaUploadRequestExecutionError::RequestExecutionFailed { status_code: None, @@ -298,6 +300,8 @@ mod tests { } async fn sleep(&self, _: u64) {} + + fn cancel(&self, _: Arc) {} } #[tokio::test] @@ -396,7 +400,6 @@ mod tests { use super::*; use crate::{ api_error::MediaUploadRequestExecutionError, - cancellation::CancellationToken, request::{ WpNetworkHeaderMap, endpoint::{WpEndpointUrl, media_endpoint::MediaUploadRequest}, @@ -417,7 +420,6 @@ mod tests { async fn execute( &self, _: Arc, - _cancellation_token: Option>, ) -> Result { if self.first_request.load(Ordering::Relaxed) { println!("First mock request; returning 429.."); @@ -438,7 +440,6 @@ mod tests { async fn upload_media( &self, _: Arc, - _cancellation_token: Option>, ) -> Result { Err(MediaUploadRequestExecutionError::RequestExecutionFailed { status_code: None, @@ -450,6 +451,8 @@ mod tests { } async fn sleep(&self, _: u64) {} + + fn cancel(&self, _: Arc) {} } #[tokio::test] diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index bd64216ab..fbb5885d0 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -5,7 +5,7 @@ use crate::{ RequestExecutionErrorReason, WpApiError, WpErrorCode, }, auth::WpAuthenticationProvider, - cancellation::CancellationToken, + cancellation::RequestContext, url_query::{FromUrlQueryPairs, UrlQueryPairsMap}, }; use base64::Engine; @@ -144,16 +144,16 @@ pub trait RequestExecutor: Send + Sync { async fn execute( &self, request: Arc, - cancellation_token: Option>, ) -> Result; async fn upload_media( &self, media_upload_request: Arc, - cancellation_token: Option>, ) -> Result; async fn sleep(&self, millis: u64); + + fn cancel(&self, context: Arc); } #[derive(uniffi::Object)] @@ -790,7 +790,7 @@ pub async fn fetch_authentication_state( ApplicationPasswordsRequestBuilder::new(api_url_resolver, authentication_provider) .retrieve_current_with_edit_context() .into(); - let response = request_executor.execute(request, None).await?; + let response = request_executor.execute(request).await?; let parsed_res: Result< ApplicationPasswordsRequestRetrieveCurrentWithEditContextResponse, WpApiError, diff --git a/wp_api/src/request/endpoint/media_endpoint.rs b/wp_api/src/request/endpoint/media_endpoint.rs index 381415c7d..72844035b 100644 --- a/wp_api/src/request/endpoint/media_endpoint.rs +++ b/wp_api/src/request/endpoint/media_endpoint.rs @@ -1,7 +1,6 @@ use super::{AsNamespace, DerivedRequest, WpEndpointUrl, WpNamespace}; use crate::{ api_error::WpApiError, - cancellation::CancellationToken, media::{MediaCreateParams, MediaId, MediaListParams, MediaUpdateParams, MediaWithEditContext}, request::{ CONTENT_TYPE_MULTIPART, NetworkRequestAccessor, ParsedResponse, RequestMethod, @@ -189,14 +188,13 @@ impl MediaRequestExecutor { file_path: String, file_content_type: String, request_id: Option>, - cancellation_token: Option>, ) -> Result { let request = self .request_builder .create(params, file_path, file_content_type, request_id); self.delegate .request_executor - .upload_media(Arc::new(request), cancellation_token) + .upload_media(Arc::new(request)) .await? .parse() } diff --git a/wp_api/src/reqwest_request_executor.rs b/wp_api/src/reqwest_request_executor.rs index 057ad9a32..43e556814 100644 --- a/wp_api/src/reqwest_request_executor.rs +++ b/wp_api/src/reqwest_request_executor.rs @@ -3,7 +3,7 @@ use crate::{ InvalidSslErrorReason, MediaUploadRequestExecutionError, RequestExecutionError, RequestExecutionErrorReason, }, - cancellation::CancellationToken, + cancellation::RequestContext, request::{ NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, WpNetworkRequest, WpNetworkResponse, endpoint::media_endpoint::MediaUploadRequest, @@ -144,7 +144,6 @@ impl RequestExecutor for ReqwestRequestExecutor { async fn execute( &self, request: Arc, - _cancellation_token: Option>, ) -> Result { self.async_request(request).await.map_err(|e| e.into()) } @@ -152,7 +151,6 @@ impl RequestExecutor for ReqwestRequestExecutor { async fn upload_media( &self, media_upload_request: Arc, - _cancellation_token: Option>, ) -> Result { self.upload_media_request(media_upload_request) .await @@ -170,6 +168,10 @@ impl RequestExecutor for ReqwestRequestExecutor { async fn sleep(&self, millis: u64) { tokio::time::sleep(std::time::Duration::from_millis(millis)).await; } + + fn cancel(&self, _context: Arc) { + // No-op for reqwest + } } impl From for RequestExecutionError { diff --git a/wp_api/src/wordpress_org/client.rs b/wp_api/src/wordpress_org/client.rs index 340ab8fc6..b92a895f6 100644 --- a/wp_api/src/wordpress_org/client.rs +++ b/wp_api/src/wordpress_org/client.rs @@ -134,10 +134,7 @@ impl WordPressOrgApiClient { where T: DeserializeOwned, { - let response = self - .request_executor - .execute(Arc::new(request), None) - .await?; + let response = self.request_executor.execute(Arc::new(request)).await?; Self::parse(response) } diff --git a/wp_api_integration_tests/src/mock.rs b/wp_api_integration_tests/src/mock.rs index 573666f56..429e8f71b 100644 --- a/wp_api_integration_tests/src/mock.rs +++ b/wp_api_integration_tests/src/mock.rs @@ -1,8 +1,7 @@ use async_trait::async_trait; use std::sync::Arc; use wp_api::{ - cancellation::CancellationToken, prelude::*, - request::endpoint::media_endpoint::MediaUploadRequest, + cancellation::RequestContext, prelude::*, request::endpoint::media_endpoint::MediaUploadRequest, }; #[derive(Debug)] @@ -30,7 +29,6 @@ impl RequestExecutor for MockExecutor { async fn execute( &self, request: Arc, - _cancellation_token: Option>, ) -> Result { (self.execute_fn)(request) } @@ -38,12 +36,13 @@ impl RequestExecutor for MockExecutor { async fn upload_media( &self, media_upload_request: Arc, - _cancellation_token: Option>, ) -> Result { (self.upload_media_fn)(media_upload_request) } async fn sleep(&self, _: u64) {} + + fn cancel(&self, _: Arc) {} } pub mod response_helpers { diff --git a/wp_api_integration_tests/tests/test_app_notifier_immut.rs b/wp_api_integration_tests/tests/test_app_notifier_immut.rs index fe2832c54..713a6b2db 100644 --- a/wp_api_integration_tests/tests/test_app_notifier_immut.rs +++ b/wp_api_integration_tests/tests/test_app_notifier_immut.rs @@ -2,7 +2,7 @@ use std::sync::{ Mutex, atomic::{AtomicBool, Ordering}, }; -use wp_api::{cancellation::CancellationToken, users::UserListParams}; +use wp_api::{cancellation::RequestContext, users::UserListParams}; use wp_api_integration_tests::prelude::*; #[tokio::test] @@ -117,23 +117,22 @@ impl RequestExecutor for TrackedRequestExecutor { async fn execute( &self, request: Arc, - cancellation_token: Option>, ) -> Result { self.requested_urls .lock() .unwrap() .push(request.url().0.clone()); - self.executor.execute(request, cancellation_token).await + self.executor.execute(request).await } async fn upload_media( &self, media_upload_request: Arc, - cancellation_token: Option>, ) -> Result { - self.upload_media(media_upload_request, cancellation_token) - .await + self.upload_media(media_upload_request).await } async fn sleep(&self, _: u64) {} + + fn cancel(&self, _: Arc) {} } diff --git a/wp_api_integration_tests/tests/test_media_err.rs b/wp_api_integration_tests/tests/test_media_err.rs index d21657f4d..56ed5f168 100644 --- a/wp_api_integration_tests/tests/test_media_err.rs +++ b/wp_api_integration_tests/tests/test_media_err.rs @@ -1,6 +1,6 @@ use wp_api::{ auth::WpAuthenticationProvider, - cancellation::CancellationToken, + cancellation::RequestContext, media::{MediaCreateParams, MediaId, MediaListParams, MediaUpdateParams}, posts::WpApiParamPostsOrderBy, prelude::*, @@ -19,7 +19,6 @@ async fn create_media_err_cannot_create() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, - None, ) .await .assert_wp_error(WpErrorCode::CannotCreate) @@ -35,7 +34,6 @@ async fn create_media_err_upload_no_data() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, - None, ) .await .assert_wp_error(WpErrorCode::UploadNoData) @@ -202,7 +200,6 @@ impl RequestExecutor for MediaErrNetworking { async fn execute( &self, _request: Arc, - _cancellation_token: Option>, ) -> Result { Err(RequestExecutionError::RequestExecutionFailed { status_code: None, @@ -216,7 +213,6 @@ impl RequestExecutor for MediaErrNetworking { async fn upload_media( &self, media_upload_request: Arc, - _cancellation_token: Option>, ) -> Result { let mut request = self .client @@ -264,4 +260,6 @@ impl RequestExecutor for MediaErrNetworking { async fn sleep(&self, millis: u64) { tokio::time::sleep(std::time::Duration::from_millis(millis)).await; } + + fn cancel(&self, _context: Arc) {} } diff --git a/wp_api_integration_tests/tests/test_media_mut.rs b/wp_api_integration_tests/tests/test_media_mut.rs index 2271644d9..fe6429c7a 100644 --- a/wp_api_integration_tests/tests/test_media_mut.rs +++ b/wp_api_integration_tests/tests/test_media_mut.rs @@ -19,7 +19,6 @@ async fn upload_media() { MEDIA_TEST_FILE_PATH.to_string(), MEDIA_TEST_FILE_CONTENT_TYPE.to_string(), None, - None, ) .await .assert_response(); diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index f720cc6c2..294b81351 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -65,7 +65,7 @@ fn generate_async_request_executor( variant.attr.request_type, &context_and_filter_handler, ); - let fn_signature_cancellable = append_cancellation_token_param(fn_signature.clone()); + let fn_signature_cancellable = append_context_param(fn_signature.clone()); let fn_signature_body = invoke_cancellation_variant(fn_signature.clone()); let response_type_ident = ident_response_type( &parsed_enum.enum_ident, @@ -86,7 +86,7 @@ fn generate_async_request_executor( let perform_request = async || { #request_from_request_builder let request_url: String = request.url().into(); - let response = self.perform(std::sync::Arc::new(request), cancellation_token.clone()).await?; + let response = self.perform(std::sync::Arc::new(request), context.clone()).await?; let response_status_code = response.status_code; let parsed_response = response.parse(); let unauthorized = parsed_response.is_unauthorized_error().unwrap_or_default() || (response_status_code == 401 && self.fetch_authentication_state().await.map(|auth_state| auth_state.is_unauthorized()).unwrap_or_default()); @@ -238,6 +238,10 @@ fn generate_async_request_executor( pub async fn fetch_authentication_state(&self) -> Result<#crate_ident::request::AuthenticationState, #error_type> { #crate_ident::request::fetch_authentication_state(self.delegate.request_executor.clone(), self.api_url_resolver.clone(), self.delegate.auth_provider.clone()).await } + + pub fn request_executor(&self) -> std::sync::Arc { + self.delegate.request_executor.clone() + } } impl #generated_request_executor_ident { 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 8e3987e43..62e5861fd 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 @@ -84,13 +84,14 @@ pub fn fn_signature( quote! { fn #fn_name(&self, #url_params #provided_param #fields_param) } } -pub fn append_cancellation_token_param(input: TokenStream) -> TokenStream { +pub fn append_context_param(input: TokenStream) -> TokenStream { let mut signature: syn::Signature = syn::parse2(input).unwrap(); let original_name = signature.ident.to_string(); signature.ident = format_ident!("{}_cancellation", original_name); - let new_arg: FnArg = parse_quote! { cancellation_token: Option> }; + let new_arg: FnArg = + parse_quote! { context: Option> }; signature.inputs.push(new_arg); quote! { #signature } @@ -1286,14 +1287,14 @@ mod tests { #[rstest] #[case( quote! { fn list(&self, params: &UserListParams) }, - "fn list_cancellation (& self , params : & UserListParams , cancellation_token : Option < std :: sync :: Arc < crate :: cancellation :: CancellationToken > >)" + "fn list_cancellation (& self , params : & UserListParams , context : Option < std :: sync :: Arc < crate :: cancellation :: RequestContext > >)" )] #[case( quote! { fn list(&self) }, - "fn list_cancellation (& self , cancellation_token : Option < std :: sync :: Arc < crate :: cancellation :: CancellationToken > >)" + "fn list_cancellation (& self , context : Option < std :: sync :: Arc < crate :: cancellation :: RequestContext > >)" )] - fn test_append_cancellation_token_param(#[case] input: TokenStream, #[case] expected: &str) { - let result = append_cancellation_token_param(input); + fn test_append_context_param(#[case] input: TokenStream, #[case] expected: &str) { + let result = append_context_param(input); assert_eq!(result.to_string(), expected); } From 180888483a29183fcc3f75c7a43a86c701fef04c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Sep 2025 11:08:45 +1200 Subject: [PATCH 17/22] Fix a swiftlint issue --- native/swift/Sources/wordpress-api/SafeRequestExecutor.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift index 545fdb854..0ab39589d 100644 --- a/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift +++ b/native/swift/Sources/wordpress-api/SafeRequestExecutor.swift @@ -55,7 +55,7 @@ public final class WpRequestExecutor: SafeRequestExecutor { self.additionalHttpHeadersForAllRequests = headers } - public func execute(_ request: WpNetworkRequest,) async -> Result { + public func execute(_ request: WpNetworkRequest) async -> Result { await perform(request) } From 40bb559532c5f42400c82668c2fdc31fbca52414 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Sep 2025 11:37:15 +1200 Subject: [PATCH 18/22] Exclude SPM buil dir from swiftlint --- .swiftlint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index 960495ece..a1bd52483 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -4,6 +4,7 @@ included: - native/swift excluded: # paths to ignore during linting. Takes precedence over `included`. - native/swift/Sources/wordpress-api-wrapper # auto-generated code + - native/swift/**/.build disabled_rules: # Don't think we should enable this rule. # See https://github.com/realm/SwiftLint/issues/5263 for context. From 95dccc0d9fb03e26f1906aa17e2533ac88605a84 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Sep 2025 11:59:24 +1200 Subject: [PATCH 19/22] Replace `RequestExecutor` getter with a `cancel` function --- native/swift/Tools/Sources/GenerateCancellable/main.swift | 2 +- wp_derive_request_builder/src/generate.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/native/swift/Tools/Sources/GenerateCancellable/main.swift b/native/swift/Tools/Sources/GenerateCancellable/main.swift index 049b4c9f7..1a10dd006 100644 --- a/native/swift/Tools/Sources/GenerateCancellable/main.swift +++ b/native/swift/Tools/Sources/GenerateCancellable/main.swift @@ -356,7 +356,7 @@ class ExtensionGenerator { return try await withTaskCancellationHandler { try await \(raw: cancellationFunctionName)(\(raw: functionCallArgs)) } onCancel: { - self.requestExecutor().cancel(context: context) + self.cancel(context: context) } """ ) diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 294b81351..9ae3ca8ca 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -239,8 +239,8 @@ fn generate_async_request_executor( #crate_ident::request::fetch_authentication_state(self.delegate.request_executor.clone(), self.api_url_resolver.clone(), self.delegate.auth_provider.clone()).await } - pub fn request_executor(&self) -> std::sync::Arc { - self.delegate.request_executor.clone() + pub fn cancel(&self, context: std::sync::Arc) { + self.delegate.request_executor.cancel(context); } } From 0d252dde14b32b7451498441062c434c78762885 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Sep 2025 12:01:34 +1200 Subject: [PATCH 20/22] Remove an unused doc --- .../main/kotlin/rs/wordpress/api/kotlin/pr.md | 86 ------------------- 1 file changed, 86 deletions(-) delete mode 100644 native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md deleted file mode 100644 index 12d2da2a5..000000000 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/pr.md +++ /dev/null @@ -1,86 +0,0 @@ -Relates to https://github.com/Automattic/wordpress-rs/issues/824. - -This PR adds cancellation support to the Swift wrapper. The same API (for example, `api.posts.create(...)`), which was not cancellable, can now be cancelled. - -To verify this change, you can checkout the first commit (Add CancellationTests), run `make xcframework-only-macos`, then run the `CancellationTests` from Xcode. The test should fail. Checkout the last commit, repeat the same steps, the same tests should pass. - -### Goal - -Use this Swift code as an example: - -```swift -let task = Task { - let newPost = try await api.posts.create(...) - viewPostDetails(newPost) -} - -// a short moment later -task.cancel() -``` - -When you cancel the task that is creating a new post and the POST HTTP request is still going, we expect the HTTP request to be cancelled, and the following `viewPostDetails` call should not be called. - -Putting this scenario into the mobile app's context, you can think of it as cancelling the publishing of a new post, replying to a comment, uploading an image, etc. When user taps a cancel button to cancel those pending jobs, we should try our best to abort the underlying HTTP request to prevent the content reaching the server. Obviously, we can't 100% guarantee that, because the request may have already reached the server when user taps the cancel button, and we can't do much from client side about that. - -### Difficulties - -There are several difficulties involved in supporting the native Swift task cancellation, when the task calls Rust's async functions underneath. Because both Swift's Task and Rust's Future API are involved, and they obviously work quite differently. - -As I mentioned in https://github.com/Automattic/wordpress-rs/issues/824, uniffi-rs does not have built-in cancellation support. We'd need to implement the mechanism ourselves. - -### Approach - -First, Rust API needs to expose a `cancellation_token: CancellationToken`. This PR modified the derive macro to add the parameter to all endpoint functions. - -The generated code now looks like this: - -```rust -impl UsersRequestExecutor { - // This is a new function, which supports cancellation when the `cancellation_token` parameter value is not None. - fn list_with_view_context_cancellation(params: UserListParams, cancellation_token: Option>) { - let request = ... - let response = self.request_executor.execute(request, cancellation_token).await; - return response.parse() - } - - // This is the existing function, which does not support cancellation, since it passes `cancellation_token: None` to the function above. - fn list_with_view_context(params: UserListParams) { - list_with_view_context_cancellation(params, None) - } -} -``` - -This change does not break the endpoint APIs. Their function signature stays the same, and they cannot be cancelled as before. The Rust code acts as a facilitator, which accepts a `CancellationToken` instance and passes it to the native implementation of `RequestExecutor`, so that the request executor can cancel HTTP requests when cancelled. - -The `RequestExecutor` API is changed to support that. Both existing functions now accept a `cancellation_token: CancellationToken?` parameter. The native implementation can use `CancellationToken.register_handler` function to get a callback when `CancellationToken` gets cancelled. The implementation here is isolated within the native `RequestExecutor`, which makes the actual HTTP request cancellation relatively straightforward. - -Finally, in order to provide a cancellable version of `list_with_view_context`, and keep the source code backward compatible, I have introduced a feature to prevent the `list_with_view_context` from being exported as a uniffi function. The native wrapper can then generate the `list_with_view_context` function according to the cancellable version: `list_with_view_context_cancellation`. It looks like this in Swift: - -```swift -extension UsersRequestExecutor { - func listWithViewContext(params: UserListParam) async throws { - let token = CancellationToken() - return try await withTaskCancellationHandler { - try await self.listWithViewContextCancellation(params, cancellationToken: token) - } onCancel: { - token.cancel() - } - } -} -``` - -### Limitations - -The cancellation support is opt-in. As in, the Rust functions need to accept a `CancellationToken` parameter, and make sure to pass the instance down to its call chain. - -```rust -fn upload_post(cancellation_token: CancellationToken) { - for image in images { - media.upload_image(image, cancellation_token).await - } - - post.create(title, content, cancellation_token).await -} -``` - -The macro change should cover the majority of the HTTP requests sent to `RequestExecutor`. But there is still code that does not support cancellation. For example, the API discovery, which we can look into later if needed. From ad55d7d5729ac5b4ff77dc1d2103af44aca75b9b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 24 Sep 2025 12:03:25 +1200 Subject: [PATCH 21/22] Remove an unnecessary unit test --- .../Tests/integration-tests/KnownIssues.swift | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 native/swift/Tests/integration-tests/KnownIssues.swift diff --git a/native/swift/Tests/integration-tests/KnownIssues.swift b/native/swift/Tests/integration-tests/KnownIssues.swift deleted file mode 100644 index 59eee69e8..000000000 --- a/native/swift/Tests/integration-tests/KnownIssues.swift +++ /dev/null @@ -1,37 +0,0 @@ -import Foundation -import Testing - -@testable import WordPressAPI -@testable import WordPressAPIInternal - -@Suite(.disabled("The tests fails due to issues in uniffi-rs")) -struct KnownIssues { - let api = WordPressAPI.admin() - - @Test - func uniffiAsyncFunctionsAreNotCancellable() async throws { - // See https://mozilla.github.io/uniffi-rs/0.29/futures.html#cancelling-async-code - - let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil)) - let task = Task { - _ = try await api.media.create( - params: .init(title: "Image", altText: "This is a test image"), - filePath: file.path, - fileContentType: "image/jpeg", - requestId: nil, - ) - } - - try await Task.sleep(nanoseconds: 50_000_000) - task.cancel() - - await #expect( - throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError), - performing: { - try await task.value - } - ) - - try await restoreTestServer() - } -} From 2d27f2e562b82393a6971007480cbc07a07ae403 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 25 Sep 2025 10:37:46 +1200 Subject: [PATCH 22/22] Move `RequestContext` to the `request` module --- wp_api/src/cancellation.rs | 30 ----------------- wp_api/src/lib.rs | 1 - wp_api/src/middleware.rs | 2 +- wp_api/src/request.rs | 32 +++++++++++++++++-- wp_api/src/reqwest_request_executor.rs | 2 +- wp_api_integration_tests/src/mock.rs | 2 +- .../tests/test_app_notifier_immut.rs | 2 +- .../tests/test_media_err.rs | 2 +- wp_derive_request_builder/src/generate.rs | 2 +- .../generate/helpers_to_generate_tokens.rs | 6 ++-- 10 files changed, 39 insertions(+), 42 deletions(-) delete mode 100644 wp_api/src/cancellation.rs diff --git a/wp_api/src/cancellation.rs b/wp_api/src/cancellation.rs deleted file mode 100644 index f6ecea450..000000000 --- a/wp_api/src/cancellation.rs +++ /dev/null @@ -1,30 +0,0 @@ -use std::sync::Mutex; - -#[derive(Debug, Default, uniffi::Object)] -pub struct RequestContext { - request_ids: Mutex>, -} - -#[uniffi::export] -impl RequestContext { - #[uniffi::constructor] - pub fn new() -> Self { - Self { - request_ids: Mutex::new(Vec::new()), - } - } - - pub fn add_request_id(&self, request_id: String) { - if let Ok(mut ids) = self.request_ids.lock() { - ids.push(request_id); - } - } - - pub fn request_ids(&self) -> Vec { - if let Ok(ids) = self.request_ids.lock() { - return (*ids).clone(); - } - - vec![] - } -} diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 245eb3690..50756d6a8 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -13,7 +13,6 @@ pub mod api_client; pub mod api_error; pub mod application_passwords; pub mod auth; -pub mod cancellation; pub mod categories; pub mod comments; pub mod date; diff --git a/wp_api/src/middleware.rs b/wp_api/src/middleware.rs index c5847ab63..e3f905d7a 100644 --- a/wp_api/src/middleware.rs +++ b/wp_api/src/middleware.rs @@ -1,7 +1,7 @@ use crate::{ api_client::IsWpApiClientDelegate, api_error::{RequestExecutionError, RequestExecutionErrorReason}, - cancellation::RequestContext, + request::RequestContext, request::{RequestExecutor, WpNetworkRequest, WpNetworkResponse}, }; use std::{fmt::Debug, sync::Arc, time::Duration}; diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index fbb5885d0..dff87550a 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -5,7 +5,6 @@ use crate::{ RequestExecutionErrorReason, WpApiError, WpErrorCode, }, auth::WpAuthenticationProvider, - cancellation::RequestContext, url_query::{FromUrlQueryPairs, UrlQueryPairsMap}, }; use base64::Engine; @@ -25,7 +24,7 @@ use std::{ collections::HashMap, fmt::Debug, str::{FromStr, Utf8Error}, - sync::Arc, + sync::{Arc, Mutex}, }; use url::Url; use uuid::Uuid; @@ -823,6 +822,35 @@ impl AuthenticationState { } } +#[derive(Debug, Default, uniffi::Object)] +pub struct RequestContext { + request_ids: Mutex>, +} + +#[uniffi::export] +impl RequestContext { + #[uniffi::constructor] + pub fn new() -> Self { + Self { + request_ids: Mutex::new(Vec::new()), + } + } + + pub fn add_request_id(&self, request_id: String) { + if let Ok(mut ids) = self.request_ids.lock() { + ids.push(request_id); + } + } + + pub fn request_ids(&self) -> Vec { + if let Ok(ids) = self.request_ids.lock() { + return (*ids).clone(); + } + + vec![] + } +} + pub mod user_agent { #[uniffi::export] pub fn default_user_agent(client_specific_postfix: &str) -> String { diff --git a/wp_api/src/reqwest_request_executor.rs b/wp_api/src/reqwest_request_executor.rs index 43e556814..29b49a242 100644 --- a/wp_api/src/reqwest_request_executor.rs +++ b/wp_api/src/reqwest_request_executor.rs @@ -3,7 +3,7 @@ use crate::{ InvalidSslErrorReason, MediaUploadRequestExecutionError, RequestExecutionError, RequestExecutionErrorReason, }, - cancellation::RequestContext, + request::RequestContext, request::{ NetworkRequestAccessor, RequestExecutor, RequestMethod, WpNetworkHeaderMap, WpNetworkRequest, WpNetworkResponse, endpoint::media_endpoint::MediaUploadRequest, diff --git a/wp_api_integration_tests/src/mock.rs b/wp_api_integration_tests/src/mock.rs index 429e8f71b..212d969a0 100644 --- a/wp_api_integration_tests/src/mock.rs +++ b/wp_api_integration_tests/src/mock.rs @@ -1,7 +1,7 @@ use async_trait::async_trait; use std::sync::Arc; use wp_api::{ - cancellation::RequestContext, prelude::*, request::endpoint::media_endpoint::MediaUploadRequest, + prelude::*, request::RequestContext, request::endpoint::media_endpoint::MediaUploadRequest, }; #[derive(Debug)] diff --git a/wp_api_integration_tests/tests/test_app_notifier_immut.rs b/wp_api_integration_tests/tests/test_app_notifier_immut.rs index 713a6b2db..f55408904 100644 --- a/wp_api_integration_tests/tests/test_app_notifier_immut.rs +++ b/wp_api_integration_tests/tests/test_app_notifier_immut.rs @@ -2,7 +2,7 @@ use std::sync::{ Mutex, atomic::{AtomicBool, Ordering}, }; -use wp_api::{cancellation::RequestContext, users::UserListParams}; +use wp_api::{request::RequestContext, users::UserListParams}; use wp_api_integration_tests::prelude::*; #[tokio::test] diff --git a/wp_api_integration_tests/tests/test_media_err.rs b/wp_api_integration_tests/tests/test_media_err.rs index 56ed5f168..a51c5285d 100644 --- a/wp_api_integration_tests/tests/test_media_err.rs +++ b/wp_api_integration_tests/tests/test_media_err.rs @@ -1,9 +1,9 @@ use wp_api::{ auth::WpAuthenticationProvider, - cancellation::RequestContext, media::{MediaCreateParams, MediaId, MediaListParams, MediaUpdateParams}, posts::WpApiParamPostsOrderBy, prelude::*, + request::RequestContext, request::endpoint::media_endpoint::MediaUploadRequest, users::UserId, }; diff --git a/wp_derive_request_builder/src/generate.rs b/wp_derive_request_builder/src/generate.rs index 9ae3ca8ca..fdd0a0f70 100644 --- a/wp_derive_request_builder/src/generate.rs +++ b/wp_derive_request_builder/src/generate.rs @@ -239,7 +239,7 @@ fn generate_async_request_executor( #crate_ident::request::fetch_authentication_state(self.delegate.request_executor.clone(), self.api_url_resolver.clone(), self.delegate.auth_provider.clone()).await } - pub fn cancel(&self, context: std::sync::Arc) { + pub fn cancel(&self, context: std::sync::Arc) { self.delegate.request_executor.cancel(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 62e5861fd..e4d68e48b 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 @@ -91,7 +91,7 @@ pub fn append_context_param(input: TokenStream) -> TokenStream { signature.ident = format_ident!("{}_cancellation", original_name); let new_arg: FnArg = - parse_quote! { context: Option> }; + parse_quote! { context: Option> }; signature.inputs.push(new_arg); quote! { #signature } @@ -1287,11 +1287,11 @@ mod tests { #[rstest] #[case( quote! { fn list(&self, params: &UserListParams) }, - "fn list_cancellation (& self , params : & UserListParams , context : Option < std :: sync :: Arc < crate :: cancellation :: RequestContext > >)" + "fn list_cancellation (& self , params : & UserListParams , context : Option < std :: sync :: Arc < crate :: request :: RequestContext > >)" )] #[case( quote! { fn list(&self) }, - "fn list_cancellation (& self , context : Option < std :: sync :: Arc < crate :: cancellation :: RequestContext > >)" + "fn list_cancellation (& self , context : Option < std :: sync :: Arc < crate :: request :: RequestContext > >)" )] fn test_append_context_param(#[case] input: TokenStream, #[case] expected: &str) { let result = append_context_param(input);