Skip to content

Conversation

@irfano
Copy link
Contributor

@irfano irfano commented Sep 23, 2025

Closes WOOMOB-1037

Description

This adds support for parsing "meta_data": {"1": {...}, "2": {...}} when the expected format is "meta_data": […]. This unexpected format can sometimes be sent by the server due to PHP behavior.
Previously, we added this for the List<*> type, but we're still seeing crashes, so we're now adding it for Array<*> type as well. This was discussed in #14553 (comment)

Steps to reproduce

I don’t know how to reproduce this case. Reviewing the unit tests should be enough.

The tests that have been performed

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@irfano irfano added this to the 23.4 milestone Sep 23, 2025
@irfano irfano added the type: crash The worst kind of bug. label Sep 23, 2025
@irfano irfano changed the title Issue/woomob 1037 deserializer for unexpected array format 2 Deserializer for unexpected array format from endpoints 2 Sep 23, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 23, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit6cf1182
Direct Downloadwoocommerce-wear-prototype-build-pr14653-6cf1182.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 23, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit6cf1182
Direct Downloadwoocommerce-prototype-build-pr14653-6cf1182.apk

@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.52%. Comparing base (2c17432) to head (6cf1182).
⚠️ Report is 92 commits behind head on trunk.

Files with missing lines Patch % Lines
...id/fluxc/network/rest/ArrayOrObjectDeserializer.kt 88.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #14653   +/-   ##
=========================================
  Coverage     38.52%   38.52%           
+ Complexity     9783     9780    -3     
=========================================
  Files          2068     2068           
  Lines        115558   115567    +9     
  Branches      15394    15394           
=========================================
+ Hits          44522    44526    +4     
- Misses        66899    66902    +3     
- Partials       4137     4139    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hichamboushaba hichamboushaba self-assigned this Sep 24, 2025
Copy link
Member

@hichamboushaba hichamboushaba left a comment

Choose a reason for hiding this comment

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

Nice work @irfano, tests work as expected, and ready to ship.

I'm just leaving an optional suggestion for you, I think we can simplify the code a bit and reduce the code repetition between this new component and the existing ListOrObjectDeserializer, please check the following patch:

Index: libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ArrayOrObjectDeserializer.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ArrayOrObjectDeserializer.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ArrayOrObjectDeserializer.kt
--- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ArrayOrObjectDeserializer.kt	(revision 88217ddda7dd0897c76a173d49272a1e6133797e)
+++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ArrayOrObjectDeserializer.kt	(date 1758795060680)
@@ -4,9 +4,11 @@
 import com.google.gson.JsonDeserializer
 import com.google.gson.JsonElement
 import com.google.gson.JsonParseException
+import com.google.gson.internal.GsonTypes.getRawType
 import java.lang.reflect.GenericArrayType
 import java.lang.reflect.ParameterizedType
 import java.lang.reflect.Type
+import java.lang.reflect.Array as JArray
 
 /**
  * Deserializes a JSON value that can be either an array or an object with numeric string keys.
@@ -27,36 +29,68 @@
         typeOfT: Type,
         context: JsonDeserializationContext
     ): Array<*>? {
-        val componentType: Type = when (typeOfT) {
-            is GenericArrayType -> typeOfT.genericComponentType
-            is Class<*> -> if (typeOfT.isArray) {
-                typeOfT.componentType
-            } else {
-                throw JsonParseException("Expected array: $typeOfT")
-            }
+        val componentType: Type = (typeOfT as? GenericArrayType)?.genericComponentType
+            ?: throw JsonParseException("Expected a GenericArrayType, but got: $typeOfT")
+
+        val items: List<Any?> = jsonToList(json, componentType, context)
+
+        return items.listToArray(componentType)
+    }
+
+    private fun List<Any?>.listToArray(componentType: Type): Array<Any?> {
+        val arr = JArray.newInstance(getRawType(componentType), size)
+        forEachIndexed { i, v -> JArray.set(arr, i, v) }
+        @Suppress("UNCHECKED_CAST")
+        return arr as Array<Any?>
+    }
+}
 
-            else -> throw JsonParseException("Unsupported type: $typeOfT")
-        }
-        val raw: Class<*> = when (componentType) {
-            is Class<*> -> componentType
-            is ParameterizedType -> componentType.rawType as Class<*>
-            else -> Any::class.java
+/**
+ * Deserializes a JSON value that can be either an array or an object with numeric string keys.
+ * Examples:
+ * - Array: ["item1", "item2"]
+ * - Object with numeric keys: {"0": "item1", "1": "item2"}
+ * - Non-ordered numeric keys example: {"2":"a","1":"b","10":"z"} -> ["b", "a", "z"]
+ *
+ * This deserializer handles both cases and converts them into a List<T>.
+ * - For arrays: elements are deserialized in their natural order.
+ * - For objects: keys must be numeric strings (0, 1, 2, ...). Values are read by sorting keys numerically
+ * (0, 1, 2, ...), not by insertion order. Any non-numeric key results in a JsonParseException.
+ * If the JSON value is null, null is returned.
+ */
+class ListOrObjectDeserializer : JsonDeserializer<List<*>> {
+    override fun deserialize(
+        json: JsonElement,
+        typeOfT: Type,
+        context: JsonDeserializationContext
+    ): List<*>? {
+        val itemType = (typeOfT as ParameterizedType).actualTypeArguments[0]
+        return jsonToList(json, itemType, context)
+    }
+}
+
+private fun jsonToList(
+    json: JsonElement,
+    itemType: Type,
+    context: JsonDeserializationContext
+): List<Any?> {
+    return when {
+        json.isJsonArray -> json.asJsonArray.map { element ->
+            context.deserialize(element, itemType)
         }
-        val items: List<Any?> = when {
-            json.isJsonArray -> json.asJsonArray.map { context.deserialize(it, componentType) }
-            json.isJsonObject -> json.asJsonObject.entrySet()
-                .onEach {
-                    it.key.toIntOrNull() ?: throw JsonParseException("Unexpected key: ${it.key}")
-                }
-                .sortedBy { it.key.toInt() }
-                .map { context.deserialize(it.value, componentType) }
-
-            json.isJsonNull -> return null
-            else -> throw JsonParseException("Unexpected JSON for Array: $json")
-        }
-        val arr = java.lang.reflect.Array.newInstance(raw, items.size)
-        items.forEachIndexed { i, v -> java.lang.reflect.Array.set(arr, i, v) }
-        @Suppress("UNCHECKED_CAST")
-        return arr as Array<*>
+
+        json.isJsonObject -> json.asJsonObject.entrySet()
+            .onEach { entry ->
+                val key = entry.key
+                key.toIntOrNull()
+                    ?: throw JsonParseException("Unexpected key in JSON object matching array: $key")
+            }
+            .sortedBy { it.key.toInt() }
+            .map { entry ->
+                context.deserialize(entry.value, itemType)
+            }
+
+        json.isJsonNull -> emptyList()
+        else -> throw JsonParseException("Unexpected JSON type for List, $json")
     }
 }
