From 792667c9a4c6b689669c8214c7dd9f96ebc6df8e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Jul 2024 18:00:25 +0200 Subject: [PATCH 1/9] Iterate on MatrixPatterns. --- .../matrix/api/core/MatrixPatterns.kt | 36 ++++++++----------- .../matrix/api/core/MatrixPatternsTest.kt | 2 +- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt index 20e7746e3f..e9ac911330 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt @@ -27,32 +27,29 @@ object MatrixPatterns { // Note: TLD is not mandatory (localhost, IP address...) private const val DOMAIN_REGEX = ":[A-Za-z0-9.-]+(:[0-9]{2,5})?" + // See https://spec.matrix.org/v1.11/appendices/#opaque-identifiers + private const val OPAQUE_ID_REGEX = "[0-9A-Za-z-\\._~]+" + // regex pattern to find matrix user ids in a string. // See https://matrix.org/docs/spec/appendices#historical-user-ids // Sadly, we need to relax the regex pattern a bit as there already exist some ids that don't match the spec. - private const val MATRIX_USER_IDENTIFIER_REGEX = "^@.*?$DOMAIN_REGEX$" + private const val MATRIX_USER_IDENTIFIER_REGEX = "^@\\S+?$DOMAIN_REGEX$" private val PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER = MATRIX_USER_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) - // regex pattern to find room ids in a string. - private const val MATRIX_ROOM_IDENTIFIER_REGEX = "![A-Z0-9.-]+$DOMAIN_REGEX" + // regex pattern to match room ids. + private const val MATRIX_ROOM_IDENTIFIER_REGEX = "^!$OPAQUE_ID_REGEX$DOMAIN_REGEX$" private val PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER = MATRIX_ROOM_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) - // regex pattern to find room aliases in a string. - private const val MATRIX_ROOM_ALIAS_REGEX = "#[A-Z0-9._%#@=+-]+$DOMAIN_REGEX" + // regex pattern to match room aliases. + private const val MATRIX_ROOM_ALIAS_REGEX = "^#\\S+$DOMAIN_REGEX$" private val PATTERN_CONTAIN_MATRIX_ALIAS = MATRIX_ROOM_ALIAS_REGEX.toRegex(RegexOption.IGNORE_CASE) - // regex pattern to find message ids in a string. + // regex pattern to match event ids. // Sadly, we need to relax the regex pattern a bit as there already exist some ids that don't match the spec. - private const val MATRIX_EVENT_IDENTIFIER_REGEX = "^\\$.+$DOMAIN_REGEX$" + private const val MATRIX_EVENT_IDENTIFIER_REGEX = "^\\$$OPAQUE_ID_REGEX$DOMAIN_REGEX$" private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER = MATRIX_EVENT_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) - // regex pattern to find message ids in a string. - private const val MATRIX_EVENT_IDENTIFIER_V3_REGEX = "\\$[A-Z0-9/+]+" - private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V3 = MATRIX_EVENT_IDENTIFIER_V3_REGEX.toRegex(RegexOption.IGNORE_CASE) - - // Ref: https://matrix.org/docs/spec/rooms/v4#event-ids - private const val MATRIX_EVENT_IDENTIFIER_V4_REGEX = "\\$[A-Z0-9\\-_]+" - private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4 = MATRIX_EVENT_IDENTIFIER_V4_REGEX.toRegex(RegexOption.IGNORE_CASE) + private const val MAX_IDENTIFIER_LENGTH = 255 /** * Tells if a string is a valid user Id. @@ -61,7 +58,7 @@ object MatrixPatterns { * @return true if the string is a valid user id */ fun isUserId(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER + return str != null && str matches PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH } /** @@ -79,7 +76,7 @@ object MatrixPatterns { * @return true if the string is a valid room Id */ fun isRoomId(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER + return str != null && str matches PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH } /** @@ -89,7 +86,7 @@ object MatrixPatterns { * @return true if the string is a valid room alias. */ fun isRoomAlias(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_ALIAS + return str != null && str matches PATTERN_CONTAIN_MATRIX_ALIAS && str.length <= MAX_IDENTIFIER_LENGTH } /** @@ -99,10 +96,7 @@ object MatrixPatterns { * @return true if the string is a valid event id. */ fun isEventId(str: String?): Boolean { - return str != null && - (str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER || - str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V3 || - str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4) + return str != null && str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH } /** diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt index d195789d14..64e66372ab 100644 --- a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt @@ -54,7 +54,7 @@ class MatrixPatternsTest { } @Test - fun `findPatterns - returns raw room event ids`() { + fun `findPatterns - returns raw event ids`() { val text = "A \$event:server.com and \$event2:server.com" val patterns = MatrixPatterns.findPatterns(text, aPermalinkParser()) assertThat(patterns).containsExactly( From 52900076bd3aa92e590959d31f43c35a8cfc5891 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Jul 2024 18:17:21 +0200 Subject: [PATCH 2/9] Add test on MatrixPatterns functions. --- .../matrix/api/core/MatrixPatternsTest.kt | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt index 64e66372ab..d8399b4823 100644 --- a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt @@ -23,6 +23,8 @@ import io.element.android.libraries.matrix.api.permalink.PermalinkParser import org.junit.Test class MatrixPatternsTest { + private val longLocalPart = "a".repeat(255 - ":server.com".length - 1) + @Test fun `findPatterns - returns raw user ids`() { val text = "A @user:server.com and @user2:server.com" @@ -90,6 +92,70 @@ class MatrixPatternsTest { assertThat(patterns).containsExactly(MatrixPatternResult(MatrixPatternType.ROOM_ALIAS, "#room:server.com", 2, 46)) } + @Test + fun `test isRoomId`() { + assertThat(MatrixPatterns.isRoomId(null)).isFalse() + assertThat(MatrixPatterns.isRoomId("")).isFalse() + assertThat(MatrixPatterns.isRoomId("not a room id")).isFalse() + assertThat(MatrixPatterns.isRoomId(" !room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomId("!room:server.com ")).isFalse() + assertThat(MatrixPatterns.isRoomId("@room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomId("#room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomId("\$room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomId("!${longLocalPart}a:server.com")).isFalse() + + assertThat(MatrixPatterns.isRoomId("!room:server.com")).isTrue() + assertThat(MatrixPatterns.isRoomId("!$longLocalPart:server.com")).isTrue() + } + + @Test + fun `test isRoomAlias`() { + assertThat(MatrixPatterns.isRoomAlias(null)).isFalse() + assertThat(MatrixPatterns.isRoomAlias("")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("not a room alias")).isFalse() + assertThat(MatrixPatterns.isRoomAlias(" #room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("#room:server.com ")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("@room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("!room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("\$room:server.com")).isFalse() + assertThat(MatrixPatterns.isRoomAlias("#${longLocalPart}a:server.com")).isFalse() + + assertThat(MatrixPatterns.isRoomAlias("#room:server.com")).isTrue() + assertThat(MatrixPatterns.isRoomAlias("#$longLocalPart:server.com")).isTrue() + } + + @Test + fun `test isEventId`() { + assertThat(MatrixPatterns.isEventId(null)).isFalse() + assertThat(MatrixPatterns.isEventId("")).isFalse() + assertThat(MatrixPatterns.isEventId("not an event id")).isFalse() + assertThat(MatrixPatterns.isEventId(" \$event:server.com")).isFalse() + assertThat(MatrixPatterns.isEventId("\$event:server.com ")).isFalse() + assertThat(MatrixPatterns.isEventId("@event:server.com")).isFalse() + assertThat(MatrixPatterns.isEventId("!event:server.com")).isFalse() + assertThat(MatrixPatterns.isEventId("#event:server.com")).isFalse() + assertThat(MatrixPatterns.isEventId("$${longLocalPart}a:server.com")).isFalse() + + assertThat(MatrixPatterns.isEventId("\$event:server.com")).isTrue() + assertThat(MatrixPatterns.isEventId("$$longLocalPart:server.com")).isTrue() + } + + @Test + fun `test isUserId`() { + assertThat(MatrixPatterns.isUserId(null)).isFalse() + assertThat(MatrixPatterns.isUserId("")).isFalse() + assertThat(MatrixPatterns.isUserId("not a user id")).isFalse() + assertThat(MatrixPatterns.isUserId(" @user:server.com")).isFalse() + assertThat(MatrixPatterns.isUserId("@user:server.com ")).isFalse() + assertThat(MatrixPatterns.isUserId("!user:server.com")).isFalse() + assertThat(MatrixPatterns.isUserId("#user:server.com")).isFalse() + assertThat(MatrixPatterns.isUserId("\$user:server.com")).isFalse() + assertThat(MatrixPatterns.isUserId("@${longLocalPart}a:server.com")).isFalse() + + assertThat(MatrixPatterns.isUserId("@user:server.com")).isTrue() + assertThat(MatrixPatterns.isUserId("@$longLocalPart:server.com")).isTrue() + } + private fun aPermalinkParser(block: (String) -> PermalinkData = { PermalinkData.FallbackLink(Uri.EMPTY) }) = object : PermalinkParser { override fun parse(uriString: String): PermalinkData { return block(uriString) From 5ae4668da5a6205a3c60d08e8d18a72d0516f888 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Jul 2024 18:23:25 +0200 Subject: [PATCH 3/9] No need to make the block optional. --- .../android/libraries/matrix/api/core/MatrixPatterns.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt index e9ac911330..46fe4f7e99 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt @@ -112,8 +112,8 @@ object MatrixPatterns { * Note not all cases are implemented. */ fun findPatterns(text: CharSequence, permalinkParser: PermalinkParser): List { - val rawTextMatches = "\\S+?$DOMAIN_REGEX".toRegex(RegexOption.IGNORE_CASE).findAll(text) - val urlMatches = "\\[\\S+?\\]\\((\\S+?)\\)".toRegex(RegexOption.IGNORE_CASE).findAll(text) + val rawTextMatches = "\\S+$DOMAIN_REGEX".toRegex(RegexOption.IGNORE_CASE).findAll(text) + val urlMatches = "\\[\\S+\\]\\((\\S+)\\)".toRegex(RegexOption.IGNORE_CASE).findAll(text) val atRoomMatches = Regex("@room").findAll(text) return buildList { for (match in rawTextMatches) { From 79d29d680b4a216e7a9198b907b56f99c7b9afa9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Jul 2024 18:25:20 +0200 Subject: [PATCH 4/9] Add extra test for room alias. --- .../android/libraries/matrix/api/core/MatrixPatternsTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt index d8399b4823..399901cf06 100644 --- a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt @@ -121,6 +121,7 @@ class MatrixPatternsTest { assertThat(MatrixPatterns.isRoomAlias("#${longLocalPart}a:server.com")).isFalse() assertThat(MatrixPatterns.isRoomAlias("#room:server.com")).isTrue() + assertThat(MatrixPatterns.isRoomAlias("#nico's-stickers:neko.dev")).isTrue() assertThat(MatrixPatterns.isRoomAlias("#$longLocalPart:server.com")).isTrue() } From 67e09295de34fc04744f89961cb270123fdb79a4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Jul 2024 18:40:03 +0200 Subject: [PATCH 5/9] Still need to support both eventId legacy and v4 --- .../android/libraries/matrix/api/core/MatrixPatterns.kt | 8 +++++++- .../libraries/matrix/api/core/MatrixPatternsTest.kt | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt index 46fe4f7e99..d706ff9e3a 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt @@ -49,6 +49,9 @@ object MatrixPatterns { private const val MATRIX_EVENT_IDENTIFIER_REGEX = "^\\$$OPAQUE_ID_REGEX$DOMAIN_REGEX$" private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER = MATRIX_EVENT_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) + private const val MATRIX_EVENT_IDENTIFIER_V4_REGEX = "\\$$OPAQUE_ID_REGEX" + private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4 = MATRIX_EVENT_IDENTIFIER_V4_REGEX.toRegex(RegexOption.IGNORE_CASE) + private const val MAX_IDENTIFIER_LENGTH = 255 /** @@ -96,7 +99,10 @@ object MatrixPatterns { * @return true if the string is a valid event id. */ fun isEventId(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH + return str != null && + (str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER || + str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4) && + str.length <= MAX_IDENTIFIER_LENGTH } /** diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt index 399901cf06..a29714764f 100644 --- a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt @@ -136,9 +136,12 @@ class MatrixPatternsTest { assertThat(MatrixPatterns.isEventId("!event:server.com")).isFalse() assertThat(MatrixPatterns.isEventId("#event:server.com")).isFalse() assertThat(MatrixPatterns.isEventId("$${longLocalPart}a:server.com")).isFalse() + assertThat(MatrixPatterns.isEventId("\$" + "a".repeat(255))).isFalse() assertThat(MatrixPatterns.isEventId("\$event:server.com")).isTrue() assertThat(MatrixPatterns.isEventId("$$longLocalPart:server.com")).isTrue() + assertThat(MatrixPatterns.isEventId("\$9BozuV4TBw6rfRW3rMEgZ5v-jNk1D6FA8Hd1OsWqT9k")).isTrue() + assertThat(MatrixPatterns.isEventId("\$" + "a".repeat(254))).isTrue() } @Test From d6c99870f9084667f928ad699517a55da397cdba Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Jul 2024 10:06:44 +0200 Subject: [PATCH 6/9] Be more lenient on Matrix pattern to support existing rooms in the wild (will fix crash on debug build). --- .../matrix/api/core/MatrixPatterns.kt | 47 ++++++++++++------- .../matrix/api/core/MatrixPatternsTest.kt | 2 + 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt index d706ff9e3a..023fb9322b 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatterns.kt @@ -27,18 +27,20 @@ object MatrixPatterns { // Note: TLD is not mandatory (localhost, IP address...) private const val DOMAIN_REGEX = ":[A-Za-z0-9.-]+(:[0-9]{2,5})?" - // See https://spec.matrix.org/v1.11/appendices/#opaque-identifiers - private const val OPAQUE_ID_REGEX = "[0-9A-Za-z-\\._~]+" + private const val BASE_64_ALPHABET = "[0-9A-Za-z/\\+=]+" + private const val BASE_64_URL_SAFE_ALPHABET = "[0-9A-Za-z/\\-_]+" // regex pattern to find matrix user ids in a string. // See https://matrix.org/docs/spec/appendices#historical-user-ids // Sadly, we need to relax the regex pattern a bit as there already exist some ids that don't match the spec. - private const val MATRIX_USER_IDENTIFIER_REGEX = "^@\\S+?$DOMAIN_REGEX$" - private val PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER = MATRIX_USER_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) + // Note: local part can be empty + private const val MATRIX_USER_IDENTIFIER_REGEX = "^@\\S*?$DOMAIN_REGEX$" + private val PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER = MATRIX_USER_IDENTIFIER_REGEX.toRegex() // regex pattern to match room ids. - private const val MATRIX_ROOM_IDENTIFIER_REGEX = "^!$OPAQUE_ID_REGEX$DOMAIN_REGEX$" - private val PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER = MATRIX_ROOM_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) + // Note: roomId can be arbitrary strings, including space and new line char + private const val MATRIX_ROOM_IDENTIFIER_REGEX = "^!.+$DOMAIN_REGEX$" + private val PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER = MATRIX_ROOM_IDENTIFIER_REGEX.toRegex(RegexOption.DOT_MATCHES_ALL) // regex pattern to match room aliases. private const val MATRIX_ROOM_ALIAS_REGEX = "^#\\S+$DOMAIN_REGEX$" @@ -46,11 +48,17 @@ object MatrixPatterns { // regex pattern to match event ids. // Sadly, we need to relax the regex pattern a bit as there already exist some ids that don't match the spec. - private const val MATRIX_EVENT_IDENTIFIER_REGEX = "^\\$$OPAQUE_ID_REGEX$DOMAIN_REGEX$" - private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER = MATRIX_EVENT_IDENTIFIER_REGEX.toRegex(RegexOption.IGNORE_CASE) + // v1 and v2: arbitrary string + domain + private const val MATRIX_EVENT_IDENTIFIER_REGEX = "^\\$.+$DOMAIN_REGEX$" + private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER = MATRIX_EVENT_IDENTIFIER_REGEX.toRegex() - private const val MATRIX_EVENT_IDENTIFIER_V4_REGEX = "\\$$OPAQUE_ID_REGEX" - private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4 = MATRIX_EVENT_IDENTIFIER_V4_REGEX.toRegex(RegexOption.IGNORE_CASE) + // v3: base64 + private const val MATRIX_EVENT_IDENTIFIER_V3_REGEX = "\\$$BASE_64_ALPHABET" + private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V3 = MATRIX_EVENT_IDENTIFIER_V3_REGEX.toRegex() + + // v4: url-safe base64 + private const val MATRIX_EVENT_IDENTIFIER_V4_REGEX = "\\$$BASE_64_URL_SAFE_ALPHABET" + private val PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4 = MATRIX_EVENT_IDENTIFIER_V4_REGEX.toRegex() private const val MAX_IDENTIFIER_LENGTH = 255 @@ -61,7 +69,9 @@ object MatrixPatterns { * @return true if the string is a valid user id */ fun isUserId(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH + return str != null && + str.length <= MAX_IDENTIFIER_LENGTH && + str matches PATTERN_CONTAIN_MATRIX_USER_IDENTIFIER } /** @@ -79,7 +89,9 @@ object MatrixPatterns { * @return true if the string is a valid room Id */ fun isRoomId(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER && str.length <= MAX_IDENTIFIER_LENGTH + return str != null && + str.length <= MAX_IDENTIFIER_LENGTH && + str matches PATTERN_CONTAIN_MATRIX_ROOM_IDENTIFIER } /** @@ -89,7 +101,9 @@ object MatrixPatterns { * @return true if the string is a valid room alias. */ fun isRoomAlias(str: String?): Boolean { - return str != null && str matches PATTERN_CONTAIN_MATRIX_ALIAS && str.length <= MAX_IDENTIFIER_LENGTH + return str != null && + str.length <= MAX_IDENTIFIER_LENGTH && + str matches PATTERN_CONTAIN_MATRIX_ALIAS } /** @@ -100,9 +114,10 @@ object MatrixPatterns { */ fun isEventId(str: String?): Boolean { return str != null && - (str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER || - str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4) && - str.length <= MAX_IDENTIFIER_LENGTH + str.length <= MAX_IDENTIFIER_LENGTH && + (str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V4 || + str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER_V3 || + str matches PATTERN_CONTAIN_MATRIX_EVENT_IDENTIFIER) } /** diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt index a29714764f..68f938adc9 100644 --- a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/core/MatrixPatternsTest.kt @@ -106,6 +106,7 @@ class MatrixPatternsTest { assertThat(MatrixPatterns.isRoomId("!room:server.com")).isTrue() assertThat(MatrixPatterns.isRoomId("!$longLocalPart:server.com")).isTrue() + assertThat(MatrixPatterns.isRoomId("!#test/room\nversion 11, with @🐈️:maunium.net")).isTrue() } @Test @@ -157,6 +158,7 @@ class MatrixPatternsTest { assertThat(MatrixPatterns.isUserId("@${longLocalPart}a:server.com")).isFalse() assertThat(MatrixPatterns.isUserId("@user:server.com")).isTrue() + assertThat(MatrixPatterns.isUserId("@:server.com")).isTrue() assertThat(MatrixPatterns.isUserId("@$longLocalPart:server.com")).isTrue() } From 10717b5e48683e0b39f4d73d52a1e3449515ab0b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Jul 2024 10:58:58 +0200 Subject: [PATCH 7/9] Render errors in room member list view. --- .../impl/members/RoomMemberListPresenter.kt | 39 ++++-- .../impl/members/RoomMemberListState.kt | 18 +-- .../members/RoomMemberListStateProvider.kt | 81 ++++++----- .../impl/members/RoomMemberListView.kt | 130 +++++++++++------- 4 files changed, 159 insertions(+), 109 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 305393c822..51f9845a49 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -31,6 +31,7 @@ import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationEvents import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationPresenter +import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.designsystem.theme.components.SearchBarResultState @@ -59,10 +60,10 @@ class RoomMemberListPresenter @AssistedInject constructor( @Composable override fun present(): RoomMemberListState { val coroutineScope = rememberCoroutineScope() - var roomMembers by remember { mutableStateOf(RoomMembers.loading()) } + var roomMembers: AsyncData by remember { mutableStateOf(AsyncData.Loading()) } var searchQuery by rememberSaveable { mutableStateOf("") } var searchResults by remember { - mutableStateOf>(SearchBarResultState.Initial()) + mutableStateOf>>(SearchBarResultState.Initial()) } var isSearchActive by rememberSaveable { mutableStateOf(false) } @@ -82,6 +83,12 @@ class RoomMemberListPresenter @AssistedInject constructor( if (membersState is MatrixRoomMembersState.Unknown) { return@LaunchedEffect } + val _membersState = membersState + if (_membersState is MatrixRoomMembersState.Error && _membersState.roomMembers().orEmpty().isEmpty()) { + // Cannot fetch members and no cached members, display the error + roomMembers = AsyncData.Failure(_membersState.failure) + return@LaunchedEffect + } withContext(coroutineDispatchers.io) { val members = membersState.roomMembers().orEmpty().groupBy { it.membership } val info = room.roomInfoFlow.first() @@ -90,14 +97,18 @@ class RoomMemberListPresenter @AssistedInject constructor( // This result will come from the timeline loading membership events and it'll be wrong. return@withContext } - roomMembers = RoomMembers( + val result = RoomMembers( invited = members.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(), joined = members.getOrDefault(RoomMembershipState.JOIN, emptyList()) .sortedWith(PowerLevelRoomMemberComparator()) .toImmutableList(), banned = members.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(), - isLoading = membersState is MatrixRoomMembersState.Pending, ) + roomMembers = if (membersState is MatrixRoomMembersState.Pending) { + AsyncData.Loading(result) + } else { + AsyncData.Success(result) + } } } @@ -110,15 +121,19 @@ class RoomMemberListPresenter @AssistedInject constructor( if (results.isEmpty()) { SearchBarResultState.NoResultsFound() } else { + val result = RoomMembers( + invited = results.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(), + joined = results.getOrDefault(RoomMembershipState.JOIN, emptyList()) + .sortedWith(PowerLevelRoomMemberComparator()) + .toImmutableList(), + banned = results.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(), + ) SearchBarResultState.Results( - RoomMembers( - invited = results.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(), - joined = results.getOrDefault(RoomMembershipState.JOIN, emptyList()) - .sortedWith(PowerLevelRoomMemberComparator()) - .toImmutableList(), - banned = results.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(), - isLoading = membersState is MatrixRoomMembersState.Pending, - ) + if (membersState is MatrixRoomMembersState.Pending) { + AsyncData.Loading(result) + } else { + AsyncData.Success(result) + } ) } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt index 67a0802f02..b71da8bfd8 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt @@ -17,15 +17,15 @@ package io.element.android.features.roomdetails.impl.members import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationState +import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.designsystem.theme.components.SearchBarResultState import io.element.android.libraries.matrix.api.room.RoomMember import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.persistentListOf data class RoomMemberListState( - val roomMembers: RoomMembers, + val roomMembers: AsyncData, val searchQuery: String, - val searchResults: SearchBarResultState, + val searchResults: SearchBarResultState>, val isSearchActive: Boolean, val canInvite: Boolean, val moderationState: RoomMembersModerationState, @@ -36,14 +36,4 @@ data class RoomMembers( val invited: ImmutableList, val joined: ImmutableList, val banned: ImmutableList, - val isLoading: Boolean, -) { - companion object { - fun loading() = RoomMembers( - invited = persistentListOf(), - joined = persistentListOf(), - banned = persistentListOf(), - isLoading = true, - ) - } -} +) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt index 8be4124e0d..11132040b0 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt @@ -19,6 +19,7 @@ package io.element.android.features.roomdetails.impl.members import androidx.compose.ui.tooling.preview.PreviewParameterProvider import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationState import io.element.android.features.roomdetails.impl.members.moderation.aRoomMembersModerationState +import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.designsystem.theme.components.SearchBarResultState import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.RoomMember @@ -29,14 +30,15 @@ internal class RoomMemberListStateProvider : PreviewParameterProvider get() = sequenceOf( aRoomMemberListState( - roomMembers = RoomMembers( - invited = persistentListOf(aVictor(), aWalter()), - joined = persistentListOf(anAlice(), aBob(), aWalter()), - banned = persistentListOf(), - isLoading = false, + roomMembers = AsyncData.Success( + RoomMembers( + invited = persistentListOf(aVictor(), aWalter()), + joined = persistentListOf(anAlice(), aBob(), aWalter()), + banned = persistentListOf(), + ) ) ), - aRoomMemberListState(roomMembers = RoomMembers.loading()), + aRoomMemberListState(roomMembers = AsyncData.Loading()), aRoomMemberListState().copy(canInvite = true), aRoomMemberListState().copy(isSearchActive = false), aRoomMemberListState().copy(isSearchActive = true), @@ -45,11 +47,12 @@ internal class RoomMemberListStateProvider : PreviewParameterProvider get() = sequenceOf( aRoomMemberListState( - roomMembers = RoomMembers( - invited = persistentListOf(), - joined = persistentListOf(), - banned = persistentListOf( - aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"), - aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"), - aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"), - ), - isLoading = false, + roomMembers = AsyncData.Success( + RoomMembers( + invited = persistentListOf(), + joined = persistentListOf(), + banned = persistentListOf( + aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"), + aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"), + aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"), + ), + ) ), moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true), ), aRoomMemberListState( - roomMembers = RoomMembers( - invited = persistentListOf(), - joined = persistentListOf(), - banned = persistentListOf( - aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"), - aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"), - aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"), - ), - isLoading = true, + roomMembers = AsyncData.Loading( + RoomMembers( + invited = persistentListOf(), + joined = persistentListOf(), + banned = persistentListOf( + aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"), + aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"), + aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"), + ), + ) ), moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true), ), aRoomMemberListState( - roomMembers = RoomMembers( - invited = persistentListOf(), - joined = persistentListOf(), - banned = persistentListOf(), - isLoading = false, + roomMembers = AsyncData.Success( + RoomMembers( + invited = persistentListOf(), + joined = persistentListOf(), + banned = persistentListOf(), + ) ), moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true), ) @@ -103,8 +112,8 @@ internal class RoomMemberListStateBannedProvider : PreviewParameterProvider = SearchBarResultState.Initial(), + roomMembers: AsyncData = AsyncData.Loading(), + searchResults: SearchBarResultState> = SearchBarResultState.Initial(), moderationState: RoomMembersModerationState = aRoomMembersModerationState(), ) = RoomMemberListState( roomMembers = roomMembers, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt index b12ecf4ec8..b0110b7039 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt @@ -53,6 +53,7 @@ import androidx.compose.ui.unit.dp import io.element.android.compound.theme.ElementTheme import io.element.android.features.roomdetails.impl.R import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationView +import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.components.button.BackButton import io.element.android.libraries.designsystem.preview.ElementPreview @@ -128,7 +129,6 @@ fun RoomMemberListView( if (!state.isSearchActive) { RoomMemberList( - isLoading = state.roomMembers.isLoading, roomMembers = state.roomMembers, showMembersCount = true, canDisplayBannedUsersControls = state.moderationState.canDisplayBannedUsers, @@ -149,8 +149,7 @@ fun RoomMemberListView( @OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class) @Composable private fun RoomMemberList( - isLoading: Boolean, - roomMembers: RoomMembers, + roomMembers: AsyncData, showMembersCount: Boolean, selectedSection: SelectedSection, onSelectedSectionChange: (SelectedSection) -> Unit, @@ -183,7 +182,7 @@ private fun RoomMemberList( } } AnimatedVisibility( - visible = isLoading, + visible = roomMembers.isLoading(), enter = fadeIn() + expandVertically(), exit = fadeOut() + shrinkVertically(), ) { @@ -191,47 +190,72 @@ private fun RoomMemberList( } } } - when (selectedSection) { - SelectedSection.MEMBERS -> { - if (roomMembers.invited.isNotEmpty()) { - roomMemberListSection( - headerText = { stringResource(id = R.string.screen_room_member_list_pending_header_title) }, - members = roomMembers.invited, - onMemberSelected = { onSelectUser(it) } - ) - } - if (roomMembers.joined.isNotEmpty()) { - roomMemberListSection( - headerText = { - if (showMembersCount) { - val memberCount = roomMembers.joined.count() - pluralStringResource(id = R.plurals.screen_room_member_list_header_title, count = memberCount, memberCount) - } else { - stringResource(id = R.string.screen_room_member_list_room_members_header_title) - } - }, - members = roomMembers.joined, - onMemberSelected = { onSelectUser(it) } - ) - } + when (roomMembers) { + is AsyncData.Failure -> failureItem(roomMembers.error) + is AsyncData.Loading, + is AsyncData.Success -> memberItems( + roomMembers = roomMembers.dataOrNull() ?: return@LazyColumn, + selectedSection = selectedSection, + onSelectUser = onSelectUser, + showMembersCount = showMembersCount, + ) + AsyncData.Uninitialized -> Unit + } + } +} + +private fun LazyListScope.memberItems( + roomMembers: RoomMembers, + selectedSection: SelectedSection, + onSelectUser: (RoomMember) -> Unit, + showMembersCount: Boolean, +) { + when (selectedSection) { + SelectedSection.MEMBERS -> { + if (roomMembers.invited.isNotEmpty()) { + roomMemberListSection( + headerText = { stringResource(id = R.string.screen_room_member_list_pending_header_title) }, + members = roomMembers.invited, + onMemberSelected = { onSelectUser(it) } + ) } - SelectedSection.BANNED -> { // Banned users - if (roomMembers.banned.isNotEmpty()) { - roomMemberListSection( - headerText = null, - members = roomMembers.banned, - onMemberSelected = { onSelectUser(it) } - ) - } else { - item { - Box(Modifier.fillParentMaxSize().padding(horizontal = 16.dp)) { - Text( - modifier = Modifier.padding(bottom = 56.dp).align(Alignment.Center), - text = stringResource(id = R.string.screen_room_member_list_banned_empty), - color = ElementTheme.colors.textSecondary, - textAlign = TextAlign.Center, - ) + if (roomMembers.joined.isNotEmpty()) { + roomMemberListSection( + headerText = { + if (showMembersCount) { + val memberCount = roomMembers.joined.count() + pluralStringResource(id = R.plurals.screen_room_member_list_header_title, count = memberCount, memberCount) + } else { + stringResource(id = R.string.screen_room_member_list_room_members_header_title) } + }, + members = roomMembers.joined, + onMemberSelected = { onSelectUser(it) } + ) + } + } + SelectedSection.BANNED -> { // Banned users + if (roomMembers.banned.isNotEmpty()) { + roomMemberListSection( + headerText = null, + members = roomMembers.banned, + onMemberSelected = { onSelectUser(it) } + ) + } else { + item { + Box( + Modifier + .fillParentMaxSize() + .padding(horizontal = 16.dp) + ) { + Text( + modifier = Modifier + .padding(bottom = 56.dp) + .align(Alignment.Center), + text = stringResource(id = R.string.screen_room_member_list_banned_empty), + color = ElementTheme.colors.textSecondary, + textAlign = TextAlign.Center, + ) } } } @@ -239,9 +263,22 @@ private fun RoomMemberList( } } +private fun LazyListScope.failureItem(failure: Throwable) { + item { + Text( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 32.dp), + text = stringResource(id = CommonStrings.error_unknown) + "\n\n" + failure.localizedMessage, + color = ElementTheme.colors.textCriticalPrimary, + textAlign = TextAlign.Center, + ) + } +} + private fun LazyListScope.roomMemberListSection( headerText: @Composable (() -> String)?, - members: ImmutableList, + members: ImmutableList?, onMemberSelected: (RoomMember) -> Unit, ) { headerText?.let { @@ -254,7 +291,7 @@ private fun LazyListScope.roomMemberListSection( ) } } - items(members) { matrixUser -> + items(members.orEmpty()) { matrixUser -> RoomMemberListItem( modifier = Modifier.fillMaxWidth(), roomMember = matrixUser, @@ -320,7 +357,7 @@ private fun RoomMemberListTopBar( @Composable private fun RoomMemberSearchBar( query: String, - state: SearchBarResultState, + state: SearchBarResultState>, active: Boolean, placeHolderTitle: String, onActiveChange: (Boolean) -> Unit, @@ -339,7 +376,6 @@ private fun RoomMemberSearchBar( resultState = state, resultHandler = { results -> RoomMemberList( - isLoading = false, roomMembers = results, showMembersCount = false, onSelectUser = { onSelectUser(it) }, From 26aaa9e07018d19083e0b3fe07f09ee15bd720a9 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Wed, 17 Jul 2024 09:10:50 +0000 Subject: [PATCH 8/9] Update screenshots --- ...es.roomdetails.impl.members_RoomMemberListView_Day_8_en.png | 3 +++ ....roomdetails.impl.members_RoomMemberListView_Night_8_en.png | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_8_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_8_en.png diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_8_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_8_en.png new file mode 100644 index 0000000000..3dafa23bb0 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_8_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cd04abc2eec057ef2dbd02e06c3f0ecd8e66fed3f760f293af4bb86c91e031f6 +size 18284 diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_8_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_8_en.png new file mode 100644 index 0000000000..516705b7ee --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_8_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:93cd01b7fe046a90b49a20311941bb3a07e8d2bd71eb9335343f2c15713ce73e +size 17222 From 263c0588437d9cffd0b743765e2bb38d8b9e2c89 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Jul 2024 11:31:25 +0200 Subject: [PATCH 9/9] Fix quality and test compilation. --- .../impl/members/RoomMemberListPresenter.kt | 6 +++--- .../roomdetails/members/RoomMemberListPresenterTest.kt | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 51f9845a49..01e1cf99c2 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -83,10 +83,10 @@ class RoomMemberListPresenter @AssistedInject constructor( if (membersState is MatrixRoomMembersState.Unknown) { return@LaunchedEffect } - val _membersState = membersState - if (_membersState is MatrixRoomMembersState.Error && _membersState.roomMembers().orEmpty().isEmpty()) { + val finalMembersState = membersState + if (finalMembersState is MatrixRoomMembersState.Error && finalMembersState.roomMembers().orEmpty().isEmpty()) { // Cannot fetch members and no cached members, display the error - roomMembers = AsyncData.Failure(_membersState.failure) + roomMembers = AsyncData.Failure(finalMembersState.failure) return@LaunchedEffect } withContext(coroutineDispatchers.io) { diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTest.kt index c6862f3e2e..a352d8bf27 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTest.kt @@ -62,7 +62,7 @@ class RoomMemberListPresenterTest { }.test { skipItems(1) val initialState = awaitItem() - assertThat(initialState.roomMembers.isLoading).isTrue() + assertThat(initialState.roomMembers.isLoading()).isTrue() assertThat(initialState.searchQuery).isEmpty() assertThat(initialState.searchResults).isInstanceOf(SearchBarResultState.Initial::class.java) assertThat(initialState.isSearchActive).isFalse() @@ -70,9 +70,9 @@ class RoomMemberListPresenterTest { // Skip item while the new members state is processed skipItems(1) val loadedMembersState = awaitItem() - assertThat(loadedMembersState.roomMembers.isLoading).isFalse() - assertThat(loadedMembersState.roomMembers.invited).isEqualTo(listOf(aVictor(), aWalter())) - assertThat(loadedMembersState.roomMembers.joined).isNotEmpty() + assertThat(loadedMembersState.roomMembers.isLoading()).isFalse() + assertThat(loadedMembersState.roomMembers.dataOrNull()?.invited).isEqualTo(listOf(aVictor(), aWalter())) + assertThat(loadedMembersState.roomMembers.dataOrNull()?.joined).isNotEmpty() } } @@ -126,7 +126,7 @@ class RoomMemberListPresenterTest { assertThat(searchQueryUpdatedState.searchQuery).isEqualTo("Alice") val searchSearchResultDelivered = awaitItem() assertThat(searchSearchResultDelivered.searchResults).isInstanceOf(SearchBarResultState.Results::class.java) - assertThat((searchSearchResultDelivered.searchResults as SearchBarResultState.Results).results.joined.first().displayName) + assertThat((searchSearchResultDelivered.searchResults as SearchBarResultState.Results).results.dataOrNull()!!.joined.first().displayName) .isEqualTo("Alice") } }