From c7d021ece6a577816a289e1018a9a4127e15a743 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoitm@matrix.org> Date: Mon, 13 Jun 2022 13:59:09 +0200 Subject: [PATCH] Extract parser to its own file and add unit test. --- .../im/vector/app/core/pushers/PushParser.kt | 51 +++++++++++ .../core/pushers/VectorMessagingReceiver.kt | 42 ++------- .../vector/app/core/pushers/model/PushData.kt | 2 +- .../app/core/pushers/model/PushDataFcm.kt | 22 +++-- .../core/pushers/model/PushDataUnifiedPush.kt | 26 +++--- .../vector/app/core/pushers/PushParserTest.kt | 85 +++++++++++++++++++ 6 files changed, 173 insertions(+), 55 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/core/pushers/PushParser.kt create mode 100644 vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt diff --git a/vector/src/main/java/im/vector/app/core/pushers/PushParser.kt b/vector/src/main/java/im/vector/app/core/pushers/PushParser.kt new file mode 100644 index 0000000000..6f141e3736 --- /dev/null +++ b/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() + } + } +} diff --git a/vector/src/main/java/im/vector/app/core/pushers/VectorMessagingReceiver.kt b/vector/src/main/java/im/vector/app/core/pushers/VectorMessagingReceiver.kt index 37845337be..110e5ac386 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/VectorMessagingReceiver.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/core/pushers/model/PushData.kt b/vector/src/main/java/im/vector/app/core/pushers/model/PushData.kt index 9f7b710c91..d6e51ddab2 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/model/PushData.kt +++ b/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, ) diff --git a/vector/src/main/java/im/vector/app/core/pushers/model/PushDataFcm.kt b/vector/src/main/java/im/vector/app/core/pushers/model/PushDataFcm.kt index b3bcc69309..8f5c1fb700 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/model/PushDataFcm.kt +++ b/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( - eventId = eventId, - roomId = roomId, - unread = unread -) +fun PushDataFcm.toPushData(): PushData? { + if (eventId.isEmpty()) return null + if (roomId.isEmpty()) return null + return PushData( + eventId = eventId, + roomId = roomId, + unread = unread + ) +} diff --git a/vector/src/main/java/im/vector/app/core/pushers/model/PushDataUnifiedPush.kt b/vector/src/main/java/im/vector/app/core/pushers/model/PushDataUnifiedPush.kt index b1410e048f..5883bfc245 100644 --- a/vector/src/main/java/im/vector/app/core/pushers/model/PushDataUnifiedPush.kt +++ b/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( - eventId = notification.eventId, - roomId = notification.roomId, - unread = notification.counts.unread -) +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 + ) +} diff --git a/vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt b/vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt new file mode 100644 index 0000000000..f247cf55e1 --- /dev/null +++ b/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 + } +}