Extract parser to its own file and add unit test.

vector/src/main/java/im/vector/app/core/pushers/PushParser.kt

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.core.pushers
+
+import im.vector.app.core.pushers.model.PushData
+import im.vector.app.core.pushers.model.PushDataFcm
+import im.vector.app.core.pushers.model.PushDataUnifiedPush
+import im.vector.app.core.pushers.model.toPushData
+import org.matrix.android.sdk.api.extensions.tryOrNull
+import org.matrix.android.sdk.api.util.MatrixJsonParser
+import javax.inject.Inject
+
+class PushParser @Inject constructor() {
+    /**
+     * Parse the received data from Push. Json format are different depending on the source.
+     *
+     * Notifications received by FCM are formatted by the matrix gateway [1]. The data send to FCM is the content
+     * of the "notification" attribute of the json sent to the gateway [2][3].
+     * On the other side, with UnifiedPush, the content of the message received is the content posted to the push
+     * gateway endpoint [3].
+     *
+     * *Note*: If we want to get the same content with FCM and unifiedpush, we can do a new sygnal pusher [4].
+     *
+     * [1] https://github.com/matrix-org/sygnal/blob/main/sygnal/gcmpushkin.py
+     * [2] https://github.com/matrix-org/sygnal/blob/main/sygnal/gcmpushkin.py#L366
+     * [3] https://spec.matrix.org/latest/push-gateway-api/
+     * [4] https://github.com/p1gp1g/sygnal/blob/unifiedpush/sygnal/upfcmpushkin.py (Not tested for a while)
+     */
+    fun parseData(message: String, firebaseFormat: Boolean): PushData? {
+        val moshi = MatrixJsonParser.getMoshi()
+        return if (firebaseFormat) {
+            tryOrNull { moshi.adapter(PushDataFcm::class.java).fromJson(message) }?.toPushData()
+        } else {
+            tryOrNull { moshi.adapter(PushDataUnifiedPush::class.java).fromJson(message) }?.toPushData()
+        }
+    }
+}

vector/src/main/java/im/vector/app/core/pushers/VectorMessagingReceiver.kt

@@ -29,9 +29,6 @@ import im.vector.app.BuildConfig
 import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.network.WifiDetector
 import im.vector.app.core.pushers.model.PushData
-import im.vector.app.core.pushers.model.PushDataFcm
-import im.vector.app.core.pushers.model.PushDataUnifiedPush
-import im.vector.app.core.pushers.model.toPushData
 import im.vector.app.core.services.GuardServiceStarter
 import im.vector.app.features.notifications.NotifiableEventResolver
 import im.vector.app.features.notifications.NotificationDrawerManager
@@ -48,7 +45,6 @@ import org.matrix.android.sdk.api.logger.LoggerTag
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.getRoom
 import org.matrix.android.sdk.api.session.room.getTimelineEvent
-import org.matrix.android.sdk.api.util.MatrixJsonParser
 import org.unifiedpush.android.connector.MessagingReceiver
 import timber.log.Timber
 import javax.inject.Inject
