Merge pull request #14813 from woocommerce/issue/WOOMOB-1560-update-json-object-or-empty-array

Fix unexpected API response error for media upload

Author: irfano

RELEASE-NOTES.txt

@@ -11,6 +11,7 @@
 - [Internal] Remove unused dependencies and optimize dependency scopes [https://github.com/woocommerce/woocommerce-android/pull/14710]
 - [*] Fix a rare crash in order refund flow [https://github.com/woocommerce/woocommerce-android/pull/14742]
 - [*] Fix customer name display when filtering orders from order details  [https://github.com/woocommerce/woocommerce-android/pull/14761]
+- [*] Fixed a rare media upload error [https://github.com/woocommerce/woocommerce-android/pull/14813]
 
 23.4
 -----

libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ReleaseNetworkModule.java

@@ -13,8 +13,7 @@
 import org.wordpress.android.fluxc.network.OkHttpStack;
 import org.wordpress.android.fluxc.network.OpenJdkCookieManager;
 import org.wordpress.android.fluxc.network.RetryOnRedirectBasicNetwork;
-import org.wordpress.android.fluxc.network.rest.JsonObjectOrEmptyArray;
-import org.wordpress.android.fluxc.network.rest.JsonObjectOrEmptyArrayDeserializer;
+import org.wordpress.android.fluxc.network.rest.JsonObjectOrNullAdapterFactory;
 import org.wordpress.android.fluxc.network.rest.JsonObjectOrFalse;
 import org.wordpress.android.fluxc.network.rest.JsonObjectOrFalseDeserializer;
 
@@ -132,8 +131,7 @@ public Gson provideGson() {
         GsonBuilder gsonBuilder = new GsonBuilder();
         gsonBuilder.setLenient();
         gsonBuilder.registerTypeHierarchyAdapter(JsonObjectOrFalse.class, new JsonObjectOrFalseDeserializer());
-        gsonBuilder.registerTypeHierarchyAdapter(JsonObjectOrEmptyArray.class,
-                new JsonObjectOrEmptyArrayDeserializer());
+        gsonBuilder.registerTypeAdapterFactory(new JsonObjectOrNullAdapterFactory());
         return gsonBuilder.create();
     }
 }

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/discovery/RootWPAPIRestResponse.kt

@@ -2,7 +2,7 @@ package org.wordpress.android.fluxc.network.discovery
 
 import com.google.gson.annotations.SerializedName
 import org.wordpress.android.fluxc.network.Response
-import org.wordpress.android.fluxc.network.rest.JsonObjectOrEmptyArray
+import org.wordpress.android.fluxc.network.rest.JsonObjectOrNull
 
 class RootWPAPIRestResponse(
     val name: String? = null,
@@ -14,7 +14,7 @@ class RootWPAPIRestResponse(
 ) : Response {
     class Authentication(
         @SerializedName("application-passwords") val applicationPasswords: ApplicationPasswords? = null
-    ) : JsonObjectOrEmptyArray() {
+    ) : JsonObjectOrNull() {
         class ApplicationPasswords(
             val endpoints: Endpoints?
         ) {

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/GsonRequest.java

@@ -176,8 +176,7 @@ public static GsonBuilder getDefaultGsonBuilder() {
         GsonBuilder gsonBuilder = new GsonBuilder();
         gsonBuilder.setStrictness(Strictness.LENIENT);
         gsonBuilder.registerTypeHierarchyAdapter(JsonObjectOrFalse.class, new JsonObjectOrFalseDeserializer());
-        gsonBuilder.registerTypeHierarchyAdapter(JsonObjectOrEmptyArray.class,
-                new JsonObjectOrEmptyArrayDeserializer());
+        gsonBuilder.registerTypeAdapterFactory(new JsonObjectOrNullAdapterFactory());
         gsonBuilder.registerTypeHierarchyAdapter(List.class, new ListOrObjectDeserializer());
         gsonBuilder.registerTypeHierarchyAdapter(Object[].class, new ArrayOrObjectDeserializer());
         return gsonBuilder;

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/JsonObjectOrEmptyArrayDeserializer.java

@@ -1,3 +1,3 @@
 package org.wordpress.android.fluxc.network.rest;
 
-public abstract class JsonObjectOrEmptyArray {}
+public abstract class JsonObjectOrNull {}

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/JsonObjectOrEmptyArray.java renamed to libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/JsonObjectOrNull.java

@@ -0,0 +1,35 @@
+package org.wordpress.android.fluxc.network.rest
+
+import com.google.gson.Gson
+import com.google.gson.TypeAdapter
+import com.google.gson.TypeAdapterFactory
+import com.google.gson.reflect.TypeToken
+import com.google.gson.stream.JsonReader
+import com.google.gson.stream.JsonToken
+import com.google.gson.stream.JsonWriter
+
+/**
+ * Parses 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.
+ */
+class JsonObjectOrNullAdapterFactory : TypeAdapterFactory {
+    override fun <T> create(gson: Gson, type: TypeToken<T>): TypeAdapter<T>? {
+        if (!JsonObjectOrNull::class.java.isAssignableFrom(type.rawType)) return null
+
+        val delegate: TypeAdapter<T> = gson.getDelegateAdapter(this, type)
+        return object : TypeAdapter<T>() {
+            override fun write(out: JsonWriter, value: T?) {
+                delegate.write(out, value)
+            }
+
+            override fun read(`in`: JsonReader): T? = if (`in`.peek() == JsonToken.BEGIN_OBJECT) {
+                delegate.read(`in`)
+            } else {
+                // BEGIN_ARRAY, NULL, primitive vs.
+                `in`.skipValue()
+                null
+            }
+        }.nullSafe()
+    }
+}

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/JsonObjectOrNullAdapterFactory.kt

@@ -6,7 +6,7 @@ import com.google.gson.annotations.SerializedName
 import org.apache.commons.text.StringEscapeUtils
 import org.wordpress.android.fluxc.model.MediaModel
 import org.wordpress.android.fluxc.model.MediaModel.MediaUploadState
-import org.wordpress.android.fluxc.network.rest.JsonObjectOrEmptyArray
+import org.wordpress.android.fluxc.network.rest.JsonObjectOrNull
 import org.wordpress.android.fluxc.network.rest.wpcom.media.MediaWPComRestResponse
 import org.wordpress.android.util.DateTimeUtils
 import java.text.SimpleDateFormat
@@ -28,7 +28,7 @@ data class MediaWPRESTResponse(
     @SerializedName("alt_text") val altText: String,
     @SerializedName("media_type") val mediaType: String,
     @SerializedName("mime_type") val mimeType: String,
-    @SerializedName("media_details") val mediaDetails: MediaDetails,
+    @SerializedName("media_details") val mediaDetails: MediaDetails?,
     @SerializedName("source_url") val sourceURL: String?
 ) {
     data class Attribute(
@@ -40,13 +40,13 @@ data class MediaWPRESTResponse(
         val height: Int,
         val file: String?,
         val sizes: Sizes?
-    ) : JsonObjectOrEmptyArray()
+    ) : JsonObjectOrNull()
 
     data class Sizes(
         val medium: ImageSize?,
         val thumbnail: ImageSize?,
         val full: ImageSize?
-    ) : JsonObjectOrEmptyArray()
+    ) : JsonObjectOrNull()
 
     data class ImageSize(
         val path: String?,
@@ -70,16 +70,16 @@ fun MediaWPRESTResponse.toMediaModel(localSiteId: Int) = MediaModel(
         SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.ROOT).parse(dateGmt)
     ),
     sourceURL.orEmpty(),
-    mediaDetails.sizes?.thumbnail?.sourceURL,
-    mediaDetails.file,
-    mediaDetails.file?.substringAfterLast('.', ""),
+    mediaDetails?.sizes?.thumbnail?.sourceURL,
+    mediaDetails?.file,
+    mediaDetails?.file?.substringAfterLast('.', ""),
     mimeType,
     StringEscapeUtils.unescapeHtml4(title.rendered),
     StringEscapeUtils.unescapeHtml4(caption.rendered),
     StringEscapeUtils.unescapeHtml4(description.rendered),
     StringEscapeUtils.unescapeHtml4(altText),
-    mediaDetails.width,
-    mediaDetails.height,
+    mediaDetails?.width ?: 0,
+    mediaDetails?.height ?: 0,
     0,
     null,
     false,

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/media/MediaWPRESTResponse.kt

@@ -0,0 +1,57 @@
+package org.wordpress.android.fluxc.network.rest
+
+import com.google.gson.Gson
+import com.google.gson.GsonBuilder
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+class JsonObjectOrNullAdapterFactoryTest {
+    data class SampleModel(val id: Int, val name: String?) : JsonObjectOrNull()
+
+    private fun gson(): Gson = GsonBuilder()
+        .registerTypeAdapterFactory(JsonObjectOrNullAdapterFactory())
+        .create()
+
+    @Test
+    fun `when json is a valid object, then it is deserialized`() {
+        val json = """{"id": 7, "name": "woo"}"""
+        val result: SampleModel = gson().fromJson(json, SampleModel::class.java)
+        assertEquals(SampleModel(7, "woo"), result)
+    }
+
+    @Test
+    fun `when json is an empty array, then returns null`() {
+        val json = "[]"
+        val result: SampleModel? = gson().fromJson(json, SampleModel::class.java)
+        assertNull(result)
+    }
+
+    @Test
+    fun `when json is null, then returns null`() {
+        val json = "null"
+        val result: SampleModel? = gson().fromJson(json, SampleModel::class.java)
+        assertNull(result)
+    }
+
+    @Test
+    fun `when json is a primitive string, then returns null`() {
+        val json = "\"primitive\""
+        val result: SampleModel? = gson().fromJson(json, SampleModel::class.java)
+        assertNull(result)
+    }
+
+    @Test
+    fun `when json is a primitive number, then returns null`() {
+        val json = "123"
+        val result: SampleModel? = gson().fromJson(json, SampleModel::class.java)
+        assertNull(result)
+    }
+
+    @Test
+    fun `when json is a primitive boolean, then returns null`() {
+        val json = "true"
+        val result: SampleModel? = gson().fromJson(json, SampleModel::class.java)
+        assertNull(result)
+    }
+}