diff --git a/core/util/src/main/kotlin/de/davis/keygo/core/util/Combine.kt b/core/util/src/main/kotlin/de/davis/keygo/core/util/Combine.kt new file mode 100644 index 000000000..77bf76151 --- /dev/null +++ b/core/util/src/main/kotlin/de/davis/keygo/core/util/Combine.kt @@ -0,0 +1,46 @@ +package de.davis.keygo.core.util + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.combine as combineArray + +fun combine( + flow1: Flow, + flow2: Flow, + flow3: Flow, + flow4: Flow, + flow5: Flow, + flow6: Flow, + transform: (T1, T2, T3, T4, T5, T6) -> R, +): Flow = combineArray(flow1, flow2, flow3, flow4, flow5, flow6) { values -> + @Suppress("UNCHECKED_CAST") + transform( + values[0] as T1, + values[1] as T2, + values[2] as T3, + values[3] as T4, + values[4] as T5, + values[5] as T6, + ) +} + +fun combine( + flow1: Flow, + flow2: Flow, + flow3: Flow, + flow4: Flow, + flow5: Flow, + flow6: Flow, + flow7: Flow, + transform: (T1, T2, T3, T4, T5, T6, T7) -> R, +): Flow = combineArray(flow1, flow2, flow3, flow4, flow5, flow6, flow7) { values -> + @Suppress("UNCHECKED_CAST") + transform( + values[0] as T1, + values[1] as T2, + values[2] as T3, + values[3] as T4, + values[4] as T5, + values[5] as T6, + values[6] as T7, + ) +} diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardViewModel.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardViewModel.kt index a78f57c4b..8965f06f7 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardViewModel.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardViewModel.kt @@ -5,6 +5,8 @@ import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator +import de.davis.keygo.core.item.domain.model.PasswordScore +import de.davis.keygo.core.util.combine import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.onFailure @@ -33,13 +35,10 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.mapLatest -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn @@ -62,71 +61,66 @@ internal class ExportWizardViewModel( private val _formatState = MutableStateFlow(SelectFormatState()) private val _scheduleState = MutableStateFlow(SelectScheduleState()) private val _destinationState = MutableStateFlow(SelectDestinationState()) - private val _providePassphraseState = MutableStateFlow( + private val _step = MutableStateFlow(ExportWizardStep.SelectFormat) + + private val _providePassphraseBaseState = MutableStateFlow( ProvidePassphraseState( passphraseTextFieldState = passphraseTextFieldState, confirmPassphraseTextFieldState = confirmPassphraseTextFieldState, - ) + ), ) - private val _step = MutableStateFlow(ExportWizardStep.SelectFormat) + @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) + private val _passphraseMetricsFlow = snapshotFlow { + val passphrase = passphraseTextFieldState.text + val valid = + passphrase.isNotEmpty() && passphrase.contentEquals(confirmPassphraseTextFieldState.text) + passphrase to valid + } + .debounce(150.milliseconds) + .distinctUntilChanged() + .mapLatest { (pwd, valid) -> + val score = passwordStrengthEstimator(pwd.toString()) + score to valid + } + .flowOn(Dispatchers.Default) + // combine withholds its first emission until every input has emitted, so without a value + // up front the whole wizard would sit on its initialValue until the debounce elapses. + .onStart { emit(PasswordScore.None to false) } val state = combine( _formatState, _scheduleState, _destinationState, - _providePassphraseState, - _step - ) { formatState, scheduleState, destinationState, providePassphraseState, step -> + _providePassphraseBaseState, + _passphraseMetricsFlow, + _step, + ) { formatState, scheduleState, destinationState, basePassphraseState, (score, valid), step -> ExportWizardUiState( formatState = formatState, scheduleState = scheduleState, destinationState = destinationState, - providePassphraseState = providePassphraseState, + providePassphraseState = basePassphraseState.copy( + passphraseScore = score, + valid = valid, + ), step = step, ) - } - .onStart { - observePassphrase() - } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = ExportWizardUiState( - formatState = _formatState.value, - scheduleState = _scheduleState.value, - destinationState = _destinationState.value, - providePassphraseState = _providePassphraseState.value, - step = _step.value, - ) - ) + }.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = ExportWizardUiState( + formatState = _formatState.value, + scheduleState = _scheduleState.value, + destinationState = _destinationState.value, + providePassphraseState = _providePassphraseBaseState.value, + step = _step.value, + ), + ) private val _event = Channel(Channel.BUFFERED) val event = _event.receiveAsFlow() - @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) - private fun observePassphrase() { - snapshotFlow { - val passphrase = passphraseTextFieldState.text - val valid = - passphrase.isNotEmpty() && passphrase.contentEquals(confirmPassphraseTextFieldState.text) - passphrase to valid - } - .debounce(150.milliseconds) - .distinctUntilChanged() - .mapLatest { (pwd, valid) -> passwordStrengthEstimator(pwd.toString()) to valid } - .onEach { (score, valid) -> - _providePassphraseState.update { - it.copy( - passphraseScore = score, - valid = valid - ) - } - } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - fun onEvent(event: ExportWizardUiEvent) { when (event) { ExportWizardUiEvent.Back -> previousStep() @@ -173,7 +167,7 @@ internal class ExportWizardViewModel( it.copy(keepAll = event.keepAll) } - is ExportWizardUiEvent.EncryptionMethodSelected -> _providePassphraseState.update { + is ExportWizardUiEvent.EncryptionMethodSelected -> _providePassphraseBaseState.update { it.copy(method = event.method) } @@ -231,7 +225,7 @@ internal class ExportWizardViewModel( interval = if (recurring) schedule.interval else null, keepCount = if (recurring && !schedule.keepAll) schedule.keepCount else null, passphrase = passphraseTextFieldState.text.toString(), - encryption = if (format.encrypted) _providePassphraseState.value.method else null, + encryption = if (format.encrypted) _providePassphraseBaseState.value.method else null, csvPreset = if (format == FileFormat.CSV) _formatState.value.csvPreset else null, ) } @@ -245,4 +239,4 @@ internal class ExportWizardViewModel( val steps = exportStepsFor(_formatState.value.format) steps[(steps.indexOf(current) - 1).coerceAtLeast(0)] } -} \ No newline at end of file +} diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModel.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModel.kt index 60b2b0469..0675b8377 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModel.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModel.kt @@ -31,10 +31,10 @@ import de.davisalessandro.keygo.rust.ColumnMapping import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.koin.core.annotation.KoinViewModel @@ -44,20 +44,9 @@ internal class ImportWizardViewModel( private val backupDestinationResolver: BackupDestinationResolver, private val importBackup: ImportBackupUseCase, private val analyzeCsv: AnalyzeCsvUseCase, - private val observeVaultsAndSelection: ObserveVaultsAndSelectionUseCase, + observeVaultsAndSelection: ObserveVaultsAndSelectionUseCase, ) : ViewModel() { - private val passphraseState = TextFieldState() - private val newVaultNameState = TextFieldState() - - private val _state = MutableStateFlow( - ImportWizardUiState( - passphraseState = passphraseState, - newVaultNameState = newVaultNameState, - ), - ) - val state = _state.asStateFlow() - private val _event = Channel(Channel.BUFFERED) val event = _event.receiveAsFlow() @@ -67,23 +56,36 @@ internal class ImportWizardViewModel( private var vaultStepSeeded = false private var seededUri: BackupDestinationUri? = null - init { - snapshotFlow { passphraseState.text.toString() } - .onEach { text -> _state.update { it.copy(passphraseValid = text.isNotBlank()) } } - .launchIn(viewModelScope) - snapshotFlow { newVaultNameState.text.toString() } - .onEach { text -> _state.update { it.copy(newVaultNameValid = text.isNotBlank()) } } - .launchIn(viewModelScope) + private val passphraseState = TextFieldState() + private val newVaultNameState = TextFieldState() + + private val _state = MutableStateFlow( + ImportWizardUiState( + passphraseState = passphraseState, + newVaultNameState = newVaultNameState, + ), + ) + val state = combine( + _state, + snapshotFlow { passphraseState.text.toString() }, + snapshotFlow { newVaultNameState.text.toString() }, observeVaultsAndSelection() - .onEach { (vaults, selection) -> - _state.update { - it.copy(vaults = vaults, contextVaultId = selection.getIdOrNull()) - } - } - .launchIn(viewModelScope) - } + ) { baseState, passphrase, vaultName, vaultData -> + val (vaults, selection) = vaultData + + baseState.copy( + passphraseValid = passphrase.isNotBlank(), + newVaultNameValid = vaultName.isNotBlank(), + vaults = vaults, + contextVaultId = selection.getIdOrNull() + ) + }.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = _state.value + ) fun onEvent(event: ImportWizardUiEvent) { when (event) { @@ -182,7 +184,11 @@ internal class ImportWizardViewModel( analysisJob = viewModelScope.launch { analyzeCsv(uri).fold( onSuccess = ::onAnalyzed, - onFailure = { error -> _state.update { it.copy(progress = ImportProgress.Failed(error)) } }, + onFailure = { error -> + _state.update { + it.copy(progress = ImportProgress.Failed(error)) + } + }, ) } } @@ -220,7 +226,10 @@ internal class ImportWizardViewModel( } vaultStepSeeded = true - val current = _state.value + // state, not _state: the vault list and the vault context are folded in by the flow above + // and never written back, so _state has neither. This runs from a Continue tap, so the + // screen is collecting and the values are present. + val current = state.value val contextVault = current.contextVaultId ?.takeIf { id -> current.vaults.any { it.vaultId == id } } @@ -242,7 +251,8 @@ internal class ImportWizardViewModel( startImport( passphrase = null, - csvMapping = current.columns.associate { it.index to it.selectedType }.toColumnMapping(), + csvMapping = current.columns.associate { it.index to it.selectedType } + .toColumnMapping(), target = target, ) } diff --git a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModelTest.kt b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModelTest.kt index bd7f559dc..a897501b2 100644 --- a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModelTest.kt +++ b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportWizardViewModelTest.kt @@ -40,7 +40,9 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runCurrent @@ -89,7 +91,11 @@ class ImportWizardViewModelTest { passkeys = emptyList(), ) - private fun viewModel( + /** + * `state` is `WhileSubscribed`, so it only tracks `_state` while something collects it. Tests + * that read `viewModel.state.value` right after an event need that subscription to exist. + */ + private fun TestScope.viewModel( resolver: FakeBackupDestinationResolver = FakeBackupDestinationResolver(), session: FakeSession = FakeSession(startOnConstruct = true), contextRepo: FakeVaultContextRepository = FakeVaultContextRepository(), @@ -98,7 +104,7 @@ class ImportWizardViewModelTest { ImportBackupUseCase(fileStore, json, csv, env.restorer, session), AnalyzeCsvUseCase(fileStore, csv), ObserveVaultsAndSelectionUseCase(env.vaultRepo, contextRepo, SortUseCase()), - ) + ).also { it.state.launchIn(backgroundScope) } private fun ImportWizardViewModel.selectJson() { onFilePicked(BackupDestinationUri("content://doc/keygo.json")) @@ -162,6 +168,7 @@ class ImportWizardViewModelTest { val viewModel = viewModel() viewModel.onFilePicked(null) + advanceUntilIdle() assertNull(viewModel.state.value.backupDestination) assertNull(viewModel.state.value.uri) @@ -262,6 +269,7 @@ class ImportWizardViewModelTest { advanceUntilIdle() viewModel.onEvent(ImportWizardUiEvent.Continue) + advanceUntilIdle() assertEquals( ImportProgress.Failed(ImportError.UnsupportedFormat), @@ -280,6 +288,7 @@ class ImportWizardViewModelTest { viewModel.state.first { it.step == ImportWizardStep.MapColumns } viewModel.onEvent(ImportWizardUiEvent.ChangeColumnType(1, CsvColumnType.Username)) + advanceUntilIdle() assertEquals(CsvColumnType.Username, viewModel.state.value.columns[1].selectedType) } @@ -570,6 +579,7 @@ class ImportWizardViewModelTest { viewModel.state.first { it.step == ImportWizardStep.MapColumns } viewModel.onEvent(ImportWizardUiEvent.Back) + advanceUntilIdle() assertEquals(ImportWizardStep.SelectFile, viewModel.state.value.step) } @@ -585,6 +595,7 @@ class ImportWizardViewModelTest { viewModel.state.first { it.step == ImportWizardStep.ProvidePassphrase } viewModel.onEvent(ImportWizardUiEvent.Back) + advanceUntilIdle() assertEquals(ImportWizardStep.SelectFile, viewModel.state.value.step) } @@ -660,6 +671,9 @@ class ImportWizardViewModelTest { viewModel.onEvent(ImportWizardUiEvent.ChangeColumnType(1, CsvColumnType.Username)) viewModel.onEvent(ImportWizardUiEvent.Back) viewModel.event.first() + // Let state catch up to the reset exit() made. It still reports MapColumns until it does, + // and the await below would match that stale value instead of the second seed's. + advanceUntilIdle() viewModel.seedFile(uri) val state = viewModel.state.first { it.step == ImportWizardStep.MapColumns } @@ -690,6 +704,7 @@ class ImportWizardViewModelTest { viewModel.onEvent(ImportWizardUiEvent.Back) viewModel.event.first() + advanceUntilIdle() // This ViewModel is scoped to the host's back stack entry, so it outlives the visit. The // gap between handing control back and the host seeding a new file is exactly what a second @@ -714,6 +729,7 @@ class ImportWizardViewModelTest { viewModel.onEvent(ImportWizardUiEvent.Back) viewModel.event.first() + advanceUntilIdle() // Same gap as above, but from a mapping rather than an error: the previous file's // column names are the tell if exit() left them behind. diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/ItemViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/ItemViewModel.kt index 613ab8d57..4f98c7e1e 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/ItemViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/ItemViewModel.kt @@ -27,8 +27,6 @@ import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn @@ -60,7 +58,22 @@ internal abstract class ItemViewModel( protected val selectedVaultId = MutableStateFlow(null) private val assignedTags = MutableStateFlow>(emptySet()) - private val nameExists = MutableStateFlow(false) + + @OptIn(FlowPreview::class) + private val nameExists = snapshotFlow { nameTextFieldState.text } + .debounce(150.milliseconds) + .combine(selectedVaultId.filterNotNull()) { input, vaultId -> + itemRepository.doesNameExist( + input.toString(), + excludeId = itemId, + vaultId = vaultId, + ) + } + .distinctUntilChanged() + .flowOn(Dispatchers.Default) + // A name the user has not typed yet cannot collide. Without a value up front, combine would + // hold the screen on ItemUiState.Loading for the debounce plus a doesNameExist round trip. + .onStart { emit(false) } private val vaults = combine( vaultRepository.observeAllVaultMetadata(), @@ -97,8 +110,6 @@ internal abstract class ItemViewModel( combine(itemState, shared) { item, shared -> ItemUiState.Ready(item, shared) } .onStart { primeActiveVaultId() - observeNameTextField() - onSubscribed() } .stateIn( scope = viewModelScope, @@ -110,26 +121,6 @@ internal abstract class ItemViewModel( private val itemCreatedEventChannel = Channel() val itemCreatedEvent = itemCreatedEventChannel.receiveAsFlow() - /** Hook for subclasses to start their own observers when [state] gains its first subscriber. */ - protected open suspend fun onSubscribed() {} - - @OptIn(FlowPreview::class) - private fun observeNameTextField() { - snapshotFlow { nameTextFieldState.text } - .debounce(150.milliseconds) - .combine(selectedVaultId.filterNotNull()) { input, vaultId -> - itemRepository.doesNameExist( - input.toString(), - excludeId = itemId, - vaultId = vaultId, - ) - } - .distinctUntilChanged() - .onEach { exists -> nameExists.value = exists } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - protected fun setSelectedVaultId(vaultId: VaultId) { selectedVaultId.value = vaultId } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index b52b8ae67..1b63f3400 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -8,6 +8,7 @@ import androidx.lifecycle.viewModelScope import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator import de.davis.keygo.core.item.domain.model.DomainInfo +import de.davis.keygo.core.item.domain.model.PasswordScore import de.davis.keygo.core.item.domain.repository.ItemRepository import de.davis.keygo.core.item.domain.repository.LoginRepository import de.davis.keygo.core.item.domain.repository.VaultContextRepository @@ -51,12 +52,12 @@ import kotlinx.coroutines.async import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.mapLatest -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.koin.core.annotation.KoinViewModel @@ -90,30 +91,26 @@ internal class LoginViewModel( ) ) - override val itemState: Flow = _base - - override suspend fun onSubscribed() { - observePasswordTextField() + @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) + private val passwordScoreFlow = snapshotFlow { passwordTextFieldState.text } + .debounce(150.milliseconds) + .mapLatest { passwordStrengthEstimator(it.toString()) } + .distinctUntilChanged() + .flowOn(Dispatchers.Default) + // Scoring is dictionary-backed and debounced. Without a value up front, combine would keep + // the whole form on ItemUiState.Loading until the first estimate lands. + .onStart { emit(PasswordScore.None) } + + override val itemState: Flow = combine( + passwordScoreFlow, + _base, + ) { score, base -> + base.copy(strengthScore = score) } private var totpSecretInformation: TotpInfo? = null private var totpOriginalUri: String? = null - @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) - private fun observePasswordTextField() { - snapshotFlow { passwordTextFieldState.text } - .debounce(150.milliseconds) - .mapLatest { passwordStrengthEstimator(it.toString()) } - .distinctUntilChanged() - .onEach { score -> - _base.update { - it.copy(strengthScore = score) - } - } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - /** * Shows a passkey for [rp] as pending until the item is saved. * diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/password/GeneratePasswordViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/password/GeneratePasswordViewModel.kt index 5363bb53c..1be84b25f 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/password/GeneratePasswordViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/password/GeneratePasswordViewModel.kt @@ -16,14 +16,10 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -44,44 +40,16 @@ internal class GeneratePasswordViewModel( private val finalPasswordChannel = Channel() val finalPassword = finalPasswordChannel.receiveAsFlow() - private val _generationState = MutableStateFlow(GeneratePasswordUiState()) - val generationState = _generationState - .onStart { - observeLength() - observeCharacterSet() - } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = GeneratePasswordUiState() - ) + private val _characterSetFlow = MutableStateFlow(UiCharacterSet.ALL) + private val _manualGenerationTrigger = MutableStateFlow(0) - private fun observeLength() { + val generationState = combine( snapshotFlow { sliderState.value.toInt() } .debounce(150.milliseconds) - .distinctUntilChanged() - .onEach { newLength -> - generateAndUpdatePassword(length = newLength) - } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - - private fun observeCharacterSet() { - generationState - .distinctUntilChangedBy { it.characterSet } - .map { it.characterSet } - .onEach { characterSet -> - generateAndUpdatePassword(characterSet = characterSet) - } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - - private suspend fun generateAndUpdatePassword( - length: Int = sliderState.value.toInt(), - characterSet: UiCharacterSet = _generationState.value.characterSet, - ) { + .distinctUntilChanged(), + _characterSetFlow, + _manualGenerationTrigger, + ) { length, characterSet, _ -> val newPassword = passwordGenerator.generatePassword( length = length, useLowercase = characterSet.selected(UiCharacterSet.LOWERCASE), @@ -90,31 +58,33 @@ internal class GeneratePasswordViewModel( useSymbols = characterSet.selected(UiCharacterSet.PUNCTUATIONS), ) val score = passwordStrengthEstimator(newPassword) - _generationState.update { ui -> - ui.copy(generatedPassword = newPassword.asUiPassword(), passwordStrength = score) - } + + GeneratePasswordUiState( + generatedPassword = newPassword.asUiPassword(), + passwordStrength = score, + characterSet = characterSet, + showCaution = characterSet != UiCharacterSet.ALL, + ) } + .flowOn(Dispatchers.Default) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = GeneratePasswordUiState(), + ) @OptIn(ExperimentalMaterial3Api::class) fun onEvent(event: GeneratePasswordUiEvent) { when (event) { is GeneratePasswordUiEvent.OnCharacterSetClick -> { - _generationState.update { - val newCharacterSet = it.characterSet.toggle(event.uiCharacterSet) - if (newCharacterSet == UiCharacterSet.NONE) return - - it.copy( - characterSet = newCharacterSet, - showCaution = newCharacterSet != UiCharacterSet.ALL, - ) + _characterSetFlow.update { currentSet -> + val newCharacterSet = currentSet.toggle(event.uiCharacterSet) + // Only update if it's valid, otherwise keep the old one + if (newCharacterSet != UiCharacterSet.NONE) newCharacterSet else currentSet } } - is GeneratePasswordUiEvent.OnGeneratePasswordClick -> { - viewModelScope.launch { - generateAndUpdatePassword() - } - } + is GeneratePasswordUiEvent.OnGeneratePasswordClick -> _manualGenerationTrigger.update { it + 1 } is GeneratePasswordUiEvent.OnUseClick -> { viewModelScope.launch { diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt index dc7fdebcd..845408f00 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt @@ -13,6 +13,7 @@ import de.davis.keygo.core.item.domain.repository.ItemRepository import de.davis.keygo.core.item.domain.repository.LoginRepository import de.davis.keygo.core.item.domain.usecase.ObserveAllTagsSortedUseCase import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import de.davis.keygo.core.util.combine import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.feature.list_screen.domain.model.FilterState import de.davis.keygo.feature.list_screen.domain.usecase.FilterUseCase @@ -39,9 +40,7 @@ import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.getAndUpdate -import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn @@ -51,6 +50,12 @@ import org.koin.core.annotation.InjectedParam import org.koin.core.annotation.KoinViewModel import kotlin.time.Duration.Companion.milliseconds +/** + * Paces the search query rather than the keystroke: each fire runs a cross-vault `LIKE` scan with a + * tag join, so halving this doubles those scans. + */ +private val SEARCH_DEBOUNCE = 300.milliseconds + @KoinViewModel internal class ItemListViewModel( @InjectedParam private val enableSelection: Boolean, @@ -105,12 +110,24 @@ internal class ItemListViewModel( filterUseCase(filter, items, scores, tagIds) }.distinctUntilChanged() - private val searchResults = MutableStateFlow(listOf()) private val selectedItemIds = MutableStateFlow(emptySet()) private val highlightedId = MutableStateFlow(null) private val _isVaultFlowVisible = MutableStateFlow(false) - val listItemState = combine7( + @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) + private val searchResults = snapshotFlow { searchTextFieldState.text } + .debounce(SEARCH_DEBOUNCE) + .flatMapLatest { + queryToItems(it.toString(), forceSearchAllVaults = true) + } + .distinctUntilChanged() + .flowOn(Dispatchers.Default) + // Nobody has searched yet, and combine withholds its first emission until every input has + // emitted: without this the list screen's first render would wait on a full cross-vault + // search for the empty query. + .onStart { emit(emptyList()) } + + val listItemState = combine( vaultsAndSelection, filteredItems, searchResults, @@ -130,12 +147,10 @@ internal class ItemListViewModel( vaultContext = vaultsAndSel.selection, ) }.distinctUntilChanged() - .onStart { - observeSearchState() - }.stateIn( + .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), - initialValue = ListItemState() + initialValue = ListItemState(), ) private val availableFilterOptions = combine( @@ -207,21 +222,6 @@ internal class ItemListViewModel( (if (!forceSearchAllVaults && query.isBlank()) vaultSpecificItems else flowOf(itemRepository.searchVaultItem(query, restrictedItemType))) - @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) - private fun observeSearchState() { - snapshotFlow { searchTextFieldState.text } - .debounce(300.milliseconds) - .flatMapLatest { - queryToItems(it.toString(), forceSearchAllVaults = true) - } - .distinctUntilChanged() - .onEach { items -> - searchResults.update { items } - } - .flowOn(Dispatchers.Default) - .launchIn(viewModelScope) - } - fun onSubmitQuery() { submittedSearchQuery.update { searchTextFieldState.text.toString() } } @@ -311,25 +311,3 @@ internal class ItemListViewModel( } } } - -private fun combine7( - flow1: Flow, - flow2: Flow, - flow3: Flow, - flow4: Flow, - flow5: Flow, - flow6: Flow, - flow7: Flow, - transform: (T1, T2, T3, T4, T5, T6, T7) -> R -): Flow = combine(flow1, flow2, flow3, flow4, flow5, flow6, flow7) { arrayOfFlows -> - @Suppress("UNCHECKED_CAST") - transform( - arrayOfFlows[0] as T1, - arrayOfFlows[1] as T2, - arrayOfFlows[2] as T3, - arrayOfFlows[3] as T4, - arrayOfFlows[4] as T5, - arrayOfFlows[5] as T6, - arrayOfFlows[6] as T7, - ) -} diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index f531071d0..b5f7ce2ce 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt @@ -8,6 +8,7 @@ import de.davis.keygo.core.identity.domain.model.Reauthentication import de.davis.keygo.core.identity.domain.repository.AccountRepository import de.davis.keygo.core.identity.domain.usecase.ChangePasswordUseCase import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator +import de.davis.keygo.core.item.domain.model.PasswordScore import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.security.domain.model.CiphertextData import de.davis.keygo.core.security.domain.repository.BiometricAvailabilityRepository @@ -15,15 +16,18 @@ import de.davis.keygo.core.ui.model.UiFieldError import de.davis.keygo.core.util.Result import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.onSuccess +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.mapLatest +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.koin.core.annotation.KoinViewModel @@ -39,7 +43,25 @@ internal class ChangePasswordViewModel( ) : ViewModel() { private val _state = MutableStateFlow(ChangePasswordState()) - val state = _state.asStateFlow() + + @OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) + private val passwordStrength = snapshotFlow { _state.value.newPassword.text } + .debounce(150.milliseconds) + .distinctUntilChanged() + .mapLatest { text -> + passwordStrengthEstimator(text.toString()) + } + // combine withholds its first emission until every input has emitted, so without a value + // up front the form would sit on initialValue until the debounce elapses. + .onStart { emit(PasswordScore.None) } + + val state = combine(_state, passwordStrength) { baseState, score -> + baseState.copy(passwordScore = score) + }.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = _state.value, + ) // Buffered (not rendezvous): Success/GenericError are emitted from a background coroutine that // may complete before a collector subscribes; a one-shot navigation/error signal must not drop. @@ -48,7 +70,6 @@ internal class ChangePasswordViewModel( init { resolveBiometricAvailability() - observePasswordStrength() } private fun resolveBiometricAvailability() { @@ -67,18 +88,6 @@ internal class ChangePasswordViewModel( } } - @OptIn(FlowPreview::class) - private fun observePasswordStrength() { - snapshotFlow { _state.value.newPassword.text } - .debounce(150.milliseconds) - .distinctUntilChanged() - .onEach { text -> - val score = passwordStrengthEstimator(text.toString()) - _state.update { it.copy(passwordScore = score) } - } - .launchIn(viewModelScope) - } - /** * Primary "Change password" action. Validates the new passwords first so we never fire a * biometric prompt for an invalid form, then routes to biometric (if available) or the diff --git a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt index 0d57050f4..8b122172f 100644 --- a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt +++ b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt @@ -16,7 +16,9 @@ import de.davis.keygo.rust.FakeKeyWrapper import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest @@ -79,12 +81,16 @@ class ChangePasswordViewModelTest { ) } - private fun viewModel() = ChangePasswordViewModel( + /** + * `state` is `WhileSubscribed`, so it only tracks `_state` while something collects it. Every + * test reads `vm.state.value`, so the subscription belongs here rather than in each test. + */ + private fun TestScope.viewModel() = ChangePasswordViewModel( accountRepository = accountRepository, biometricAvailabilityRepository = biometricAvailability, passwordStrengthEstimator = estimator, changePassword = changePassword, - ) + ).also { it.state.launchIn(backgroundScope) } @Test fun `blank new password sets Empty error and does not change password`() = runTest(dispatcher) { @@ -211,6 +217,7 @@ class ChangePasswordViewModelTest { vm.state.first { it.currentPasswordError == UiFieldError.Incorrect } vm.dismissReauthDialog() + advanceUntilIdle() assertEquals(false, vm.state.value.showReauthDialog) assertEquals(null, vm.state.value.currentPasswordError) @@ -277,6 +284,7 @@ class ChangePasswordViewModelTest { val failure: Result = Result.Failure(BiometricAuthError.NoCipher) vm.onBiometricResult(failure) + advanceUntilIdle() assertEquals(true, vm.state.value.showReauthDialog) } @@ -289,6 +297,7 @@ class ChangePasswordViewModelTest { val failure: Result = Result.Failure(BiometricAuthError.Declined) vm.onBiometricResult(failure) + advanceUntilIdle() assertEquals(true, vm.state.value.showReauthDialog) }