From 294f1f2d96e3d1b61ea9d954f376a134417e6ef7 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 9 Apr 2024 11:45:43 +0200 Subject: [PATCH] Use SDK API to build room and event permalinks. --- .../messages/impl/MessagesPresenterTest.kt | 2 +- .../roomdetails/impl/RoomDetailsNode.kt | 32 +++++++++-------- .../matrix/api/permalink/PermalinkBuilder.kt | 5 --- .../libraries/matrix/api/room/MatrixRoom.kt | 5 +++ .../impl/permalink/DefaultPermalinkBuilder.kt | 31 ---------------- .../matrix/impl/room/RustMatrixRoom.kt | 18 ++++------ .../permalink/DefaultPermalinkBuilderTest.kt | 35 +------------------ .../test/permalink/FakePermalinkBuilder.kt | 9 ----- .../matrix/test/room/FakeMatrixRoom.kt | 9 +++-- 9 files changed, 37 insertions(+), 109 deletions(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt index ab604e468e..002b34f0eb 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt @@ -236,7 +236,7 @@ class MessagesPresenterTest { val clipboardHelper = FakeClipboardHelper() val event = aMessageEvent() val matrixRoom = FakeMatrixRoom( - permalinkResult = { Result.success("a link") }, + eventPermalinkResult = { Result.success("a link") }, ) val presenter = createMessagesPresenter( clipboardHelper = clipboardHelper, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt index 6bf8b00dd7..562e34bb38 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt @@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.impl import android.content.Context import androidx.compose.runtime.Composable +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import com.bumble.appyx.core.lifecycle.subscribe @@ -35,6 +36,8 @@ import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.services.analytics.api.AnalyticsService +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import timber.log.Timber import io.element.android.libraries.androidutils.R as AndroidUtilsR @@ -83,20 +86,18 @@ class RoomDetailsNode @AssistedInject constructor( callbacks.forEach { it.openPollHistory() } } - private fun onShareRoom(context: Context) { - val alias = room.alias ?: room.alternativeAliases.firstOrNull() - val permalinkResult = alias?.let { permalinkBuilder.permalinkForRoomAlias(it) } - ?: permalinkBuilder.permalinkForRoomId(room.roomId) - permalinkResult.onSuccess { permalink -> - context.startSharePlainTextIntent( - activityResultLauncher = null, - chooserTitle = context.getString(R.string.screen_room_details_share_room_title), - text = permalink, - noActivityFoundMessage = context.getString(AndroidUtilsR.string.error_no_compatible_app_found) - ) - }.onFailure { - Timber.e(it) - } + private fun CoroutineScope.onShareRoom(context: Context) = launch { + room.getPermalink() + .onSuccess { permalink -> + context.startSharePlainTextIntent( + activityResultLauncher = null, + chooserTitle = context.getString(R.string.screen_room_details_share_room_title), + text = permalink, + noActivityFoundMessage = context.getString(AndroidUtilsR.string.error_no_compatible_app_found) + ) + }.onFailure { + Timber.e(it) + } } private fun onShareMember(context: Context, member: RoomMember) { @@ -129,9 +130,10 @@ class RoomDetailsNode @AssistedInject constructor( override fun View(modifier: Modifier) { val context = LocalContext.current val state = presenter.present() + val coroutineScope = rememberCoroutineScope() fun onShareRoom() { - this.onShareRoom(context) + coroutineScope.onShareRoom(context) } fun onShareMember(roomMember: RoomMember) { diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/permalink/PermalinkBuilder.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/permalink/PermalinkBuilder.kt index 14c29f2de5..76bb327c2e 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/permalink/PermalinkBuilder.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/permalink/PermalinkBuilder.kt @@ -16,17 +16,12 @@ package io.element.android.libraries.matrix.api.permalink -import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId interface PermalinkBuilder { fun permalinkForUser(userId: UserId): Result - fun permalinkForRoomAlias(roomAlias: String): Result - fun permalinkForRoomId(roomId: RoomId): Result } sealed class PermalinkBuilderError : Throwable() { - data object InvalidRoomAlias : PermalinkBuilderError() - data object InvalidRoomId : PermalinkBuilderError() data object InvalidUserId : PermalinkBuilderError() } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 07b5b310bc..761d87445a 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -317,6 +317,11 @@ interface MatrixRoom : Closeable { */ fun getWidgetDriver(widgetSettings: MatrixWidgetSettings): Result + /** + * Get the permalink for the room. + */ + suspend fun getPermalink(): Result + /** * Get the permalink for the provided [eventId]. * @param eventId The event id to get the permalink for. diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilder.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilder.kt index ae30c94c9c..42599c5e64 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilder.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilder.kt @@ -20,7 +20,6 @@ import com.squareup.anvil.annotations.ContributesBinding import io.element.android.appconfig.MatrixConfiguration import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.core.MatrixPatterns -import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder import io.element.android.libraries.matrix.api.permalink.PermalinkBuilderError @@ -54,39 +53,9 @@ class DefaultPermalinkBuilder @Inject constructor() : PermalinkBuilder { } } - override fun permalinkForRoomAlias(roomAlias: String): Result { - return if (MatrixPatterns.isRoomAlias(roomAlias)) { - Result.success(permalinkForRoomAliasOrId(roomAlias)) - } else { - Result.failure(PermalinkBuilderError.InvalidRoomAlias) - } - } - - override fun permalinkForRoomId(roomId: RoomId): Result { - return if (MatrixPatterns.isRoomId(roomId.value)) { - Result.success(permalinkForRoomAliasOrId(roomId.value)) - } else { - Result.failure(PermalinkBuilderError.InvalidRoomId) - } - } - - private fun permalinkForRoomAliasOrId(value: String): String { - val id = escapeId(value) - return buildString { - append(permalinkBaseUrl) - if (!isMatrixTo()) { - append(ROOM_PATH) - } - append(id) - } - } - - private fun escapeId(value: String) = value.replace("/", "%2F") - private fun isMatrixTo(): Boolean = permalinkBaseUrl.startsWith(MatrixConfiguration.MATRIX_TO_PERMALINK_BASE_URL) companion object { - private const val ROOM_PATH = "room/" private const val USER_PATH = "user/" } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 0531c59d4a..59e5c99efa 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -16,7 +16,6 @@ package io.element.android.libraries.matrix.impl.room -import io.element.android.appconfig.MatrixConfiguration import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.core.coroutine.childScope import io.element.android.libraries.matrix.api.core.EventId @@ -731,17 +730,12 @@ class RustMatrixRoom( ) } - override suspend fun getPermalinkFor(eventId: EventId): Result { - // FIXME Use the SDK API once https://github.com/matrix-org/matrix-rust-sdk/issues/3259 has been done - // Now use a simple builder - return runCatching { - buildString { - append(MatrixConfiguration.MATRIX_TO_PERMALINK_BASE_URL) - append(roomId.value) - append("/") - append(eventId.value) - } - } + override suspend fun getPermalink(): Result = runCatching { + innerRoom.matrixToPermalink() + } + + override suspend fun getPermalinkFor(eventId: EventId): Result = runCatching { + innerRoom.matrixToEventPermalink(eventId.value) } private fun sendAttachment(files: List, handle: () -> SendAttachmentJoinHandle): Result { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilderTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilderTest.kt index c861b3105c..bebb46cf85 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilderTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/permalink/DefaultPermalinkBuilderTest.kt @@ -18,12 +18,12 @@ package io.element.android.libraries.matrix.impl.permalink import com.google.common.truth.Truth.assertThat import io.element.android.libraries.androidutils.metadata.withReleaseBehavior -import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.tests.testutils.assertThrowsInDebug import org.junit.Test class DefaultPermalinkBuilderTest { + @Test fun `building a permalink for an invalid user id throws when verifying the id`() { assertThrowsInDebug { val userId = UserId("some invalid user id") @@ -31,13 +31,6 @@ class DefaultPermalinkBuilderTest { } } - fun `building a permalink for an invalid room id throws when verifying the id`() { - assertThrowsInDebug { - val roomId = RoomId("some invalid room id") - DefaultPermalinkBuilder().permalinkForRoomId(roomId) - } - } - @Test fun `building a permalink for an invalid user id returns failure when not verifying the id`() { withReleaseBehavior { @@ -46,35 +39,9 @@ class DefaultPermalinkBuilderTest { } } - @Test - fun `building a permalink for an invalid room id returns failure when not verifying the id`() { - withReleaseBehavior { - val roomId = RoomId("some invalid room id") - assertThat(DefaultPermalinkBuilder().permalinkForRoomId(roomId).isFailure).isTrue() - } - } - - @Test - fun `building a permalink for an invalid room alias returns failure`() { - val roomAlias = "an invalid room alias" - assertThat(DefaultPermalinkBuilder().permalinkForRoomAlias(roomAlias).isFailure).isTrue() - } - @Test fun `building a permalink for a valid user id returns a matrix-to url`() { val userId = UserId("@user:matrix.org") assertThat(DefaultPermalinkBuilder().permalinkForUser(userId).getOrNull()).isEqualTo("https://matrix.to/#/@user:matrix.org") } - - @Test - fun `building a permalink for a valid room id returns a matrix-to url`() { - val roomId = RoomId("!aBCdEFG1234:matrix.org") - assertThat(DefaultPermalinkBuilder().permalinkForRoomId(roomId).getOrNull()).isEqualTo("https://matrix.to/#/!aBCdEFG1234:matrix.org") - } - - @Test - fun `building a permalink for a valid room alias returns a matrix-to url`() { - val roomAlias = "#room:matrix.org" - assertThat(DefaultPermalinkBuilder().permalinkForRoomAlias(roomAlias).getOrNull()).isEqualTo("https://matrix.to/#/#room:matrix.org") - } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/permalink/FakePermalinkBuilder.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/permalink/FakePermalinkBuilder.kt index 4e6d3375e1..273899e762 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/permalink/FakePermalinkBuilder.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/permalink/FakePermalinkBuilder.kt @@ -16,7 +16,6 @@ package io.element.android.libraries.matrix.test.permalink -import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder @@ -26,12 +25,4 @@ class FakePermalinkBuilder( override fun permalinkForUser(userId: UserId): Result { return result() } - - override fun permalinkForRoomAlias(roomAlias: String): Result { - return result() - } - - override fun permalinkForRoomId(roomId: RoomId): Result { - return result() - } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index 0de64514b7..5938cd300b 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -84,7 +84,8 @@ class FakeMatrixRoom( override val activeMemberCount: Long = 234L, val notificationSettingsService: NotificationSettingsService = FakeNotificationSettingsService(), private val matrixTimeline: MatrixTimeline = FakeMatrixTimeline(), - private var permalinkResult: () -> Result = { Result.success("link") }, + private var roomPermalinkResult: () -> Result = { Result.success("room link") }, + private var eventPermalinkResult: (EventId) -> Result = { Result.success("event link") }, canRedactOwn: Boolean = false, canRedactOther: Boolean = false, ) : MatrixRoom { @@ -278,8 +279,12 @@ class FakeMatrixRoom( return cancelSendResult } + override suspend fun getPermalink(): Result { + return roomPermalinkResult() + } + override suspend fun getPermalinkFor(eventId: EventId): Result { - return permalinkResult() + return eventPermalinkResult(eventId) } override suspend fun editMessage(