Merge pull request #3077 from wordpress-mobile/fix/metadata-no-id-npe

FromJson returns null when json doesn't contain required fields

Author: atorresveiga

plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/model/WCMetaData.kt

@@ -52,7 +52,7 @@ data class WCMetaData(
     }
 
     companion object {
-        private const val ID = "id"
+        const val ID = "id"
         const val KEY = "key"
         const val VALUE = "value"
         private const val DISPLAY_KEY = "display_key"
@@ -68,14 +68,16 @@ data class WCMetaData(
             addAll(OrderAttributionInfoKeys.ALL_KEYS)
         }
 
-        internal fun fromJson(json: JsonObject): WCMetaData {
-            return WCMetaData(
-                id = json[ID].asLong,
-                key = json[KEY].asString,
-                value = WCMetaDataValue.fromJsonElement(json[VALUE]),
-                displayKey = json[DISPLAY_KEY]?.asString,
-                displayValue = json[DISPLAY_VALUE]?.let { WCMetaDataValue.fromJsonElement(it) }
-            )
+        internal fun fromJson(json: JsonObject): WCMetaData? {
+            return if (json.has(ID) && json.has(KEY) && json.has(VALUE)) {
+                WCMetaData(
+                    id = json[ID].asLong,
+                    key = json[KEY].asString,
+                    value = WCMetaDataValue.fromJsonElement(json[VALUE]),
+                    displayKey = json[DISPLAY_KEY]?.asString,
+                    displayValue = json[DISPLAY_VALUE]?.let { WCMetaDataValue.fromJsonElement(it) }
+                )
+            } else null
         }
 
         fun addAsMetadata(metadata: JsonArray, key: String, value: String) {

plugins/woocommerce/src/test/java/org/wordpress/android/fluxc/model/WCMetaDataTest.kt

@@ -0,0 +1,75 @@
+package org.wordpress.android.fluxc.model
+
+import com.google.gson.JsonObject
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+import org.wordpress.android.fluxc.model.WCMetaData.Companion.ID
+import org.wordpress.android.fluxc.model.WCMetaData.Companion.KEY
+import org.wordpress.android.fluxc.model.WCMetaData.Companion.VALUE
+
+class WCMetaDataTest {
+    @Test
+    fun `when all required fields are present, should return a WCMetaData`() {
+        val id = 1234L
+        val key = "this_is_the_key"
+        val value = "random value"
+
+        val metadataJson = JsonObject().apply {
+            addProperty(ID, id)
+            addProperty(KEY, key)
+            addProperty(VALUE, value)
+        }
+
+        val metadata = WCMetaData.fromJson(metadataJson)
+
+        assertThat(metadata).isNotNull
+        assertThat(metadata!!.id).isEqualTo(id)
+        assertThat(metadata.key).isEqualTo(key)
+        assertThat(metadata.value.stringValue).isEqualTo(value)
+    }
+
+    @Test
+    fun `when ID is missing, should return null`() {
+        val key = "this_is_the_key"
+        val value = "random value"
+
+        val metadataJson = JsonObject().apply {
+            addProperty(KEY, key)
+            addProperty(VALUE, value)
+        }
+
+        val metadata = WCMetaData.fromJson(metadataJson)
+
+        assertThat(metadata).isNull()
+    }
+
+    @Test
+    fun `when Key is missing, should return null`() {
+        val id = 1234L
+        val value = "random value"
+
+        val metadataJson = JsonObject().apply {
+            addProperty(ID, id)
+            addProperty(VALUE, value)
+        }
+
+        val metadata = WCMetaData.fromJson(metadataJson)
+
+        assertThat(metadata).isNull()
+    }
+
+    @Test
+    fun `when Value is missing, should return null`() {
+        val id = 1234L
+        val key = "this_is_the_key"
+
+        val metadataJson = JsonObject().apply {
+            addProperty(ID, id)
+            addProperty(KEY, key)
+        }
+
+        val metadata = WCMetaData.fromJson(metadataJson)
+
+        assertThat(metadata).isNull()
+    }
+}