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 110e5ac386..723d9c2480 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 @@ -187,12 +187,12 @@ class VectorMessagingReceiver : MessagingReceiver() { if (session == null) { Timber.tag(loggerTag.value).w("## Can't sync from push, no current session") } else { - if (isEventAlreadyKnown(pushData.eventId, pushData.roomId)) { + if (isEventAlreadyKnown(pushData)) { Timber.tag(loggerTag.value).d("Ignoring push, event already known") } else { // Try to get the Event content faster Timber.tag(loggerTag.value).d("Requesting event in fast lane") - getEventFastLane(session, pushData.roomId, pushData.eventId) + getEventFastLane(session, pushData) Timber.tag(loggerTag.value).d("Requesting background sync") session.syncService().requireBackgroundSync() @@ -203,12 +203,12 @@ class VectorMessagingReceiver : MessagingReceiver() { } } - private fun getEventFastLane(session: Session, roomId: String?, eventId: String?) { - roomId?.takeIf { it.isNotEmpty() } ?: return - eventId?.takeIf { it.isNotEmpty() } ?: return + private fun getEventFastLane(session: Session, pushData: PushData) { + pushData.roomId ?: return + pushData.eventId ?: return // If the room is currently displayed, we will not show a notification, so no need to get the Event faster - if (notificationDrawerManager.shouldIgnoreMessageEventInRoom(roomId)) { + if (notificationDrawerManager.shouldIgnoreMessageEventInRoom(pushData.roomId)) { return } @@ -219,7 +219,7 @@ class VectorMessagingReceiver : MessagingReceiver() { coroutineScope.launch { Timber.tag(loggerTag.value).d("Fast lane: start request") - val event = tryOrNull { session.eventService().getEvent(roomId, eventId) } ?: return@launch + val event = tryOrNull { session.eventService().getEvent(pushData.roomId, pushData.eventId) } ?: return@launch val resolvedEvent = notifiableEventResolver.resolveInMemoryEvent(session, event, canBeReplaced = true) @@ -233,12 +233,12 @@ class VectorMessagingReceiver : MessagingReceiver() { // check if the event was not yet received // a previous catchup might have already retrieved the notified event - private fun isEventAlreadyKnown(eventId: String?, roomId: String?): Boolean { - if (null != eventId && null != roomId) { + private fun isEventAlreadyKnown(pushData: PushData): Boolean { + if (pushData.eventId != null && pushData.roomId != null) { try { val session = activeSessionHolder.getSafeActiveSession() ?: return false - val room = session.getRoom(roomId) ?: return false - return room.getTimelineEvent(eventId) != null + val room = session.getRoom(pushData.roomId) ?: return false + return room.getTimelineEvent(pushData.eventId) != null } catch (e: Exception) { Timber.tag(loggerTag.value).e(e, "## isEventAlreadyKnown() : failed to check if the event was already defined") } 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 d6e51ddab2..d1d095a6fa 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 @@ -16,8 +16,15 @@ package im.vector.app.core.pushers.model +/** + * 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 unread Number of unread message. + */ data class PushData( - val eventId: String, - val roomId: String, - val unread: Int, + val eventId: String?, + val roomId: String?, + 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 8f5c1fb700..1b9c37ae0a 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 @@ -18,6 +18,7 @@ package im.vector.app.core.pushers.model import com.squareup.moshi.Json import com.squareup.moshi.JsonClass +import org.matrix.android.sdk.api.MatrixPatterns /** * In this case, the format is: @@ -33,17 +34,13 @@ import com.squareup.moshi.JsonClass */ @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, + @Json(name = "event_id") val eventId: String?, + @Json(name = "room_id") val roomId: String?, + @Json(name = "unread") var unread: Int?, ) -fun PushDataFcm.toPushData(): PushData? { - if (eventId.isEmpty()) return null - if (roomId.isEmpty()) return null - return PushData( - eventId = eventId, - roomId = roomId, - unread = unread - ) -} +fun PushDataFcm.toPushData() = PushData( + eventId = eventId?.takeIf { MatrixPatterns.isEventId(it) }, + roomId = roomId?.takeIf { MatrixPatterns.isRoomId(it) }, + 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 5883bfc245..f4a2f6741d 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 @@ -18,6 +18,7 @@ package im.vector.app.core.pushers.model import com.squareup.moshi.Json import com.squareup.moshi.JsonClass +import org.matrix.android.sdk.api.extensions.ensureNotEmpty /** * In this case, the format is: @@ -37,27 +38,23 @@ import com.squareup.moshi.JsonClass */ @JsonClass(generateAdapter = true) data class PushDataUnifiedPush( - @Json(name = "notification") val notification: 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, + @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 + @Json(name = "unread") val unread: Int? ) -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 - ) -} +fun PushDataUnifiedPush.toPushData() = PushData( + eventId = notification?.eventId?.ensureNotEmpty(), + roomId = notification?.roomId?.ensureNotEmpty(), + 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 index f247cf55e1..62875bb26d 100644 --- a/vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt +++ b/vector/src/test/java/im/vector/app/core/pushers/PushParserTest.kt @@ -29,12 +29,18 @@ class PushParserTest { "{\"event_id\":\"\$anEventId\",\"room_id\":\"!aRoomId\",\"unread\":\"1\",\"prio\":\"high\"}" } - private val parsedData = PushData( + private val validData = PushData( eventId = "\$anEventId", roomId = "!aRoomId", unread = 1 ) + private val emptyData = PushData( + eventId = null, + roomId = null, + unread = null + ) + @Test fun `test edge cases`() { doAllEdgeTests(true) @@ -46,7 +52,7 @@ class PushParserTest { // Empty string pushParser.parseData("", firebaseFormat) shouldBe null // Empty Json - pushParser.parseData("{}", firebaseFormat) shouldBe null + pushParser.parseData("{}", firebaseFormat) shouldBeEqualTo emptyData // Bad Json pushParser.parseData("ABC", firebaseFormat) shouldBe null } @@ -55,31 +61,47 @@ class PushParserTest { fun `test unified push format`() { val pushParser = PushParser() - pushParser.parseData(UNIFIED_PUSH_DATA, false) shouldBeEqualTo parsedData - pushParser.parseData(UNIFIED_PUSH_DATA, true) shouldBe null + pushParser.parseData(UNIFIED_PUSH_DATA, false) shouldBeEqualTo validData + pushParser.parseData(UNIFIED_PUSH_DATA, true) shouldBeEqualTo emptyData } @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 + pushParser.parseData(FIREBASE_PUSH_DATA, true) shouldBeEqualTo validData + pushParser.parseData(FIREBASE_PUSH_DATA, false) shouldBeEqualTo emptyData } @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 + pushParser.parseData(FIREBASE_PUSH_DATA.replace("!aRoomId", ""), true) shouldBeEqualTo validData.copy(roomId = null) + pushParser.parseData(UNIFIED_PUSH_DATA.replace("!aRoomId", ""), false) shouldBeEqualTo validData.copy(roomId = null) + } + + @Test + fun `test invalid roomId`() { + val pushParser = PushParser() + + pushParser.parseData(FIREBASE_PUSH_DATA.replace("!aRoomId", "aRoomId"), true) shouldBeEqualTo validData.copy(roomId = null) + pushParser.parseData(UNIFIED_PUSH_DATA.replace("!aRoomId", "aRoomId"), false) shouldBeEqualTo validData.copy(roomId = 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 + pushParser.parseData(FIREBASE_PUSH_DATA.replace("\$anEventId", ""), true) shouldBeEqualTo validData.copy(eventId = null) + pushParser.parseData(UNIFIED_PUSH_DATA.replace("\$anEventId", ""), false) shouldBeEqualTo validData.copy(eventId = null) + } + + @Test + fun `test invalid eventId`() { + val pushParser = PushParser() + + pushParser.parseData(FIREBASE_PUSH_DATA.replace("\$anEventId", "anEventId"), true) shouldBeEqualTo validData.copy(eventId = null) + pushParser.parseData(UNIFIED_PUSH_DATA.replace("\$anEventId", "anEventId"), false) shouldBeEqualTo validData.copy(eventId = null) } }