Skip to content

Commit 3f1543e

Browse files
BillCarsonFrBillCarsonFr
committed
code review: renaming, comments, extract common code
1 parent 31ab669 commit 3f1543e

File tree

10 files changed

+90
-69
lines changed

10 files changed

+90
-69
lines changed

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeEvent.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ package io.element.android.features.messages.impl.crypto.identity
1010
import io.element.android.libraries.matrix.api.core.UserId
1111

1212
sealed interface IdentityChangeEvent {
13-
data class PinViolation(val userId: UserId) : IdentityChangeEvent
14-
data class VerificationViolation(val userId: UserId) : IdentityChangeEvent
13+
data class PinIdentity(val userId: UserId) : IdentityChangeEvent
14+
data class WithdrawVerification(val userId: UserId) : IdentityChangeEvent
1515
}

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt‎

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,20 @@
88
package io.element.android.features.messages.impl.crypto.identity
99

1010
import androidx.compose.runtime.Composable
11-
import androidx.compose.runtime.ProduceStateScope
1211
import androidx.compose.runtime.getValue
1312
import androidx.compose.runtime.produceState
1413
import androidx.compose.runtime.rememberCoroutineScope
14+
import io.element.android.features.messages.impl.messagecomposer.observeRoomMemberIdentityStateChange
1515
import io.element.android.libraries.architecture.Presenter
1616
import io.element.android.libraries.designsystem.components.avatar.AvatarData
1717
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
1818
import io.element.android.libraries.matrix.api.core.UserId
1919
import io.element.android.libraries.matrix.api.encryption.EncryptionService
2020
import io.element.android.libraries.matrix.api.room.MatrixRoom
2121
import io.element.android.libraries.matrix.api.room.RoomMember
22-
import io.element.android.libraries.matrix.api.room.roomMembers
2322
import io.element.android.libraries.matrix.ui.model.getAvatarData
24-
import kotlinx.collections.immutable.PersistentList
2523
import kotlinx.collections.immutable.persistentListOf
26-
import kotlinx.collections.immutable.toPersistentList
2724
import kotlinx.coroutines.CoroutineScope
28-
import kotlinx.coroutines.ExperimentalCoroutinesApi
29-
import kotlinx.coroutines.flow.combine
30-
import kotlinx.coroutines.flow.distinctUntilChanged
31-
import kotlinx.coroutines.flow.filter
32-
import kotlinx.coroutines.flow.flatMapLatest
33-
import kotlinx.coroutines.flow.launchIn
34-
import kotlinx.coroutines.flow.onEach
3525
import kotlinx.coroutines.launch
3626
import timber.log.Timber
3727
import javax.inject.Inject
@@ -44,15 +34,15 @@ class IdentityChangeStatePresenter @Inject constructor(
4434
override fun present(): IdentityChangeState {
4535
val coroutineScope = rememberCoroutineScope()
4636
val roomMemberIdentityStateChange by produceState(persistentListOf()) {
47-
observeRoomMemberIdentityStateChange()
37+
observeRoomMemberIdentityStateChange(room)
4838
}
4939

5040
fun handleEvent(event: IdentityChangeEvent) {
5141
when (event) {
52-
is IdentityChangeEvent.VerificationViolation -> {
53-
coroutineScope.withdrawVerificationRequirement(event.userId)
42+
is IdentityChangeEvent.WithdrawVerification -> {
43+
coroutineScope.withdrawVerification(event.userId)
5444
}
55-
is IdentityChangeEvent.PinViolation -> {
45+
is IdentityChangeEvent.PinIdentity -> {
5646
coroutineScope.pinUserIdentity(event.userId)
5747
}
5848
}
@@ -64,44 +54,15 @@ class IdentityChangeStatePresenter @Inject constructor(
6454
)
6555
}
6656

67-
@OptIn(ExperimentalCoroutinesApi::class)
68-
private fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange() {
69-
room.syncUpdateFlow
70-
.filter {
71-
// Room cannot become unencrypted, so we can just apply a filter here.
72-
room.isEncrypted
73-
}
74-
.distinctUntilChanged()
75-
.flatMapLatest {
76-
combine(room.identityStateChangesFlow, room.membersStateFlow) { identityStateChanges, membersState ->
77-
identityStateChanges.map { identityStateChange ->
78-
val member = membersState.roomMembers()
79-
?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId }
80-
?.toIdentityRoomMember()
81-
?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId)
82-
RoomMemberIdentityStateChange(
83-
identityRoomMember = member,
84-
identityState = identityStateChange.identityState,
85-
)
86-
}
87-
}
88-
.distinctUntilChanged()
89-
.onEach { roomMemberIdentityStateChanges ->
90-
value = roomMemberIdentityStateChanges.toPersistentList()
91-
}
92-
}
93-
.launchIn(this)
94-
}
95-
9657
private fun CoroutineScope.pinUserIdentity(userId: UserId) = launch {
9758
encryptionService.pinUserIdentity(userId)
9859
.onFailure {
9960
Timber.e(it, "Failed to pin identity for user $userId")
10061
}
10162
}
10263

103-
private fun CoroutineScope.withdrawVerificationRequirement(userId: UserId) = launch {
104-
encryptionService.withdrawVerificationRequirement(userId)
64+
private fun CoroutineScope.withdrawVerification(userId: UserId) = launch {
65+
encryptionService.withdrawVerification(userId)
10566
.onFailure {
10667
Timber.e(it, "Failed to withdraw verification for user $userId")
10768
}

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,24 @@ fun IdentityChangeStateView(
3030
onLinkClick: (String, Boolean) -> Unit,
3131
modifier: Modifier = Modifier,
3232
) {
33-
// Pick the first identity change to PinViolation
34-
val pinViolationIdentityChange = state.roomMemberIdentityStateChanges.firstOrNull {
35-
// For now only render PinViolation
33+
// Pick the first identity change that is in Pin or Verification violation
34+
val maybeIdentityChangeViolation = state.roomMemberIdentityStateChanges.firstOrNull {
3635
it.identityState == IdentityState.PinViolation ||
3736
it.identityState == IdentityState.VerificationViolation
3837
}
39-
if (pinViolationIdentityChange != null) {
38+
if (maybeIdentityChangeViolation != null) {
4039
ComposerAlertMolecule(
4140
modifier = modifier,
42-
avatar = pinViolationIdentityChange.identityRoomMember.avatarData,
41+
avatar = maybeIdentityChangeViolation.identityRoomMember.avatarData,
4342
content = buildAnnotatedString {
4443
val learnMoreStr = stringResource(CommonStrings.action_learn_more)
45-
val displayName = pinViolationIdentityChange.identityRoomMember.displayNameOrDefault
44+
val displayName = maybeIdentityChangeViolation.identityRoomMember.displayNameOrDefault
4645
val userIdStr = stringResource(
4746
CommonStrings.crypto_identity_change_pin_violation_new_user_id,
48-
pinViolationIdentityChange.identityRoomMember.userId,
47+
maybeIdentityChangeViolation.identityRoomMember.userId,
4948
)
5049

51-
val fullText = if (pinViolationIdentityChange.identityState == IdentityState.PinViolation) {
50+
val fullText = if (maybeIdentityChangeViolation.identityState == IdentityState.PinViolation) {
5251
stringResource(
5352
id = CommonStrings.crypto_identity_change_pin_violation_new,
5453
displayName,
@@ -93,19 +92,19 @@ fun IdentityChangeStateView(
9392
end = learnMoreStartIndex + learnMoreStr.length,
9493
)
9594
},
96-
submitText = if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
95+
submitText = if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
9796
stringResource(CommonStrings.crypto_identity_change_withdraw_verification_action)
9897
} else {
9998
stringResource(CommonStrings.action_ok)
10099
},
101100
onSubmitClick = {
102-
if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
103-
state.eventSink(IdentityChangeEvent.VerificationViolation(pinViolationIdentityChange.identityRoomMember.userId))
101+
if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
102+
state.eventSink(IdentityChangeEvent.WithdrawVerification(maybeIdentityChangeViolation.identityRoomMember.userId))
104103
} else {
105-
state.eventSink(IdentityChangeEvent.PinViolation(pinViolationIdentityChange.identityRoomMember.userId))
104+
state.eventSink(IdentityChangeEvent.PinIdentity(maybeIdentityChangeViolation.identityRoomMember.userId))
106105
}
107106
},
108-
isCritical = pinViolationIdentityChange.identityState == IdentityState.VerificationViolation,
107+
isCritical = maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation,
109108
)
110109
}
111110
}

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ class MessageComposerPresenter @AssistedInject constructor(
403403
}
404404

405405
val roomMemberIdentityStateChange by produceState(persistentListOf()) {
406-
observeRoomMemberIdentityStateChange()
406+
observeRoomMemberIdentityStateChange(room)
407407
}
408408

409409
return MessageComposerState(

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerUtils.kt‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
package io.element.android.features.messages.impl.messagecomposer
9+
10+
import androidx.compose.runtime.ProduceStateScope
11+
import io.element.android.features.messages.impl.crypto.identity.RoomMemberIdentityStateChange
12+
import io.element.android.features.messages.impl.crypto.identity.createDefaultRoomMemberForIdentityChange
13+
import io.element.android.features.messages.impl.crypto.identity.toIdentityRoomMember
14+
import io.element.android.libraries.matrix.api.room.MatrixRoom
15+
import io.element.android.libraries.matrix.api.room.roomMembers
16+
import kotlinx.collections.immutable.PersistentList
17+
import kotlinx.collections.immutable.toPersistentList
18+
import kotlinx.coroutines.ExperimentalCoroutinesApi
19+
import kotlinx.coroutines.flow.combine
20+
import kotlinx.coroutines.flow.distinctUntilChanged
21+
import kotlinx.coroutines.flow.filter
22+
import kotlinx.coroutines.flow.flatMapLatest
23+
import kotlinx.coroutines.flow.launchIn
24+
import kotlinx.coroutines.flow.onEach
25+
26+
@OptIn(ExperimentalCoroutinesApi::class)
27+
fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange(room: MatrixRoom) {
28+
room.syncUpdateFlow
29+
.filter {
30+
// Room cannot become unencrypted, so we can just apply a filter here.
31+
room.isEncrypted
32+
}
33+
.distinctUntilChanged()
34+
.flatMapLatest {
35+
combine(room.identityStateChangesFlow, room.membersStateFlow) { identityStateChanges, membersState ->
36+
identityStateChanges.map { identityStateChange ->
37+
val member = membersState.roomMembers()
38+
?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId }
39+
?.toIdentityRoomMember()
40+
?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId)
41+
RoomMemberIdentityStateChange(
42+
identityRoomMember = member,
43+
identityState = identityStateChange.identityState,
44+
)
45+
}
46+
}
47+
.distinctUntilChanged()
48+
.onEach { roomMemberIdentityStateChanges ->
49+
value = roomMemberIdentityStateChanges.toPersistentList()
50+
}
51+
}
52+
.launchIn(this)
53+
}

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ class IdentityChangeStatePresenterTest {
143143
val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
144144
presenter.test {
145145
val initialState = awaitItem()
146-
initialState.eventSink(IdentityChangeEvent.PinViolation(A_USER_ID))
146+
initialState.eventSink(IdentityChangeEvent.PinIdentity(A_USER_ID))
147147
lambda.assertions().isCalledOnce().with(value(A_USER_ID))
148148
}
149149
}
@@ -158,7 +158,7 @@ class IdentityChangeStatePresenterTest {
158158
val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
159159
presenter.test {
160160
val initialState = awaitItem()
161-
initialState.eventSink(IdentityChangeEvent.VerificationViolation(A_USER_ID))
161+
initialState.eventSink(IdentityChangeEvent.WithdrawVerification(A_USER_ID))
162162
lambda.assertions().isCalledOnce().with(value(A_USER_ID))
163163
}
164164
}

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateViewTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class IdentityChangeStateViewTest {
4747
rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
4848

4949
rule.clickOn(res = CommonStrings.action_ok)
50-
eventsRecorder.assertSingle(IdentityChangeEvent.PinViolation(UserId("@alice:localhost")))
50+
eventsRecorder.assertSingle(IdentityChangeEvent.PinIdentity(UserId("@alice:localhost")))
5151
}
5252

5353
@Test
@@ -70,7 +70,7 @@ class IdentityChangeStateViewTest {
7070
rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
7171

7272
rule.clickOn(res = CommonStrings.crypto_identity_change_withdraw_verification_action)
73-
eventsRecorder.assertSingle(IdentityChangeEvent.VerificationViolation(UserId("@alice:localhost")))
73+
eventsRecorder.assertSingle(IdentityChangeEvent.WithdrawVerification(UserId("@alice:localhost")))
7474
}
7575

7676
@Test

‎libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/encryption/EncryptionService.kt‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,15 @@ interface EncryptionService {
6666
* Remember this identity, ensuring it does not result in a pin violation.
6767
*/
6868
suspend fun pinUserIdentity(userId: UserId): Result<Unit>
69-
suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit>
69+
70+
/**
71+
* Withdraw the verification for that user (also pin the identity).
72+
*
73+
* Useful when a user that was verified is not anymore, but it is not
74+
* possible to re-verify immediately. This allows to restore communication by reverting the
75+
* user trust from verified to TOFU verified.
76+
*/
77+
suspend fun withdrawVerification(userId: UserId): Result<Unit>
7078
}
7179

7280
/**

‎libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ internal class RustEncryptionService(
209209
getUserIdentity(userId).pin()
210210
}
211211

212-
override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> = runCatching {
212+
override suspend fun withdrawVerification(userId: UserId): Result<Unit> = runCatching {
213213
getUserIdentity(userId).withdrawVerification()
214214
}
215215

‎libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/encryption/FakeEncryptionService.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ class FakeEncryptionService(
125125
return pinUserIdentityResult(userId)
126126
}
127127

128-
override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> {
128+
override suspend fun withdrawVerification(userId: UserId): Result<Unit> {
129129
return withdrawVerificationResult(userId)
130130
}
131131

0 commit comments

Comments
 (0)