From ef4aa8f91e6b941ac1839ce2f0931a88c2e66134 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 8 Oct 2024 18:30:08 +0200 Subject: [PATCH] Do not render pin violation in clear room. --- .../identity/IdentityChangeStatePresenter.kt | 39 ++++++++++++------- .../IdentityChangeStatePresenterTest.kt | 31 ++++++++++++++- .../matrix/test/room/FakeMatrixRoom.kt | 15 ++++++- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt index 9b338b4833..cded6e38de 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt @@ -25,9 +25,13 @@ import kotlinx.collections.immutable.PersistentList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toPersistentList import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import timber.log.Timber @@ -56,22 +60,31 @@ class IdentityChangeStatePresenter @Inject constructor( ) } + @OptIn(ExperimentalCoroutinesApi::class) private fun ProduceStateScope>.observeRoomMemberIdentityStateChange() { - combine(room.identityStateChangesFlow, room.membersStateFlow) { identityStateChanges, membersState -> - identityStateChanges.map { identityStateChange -> - val member = membersState.roomMembers() - ?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId } - ?.toIdentityRoomMember() - ?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId) - RoomMemberIdentityStateChange( - identityRoomMember = member, - identityState = identityStateChange.identityState, - ) + room.syncUpdateFlow + .filter { + // Room cannot become unencrypted, so we can just apply a filter here. + room.isEncrypted } - } .distinctUntilChanged() - .onEach { roomMemberIdentityStateChanges -> - value = roomMemberIdentityStateChanges.toPersistentList() + .flatMapLatest { + combine(room.identityStateChangesFlow, room.membersStateFlow,) { identityStateChanges, membersState -> + identityStateChanges.map { identityStateChange -> + val member = membersState.roomMembers() + ?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId } + ?.toIdentityRoomMember() + ?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId) + RoomMemberIdentityStateChange( + identityRoomMember = member, + identityState = identityStateChange.identityState, + ) + } + } + .distinctUntilChanged() + .onEach { roomMemberIdentityStateChanges -> + value = roomMemberIdentityStateChanges.toPersistentList() + } } .launchIn(this) } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt index 5235361870..afc21efd97 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt @@ -44,7 +44,7 @@ class IdentityChangeStatePresenterTest { @Test fun `present - when the room emits identity change, the presenter emits new state`() = runTest { - val room = FakeMatrixRoom() + val room = FakeMatrixRoom(isEncrypted = true) val presenter = createIdentityChangeStatePresenter(room) presenter.test { val initialState = awaitItem() @@ -65,10 +65,37 @@ class IdentityChangeStatePresenterTest { } } + @Test + fun `present - when the clear room emits identity change, the presenter does not emits new state`() = runTest { + val room = FakeMatrixRoom(isEncrypted = false) + val presenter = createIdentityChangeStatePresenter(room) + presenter.test { + val initialState = awaitItem() + assertThat(initialState.roomMemberIdentityStateChanges).isEmpty() + room.emitIdentityStateChanges( + listOf( + IdentityStateChange( + userId = A_USER_ID_2, + identityState = IdentityState.PinViolation, + ), + ) + ) + // No item emitted. + expectNoEvents() + // Room become encrypted. + room.enableEncryption() + val finalItem = awaitItem() + assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1) + val value = finalItem.roomMemberIdentityStateChanges.first() + assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2) + assertThat(value.identityState).isEqualTo(IdentityState.PinViolation) + } + } + @Test fun `present - when the room emits identity change, the presenter emits new state with member details`() = runTest { - val room = FakeMatrixRoom().apply { + val room = FakeMatrixRoom(isEncrypted = true).apply { givenRoomMembersState( MatrixRoomMembersState.Ready( listOf( 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 81f276cc84..ab0ad45cac 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 @@ -60,6 +60,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import java.io.File class FakeMatrixRoom( @@ -68,7 +69,7 @@ class FakeMatrixRoom( override val displayName: String = "", override val topic: String? = null, override val avatarUrl: String? = null, - override val isEncrypted: Boolean = false, + override var isEncrypted: Boolean = false, override val alias: RoomAlias? = null, override val alternativeAliases: List = emptyList(), override val isPublic: Boolean = true, @@ -181,7 +182,17 @@ class FakeMatrixRoom( return Result.success(Unit) } - override val syncUpdateFlow: StateFlow = MutableStateFlow(0L) + fun enableEncryption() { + isEncrypted = true + emitSyncUpdate() + } + + private val _syncUpdateFlow = MutableStateFlow(0L) + override val syncUpdateFlow: StateFlow = _syncUpdateFlow.asStateFlow() + + fun emitSyncUpdate() { + _syncUpdateFlow.tryEmit(_syncUpdateFlow.value + 1) + } override suspend fun timelineFocusedOnEvent(eventId: EventId): Result = simulateLongTask { timelineFocusedOnEventResult(eventId)