Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2026-07-21 04:54:41 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix race condition with event stream in regV5.

Browse Source
This commit is contained in:
Greyson Parrelli
2026-07-09 10:52:33 -04:00
parent 31216592de
commit 908bb8dbe6
3 changed files with 115 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryScreenEvents.kt
+7
View File
@@ -6,9 +6,16 @@
package org.signal.registration.screens.phonenumber
import org.signal.core.util.censor
import org.signal.registration.RegistrationFlowState
import org.signal.registration.screens.localbackuprestore.LocalBackupRestoreResult
sealed class PhoneNumberEntryScreenEvents {
/** Emitted once when the screen is created to load initial data into the state. */
data object Initialize : PhoneNumberEntryScreenEvents()
/** The parent registration flow state changed and needs to be merged into this screen's state. */
data class ParentStateChanged(val parentState: RegistrationFlowState) : PhoneNumberEntryScreenEvents()
/** The phone country code prefix (i.e. +1) was changed by the user. */
data class CountryCodeChanged(val value: String) : PhoneNumberEntryScreenEvents()
feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt
+48 -45
View File
@@ -13,14 +13,11 @@ import com.google.i18n.phonenumbers.AsYouTypeFormatter
import com.google.i18n.phonenumbers.NumberParseException
import com.google.i18n.phonenumbers.PhoneNumberUtil
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeoutOrNull
import org.signal.core.models.AccountEntropyPool
import org.signal.core.util.E164Util
@@ -39,27 +36,6 @@ import org.signal.registration.screens.localbackuprestore.LocalBackupRestoreResu
import org.signal.registration.screens.phonenumber.PhoneNumberEntryState.OneTimeEvent
import org.signal.registration.screens.util.navigateTo
/**
* Returns how many characters a single edit inserted when going from [old] to [new], by diffing off the common prefix
* and suffix. Used to distinguish a typed character (1) from a paste or autofill (more than 1). Returns 0 for
* deletions or replacements that don't grow the differing region.
*/
private fun insertedCharCount(old: String, new: String): Int {
val max = minOf(old.length, new.length)
var prefix = 0
while (prefix < max && old[prefix] == new[prefix]) {
prefix++
}
var suffix = 0
while (suffix < max - prefix && old[old.length - 1 - suffix] == new[new.length - 1 - suffix]) {
suffix++
}
return (new.length - prefix - suffix).coerceAtLeast(0)
}
class PhoneNumberEntryViewModel(
val repository: RegistrationRepository,
private val parentState: StateFlow<RegistrationFlowState>,
@@ -75,26 +51,20 @@ class PhoneNumberEntryViewModel(
private var formatter: AsYouTypeFormatter = phoneNumberUtil.getAsYouTypeFormatter("US")
private val _state = MutableStateFlow(PhoneNumberEntryState())
val state = _state
.combine(parentState) { state, parentState -> applyParentState(state, parentState) }
.onEach { Log.d(TAG, "[State] $it") }
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), PhoneNumberEntryState())
val state: StateFlow<PhoneNumberEntryState> = _state.asStateFlow()
init {
viewModelScope.launch {
_state.value = state.value.copy(
restoredSvrCredentials = repository.getRestoredSvrCredentials()
)
setDefaultCountry()
setDefaultCountry()
parentState.firstOrNull()?.preExistingRegistrationData?.e164?.let { preExistingE164 ->
if (state.value.formattedNumber.isEmpty()) {
_state.value = applyFullPhoneNumberEntered(_state.value, preExistingE164)
}
}
_state
.onEach { Log.d(TAG, "[State] $it") }
.launchIn(viewModelScope)
_state.update { it.copy(initialized = true) }
}
onEvent(PhoneNumberEntryScreenEvents.Initialize)
parentState
.onEach { onEvent(PhoneNumberEntryScreenEvents.ParentStateChanged(it)) }
.launchIn(viewModelScope)
}
fun setDefaultCountry() {
@@ -111,7 +81,7 @@ class PhoneNumberEntryViewModel(
}
override suspend fun processEvent(event: PhoneNumberEntryScreenEvents) {
applyEvent(state.value, event, parentEventEmitter) {
applyEvent(_state.value, event, parentEventEmitter) {
_state.value = it
}
}
@@ -119,6 +89,12 @@ class PhoneNumberEntryViewModel(
@VisibleForTesting
suspend fun applyEvent(state: PhoneNumberEntryState, event: PhoneNumberEntryScreenEvents, parentEventEmitter: (RegistrationFlowEvent) -> Unit, stateEmitter: (PhoneNumberEntryState) -> Unit) {
when (event) {
is PhoneNumberEntryScreenEvents.Initialize -> {
stateEmitter(applyInitialize(state))
}
is PhoneNumberEntryScreenEvents.ParentStateChanged -> {
stateEmitter(applyParentState(state, event.parentState))
}
is PhoneNumberEntryScreenEvents.CountryCodeChanged -> {
stateEmitter(applyCountryCodeChanged(state, event.value))
}
@@ -178,8 +154,19 @@ class PhoneNumberEntryViewModel(
}
}
@VisibleForTesting
fun applyParentState(state: PhoneNumberEntryState, parentState: RegistrationFlowState): PhoneNumberEntryState {
private suspend fun applyInitialize(inputState: PhoneNumberEntryState): PhoneNumberEntryState {
var state = inputState.copy(restoredSvrCredentials = repository.getRestoredSvrCredentials())
parentState.value.preExistingRegistrationData?.e164?.let { preExistingE164 ->
if (state.formattedNumber.isEmpty()) {
state = applyFullPhoneNumberEntered(state, preExistingE164)
}
}
return state.copy(initialized = true)
}
private fun applyParentState(state: PhoneNumberEntryState, parentState: RegistrationFlowState): PhoneNumberEntryState {
return state.copy(
sessionE164 = parentState.sessionE164,
sessionMetadata = parentState.sessionMetadata,
@@ -791,6 +778,22 @@ class PhoneNumberEntryViewModel(
}
}
private fun insertedCharCount(old: String, new: String): Int {
val max = minOf(old.length, new.length)
var prefix = 0
while (prefix < max && old[prefix] == new[prefix]) {
prefix++
}
var suffix = 0
while (suffix < max - prefix && old[old.length - 1 - suffix] == new[new.length - 1 - suffix]) {
suffix++
}
return (new.length - prefix - suffix).coerceAtLeast(0)
}
/**
* Recomputes [PhoneNumberEntryState.isNumberPossible] and [PhoneNumberEntryState.isNumberInvalid] from the current
* country code and national number. Should be applied to any state that changes either of those fields.
feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt
+60 -11
View File
@@ -560,6 +560,55 @@ class PhoneNumberEntryViewModelTest {
assertThat(emittedEvents).isEmpty()
}
// ==================== Initialize Tests ====================
@Test
fun `state is populated with the default country and initialized once construction settles`() {
// The view model was constructed and its event loop advanced to idle in setup().
val state = viewModel.state.value
assertThat(state.regionCode).isEqualTo("US")
assertThat(state.countryCode).isEqualTo("1")
assertThat(state.countryName).isEqualTo("United States")
assertThat(state.initialized).isTrue()
}
@Test
fun `Initialize loads restored SVR credentials into state and marks it initialized`() = runTest {
val credentials = listOf(NetworkController.SvrCredentials(username = "user", password = "pass"))
coEvery { mockRepository.getRestoredSvrCredentials() } returns credentials
viewModel.applyEvent(PhoneNumberEntryState(), PhoneNumberEntryScreenEvents.Initialize, parentEventEmitter, stateEmitter)
assertThat(emittedStates).hasSize(1)
assertThat(emittedStates.last().restoredSvrCredentials).isEqualTo(credentials)
assertThat(emittedStates.last().initialized).isTrue()
}
@Test
fun `Initialize does not prefill when the number field is already populated`() = runTest {
val preExisting = mockk<PreExistingRegistrationData>(relaxed = true)
every { preExisting.e164 } returns "+15551234567"
parentState.value = RegistrationFlowState(preExistingRegistrationData = preExisting)
val alreadyPopulated = PhoneNumberEntryState(countryCode = "44", regionCode = "GB", nationalNumber = "2079460958", formattedNumber = "2079460958")
viewModel.applyEvent(alreadyPopulated, PhoneNumberEntryScreenEvents.Initialize, parentEventEmitter, stateEmitter)
val result = emittedStates.last()
assertThat(result.nationalNumber).isEqualTo("2079460958")
assertThat(result.countryCode).isEqualTo("44")
}
// ==================== Parent State Tests ====================
@Test
fun `parent state changes are merged into state through the event stream`() = runTest {
parentState.value = RegistrationFlowState(sessionE164 = "+15551234567")
testDispatcher.scheduler.advanceUntilIdle()
assertThat(viewModel.state.value.sessionE164).isEqualTo("+15551234567")
assertThat(viewModel.state.value.regionCode).isEqualTo("US")
}
// ==================== Pre-existing Registration Data Prefill Tests ====================
@Test
@@ -1172,43 +1221,43 @@ class PhoneNumberEntryViewModelTest {
assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PhoneNumberEntryState.OneTimeEvent.NetworkError)
}
// ==================== applyParentState Tests ====================
// ==================== ParentStateChanged Tests ====================
@Test
fun `applyParentState copies preExistingRegistrationData from parent`() {
fun `ParentStateChanged copies preExistingRegistrationData from parent`() = runTest {
val preExistingData = mockk<PreExistingRegistrationData>(relaxed = true)
val state = PhoneNumberEntryState()
val parentFlowState = RegistrationFlowState(preExistingRegistrationData = preExistingData)
val result = viewModel.applyParentState(state, parentFlowState)
viewModel.applyEvent(PhoneNumberEntryState(), PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter)
assertThat(result.preExistingRegistrationData).isEqualTo(preExistingData)
assertThat(emittedStates).hasSize(1)
assertThat(emittedStates.last().preExistingRegistrationData).isEqualTo(preExistingData)
}
@Test
fun `applyParentState clears restoredSvrCredentials when doNotAttemptRecoveryPassword is true`() {
fun `ParentStateChanged clears restoredSvrCredentials when doNotAttemptRecoveryPassword is true`() = runTest {
val credentials = listOf(
NetworkController.SvrCredentials(username = "user", password = "pass")
)
val state = PhoneNumberEntryState(restoredSvrCredentials = credentials)
val parentFlowState = RegistrationFlowState(doNotAttemptRecoveryPassword = true)
val result = viewModel.applyParentState(state, parentFlowState)
viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter)
assertThat(result.restoredSvrCredentials).isEmpty()
assertThat(emittedStates.last().restoredSvrCredentials).isEmpty()
}
@Test
fun `applyParentState keeps restoredSvrCredentials when doNotAttemptRecoveryPassword is false`() {
fun `ParentStateChanged keeps restoredSvrCredentials when doNotAttemptRecoveryPassword is false`() = runTest {
val credentials = listOf(
NetworkController.SvrCredentials(username = "user", password = "pass")
)
val state = PhoneNumberEntryState(restoredSvrCredentials = credentials)
val parentFlowState = RegistrationFlowState(doNotAttemptRecoveryPassword = false)
val result = viewModel.applyParentState(state, parentFlowState)
viewModel.applyEvent(state, PhoneNumberEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter)
assertThat(result.restoredSvrCredentials).isEqualTo(credentials)
assertThat(emittedStates.last().restoredSvrCredentials).isEqualTo(credentials)
}
// ==================== Pre-existing Registration Data (RRP) Tests ====================