Support media single fetch, upload, and delete using WordPress-rs #22036
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,16 +4,24 @@ import kotlinx.coroutines.CoroutineScope | |
| import kotlinx.coroutines.launch | ||
| import org.wordpress.android.fluxc.Dispatcher | ||
| import org.wordpress.android.fluxc.generated.MediaActionBuilder | ||
| import org.wordpress.android.fluxc.generated.UploadActionBuilder | ||
| import org.wordpress.android.fluxc.model.MediaModel | ||
| import org.wordpress.android.fluxc.model.MediaModel.MediaUploadState | ||
| import org.wordpress.android.fluxc.model.SiteModel | ||
| import org.wordpress.android.fluxc.module.FLUXC_SCOPE | ||
| import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.WpAppNotifierHandler | ||
| import org.wordpress.android.fluxc.store.MediaStore.FetchMediaListResponsePayload | ||
| import org.wordpress.android.fluxc.store.MediaStore.MediaError | ||
| import org.wordpress.android.fluxc.store.MediaStore.MediaErrorType | ||
| import org.wordpress.android.fluxc.store.MediaStore.MediaPayload | ||
| import org.wordpress.android.fluxc.store.MediaStore.ProgressPayload | ||
| import org.wordpress.android.fluxc.utils.AppLogWrapper | ||
| import org.wordpress.android.fluxc.utils.MediaUtils | ||
| import org.wordpress.android.fluxc.utils.MimeType | ||
| import org.wordpress.android.util.AppLog | ||
| import rs.wordpress.api.kotlin.WpApiClient | ||
| import rs.wordpress.api.kotlin.WpRequestResult | ||
| import uniffi.wp_api.MediaCreateParams | ||
| import uniffi.wp_api.MediaDetailsPayload | ||
| import uniffi.wp_api.MediaListParams | ||
| import uniffi.wp_api.MediaWithEditContext | ||
|
|
@@ -36,19 +44,7 @@ class MediaRSApiRestClient @Inject constructor( | |
| ) { | ||
| fun fetchMediaList(site: SiteModel, number: Int, offset: Int, mimeType: MimeType.Type?) { | ||
| scope.launch { | ||
| val client = getWpApiClient(site) | ||
| val mediaResponse = client.request { requestBuilder -> | ||
| requestBuilder.media().listWithEditContext( | ||
| MediaListParams( | ||
|
|
@@ -62,12 +58,12 @@ class MediaRSApiRestClient @Inject constructor( | |
|
|
||
| val mediaModelList = when (mediaResponse) { | ||
| is WpRequestResult.Success -> { | ||
| appLogWrapper.d(AppLog.T.MEDIA, "Fetched media list: ${mediaResponse.response.data.size}") | ||
| mediaResponse.response.data.toMediaModelList(site.id) | ||
| } | ||
|
|
||
| else -> { | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Fetch media list failed: $mediaResponse") | ||
| emptyList() | ||
| } | ||
| } | ||
|
|
@@ -92,6 +88,207 @@ class MediaRSApiRestClient @Inject constructor( | |
| dispatcher.dispatch(MediaActionBuilder.newFetchedMediaListAction(payload)) | ||
| } | ||
|
|
||
| fun fetchMedia(site: SiteModel, media: MediaModel?) { | ||
| if (media == null) { | ||
| val error = MediaError(MediaErrorType.NULL_MEDIA_ARG) | ||
| error.logMessage = "Requested media is null" | ||
| notifyMediaFetched(site, null, error) | ||
| return | ||
| } | ||
|
|
||
| scope.launch { | ||
| val client = getWpApiClient(site) | ||
|
|
||
| val mediaResponse = client.request { requestBuilder -> | ||
| requestBuilder.media().retrieveWithEditContext(media.mediaId) | ||
| } | ||
|
|
||
|
|
||
| when (mediaResponse) { | ||
| is WpRequestResult.Success -> { | ||
| appLogWrapper.d(AppLog.T.MEDIA, "Fetched media with ID: " + media.mediaId) | ||
|
|
||
| val responseMedia: MediaModel = mediaResponse.response.data.toMediaModel(site.id).apply { | ||
| localSiteId = site.id | ||
| } | ||
| notifyMediaFetched(site, responseMedia, null) | ||
| } | ||
|
|
||
| else -> { | ||
| val mediaError = parseMediaError(mediaResponse) | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Fetch media failed: ${mediaError.message}") | ||
| notifyMediaFetched(site, media, mediaError) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Suppress("UseCheckOrError") // Allow to throw IllegalStateException | ||
| private fun parseMediaError(mediaResponse: WpRequestResult<*>): MediaError { | ||
| return when (mediaResponse) { | ||
| is WpRequestResult.Success -> { | ||
| throw IllegalStateException("Success media response should not be parsed as an error") | ||
| } | ||
| is WpRequestResult.MediaFileNotFound<*> -> { | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Media file not found: $mediaResponse") | ||
| MediaError(MediaErrorType.NOT_FOUND).apply { | ||
| message = "Media file not found" | ||
| } | ||
| } | ||
|
|
||
| is WpRequestResult.ResponseParsingError<*> -> { | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Response parsing error: $mediaResponse") | ||
| MediaError(MediaErrorType.PARSE_ERROR).apply { | ||
| message = "Failed to parse response" | ||
| } | ||
| } | ||
|
|
||
| is WpRequestResult.SiteUrlParsingError<*> -> { | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Site URL parsing error: $mediaResponse") | ||
| MediaError(MediaErrorType.MALFORMED_MEDIA_ARG).apply { | ||
| message = "Invalid site URL" | ||
| } | ||
| } | ||
|
|
||
| is WpRequestResult.InvalidHttpStatusCode<*>, | ||
| is WpRequestResult.WpError<*>, | ||
| is WpRequestResult.RequestExecutionFailed<*>, | ||
| is WpRequestResult.UnknownError<*> -> { | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Unknown error: $mediaResponse") | ||
| MediaError(MediaErrorType.GENERIC_ERROR).apply { | ||
| message = "Unknown error occurred" | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun notifyMediaFetched( | ||
| site: SiteModel, | ||
| media: MediaModel?, | ||
| error: MediaError? | ||
| ) { | ||
| val payload = MediaPayload(site, media, error) | ||
| dispatcher.dispatch(MediaActionBuilder.newFetchedMediaAction(payload)) | ||
| } | ||
|
|
||
| fun deleteMedia(site: SiteModel, media: MediaModel?) { | ||
| if (media == null) { | ||
| val error = MediaError(MediaErrorType.NULL_MEDIA_ARG) | ||
| error.logMessage = "Media to delete is null" | ||
| notifyMediaDeleted(site, null, error) | ||
| return | ||
| } | ||
|
|
||
| scope.launch { | ||
| val client = getWpApiClient(site) | ||
|
|
||
| val mediaResponse = client.request { requestBuilder -> | ||
| requestBuilder.media().delete(media.mediaId) | ||
| } | ||
|
|
||
| when (mediaResponse) { | ||
| is WpRequestResult.Success -> { | ||
| appLogWrapper.d(AppLog.T.MEDIA, "Deleted media with ID: " + media.mediaId) | ||
|
|
||
| val responseMedia: MediaModel = mediaResponse.response.data.previous.toMediaModel(site.id).apply { | ||
| localSiteId = site.id | ||
| } | ||
| notifyMediaDeleted(site, responseMedia, null) | ||
| } | ||
|
|
||
| else -> { | ||
| val mediaError = parseMediaError(mediaResponse) | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Delete media failed: ${mediaError.message}") | ||
| notifyMediaDeleted(site, media, mediaError) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun notifyMediaDeleted( | ||
| site: SiteModel, | ||
| media: MediaModel?, | ||
| error: MediaError? | ||
| ) { | ||
| val payload = MediaPayload(site, media, error) | ||
| dispatcher.dispatch(MediaActionBuilder.newDeletedMediaAction(payload)) | ||
| } | ||
|
|
||
| fun uploadMedia(site: SiteModel, media: MediaModel?) { | ||
| if (media == null || media.id == 0) { | ||
| // we can't have a MediaModel without an ID - otherwise we can't keep track of them. | ||
| val error = MediaError(MediaErrorType.INVALID_ID) | ||
| if (media == null) { | ||
| error.logMessage = "Media object is null on upload" | ||
| } else { | ||
| error.logMessage = "Media ID is 0 on upload" | ||
| } | ||
| notifyMediaUploaded(media, error) | ||
| return | ||
| } | ||
|
|
||
| if (media.filePath == null || !MediaUtils.canReadFile(media.filePath)) { | ||
| val error = MediaError(MediaErrorType.FS_READ_PERMISSION_DENIED) | ||
| error.logMessage = "Can't read file on upload" | ||
| notifyMediaUploaded(media, error) | ||
| return | ||
| } | ||
|
|
||
| scope.launch { | ||
| val client = getWpApiClient(site) | ||
|
|
||
| val mediaResponse = client.request { requestBuilder -> | ||
| requestBuilder.media().create( | ||
| params = MediaCreateParams(title = media.title), | ||
| filePath = media.filePath!!, // We have already checked the nullability but it's mutable | ||
| fileContentType = media.mimeType.orEmpty(), | ||
| requestId = null | ||
| ) | ||
| } | ||
|
|
||
| when (mediaResponse) { | ||
| is WpRequestResult.Success -> { | ||
| appLogWrapper.d(AppLog.T.MEDIA, "Uploaded media with ID: " + media.id) | ||
|
|
||
| val responseMedia: MediaModel = mediaResponse.response.data.toMediaModel(site.id).apply { | ||
| id = media.id // be sure we are using the same local id when getting the remote response | ||
| localSiteId = site.id | ||
| } | ||
| notifyMediaUploaded(responseMedia, null) | ||
| } | ||
|
|
||
| else -> { | ||
| val mediaError = parseMediaError(mediaResponse) | ||
| appLogWrapper.e(AppLog.T.MEDIA, "Upload media failed: ${mediaError.message}") | ||
| notifyMediaUploaded(media, mediaError) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun notifyMediaUploaded(media: MediaModel?, error: MediaError?) { | ||
| media?.setUploadState(if (error == null) MediaUploadState.UPLOADED else MediaUploadState.FAILED) | ||
| val payload = ProgressPayload(media, 1f, error == null, error) | ||
| dispatcher.dispatch(UploadActionBuilder.newUploadedMediaAction(payload)) | ||
| } | ||
|
|
||
| private fun getWpApiClient(site: SiteModel): WpApiClient { | ||
|
There was a problem hiding this comment. The getWpApiClient method creates a new WpApiClient instance on every call. Consider caching the client instance per site to avoid unnecessary object creation and potential connection overhead. Copilot uses AI. Check for mistakes. There was a problem hiding this comment. We don't want to cache the object because every call is associated with a specific site. Maybe we could create a map to store one client per site. I'm not sure that won't create other problems, though. |
||
| val authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword( | ||
| username = site.apiRestUsernamePlain, password = site.apiRestPasswordPlain | ||
| ) | ||
| val apiRootUrl = URL(site.buildUrl()) | ||
| val client = WpApiClient( | ||
| wpOrgSiteApiRootUrl = apiRootUrl, | ||
| authProvider = authProvider, | ||
| appNotifier = object : WpAppNotifier { | ||
| override suspend fun requestedWithInvalidAuthentication() { | ||
| wpAppNotifierHandler.notifyRequestedWithInvalidAuthentication(site) | ||
| } | ||
| } | ||
| ) | ||
| return client | ||
| } | ||
|
|
||
| private fun List<MediaWithEditContext>.toMediaModelList( | ||
| siteId: Int | ||
| ): List<MediaModel> = map { it.toMediaModel(siteId) } | ||
|
|
@@ -110,7 +307,7 @@ class MediaRSApiRestClient @Inject constructor( | |
| fileExtension = [email protected]() | ||
| uploadDate = [email protected] | ||
| authorId = [email protected] | ||
| uploadState = MediaUploadState.UPLOADED.toString() | ||
|
|
||
| // Parse the media details | ||
| when (val parsedType = [email protected]([email protected])) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ This is for my own understanding: if we retrieve media with an edit context, does that mean that users without edit privilege will be unable to fetch media?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. And probably @oguzkocer will better answer this question.
Here is the context:
We are not differentiating between media access when requesting it in the client right now, so I opted to ask for the less restrictive media mode like iOS seems to be doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. I think we shouldn't even show the
Mediapage if the user doesn't have these permissions. Best way to go about this is probably to check how wp-admin looks/works for a user with subscriber role.