Skip to content

Adding the Call object when upload media wo the Kotlin wrapperto allow cancel it #817

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 28, 2025

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented Jul 24, 2025

Add Call object to media upload for external cancellation support
Added the HTTP call object to the media upload flow in the Kotlin wrapper to enable cancellation from external callers. Previously, cancelling the job that initiated the upload didn't properly cancel the underlying HTTP request.

Technical Note: We could alternatively implement a new cancel function directly within the executor, but this is requiring me to create a new payload structure and internal cancellation flowwhich I am unable to do because of the local compilation issues in my IDE.

Note: @oguzkocer I would like to go with that approach to prevent exposing api details, but I was unable to add the new objects and functions and I would like to unblock this feature since the client is ready to use it. We will just need an extra iteration to make the mentioned improvements.

Changes:

Exposed HTTP call object in media upload methods
Enabled external cancellation of in-progress uploads

This maintains backward compatibility while providing the cancellation functionality needed for better user experience in upload scenarios.

@adalpari adalpari requested a review from Copilot July 24, 2025 10:30
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds the ability to cancel media upload operations by exposing the OkHttp Call object through a new callback interface. The change replaces the simple progress callback with a more comprehensive listener interface that provides both progress updates and call creation notifications.

  • Replaces simple upload progress callback with a structured UploadListener interface
  • Exposes the OkHttp Call object when upload starts to enable cancellation
  • Updates method signatures and parameter handling to use the new interface

Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

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

Approving to unblock to temporarily merge it, but this doesn't look right to me to be honest.

I thought you got your IDE working or at least had the ability to implement a custom executor in WPAndroid that we could port to wordpress-rs.

If your IDE was working at some point, but stopped working a couple days ago, it's likely due to the JDK 21 upgrade. So, I suggest making sure you have updated your setup and ./gradlew commands succeed and verify that the correct JDK is used in your Android Studio.

@adalpari
Copy link
Contributor Author

Approving to unblock to temporarily merge it, but this doesn't look right to me to be honest.

Yes, I would like to go with the non-exposing call approach I mentioned. But I needed to unblock this and went this way. Do you have any other suggestion about the solution to take?

I thought you got your IDE working or at least had the ability to implement a custom executor in WPAndroid that we could port to wordpress-rs.

My IDE is working half-way :( . I can run gradle commands, but cannot navigate through the code, see dependencies, and so on.
So, my solution was to implement a custom executor inside WPAndroid (see this draft) and then port it to the rs project as you mentioned.
However, I can not modify, for instance, the Executor interface (needed to add a cancel function) in the WPAndroid project. Nor I can do it in the rs project since no object inside uniffi.wp_api is accessible in my project. :( . Running the project is working, though.

Screenshot 2025-07-26 at 12 14 52

I am a bit hand-tied when interacting with the rs project, but I am more than happy to keep iterating it and becoming more familiar with the project.

@oguzkocer
Copy link
Contributor

@adalpari I only saw your latest comment just now.

As discussed on our screenshare call today, in order to address the indexing issue on Android you can add -Didea.max.intellisense.filesize=999999 to your studio.vmoptions (Help -> Edit Custom VM options…) and then Invalidate Caches + Restart. This is necessary because the generated wp_api.kt file is too large for the default settings.

@adalpari adalpari merged commit f4e2450 into trunk Jul 28, 2025
20 checks passed
@adalpari adalpari deleted the feature/adding-the-call-object-when-uplaod-media branch July 28, 2025 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants