Skip to content

Kotlin wrapper fix: use File direct access to fix the FileNotFound exception #810

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 7 commits into from
Jul 22, 2025

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented Jul 21, 2025

Problem
The Kotlin wrapper was experiencing file access issues during media uploads, throwing MediaUploadRequestExecutionException.MediaFileNotFound exceptions when attempting to upload files. This prevented successful file uploads through the WordPress REST API implementation.

Solution
This PR resolves the file access issue by modifying the Kotlin wrapper to use direct File access instead of the previous file handling approach. The change ensures that files can be properly accessed and read during the upload process, eliminating the MediaFileNotFound exception.

@adalpari adalpari requested a review from Copilot July 21, 2025 14:26
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 fixes a FileNotFound exception in the Kotlin wrapper by changing how media files are accessed during upload requests. The change removes resource-based file loading in favor of direct file system access with proper validation.

  • Replaces classloader resource loading with direct File constructor access
  • Adds explicit file readability validation using canRead()
  • Simplifies the file handling logic while maintaining error handling

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.

@adalpari It looks like the create media integration test is broken:

java.lang.AssertionError: Request wasn't successful: MediaFileNotFound(filePath=/test-data/test_media.jpg)

@adalpari
Copy link
Contributor Author

@adalpari It looks like the create media integration test is broken:

java.lang.AssertionError: Request wasn't successful: MediaFileNotFound(filePath=/test-data/test_media.jpg)

Yes, I saw it. And actually, that's the reason why the function was not working, but the test was. The function was getting the file as a resource of the project, which is available ONLY inside the project or in test build. However, in prod build we are actually sending external files paths.

Do you happen to know the absolute path of the file? Maybe the test will work that way.

There are other two alternatives, but since I cannot build the project I am a bit limited:

  1. Create a FileResolver and inject it inside WpRequestExecutor so I can mock it in test to get return the proper file.
  2. Add the File as a parameter of MediaUploadRequest instead of the path. So the client will decide how to resolve it.

As said I am a bit limited, specially for option 2.

Wdyt?

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.

I made a minor suggestion to use interface over open class.

@@ -168,3 +167,7 @@ private fun requestExecutionFailedWith(reason: RequestExecutionErrorReason) =
redirects = null,
reason = reason
)

open class FileResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be an interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

@@ -80,6 +81,7 @@ class WpRequestExecutor(
}
}

@Suppress("ComplexCondition")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this due to if (file == null || !file.exists() || !file.isFile || !file.canRead()) condition?

It's fine to suppress it, but I find it weird that it's causing a warning for this 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's because of that. I can extract the file checks though.

@adalpari
Copy link
Contributor Author

I've now made the changes you suggested

@adalpari adalpari merged commit 0d71743 into trunk Jul 22, 2025
20 checks passed
@adalpari adalpari deleted the fix/kotlin-file-accesss-when-uploading-it branch July 22, 2025 09:01
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