Skip to content
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

[Media] Migrate WPCom media upload to /wp/v2 endpoints #3109

Merged
merged 5 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class MockedStack_MediaTest : MockedStack_Base() {
val site = SiteModel()
site.id = 5
site.setIsWPCom(true)
site.origin = SiteModel.ORIGIN_WPCOM_REST
site.siteId = 6426253
return site
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ private void setupPostAttributes() {
private SiteModel getTestSite() {
SiteModel site = new SiteModel();
site.setIsWPCom(true);
site.setOrigin(SiteModel.ORIGIN_WPCOM_REST);
site.setSiteId(6426253);
return site;
}
Expand Down
243 changes: 196 additions & 47 deletions example/src/androidTest/resources/media-upload-response-success.json
Original file line number Diff line number Diff line change
@@ -1,50 +1,199 @@
{
"media":[
{
"ID":9999,
"URL":"https://place.com/photo.jpg",
"guid":"http:\/\/place.com\/photo.jpg",
"date":"2017-08-11T12:17:49+00:00",
"post_ID":0,
"author_ID":12345,
"file":"photo.jpg",
"mime_type":"image\/jpeg",
"extension":"jpg",
"title":"",
"caption":"Test Caption",
"description":"",
"alt":"",
"icon":"",
"thumbnails":{
"thumbnail":"https:\/\/place.com\/photo.jpg?w=150",
"medium":"https:\/\/place.com\/photo.jpg?w=300",
"large":"https:\/\/place.com\/photo.jpg?w=880"
},
"height":585,
"width":880,
"exif":{
"aperture":"0",
"credit":"",
"camera":"",
"caption":"",
"created_timestamp":"0",
"copyright":"",
"focal_length":"0",
"iso":"0",
"shutter_speed":"0",
"title":"",
"orientation":"0",
"keywords":[

]
},
"meta":{
"links":{
"self":"",
"help":"",
"site":""
}
"id": 9999,
"date": "2024-10-21T17:49:43",
"date_gmt": "2024-10-21T16:49:43",
"guid": {
"rendered": "https://place.com/wp-content/uploads/2024/10/img_0111.jpeg"
},
"modified": "2024-10-21T17:49:45",
"modified_gmt": "2024-10-21T16:49:45",
"slug": "img_0111",
"status": "inherit",
"type": "attachment",
"link": "https://place.com/?attachment_id=576",
"title": {
"rendered": "img_0111"
},
"author": 190912935,
"featured_media": 0,
"comment_status": "open",
"ping_status": "closed",
"template": "",
"meta": {
"_acf_changed": false,
"advanced_seo_description": "",
"jetpack_seo_html_title": "",
"jetpack_seo_noindex": false,
"jetpack_post_was_ever_published": false
},
"class_list": [
"post-576",
"attachment",
"type-attachment",
"status-inherit",
"hentry"
],
"acf": [],
"jetpack_sharing_enabled": true,
"jetpack_likes_enabled": true,
"jetpack_shortlink": "https://wp.me/afje44-9i",
"description": {
"rendered": "<p class=\"attachment\"><a href='https://place.com/wp-content/uploads/2024/10/img_0111.jpeg'><img loading=\"lazy\" decoding=\"async\" width=\"300\" height=\"225\" src=\"https://place.com/wp-content/uploads/2024/10/img_0111-300x225.jpeg\" class=\"attachment-medium size-medium\" alt=\"\" srcset=\"https://place.com/wp-content/uploads/2024/10/img_0111-300x225.jpeg 300w, https://place.com/wp-content/uploads/2024/10/img_0111-1024x768.jpeg 1024w, https://place.com/wp-content/uploads/2024/10/img_0111-768x576.jpeg 768w, https://place.com/wp-content/uploads/2024/10/img_0111-1536x1152.jpeg 1536w, https://place.com/wp-content/uploads/2024/10/img_0111-2048x1536.jpeg 2048w, https://place.com/wp-content/uploads/2024/10/img_0111-1320x990.jpeg 1320w, https://place.com/wp-content/uploads/2024/10/img_0111-600x450.jpeg?crop=1 600w\" sizes=\"auto, (max-width: 300px) 100vw, 300px\" data-attachment-id=\"576\" data-permalink=\"https://place.com/?attachment_id=576\" data-orig-file=\"https://place.com/wp-content/uploads/2024/10/img_0111.jpeg\" data-orig-size=\"3000,2250\" data-comments-opened=\"1\" data-image-meta=\"{&quot;aperture&quot;:&quot;2.4&quot;,&quot;credit&quot;:&quot;&quot;,&quot;camera&quot;:&quot;iPhone X&quot;,&quot;caption&quot;:&quot;&quot;,&quot;created_timestamp&quot;:&quot;1522412059&quot;,&quot;copyright&quot;:&quot;&quot;,&quot;focal_length&quot;:&quot;6&quot;,&quot;iso&quot;:&quot;16&quot;,&quot;shutter_speed&quot;:&quot;0.0047846889952153&quot;,&quot;title&quot;:&quot;&quot;,&quot;orientation&quot;:&quot;1&quot;}\" data-image-title=\"img_0111\" data-image-description=\"\" data-image-caption=\"\" data-medium-file=\"https://place.com/wp-content/uploads/2024/10/img_0111-300x225.jpeg\" data-large-file=\"https://place.com/wp-content/uploads/2024/10/img_0111-1024x768.jpeg\" /></a></p>\n"
},
"caption": {
"rendered": ""
},
"alt_text": "",
"media_type": "image",
"mime_type": "image/jpeg",
"media_details": {
"width": 3000,
"height": 2250,
"file": "2024/10/img_0111.jpeg",
"filesize": 2926027,
"sizes": {
"medium": {
"file": "img_0111-300x225.jpeg",
"width": 300,
"height": 225,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-300x225.jpeg"
},
"large": {
"file": "img_0111-1024x768.jpeg",
"width": 1024,
"height": 768,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-1024x768.jpeg"
},
"thumbnail": {
"file": "img_0111-150x150.jpeg",
"width": 150,
"height": 150,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-150x150.jpeg?crop=1"
},
"medium_large": {
"file": "img_0111-768x576.jpeg",
"width": 768,
"height": 576,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-768x576.jpeg"
},
"1536x1536": {
"file": "img_0111-1536x1152.jpeg",
"width": 1536,
"height": 1152,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-1536x1152.jpeg"
},
"2048x2048": {
"file": "img_0111-2048x1536.jpeg",
"width": 2048,
"height": 1536,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-2048x1536.jpeg"
},
"mailpoet_newsletter_max": {
"file": "img_0111-1320x990.jpeg",
"width": 1320,
"height": 990,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-1320x990.jpeg"
},
"woocommerce_thumbnail": {
"file": "img_0111-450x450.jpeg",
"width": 450,
"height": 450,
"virtual": true,
"uncropped": false,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-450x450.jpeg?crop=1"
},
"woocommerce_single": {
"file": "img_0111-600x450.jpeg",
"width": 600,
"height": 450,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-600x450.jpeg?crop=1"
},
"woocommerce_gallery_thumbnail": {
"file": "img_0111-100x100.jpeg",
"width": 100,
"height": 100,
"virtual": true,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111-100x100.jpeg?crop=1"
},
"full": {
"file": "img_0111.jpeg",
"width": 3000,
"height": 2250,
"mime_type": "image/jpeg",
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111.jpeg"
}
},
"image_meta": {
"aperture": "2.4",
"credit": "",
"camera": "iPhone X",
"caption": "",
"created_timestamp": "1522412059",
"copyright": "",
"focal_length": "6",
"iso": "16",
"shutter_speed": "0.0047846889952153",
"title": "",
"orientation": "1",
"keywords": []
}
]
}
},
"post": 74,
"source_url": "https://place.com/wp-content/uploads/2024/10/img_0111.jpeg",
"_links": {
"self": [
{
"href": "https://place.com/wp-json/wp/v2/media/576",
"targetHints": {
"allow": [
"GET",
"POST",
"PUT",
"PATCH",
"DELETE"
]
}
}
],
"collection": [
{
"href": "https://place.com/wp-json/wp/v2/media"
}
],
"about": [
{
"href": "https://place.com/wp-json/wp/v2/types/attachment"
}
],
"author": [
{
"embeddable": true,
"href": "https://place.com/wp-json/wp/v2/users/190912935"
}
],
"replies": [
{
"embeddable": true,
"href": "https://place.com/wp-json/wp/v2/comments?post=576"
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package org.wordpress.android.fluxc.network.rest.wpapi.media

import com.google.gson.Gson
import com.google.gson.JsonSyntaxException
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.cancel
import kotlinx.coroutines.channels.ProducerScope
Expand Down Expand Up @@ -107,25 +106,15 @@ abstract class BaseWPV2MediaRestClient constructor(
fun ProducerScope<ProgressPayload>.handleFailure(media: MediaModel, error: MediaError) {
media.setUploadState(FAILED)
val payload = ProgressPayload(media, 1f, false, error)
try {
trySendBlocking(payload)
close()
} catch (e: CancellationException) {
// Do nothing (the flow has been cancelled)
}
trySendBlocking(payload)
close()
}

return callbackFlow {
val url = WPAPI.media.getFullUrl(site)
val body = WPRestUploadRequestBody(media) { media, progress ->
if (!isClosedForSend) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I also removed all statements of isClosedForSend, it's marked as delicate API, and it's not needed, as now we use trySend instead of the old offer API.

val payload = ProgressPayload(media, progress, false, null)
try {
trySend(payload).isSuccess
} catch (e: CancellationException) {
// Do nothing (the flow has been cancelled)
}
Comment on lines -125 to -127
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed these parts about catching the CancellationException, I was the one who added these, but they are not useful, as the exception is automatically ignored by the Coroutine's exception handler.

}
val payload = ProgressPayload(media, progress, false, null)
trySend(payload)
}

val request = Request.Builder()
Expand All @@ -138,29 +127,22 @@ abstract class BaseWPV2MediaRestClient constructor(

call.enqueue(object : Callback {
override fun onFailure(call: Call, e: IOException) {
// If the upload has been canceled, then ignore errors
if (!isClosedForSend) {
val message = "media upload failed: $e"
AppLog.w(MEDIA, message)
val error = MediaError.fromIOException(e)
error.logMessage = message
handleFailure(media, error)
}
val message = "media upload failed: $e"
AppLog.w(MEDIA, message)
val error = MediaError.fromIOException(e)
error.logMessage = message
handleFailure(media, error)
}

override fun onResponse(call: Call, response: Response) {
if (isClosedForSend) return
if (response.isSuccessful) {
try {
val res = gson.fromJson(response.body!!.string(), MediaWPRESTResponse::class.java)
val uploadedMedia = res.toMediaModel(site.id)
uploadedMedia.id = media.id
val payload = ProgressPayload(uploadedMedia, 1f, true, false)
try {
trySendBlocking(payload)
close()
} catch (e: CancellationException) {
// Do nothing (the flow has been cancelled)
}
trySendBlocking(payload)
close()
} catch (e: JsonSyntaxException) {
AppLog.e(MEDIA, e)
val error = MediaError(MediaErrorType.PARSE_ERROR)
Expand Down Expand Up @@ -234,7 +216,7 @@ abstract class BaseWPV2MediaRestClient constructor(
params["offset"] = offset.toString()
}
if (mimeType != null) {
params["mime_type"] = mimeType.value
params["media_type"] = mimeType.value
Comment on lines -237 to +219
Copy link
Member Author

@hichamboushaba hichamboushaba Nov 16, 2024

Choose a reason for hiding this comment

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

Working on this PR identified a bug with our previous implementation, when I worked on this in the first time, I based my implementation on the WPCom one and then used the mime_type argument, but it turns out it's not working the same between the two APIs:

  • In WPCom, there is only mime_type, and supports both a fully qualified type, or just the base type (image/jpeg is fully qualified, while image is the base type)
  • In WordPress Core, we have two arguments, media_type which supports the base type, and mime_type which requires the fully qualified type, so when we passed the base type here, the filtering wasn't working, which meant that in the WordPress Media Library screen we would list non-image files.

I believe the same issue is happening on iOS as well (here), I'll create an issue for them to fix it.

}
val response = executeGetGsonRequest(
site,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -934,9 +934,7 @@ private void performUploadMedia(@NonNull UploadMediaPayload payload) {
MediaUtils.stripLocation(payload.media.getFilePath());
}

if (payload.site.isUsingWpComRestApi()) {
mMediaRestClient.uploadMedia(payload.site, payload.media);
} else if (payload.site.isJetpackCPConnected()) {
if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) {
mWPComV2MediaRestClient.uploadMedia(payload.site, payload.media);
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
&& mApplicationPasswordsConfiguration.isEnabled()) {
Expand All @@ -958,9 +956,7 @@ private void performFetchMediaList(@NonNull FetchMediaListPayload payload) {
offset = MediaSqlUtils.getMediaWithStates(payload.site, list).size();
}
}
if (payload.site.isUsingWpComRestApi()) {
mMediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType);
} else if (payload.site.isJetpackCPConnected()) {
if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) {
mWPComV2MediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType);
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
&& mApplicationPasswordsConfiguration.isEnabled()) {
Expand All @@ -977,9 +973,7 @@ private void performFetchMedia(@NonNull MediaPayload payload) {
return;
}

if (payload.site.isUsingWpComRestApi()) {
mMediaRestClient.fetchMedia(payload.site, payload.media);
} else if (payload.site.isJetpackCPConnected()) {
if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) {
mWPComV2MediaRestClient.fetchMedia(payload.site, payload.media);
} else {
mMediaXmlrpcClient.fetchMedia(payload.site, payload.media);
Expand Down Expand Up @@ -1008,9 +1002,7 @@ private void performCancelUpload(@NonNull CancelMediaPayload payload) {
MediaSqlUtils.insertOrUpdateMedia(media);
}

if (payload.site.isUsingWpComRestApi()) {
mMediaRestClient.cancelUpload(media);
} else if (payload.site.isJetpackCPConnected()) {
if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) {
mWPComV2MediaRestClient.cancelUpload(media);
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
&& mApplicationPasswordsConfiguration.isEnabled()) {
Expand Down