@@ -70,6 +66,7 @@ class VectorMessagingReceiver : MessagingReceiver() {
     @Inject lateinit var guardServiceStarter: GuardServiceStarter
     @Inject lateinit var unifiedPushHelper: UnifiedPushHelper
     @Inject lateinit var unifiedPushStore: UnifiedPushStore
+    @Inject lateinit var pushParser: PushParser
 
     private val coroutineScope = CoroutineScope(SupervisorJob())
 
@@ -88,11 +85,17 @@ class VectorMessagingReceiver : MessagingReceiver() {
     override fun onMessage(context: Context, message: ByteArray, instance: String) {
         Timber.tag(loggerTag.value).d("## onMessage() received")
 
+        val sMessage = String(message)
+        if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) {
+            Timber.tag(loggerTag.value).d("## onMessage() $sMessage")
+        }
+
         runBlocking {
             vectorDataStore.incrementPushCounter()
         }
 
-        val pushData = parseData(message) ?: return Unit.also { Timber.tag(loggerTag.value).w("Invalid received data Json format") }
+        val pushData = pushParser.parseData(sMessage, unifiedPushHelper.isEmbeddedDistributor())
+                ?: return Unit.also { Timber.tag(loggerTag.value).w("Invalid received data Json format") }
 
         // Diagnostic Push
         if (pushData.eventId == PushersManager.TEST_EVENT_ID) {
@@ -116,35 +119,6 @@ class VectorMessagingReceiver : MessagingReceiver() {
         }
     }
 
-    /**
-     * Parse the received data from Push. Json format are different depending on the source.
-     *
-     * Notifications received by FCM are formatted by the matrix gateway [1]. The data send to FCM is the content
-     * of the "notification" attribute of the json sent to the gateway [2][3].
-     * On the other side, with UnifiedPush, the content of the message received is the content posted to the push
-     * gateway endpoint [3].
-     *
-     * *Note*: If we want to get the same content with FCM and unifiedpush, we can do a new sygnal pusher [4].
-     *
-     * [1] https://github.com/matrix-org/sygnal/blob/main/sygnal/gcmpushkin.py
-     * [2] https://github.com/matrix-org/sygnal/blob/main/sygnal/gcmpushkin.py#L366
-     * [3] https://spec.matrix.org/latest/push-gateway-api/
-     * [4] https://github.com/p1gp1g/sygnal/blob/unifiedpush/sygnal/upfcmpushkin.py (Not tested for a while)
-     */
-    private fun parseData(message: ByteArray): PushData? {
-        val sMessage = String(message)
-        if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) {
-            Timber.tag(loggerTag.value).d("## onMessage() $sMessage")
-        }
-
-        val moshi = MatrixJsonParser.getMoshi()
-        return if (unifiedPushHelper.isEmbeddedDistributor()) {
-            moshi.adapter(PushDataFcm::class.java).fromJson(sMessage)?.toPushData()
-        } else {
-            moshi.adapter(PushDataUnifiedPush::class.java).fromJson(sMessage)?.toPushData()
-        }
-    }
-
     /**
      * Called if InstanceID token is updated. This may occur if the security of
      * the previous token had been compromised. Note that this is also called

vector/src/main/java/im/vector/app/core/pushers/model/PushData.kt

@@ -19,5 +19,5 @@ package im.vector.app.core.pushers.model
 data class PushData(
         val eventId: String,
         val roomId: String,
-        var unread: Int,
+        val unread: Int,
 )

vector/src/main/java/im/vector/app/core/pushers/model/PushDataFcm.kt

@@ -26,20 +26,24 @@ import com.squareup.moshi.JsonClass
  *     "event_id":"$anEventId",
  *     "room_id":"!aRoomId",
  *     "unread":"1",
- *     "prio":"high",
+ *     "prio":"high"
  * }
  * </pre>
  * .
  */
 @JsonClass(generateAdapter = true)
 data class PushDataFcm(
-        @Json(name = "event_id") val eventId: String = "",
-        @Json(name = "room_id") val roomId: String = "",
-        @Json(name = "unread") var unread: Int = 0,
+        @Json(name = "event_id") val eventId: String,
+        @Json(name = "room_id") val roomId: String,
+        @Json(name = "unread") var unread: Int,
 )
 
-fun PushDataFcm.toPushData() = PushData(
+fun PushDataFcm.toPushData(): PushData? {
+    if (eventId.isEmpty()) return null
+    if (roomId.isEmpty()) return null
+    return PushData(
             eventId = eventId,
             roomId = roomId,
             unread = unread
     )
+}

vector/src/main/java/im/vector/app/core/pushers/model/PushDataUnifiedPush.kt

@@ -29,7 +29,7 @@ import com.squareup.moshi.JsonClass
  *         "counts":{
  *             "unread":1
  *         },
- *         "prio":"high",
+ *         "prio":"high"
  *     }
  * }
  * </pre>
