Sulkta-OSS/element-x-ada
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

code review: renaming, comments, extract common code

Browse source
This commit is contained in:
Valere 2025-01-24 12:00:16 +01:00
parent 31ab66985e
commit 3f1543eb51
10 changed files with 90 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

4
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeEvent.kt
View file

@ -10,6 +10,6 @@ package io.element.android.features.messages.impl.crypto.identity
import io.element.android.libraries.matrix.api.core.UserId
sealed interface IdentityChangeEvent {
data class PinViolation(val userId: UserId) : IdentityChangeEvent
data class VerificationViolation(val userId: UserId) : IdentityChangeEvent
data class PinIdentity(val userId: UserId) : IdentityChangeEvent
data class WithdrawVerification(val userId: UserId) : IdentityChangeEvent
}

53
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt
View file

@ -8,10 +8,10 @@
package io.element.android.features.messages.impl.crypto.identity
import androidx.compose.runtime.Composable
import androidx.compose.runtime.ProduceStateScope
import androidx.compose.runtime.getValue
import androidx.compose.runtime.produceState
import androidx.compose.runtime.rememberCoroutineScope
import io.element.android.features.messages.impl.messagecomposer.observeRoomMemberIdentityStateChange
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
@ -19,19 +19,9 @@ import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.roomMembers
import io.element.android.libraries.matrix.ui.model.getAvatarData
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.onEach
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
@ -44,15 +34,15 @@ class IdentityChangeStatePresenter @Inject constructor(
override fun present(): IdentityChangeState {
val coroutineScope = rememberCoroutineScope()
val roomMemberIdentityStateChange by produceState(persistentListOf()) {
observeRoomMemberIdentityStateChange()
observeRoomMemberIdentityStateChange(room)
}
fun handleEvent(event: IdentityChangeEvent) {
when (event) {
is IdentityChangeEvent.VerificationViolation -> {
coroutineScope.withdrawVerificationRequirement(event.userId)
is IdentityChangeEvent.WithdrawVerification -> {
coroutineScope.withdrawVerification(event.userId)
}
is IdentityChangeEvent.PinViolation -> {
is IdentityChangeEvent.PinIdentity -> {
coroutineScope.pinUserIdentity(event.userId)
}
}
@ -64,35 +54,6 @@ class IdentityChangeStatePresenter @Inject constructor(
)
}
@OptIn(ExperimentalCoroutinesApi::class)
private fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange() {
room.syncUpdateFlow
.filter {
// Room cannot become unencrypted, so we can just apply a filter here.
room.isEncrypted
}
.distinctUntilChanged()
.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)
}
private fun CoroutineScope.pinUserIdentity(userId: UserId) = launch {
encryptionService.pinUserIdentity(userId)
.onFailure {
@ -100,8 +61,8 @@ class IdentityChangeStatePresenter @Inject constructor(
}
}
private fun CoroutineScope.withdrawVerificationRequirement(userId: UserId) = launch {
encryptionService.withdrawVerificationRequirement(userId)
private fun CoroutineScope.withdrawVerification(userId: UserId) = launch {
encryptionService.withdrawVerification(userId)
.onFailure {
Timber.e(it, "Failed to withdraw verification for user $userId")
}

25
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt
View file

@ -30,25 +30,24 @@ fun IdentityChangeStateView(
onLinkClick: (String, Boolean) -> Unit,
modifier: Modifier = Modifier,
) {
// Pick the first identity change to PinViolation
val pinViolationIdentityChange = state.roomMemberIdentityStateChanges.firstOrNull {
// For now only render PinViolation
// Pick the first identity change that is in Pin or Verification violation
val maybeIdentityChangeViolation = state.roomMemberIdentityStateChanges.firstOrNull {
it.identityState == IdentityState.PinViolation ||
it.identityState == IdentityState.VerificationViolation
}
if (pinViolationIdentityChange != null) {
if (maybeIdentityChangeViolation != null) {
ComposerAlertMolecule(
modifier = modifier,
avatar = pinViolationIdentityChange.identityRoomMember.avatarData,
avatar = maybeIdentityChangeViolation.identityRoomMember.avatarData,
content = buildAnnotatedString {
val learnMoreStr = stringResource(CommonStrings.action_learn_more)
val displayName = pinViolationIdentityChange.identityRoomMember.displayNameOrDefault
val displayName = maybeIdentityChangeViolation.identityRoomMember.displayNameOrDefault
val userIdStr = stringResource(
CommonStrings.crypto_identity_change_pin_violation_new_user_id,
pinViolationIdentityChange.identityRoomMember.userId,
maybeIdentityChangeViolation.identityRoomMember.userId,
)
val fullText = if (pinViolationIdentityChange.identityState == IdentityState.PinViolation) {
val fullText = if (maybeIdentityChangeViolation.identityState == IdentityState.PinViolation) {
stringResource(
id = CommonStrings.crypto_identity_change_pin_violation_new,
displayName,
@ -93,19 +92,19 @@ fun IdentityChangeStateView(
end = learnMoreStartIndex + learnMoreStr.length,
)
},
submitText = if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
submitText = if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
stringResource(CommonStrings.crypto_identity_change_withdraw_verification_action)
} else {
stringResource(CommonStrings.action_ok)
},
onSubmitClick = {
if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
state.eventSink(IdentityChangeEvent.VerificationViolation(pinViolationIdentityChange.identityRoomMember.userId))
if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
state.eventSink(IdentityChangeEvent.WithdrawVerification(maybeIdentityChangeViolation.identityRoomMember.userId))
} else {
state.eventSink(IdentityChangeEvent.PinViolation(pinViolationIdentityChange.identityRoomMember.userId))
state.eventSink(IdentityChangeEvent.PinIdentity(maybeIdentityChangeViolation.identityRoomMember.userId))
}
},
isCritical = pinViolationIdentityChange.identityState == IdentityState.VerificationViolation,
isCritical = maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation,
)
}
}

2
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt
View file

@ -403,7 +403,7 @@ class MessageComposerPresenter @AssistedInject constructor(
}
val roomMemberIdentityStateChange by produceState(persistentListOf()) {
observeRoomMemberIdentityStateChange()
observeRoomMemberIdentityStateChange(room)
}
return MessageComposerState(

53
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerUtils.kt Normal file
View file

@ -0,0 +1,53 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.messages.impl.messagecomposer
import androidx.compose.runtime.ProduceStateScope
import io.element.android.features.messages.impl.crypto.identity.RoomMemberIdentityStateChange
import io.element.android.features.messages.impl.crypto.identity.createDefaultRoomMemberForIdentityChange
import io.element.android.features.messages.impl.crypto.identity.toIdentityRoomMember
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.roomMembers
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
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.onEach
@OptIn(ExperimentalCoroutinesApi::class)
fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange(room: MatrixRoom) {
room.syncUpdateFlow
.filter {
// Room cannot become unencrypted, so we can just apply a filter here.
room.isEncrypted
}
.distinctUntilChanged()
.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)
}

4
features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt
View file

@ -143,7 +143,7 @@ class IdentityChangeStatePresenterTest {
val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
presenter.test {
val initialState = awaitItem()
initialState.eventSink(IdentityChangeEvent.PinViolation(A_USER_ID))
initialState.eventSink(IdentityChangeEvent.PinIdentity(A_USER_ID))
lambda.assertions().isCalledOnce().with(value(A_USER_ID))
}
}
@ -158,7 +158,7 @@ class IdentityChangeStatePresenterTest {
val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
presenter.test {
val initialState = awaitItem()
initialState.eventSink(IdentityChangeEvent.VerificationViolation(A_USER_ID))
initialState.eventSink(IdentityChangeEvent.WithdrawVerification(A_USER_ID))
lambda.assertions().isCalledOnce().with(value(A_USER_ID))
}
}

4
features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateViewTest.kt
View file

@ -47,7 +47,7 @@ class IdentityChangeStateViewTest {
rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
rule.clickOn(res = CommonStrings.action_ok)
eventsRecorder.assertSingle(IdentityChangeEvent.PinViolation(UserId("@alice:localhost")))
eventsRecorder.assertSingle(IdentityChangeEvent.PinIdentity(UserId("@alice:localhost")))
}
@Test
@ -70,7 +70,7 @@ class IdentityChangeStateViewTest {
rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
rule.clickOn(res = CommonStrings.crypto_identity_change_withdraw_verification_action)
eventsRecorder.assertSingle(IdentityChangeEvent.VerificationViolation(UserId("@alice:localhost")))
eventsRecorder.assertSingle(IdentityChangeEvent.WithdrawVerification(UserId("@alice:localhost")))
}
@Test

10
libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/encryption/EncryptionService.kt
View file

@ -66,7 +66,15 @@ interface EncryptionService {
* Remember this identity, ensuring it does not result in a pin violation.
*/
suspend fun pinUserIdentity(userId: UserId): Result<Unit>
suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit>
/**
* Withdraw the verification for that user (also pin the identity).
*
* Useful when a user that was verified is not anymore, but it is not
* possible to re-verify immediately. This allows to restore communication by reverting the
* user trust from verified to TOFU verified.
*/
suspend fun withdrawVerification(userId: UserId): Result<Unit>
}
/**

2
libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt
View file

@ -209,7 +209,7 @@ internal class RustEncryptionService(
getUserIdentity(userId).pin()
}
override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> = runCatching {
override suspend fun withdrawVerification(userId: UserId): Result<Unit> = runCatching {
getUserIdentity(userId).withdrawVerification()
}

2
libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/encryption/FakeEncryptionService.kt
View file

@ -125,7 +125,7 @@ class FakeEncryptionService(
return pinUserIdentityResult(userId)
}
override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> {
override suspend fun withdrawVerification(userId: UserId): Result<Unit> {
return withdrawVerificationResult(userId)
}