From a07286ace9ec12eb6650ea73180205e370e49353 Mon Sep 17 00:00:00 2001 From: Marco Romano Date: Fri, 27 Oct 2023 16:02:16 +0200 Subject: [PATCH] Split `VoiceMessageType` from `AudioMessageType` (#1664) Currently, for compatibility reasons, we implement MSC3245v1 which puts the voice data inside an audio message type. Though at times it seems impractical to deal with a single message type which effectively represents 2 different kinds of messages. This PR creates a new message type called `VoiceMessageType` which is used whenever we receive an event with `"msgtype": "m.audio"` which also has the `"org.matrix.msc3245.voice": {}` field. This makes it easier to process voice messages as different entities throughout the rest of the codebase. --- .../components/TimelineItemEventRow.kt | 21 +++++------ .../TimelineItemContentMessageFactory.kt | 37 ++++++++++++++----- .../impl/DefaultRoomLastMessageFormatter.kt | 10 ++--- .../DefaultRoomLastMessageFormatterTest.kt | 29 +++++---------- .../api/timeline/item/event/MessageType.kt | 7 +++- .../timeline/item/event/EventMessageMapper.kt | 25 +++++++++---- .../notifications/NotifiableEventResolver.kt | 2 + 7 files changed, 76 insertions(+), 55 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt index 304ff88662..2c92de4df9 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt @@ -100,6 +100,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.LocationMessa import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import io.element.android.libraries.matrix.ui.components.AttachmentThumbnail import io.element.android.libraries.matrix.ui.components.AttachmentThumbnailInfo import io.element.android.libraries.matrix.ui.components.AttachmentThumbnailType @@ -612,18 +613,14 @@ private fun attachmentThumbnailInfoForInReplyTo(inReplyTo: InReplyTo.Ready): Att textContent = messageContent.body, type = AttachmentThumbnailType.Location, ) - is AudioMessageType -> { - when (type.isVoiceMessage) { - true -> AttachmentThumbnailInfo( - textContent = messageContent.body, - type = AttachmentThumbnailType.Voice, - ) - false -> AttachmentThumbnailInfo( - textContent = messageContent.body, - type = AttachmentThumbnailType.Audio, - ) - } - } + is AudioMessageType -> AttachmentThumbnailInfo( + textContent = messageContent.body, + type = AttachmentThumbnailType.Audio, + ) + is VoiceMessageType -> AttachmentThumbnailInfo( + textContent = messageContent.body, + type = AttachmentThumbnailType.Voice, + ) else -> null } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt index 4f78c38393..d15a07526c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt @@ -45,6 +45,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.OtherMessageT import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType import io.element.android.libraries.matrix.api.timeline.item.event.UnknownMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import java.time.Duration @@ -110,16 +111,8 @@ class TimelineItemContentMessageFactory @Inject constructor( fileExtension = fileExtensionExtractor.extractFromName(messageType.body) ) } - is AudioMessageType -> when { - featureFlagService.isFeatureEnabled(FeatureFlags.VoiceMessages) && messageType.isVoiceMessage -> TimelineItemVoiceContent( - eventId = eventId, - body = messageType.body, - mediaSource = messageType.source, - duration = messageType.info?.duration ?: Duration.ZERO, - mimeType = messageType.info?.mimetype ?: MimeTypes.OctetStream, - waveform = messageType.details?.waveform?.toImmutableList() ?: persistentListOf(), - ) - else -> TimelineItemAudioContent( + is AudioMessageType -> { + TimelineItemAudioContent( body = messageType.body, mediaSource = messageType.source, duration = messageType.info?.duration ?: Duration.ZERO, @@ -128,6 +121,30 @@ class TimelineItemContentMessageFactory @Inject constructor( fileExtension = fileExtensionExtractor.extractFromName(messageType.body), ) } + is VoiceMessageType -> { + when (featureFlagService.isFeatureEnabled(FeatureFlags.VoiceMessages)) { + true -> { + TimelineItemVoiceContent( + eventId = eventId, + body = messageType.body, + mediaSource = messageType.source, + duration = messageType.info?.duration ?: Duration.ZERO, + mimeType = messageType.info?.mimetype ?: MimeTypes.OctetStream, + waveform = messageType.details?.waveform?.toImmutableList() ?: persistentListOf(), + ) + } + false -> { + TimelineItemAudioContent( + body = messageType.body, + mediaSource = messageType.source, + duration = messageType.info?.duration ?: Duration.ZERO, + mimeType = messageType.info?.mimetype ?: MimeTypes.OctetStream, + formattedFileSize = fileSizeFormatter.format(messageType.info?.size ?: 0), + fileExtension = fileExtensionExtractor.extractFromName(messageType.body), + ) + } + } + } is FileMessageType -> { val fileExtension = fileExtensionExtractor.extractFromName(messageType.body) TimelineItemFileContent( diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt index 99b06401d9..96626e8633 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt @@ -50,6 +50,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecry import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent import io.element.android.libraries.matrix.api.timeline.item.event.UnknownMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.services.toolbox.api.strings.StringProvider import javax.inject.Inject @@ -128,11 +129,10 @@ class DefaultRoomLastMessageFormatter @Inject constructor( sp.getString(CommonStrings.common_file) } is AudioMessageType -> { - if (messageType.isVoiceMessage) { - sp.getString(CommonStrings.common_voice_message) - } else { - sp.getString(CommonStrings.common_audio) - } + sp.getString(CommonStrings.common_audio) + } + is VoiceMessageType -> { + sp.getString(CommonStrings.common_voice_message) } is OtherMessageType -> { messageType.body diff --git a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt index 48c498c3d8..e9d91fbb8c 100644 --- a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt +++ b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt @@ -47,6 +47,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecry import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent import io.element.android.libraries.matrix.api.timeline.item.event.UnknownMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.aPollContent @@ -162,8 +163,8 @@ class DefaultRoomLastMessageFormatterTest { val sharedContentMessagesTypes = arrayOf( TextMessageType(body, null), VideoMessageType(body, MediaSource("url"), null), - AudioMessageType(body, MediaSource("url"), null, null, false), - AudioMessageType(body, MediaSource("url"), null, null, true), + AudioMessageType(body, MediaSource("url"), null), + VoiceMessageType(body, MediaSource("url"), null, null), ImageMessageType(body, MediaSource("url"), null), FileMessageType(body, MediaSource("url"), null), LocationMessageType(body, "geo:1,2", null), @@ -199,12 +200,8 @@ class DefaultRoomLastMessageFormatterTest { for ((type, result) in resultsInDm) { val expectedResult = when (type) { is VideoMessageType -> "Video" - is AudioMessageType -> { - when (type.isVoiceMessage) { - true -> "Voice message" - false -> "Audio" - } - } + is AudioMessageType -> "Audio" + is VoiceMessageType -> "Voice message" is ImageMessageType -> "Image" is FileMessageType -> "File" is LocationMessageType -> "Shared location" @@ -222,12 +219,8 @@ class DefaultRoomLastMessageFormatterTest { val string = result.toString() val expectedResult = when (type) { is VideoMessageType -> "$senderName: Video" - is AudioMessageType -> { - when (type.isVoiceMessage) { - true -> "$senderName: Voice message" - false -> "$senderName: Audio" - } - } + is AudioMessageType -> "$senderName: Audio" + is VoiceMessageType -> "$senderName: Voice message" is ImageMessageType -> "$senderName: Image" is FileMessageType -> "$senderName: File" is LocationMessageType -> "$senderName: Shared location" @@ -239,12 +232,8 @@ class DefaultRoomLastMessageFormatterTest { } val shouldCreateAnnotatedString = when (type) { is VideoMessageType -> true - is AudioMessageType -> { - when (type.isVoiceMessage) { - true -> true - false -> true - } - } + is AudioMessageType -> true + is VoiceMessageType -> true is ImageMessageType -> true is FileMessageType -> true is LocationMessageType -> false diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/MessageType.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/MessageType.kt index 09f0c00a7c..c8122935bd 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/MessageType.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/MessageType.kt @@ -48,8 +48,13 @@ data class AudioMessageType( val body: String, val source: MediaSource, val info: AudioInfo?, +) : MessageType + +data class VoiceMessageType( + val body: String, + val source: MediaSource, + val info: AudioInfo?, val details: AudioDetails?, - val isVoiceMessage: Boolean, ) : MessageType data class VideoMessageType( diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventMessageMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventMessageMapper.kt index 521ff3bd5a..7f88cc0569 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventMessageMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventMessageMapper.kt @@ -32,6 +32,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.OtherMessageT import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType import io.element.android.libraries.matrix.api.timeline.item.event.UnknownMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import io.element.android.libraries.matrix.impl.media.map import org.matrix.rustcomponents.sdk.Message import org.matrix.rustcomponents.sdk.MessageType @@ -77,13 +78,23 @@ class EventMessageMapper { fun mapMessageType(type: RustMessageType?) = when (type) { is RustMessageType.Audio -> { - AudioMessageType( - body = type.content.body, - source = type.content.source.map(), - info = type.content.info?.map(), - details = type.content.audio?.map(), - isVoiceMessage = type.content.voice != null, - ) + when (type.content.voice) { + null -> { + AudioMessageType( + body = type.content.body, + source = type.content.source.map(), + info = type.content.info?.map(), + ) + } + else -> { + VoiceMessageType( + body = type.content.body, + source = type.content.source.map(), + info = type.content.info?.map(), + details = type.content.audio?.map(), + ) + } + } } is RustMessageType.File -> { FileMessageType(type.content.body, type.content.source.map(), type.content.info?.map()) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt index aabc58befb..c8d496a64e 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt @@ -36,6 +36,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.OtherMessageT import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType import io.element.android.libraries.matrix.api.timeline.item.event.UnknownMessageType import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType +import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType import io.element.android.libraries.push.impl.R import io.element.android.libraries.push.impl.notifications.model.FallbackNotifiableEvent import io.element.android.libraries.push.impl.notifications.model.InviteNotifiableEvent @@ -210,6 +211,7 @@ class NotifiableEventResolver @Inject constructor( ): String { return when (val messageType = content.messageType) { is AudioMessageType -> messageType.body + is VoiceMessageType -> messageType.body is EmoteMessageType -> "* $senderDisplayName ${messageType.body}" is FileMessageType -> messageType.body is ImageMessageType -> messageType.body