From 8b4d3a4bc80f1cf9f3213202ff2e2bbe524c95f4 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 7 Nov 2023 21:06:26 +0100 Subject: [PATCH] Lock : fix race condition on setup pin --- .../lockscreen/impl/setup/pin/SetupPinEvents.kt | 2 +- .../impl/setup/pin/SetupPinPresenter.kt | 3 ++- .../lockscreen/impl/setup/pin/SetupPinView.kt | 4 ++-- .../impl/setup/pin/SetupPinPresenterTest.kt | 16 ++++++++++------ 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinEvents.kt b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinEvents.kt index d0105b1a21..15de84c863 100644 --- a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinEvents.kt +++ b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinEvents.kt @@ -17,6 +17,6 @@ package io.element.android.features.lockscreen.impl.setup.pin sealed interface SetupPinEvents { - data class OnPinEntryChanged(val entryAsText: String) : SetupPinEvents + data class OnPinEntryChanged(val entryAsText: String, val fromConfirmationStep: Boolean) : SetupPinEvents data object ClearFailure : SetupPinEvents } diff --git a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenter.kt b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenter.kt index c5f170404f..d48e418a59 100644 --- a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenter.kt +++ b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenter.kt @@ -85,7 +85,8 @@ class SetupPinPresenter @Inject constructor( fun handleEvents(event: SetupPinEvents) { when (event) { is SetupPinEvents.OnPinEntryChanged -> { - if (isConfirmationStep) { + // Use the fromConfirmationStep flag from ui to avoid race condition. + if (event.fromConfirmationStep) { confirmPinEntry = confirmPinEntry.fillWith(event.entryAsText) } else { choosePinEntry = choosePinEntry.fillWith(event.entryAsText) diff --git a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinView.kt b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinView.kt index 3d41d1ba1d..095848c9fb 100644 --- a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinView.kt +++ b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinView.kt @@ -116,8 +116,8 @@ private fun SetupPinContent( PinEntryTextField( pinEntry = state.activePinEntry, isSecured = true, - onValueChange = { - state.eventSink(SetupPinEvents.OnPinEntryChanged(it)) + onValueChange = { entry -> + state.eventSink(SetupPinEvents.OnPinEntryChanged(entry, state.isConfirmationStep)) }, modifier = modifier .focusRequester(focusRequester) diff --git a/features/lockscreen/impl/src/test/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenterTest.kt b/features/lockscreen/impl/src/test/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenterTest.kt index 7f3373bd4a..7ec73116c2 100644 --- a/features/lockscreen/impl/src/test/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenterTest.kt +++ b/features/lockscreen/impl/src/test/kotlin/io/element/android/features/lockscreen/impl/setup/pin/SetupPinPresenterTest.kt @@ -60,14 +60,14 @@ class SetupPinPresenterTest { state.confirmPinEntry.assertEmpty() assertThat(state.setupPinFailure).isNull() assertThat(state.isConfirmationStep).isFalse() - state.eventSink(SetupPinEvents.OnPinEntryChanged(halfCompletePin)) + state.onPinEntryChanged(halfCompletePin) } awaitItem().also { state -> state.choosePinEntry.assertText(halfCompletePin) state.confirmPinEntry.assertEmpty() assertThat(state.setupPinFailure).isNull() assertThat(state.isConfirmationStep).isFalse() - state.eventSink(SetupPinEvents.OnPinEntryChanged(blacklistedPin)) + state.onPinEntryChanged(blacklistedPin) } awaitLastSequentialItem().also { state -> state.choosePinEntry.assertText(blacklistedPin) @@ -77,7 +77,7 @@ class SetupPinPresenterTest { awaitLastSequentialItem().also { state -> state.choosePinEntry.assertEmpty() assertThat(state.setupPinFailure).isNull() - state.eventSink(SetupPinEvents.OnPinEntryChanged(completePin)) + state.onPinEntryChanged(completePin) } consumeItemsUntilPredicate { it.isConfirmationStep @@ -85,7 +85,7 @@ class SetupPinPresenterTest { state.choosePinEntry.assertText(completePin) state.confirmPinEntry.assertEmpty() assertThat(state.isConfirmationStep).isTrue() - state.eventSink(SetupPinEvents.OnPinEntryChanged(mismatchedPin)) + state.onPinEntryChanged(mismatchedPin) } awaitLastSequentialItem().also { state -> state.choosePinEntry.assertText(completePin) @@ -98,7 +98,7 @@ class SetupPinPresenterTest { state.confirmPinEntry.assertEmpty() assertThat(state.isConfirmationStep).isFalse() assertThat(state.setupPinFailure).isNull() - state.eventSink(SetupPinEvents.OnPinEntryChanged(completePin)) + state.onPinEntryChanged(completePin) } consumeItemsUntilPredicate { it.isConfirmationStep @@ -106,7 +106,7 @@ class SetupPinPresenterTest { state.choosePinEntry.assertText(completePin) state.confirmPinEntry.assertEmpty() assertThat(state.isConfirmationStep).isTrue() - state.eventSink(SetupPinEvents.OnPinEntryChanged(completePin)) + state.onPinEntryChanged(completePin) } awaitItem().also { state -> state.choosePinEntry.assertText(completePin) @@ -116,6 +116,10 @@ class SetupPinPresenterTest { } } + private fun SetupPinState.onPinEntryChanged(pinEntry: String){ + eventSink(SetupPinEvents.OnPinEntryChanged(pinEntry, isConfirmationStep)) + } + private fun createSetupPinPresenter( callback: PinCodeManager.Callback, lockScreenConfig: LockScreenConfig = aLockScreenConfig(