-
Notifications
You must be signed in to change notification settings - Fork 133
Fix unexpected API response error for media upload #14813
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
Fix unexpected API response error for media upload #14813
Conversation
Add a new Gson `TypeAdapterFactory` for handling fields that should be a JSON object but might come as an empty array (`[]`), null, or a primitive from the API. This factory ensures that if the incoming JSON token for a field is not `BEGIN_OBJECT`, it's parsed as `null` instead of causing a crash. This makes the JSON parsing more robust against inconsistent API responses.
Replaces the `JsonObjectOrEmptyArrayDeserializer` with a new `JsonObjectOrNullAdapterFactory`.
The `mediaDetails` field in the `MediaWPRESTResponse` is now nullable to prevent crashes when the API response for media items is missing this information.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
60270ff to
35123ad
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #14813 +/- ##
=========================================
Coverage 38.22% 38.22%
- Complexity 10081 10084 +3
=========================================
Files 2131 2132 +1
Lines 120391 120403 +12
Branches 16480 16484 +4
=========================================
+ Hits 46015 46025 +10
- Misses 69720 69722 +2
Partials 4656 4656 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Really nice work @irfano 👏 , TIL about the AdapterFactory pattern.
I believe we might need to apply the same pattern for the JsonObjectOrFalse item, or maybe even remove it and replace its usage with the new JsonObjectOrNull as it works for all issues, but I think this can be done in a separate PR.
|
Good suggestion! I've opened an issue for it (WOOMOB-1580) and might look into it during my rotation if I can. |
Closes WOOMOB-1560
Description
Sometimes API returns Json array while we're expecting Json object. In this case, we used
JsonObjectOrEmptyArrayfor long time. This class was changing those unexpected values tonull.In the example above,
MediaDetailsis aJsonObjectOrEmptyArray, and itsSizesfield is also aJsonObjectOrEmptyArray. When Gson encountersMediaDetails, it usesJsonObjectOrEmptyArrayDeserializerto parse it. However, becauseJsonObjectOrEmptyArrayDeserializeruses a newGsoninstance it, it can't parseSizes. I tried using the same Gson instance withJsonObjectOrEmptyArrayDeserializer, but that caused aStackOverflowError, so I switched to a TypeAdapterFactory (d6fb0bc).When the API returns
MediaDetailsas an array,JsonObjectOrEmptyArrayDeserializerresolves tonull, which is why I madeMediaDetailsnullable (180671c).Test Steps
I suggest you follow these steps on trunk first to reproduce the error, and then on the PR to verify the solution:
Case 1
Case 2
Repeat the steps in Case 1, but this time, in step 7, use the actual
"media_details": {...}"object but change only theSizesfield to[].For example:
Images/gif
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.