@@ -37,23 +37,27 @@ import com.squareup.moshi.JsonClass
  */
 @JsonClass(generateAdapter = true)
 data class PushDataUnifiedPush(
-        @Json(name = "notification") val notification: PushDataUnifiedPushNotification = PushDataUnifiedPushNotification()
+        @Json(name = "notification") val notification: PushDataUnifiedPushNotification
 )
 
 @JsonClass(generateAdapter = true)
 data class PushDataUnifiedPushNotification(
-        @Json(name = "event_id") val eventId: String = "",
-        @Json(name = "room_id") val roomId: String = "",
-        @Json(name = "counts") var counts: PushDataUnifiedPushCounts = PushDataUnifiedPushCounts(),
+        @Json(name = "event_id") val eventId: String,
+        @Json(name = "room_id") val roomId: String,
+        @Json(name = "counts") var counts: PushDataUnifiedPushCounts,
 )
 
 @JsonClass(generateAdapter = true)
 data class PushDataUnifiedPushCounts(
-        @Json(name = "unread") val unread: Int = 0
+        @Json(name = "unread") val unread: Int
 )
 
-fun PushDataUnifiedPush.toPushData() = PushData(
+fun PushDataUnifiedPush.toPushData(): PushData? {
+    if (notification.eventId.isEmpty()) return null
+    if (notification.roomId.isEmpty()) return null
+    return PushData(
             eventId = notification.eventId,
             roomId = notification.roomId,
             unread = notification.counts.unread
     )
+}

vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.core.pushers
+
+import im.vector.app.core.pushers.model.PushData
+import org.amshove.kluent.shouldBe
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+
+class PushParserTest {
+    companion object {
+        private const val UNIFIED_PUSH_DATA =
+                "{\"notification\":{\"event_id\":\"\$anEventId\",\"room_id\":\"!aRoomId\",\"counts\":{\"unread\":1},\"prio\":\"high\"}}"
+        private const val FIREBASE_PUSH_DATA =
+                "{\"event_id\":\"\$anEventId\",\"room_id\":\"!aRoomId\",\"unread\":\"1\",\"prio\":\"high\"}"
+    }
+
+    private val parsedData = PushData(
+            eventId = "\$anEventId",
+            roomId = "!aRoomId",
+            unread = 1
+    )
+
+    @Test
+    fun `test edge cases`() {
+        doAllEdgeTests(true)
+        doAllEdgeTests(false)
+    }
+
+    private fun doAllEdgeTests(firebaseFormat: Boolean) {
+        val pushParser = PushParser()
+        // Empty string
+        pushParser.parseData("", firebaseFormat) shouldBe null
+        // Empty Json
+        pushParser.parseData("{}", firebaseFormat) shouldBe null
+        // Bad Json
+        pushParser.parseData("ABC", firebaseFormat) shouldBe null
+    }
+
+    @Test
+    fun `test unified push format`() {
+        val pushParser = PushParser()
+
+        pushParser.parseData(UNIFIED_PUSH_DATA, false) shouldBeEqualTo parsedData
+        pushParser.parseData(UNIFIED_PUSH_DATA, true) shouldBe null
+    }
+
+    @Test
+    fun `test firebase push format`() {
+        val pushParser = PushParser()
+
+        pushParser.parseData(FIREBASE_PUSH_DATA, true) shouldBeEqualTo parsedData
+        pushParser.parseData(FIREBASE_PUSH_DATA, false) shouldBe null
+    }
+
+    @Test
+    fun `test empty roomId`() {
+        val pushParser = PushParser()
+
+        pushParser.parseData(FIREBASE_PUSH_DATA.replace("!aRoomId", ""), true) shouldBe null
+        pushParser.parseData(UNIFIED_PUSH_DATA.replace("!aRoomId", ""), false) shouldBe null
+    }
+
+    @Test
+    fun `test empty eventId`() {
+        val pushParser = PushParser()
+
+        pushParser.parseData(FIREBASE_PUSH_DATA.replace("\$anEventId", ""), true) shouldBe null
+        pushParser.parseData(UNIFIED_PUSH_DATA.replace("\$anEventId", ""), false) shouldBe null
+    }
+}