diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParser.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParser.kt index 1f18b20652..8659c299ae 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParser.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParser.kt @@ -20,7 +20,7 @@ import io.element.android.libraries.push.impl.push.PushData import javax.inject.Inject class FirebasePushParser @Inject constructor() { - fun parse(message: Map): PushData { + fun parse(message: Map): PushData? { val pushDataFirebase = PushDataFirebase( eventId = message["event_id"], roomId = message["room_id"], diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/PushDataFirebase.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/PushDataFirebase.kt index bcf48bab15..739c161e79 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/PushDataFirebase.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/PushDataFirebase.kt @@ -16,7 +16,6 @@ package io.element.android.libraries.push.impl.firebase -import io.element.android.libraries.matrix.api.core.MatrixPatterns import io.element.android.libraries.matrix.api.core.asEventId import io.element.android.libraries.matrix.api.core.asRoomId import io.element.android.libraries.push.impl.push.PushData @@ -41,9 +40,13 @@ data class PushDataFirebase( val clientSecret: String? ) -fun PushDataFirebase.toPushData() = PushData( - eventId = eventId?.takeIf { MatrixPatterns.isEventId(it) }?.asEventId(), - roomId = roomId?.takeIf { MatrixPatterns.isRoomId(it) }?.asRoomId(), - unread = unread, - clientSecret = clientSecret, -) +fun PushDataFirebase.toPushData(): PushData? { + val safeEventId = eventId?.asEventId() ?: return null + val safeRoomId = roomId?.asRoomId() ?: return null + return PushData( + eventId = safeEventId, + roomId = safeRoomId, + unread = unread, + clientSecret = clientSecret, + ) +} diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/VectorFirebaseMessagingService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/VectorFirebaseMessagingService.kt index 8769baa947..2ccf6f2505 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/VectorFirebaseMessagingService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/VectorFirebaseMessagingService.kt @@ -53,8 +53,11 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { override fun onMessageReceived(message: RemoteMessage) { Timber.tag(loggerTag.value).d("New Firebase message") coroutineScope.launch { - pushParser.parse(message.data).let { - pushHandler.handle(it) + val pushData = pushParser.parse(message.data) + if (pushData == null) { + Timber.tag(loggerTag.value).w("Invalid data received from Firebase") + } else { + pushHandler.handle(pushData) } } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/PushData.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/PushData.kt index 864155e522..aaf6d65c08 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/PushData.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/PushData.kt @@ -22,14 +22,14 @@ import io.element.android.libraries.matrix.api.core.RoomId /** * Represent parsed data that the app has received from a Push content. * - * @property eventId The Event ID. If not null, it will not be empty, and will have a valid format. - * @property roomId The Room ID. If not null, it will not be empty, and will have a valid format. + * @property eventId The Event Id. + * @property roomId The Room Id. * @property unread Number of unread message. - * @property clientSecret A client secret, used to determine which user should receive the notification. + * @property clientSecret data used when the pusher was configured, to be able to determine the session. */ data class PushData( - val eventId: EventId?, - val roomId: RoomId?, + val eventId: EventId, + val roomId: RoomId, val unread: Int?, val clientSecret: String?, ) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/PushDataUnifiedPush.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/PushDataUnifiedPush.kt index 0bc6a6bce5..73d5f0286a 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/PushDataUnifiedPush.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/PushDataUnifiedPush.kt @@ -16,7 +16,6 @@ package io.element.android.libraries.push.impl.unifiedpush -import io.element.android.libraries.matrix.api.core.MatrixPatterns import io.element.android.libraries.matrix.api.core.asEventId import io.element.android.libraries.matrix.api.core.asRoomId import io.element.android.libraries.push.impl.push.PushData @@ -56,9 +55,13 @@ data class PushDataUnifiedPushCounts( @SerialName("unread") val unread: Int? = null ) -fun PushDataUnifiedPush.toPushData() = PushData( - eventId = notification?.eventId?.takeIf { MatrixPatterns.isEventId(it) }?.asEventId(), - roomId = notification?.roomId?.takeIf { MatrixPatterns.isRoomId(it) }?.asRoomId(), - unread = notification?.counts?.unread, - clientSecret = null // TODO EAx check how client secret will be sent through UnifiedPush -) +fun PushDataUnifiedPush.toPushData(): PushData? { + val safeEventId = notification?.eventId?.asEventId() ?: return null + val safeRoomId = notification.roomId?.asRoomId() ?: return null + return PushData( + eventId = safeEventId, + roomId = safeRoomId, + unread = notification.counts?.unread, + clientSecret = null // TODO EAx check how client secret will be sent through UnifiedPush + ) +} diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/VectorUnifiedPushMessagingReceiver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/VectorUnifiedPushMessagingReceiver.kt index 81dd389e78..acb438cc70 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/VectorUnifiedPushMessagingReceiver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/VectorUnifiedPushMessagingReceiver.kt @@ -67,10 +67,11 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { override fun onMessage(context: Context, message: ByteArray, instance: String) { Timber.tag(loggerTag.value).d("New message") coroutineScope.launch { - pushParser.parse(message)?.let { - pushHandler.handle(it) - } ?: run { - Timber.tag(loggerTag.value).w("Invalid received data Json format") + val pushData = pushParser.parse(message) + if (pushData == null) { + Timber.tag(loggerTag.value).w("Invalid data received from UnifiedPush") + } else { + pushHandler.handle(pushData) } } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParserTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParserTest.kt index 9d02913cf8..b14e067dae 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParserTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParserTest.kt @@ -30,18 +30,11 @@ class FirebasePushParserTest { clientSecret = "a-secret" ) - private val emptyData = PushData( - eventId = null, - roomId = null, - unread = null, - clientSecret = null - ) - @Test fun `test edge cases Firebase`() { val pushParser = FirebasePushParser() // Empty Json - assertThat(pushParser.parse(emptyMap())).isEqualTo(emptyData) + assertThat(pushParser.parse(emptyMap())).isNull() // Bad Json assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("unread", "str"))).isEqualTo(validData.copy(unread = null)) // Extra data @@ -57,31 +50,27 @@ class FirebasePushParserTest { @Test fun `test empty roomId`() { val pushParser = FirebasePushParser() - val expected = validData.copy(roomId = null) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", null))).isEqualTo(expected) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", ""))).isEqualTo(expected) + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", null))).isNull() + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", ""))).isNull() } @Test fun `test invalid roomId`() { val pushParser = FirebasePushParser() - val expected = validData.copy(roomId = null) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain"))).isEqualTo(expected) + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain"))).isNull() } @Test fun `test empty eventId`() { val pushParser = FirebasePushParser() - val expected = validData.copy(eventId = null) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", null))).isEqualTo(expected) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", ""))).isEqualTo(expected) + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", null))).isNull() + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", ""))).isNull() } @Test fun `test invalid eventId`() { val pushParser = FirebasePushParser() - val expected = validData.copy(eventId = null) - assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId"))).isEqualTo(expected) + assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId"))).isNull() } companion object { diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unifiedpush/UnifiedPushParserTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unifiedpush/UnifiedPushParserTest.kt index 00c92bb08a..f9275de4b2 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unifiedpush/UnifiedPushParserTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unifiedpush/UnifiedPushParserTest.kt @@ -31,20 +31,13 @@ class UnifiedPushParserTest { clientSecret = null ) - private val emptyData = PushData( - eventId = null, - roomId = null, - unread = null, - clientSecret = null - ) - @Test fun `test edge cases UnifiedPush`() { val pushParser = UnifiedPushParser() // Empty string assertThat(pushParser.parse("".toByteArray())).isNull() // Empty Json - assertThat(pushParser.parse("{}".toByteArray())).isEqualTo(emptyData) + assertThat(pushParser.parse("{}".toByteArray())).isNull() // Bad Json assertThat(pushParser.parse("ABC".toByteArray())).isNull() } @@ -58,29 +51,25 @@ class UnifiedPushParserTest { @Test fun `test empty roomId`() { val pushParser = UnifiedPushParser() - val expected = validData.copy(roomId = null) - assertThat(pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray())).isEqualTo(expected) + assertThat(pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray())).isNull() } @Test fun `test invalid roomId`() { val pushParser = UnifiedPushParser() - val expected = validData.copy(roomId = null) - assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"))).isEqualTo(expected) + assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"))).isNull() } @Test fun `test empty eventId`() { val pushParser = UnifiedPushParser() - val expected = validData.copy(eventId = null) - assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""))).isEqualTo(expected) + assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""))).isNull() } @Test fun `test invalid eventId`() { val pushParser = UnifiedPushParser() - val expected = validData.copy(eventId = null) - assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"))).isEqualTo(expected) + assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"))).isNull() } companion object {