Index: libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ListOrObjectDeserializer.kt
===================================================================
diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ListOrObjectDeserializer.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ListOrObjectDeserializer.kt
deleted file mode 100644
--- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/ListOrObjectDeserializer.kt	(revision 88217ddda7dd0897c76a173d49272a1e6133797e)
+++ /dev/null	(revision 88217ddda7dd0897c76a173d49272a1e6133797e)
@@ -1,50 +0,0 @@
-package org.wordpress.android.fluxc.network.rest
-
-import com.google.gson.JsonDeserializationContext
-import com.google.gson.JsonDeserializer
-import com.google.gson.JsonElement
-import com.google.gson.JsonParseException
-import java.lang.reflect.ParameterizedType
-import java.lang.reflect.Type
-
-/**
- * Deserializes a JSON value that can be either an array or an object with numeric string keys.
- * Examples:
- * - Array: ["item1", "item2"]
- * - Object with numeric keys: {"0": "item1", "1": "item2"}
- * - Non-ordered numeric keys example: {"2":"a","1":"b","10":"z"} -> ["b", "a", "z"]
- *
- * This deserializer handles both cases and converts them into a List<T>.
- * - For arrays: elements are deserialized in their natural order.
- * - For objects: keys must be numeric strings (0, 1, 2, ...). Values are read by sorting keys numerically
- * (0, 1, 2, ...), not by insertion order. Any non-numeric key results in a JsonParseException.
- * If the JSON value is null, null is returned.
- */
-class ListOrObjectDeserializer : JsonDeserializer<List<*>> {
-    override fun deserialize(
-        json: JsonElement,
-        typeOfT: Type,
-        context: JsonDeserializationContext
-    ): List<*>? {
-        val itemType = (typeOfT as ParameterizedType).actualTypeArguments[0]
-        return when {
-            json.isJsonArray -> json.asJsonArray.map { element ->
-                context.deserialize<Any?>(element, itemType)
-            }
-
-            json.isJsonObject -> json.asJsonObject.entrySet()
-                .onEach { entry ->
-                    val key = entry.key
-                    key.toIntOrNull()
-                        ?: throw JsonParseException("Unexpected key in JSON object matching array: $key")
-                }
-                .sortedBy { it.key.toInt() }
-                .map { entry ->
-                    context.deserialize<Any?>(entry.value, itemType)
-                }
-
-            json.isJsonNull -> null
-            else -> throw JsonParseException("Unexpected JSON type for List, $json")
-        }
-    }
-}

Comment on lines 30 to 39
val componentType: Type = when (typeOfT) {
is GenericArrayType -> typeOfT.genericComponentType
is Class<*> -> if (typeOfT.isArray) {
typeOfT.componentType
} else {
throw JsonParseException("Expected array: $typeOfT")
}

else -> throw JsonParseException("Unsupported type: $typeOfT")
}
Copy link
Member

Choose a reason for hiding this comment

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

np, as this will be called only for arrays, we can be sure that the type here is GenericArrayType, so we can simplify this to something like this:

        val componentType: Type = (typeOfT as? GenericArrayType)?.genericComponentType
            ?: throw JsonParseException("Expected a GenericArrayType, but got: $typeOfT")

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 catch! Your patch makes it simpler and better.

Comment on lines 40 to 44
val raw: Class<*> = when (componentType) {
is Class<*> -> componentType
is ParameterizedType -> componentType.rawType as Class<*>
else -> Any::class.java
}
Copy link
Member

Choose a reason for hiding this comment

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

np, we can use the GsonTypes.getRawType function to get the raw type directly.

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 advice!

This commit refactors the `ArrayOrObjectDeserializer` and `ListOrObjectDeserializer` classes.

Additionally, `ListOrObjectDeserializer.kt` has been removed, and its contents were moved into `ArrayOrObjectDeserializer.kt` since they are closely related.
@irfano
Copy link
Contributor Author

irfano commented Sep 29, 2025

Thank you for the patch, @hichamboushaba! It includes useful improvements, and I applied it directly. 🙇🏻‍♂️

@irfano irfano enabled auto-merge September 29, 2025 12:47
@irfano irfano merged commit d1cac51 into trunk Sep 29, 2025
15 checks passed
@irfano irfano deleted the issue/WOOMOB-1037-deserializer-for-unexpected-array-format-2 branch September 29, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: crash The worst kind of bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants