diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index d0395fc7d0..54cad4ccea 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -84,8 +84,17 @@ sealed class PubkyContactError(message: String) : AppError(message) { data object CannotAddSelf : PubkyContactError("Cannot add your own pubky as a contact") data object InvalidFormat : PubkyContactError("Invalid pubky key format") data object ActiveSubscription : PubkyContactError("Contact has an active subscription") + data object SignInChanged : PubkyContactError("Pubky sign-in changed while saving the contact") } +/** + * One sign-in of a Pubky identity. Work the user starts in it, such as a contact edit that first waits for a profile + * lookup, checks [PubkyRepo.isCurrent] before it writes, so it stops once that identity signs out or the next sign-in + * starts. Every sign-in starts a new one, adopting a Ring identity included, even when it signs in the same identity + * again; restoring or refreshing the session of the identity already signed in does not. It holds no secret. + */ +class PubkySignIn internal constructor(val publicKey: String, internal val generation: Long) + private fun Throwable.containsActiveSubscriptionError(): Boolean = generateSequence(this) { it.cause }.any { it is PubkyContactError.ActiveSubscription } @@ -152,6 +161,7 @@ class PubkyRepo @Inject constructor( private val adoptedSourceCheckMutex = Mutex() private val adoptionMutex = Mutex() private val profileWriteGeneration = AtomicLong(0L) + private val signInGeneration = AtomicLong(0L) private var isServiceInitialized = false private val _profile = MutableStateFlow(null) @@ -394,7 +404,7 @@ class PubkyRepo @Inject constructor( } is InitResult.Restored -> { _sessionRestorationFailed.update { false } - _publicKey.update { result.publicKey } + continueSignIn(result.publicKey) Logger.info("Restored paykit session for '${redacted(result.publicKey)}'", context = TAG) } is InitResult.RestorationFailed -> { @@ -540,7 +550,7 @@ class PubkyRepo @Inject constructor( val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix() clearProfileIfIdentityChanged(prefixedPublicKey) - _publicKey.update { prefixedPublicKey } + startSignIn(prefixedPublicKey) notifyBackupStateChanged() Logger.info("Adopted ring identity for '${redacted(rawPublicKey)}'", context = TAG) prefixedPublicKey @@ -648,6 +658,18 @@ class PubkyRepo @Inject constructor( } } + /** The current sign-in, for work that must stop once it ends, or null when no identity is signed in. */ + fun currentSignIn(): PubkySignIn? { + // The generation is read first, so a sign-out between the two reads leaves a sign-in that is already over. + val generation = signInGeneration.get() + val publicKey = _publicKey.value ?: return null + return PubkySignIn(publicKey, generation) + } + + /** Whether [signIn] has not ended: its identity has not signed out and no other sign-in has started since. */ + fun isCurrent(signIn: PubkySignIn): Boolean = + signInGeneration.get() == signIn.generation && _publicKey.value == signIn.publicKey + // endregion // region Profile loading @@ -814,7 +836,7 @@ class PubkyRepo @Inject constructor( tags = tags, status = null, ) - _publicKey.update { publicKey } + startSignIn(publicKey) setProfile(createdProfile) cacheMetadata(createdProfile) settingsStore.setPubkyProfileSetupPending(false) @@ -1117,8 +1139,16 @@ class PubkyRepo @Inject constructor( } } + /** + * Saves a contact's label and keeps the rest of its profile as the contact's local override. The edit belongs to + * [signIn]: once that identity signs out or the next sign-in starts, it fails with + * [PubkyContactError.SignInChanged] before the save, or once the save has run, before the override and the contact + * row are written. Sign-out clears the overrides, so a save that lands after it must not write one back, least of + * all for the next identity, which may have saved a contact with the same key. + */ @Suppress("LongParameterList") suspend fun updateContact( + signIn: PubkySignIn, publicKey: String, name: String, bio: String, @@ -1137,11 +1167,16 @@ class PubkyRepo @Inject constructor( tags = tags, status = null, ) - pubkyService.saveContact(prefixedKey, name) - upsertContactProfileOverride(updatedProfile) - updateContacts { current -> - current.map { if (it.publicKey == prefixedKey) updatedProfile else it } - .sortedBy { it.name.lowercase() } + requireCurrent(signIn) + // The SDK checks the identity under the same lock as the save, so no other identity can slip in between. + pubkyService.saveContact(prefixedKey, name, expectedIdentity = signIn.publicKey) + upsertContactProfileOverride(updatedProfile, signIn) + synchronized(contactsLock) { + requireCurrent(signIn) + updateContacts { current -> + current.map { if (it.publicKey == prefixedKey) updatedProfile else it } + .sortedBy { it.name.lowercase() } + } } markContactsLoaded() Logger.info("Updated contact '${redacted(prefixedKey)}'", context = TAG) @@ -1372,7 +1407,7 @@ class PubkyRepo @Inject constructor( } } - _publicKey.update { publicKey } + startSignIn(publicKey) var pendingSaved = false try { settingsStore.setPubkyProfileSetupPending(true) @@ -1473,7 +1508,7 @@ class PubkyRepo @Inject constructor( val secretKeyHex = deriveLocalSecretKeyFromWalletSeed() keychain.upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secretKeyHex) pubkyService.signIn(secretKeyHex) - _publicKey.update { pubkyService.publicKeyFromSecret(secretKeyHex).ensurePubkyPrefix() } + startSignIn(pubkyService.publicKeyFromSecret(secretKeyHex).ensurePubkyPrefix()) } PubkySessionBackupKind.ExternalSession -> Unit @@ -1507,7 +1542,7 @@ class PubkyRepo @Inject constructor( val publicKey = pubkyService.publicKeyFromSecret(storedSecretKeyHex).ensurePubkyPrefix() notifyBackupStateChanged() - _publicKey.update { publicKey } + continueSignIn(publicKey) true } @@ -1765,18 +1800,23 @@ class PubkyRepo @Inject constructor( }.getOrNull() ?: listOf(PaykitReceiverPaths.WALLET) - private suspend fun upsertContactProfileOverride(profile: PubkyProfile) { + private suspend fun upsertContactProfileOverride(profile: PubkyProfile, signIn: PubkySignIn) { val prefixedKey = profile.publicKey.ensurePubkyPrefix() - val ownerPublicKey = requireNotNull(_publicKey.value) { "Pubky identity unavailable" } pubkyStore.update { data -> + // Checked as the store applies the write: sign-out ends the sign-in before it resets the store. + if (!isCurrent(signIn)) return@update data data.copy( - ownerPublicKey = ownerPublicKey, + ownerPublicKey = signIn.publicKey, contactProfileOverrides = data.contactProfileOverrides + (prefixedKey to profile.toProfileData()), ) } notifyBackupStateChanged() } + private fun requireCurrent(signIn: PubkySignIn) { + if (!isCurrent(signIn)) throw PubkyContactError.SignInChanged + } + private suspend fun removeContactProfileOverride(publicKey: String) { val prefixedKey = publicKey.ensurePubkyPrefix() val ownerPublicKey = requireNotNull(_publicKey.value) { "Pubky identity unavailable" } @@ -1860,10 +1900,23 @@ class PubkyRepo @Inject constructor( _profile.update { profile } } + private fun startSignIn(publicKey: String) { + // First, so the sign-in it replaces has ended before the key is published, even when it is the same identity. + signInGeneration.incrementAndGet() + _publicKey.update { publicKey } + } + + private fun continueSignIn(publicKey: String) { + // Restoring or refreshing the session already signed in keeps its sign-in, so an edit under way still saves. + if (_publicKey.value != publicKey) startSignIn(publicKey) + } + private suspend fun clearAuthenticatedState( clearCachedProfile: Boolean = true, clearRestorationFailure: Boolean = true, ) = withContext(ioDispatcher) { + // First, so work of the ending sign-in stops before the store reset below, and cannot write after it. + signInGeneration.incrementAndGet() if (clearCachedProfile) { evictPubkyImages() profileWriteGeneration.incrementAndGet() diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt index 3c470571df..43903caf0a 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt @@ -38,6 +38,7 @@ import to.bitkit.repositories.PrivatePaykitPaymentContext import to.bitkit.repositories.PrivatePaykitRepo import to.bitkit.repositories.PubkyContactError import to.bitkit.repositories.PubkyRepo +import to.bitkit.repositories.PubkySignIn import to.bitkit.repositories.PublicPaykitPaymentResult import to.bitkit.ui.shared.toast.ToastEventBus import to.bitkit.utils.Logger @@ -347,10 +348,13 @@ class ContactDetailViewModel @Inject constructor( transform: (ImmutableList) -> ImmutableList, onSuccess: () -> Unit = {}, ) { + val signIn = pubkyRepo.currentSignIn() ?: return viewModelScope.launch { tagPersistenceMutex.withLock { contactLoad?.join() + if (!isTagChangeCurrent(signIn)) return@withLock pubkyRepo.resolvePendingContactProfile(publicKey) + if (!isTagChangeCurrent(signIn)) return@withLock val state = _uiState.value val profile = pubkyRepo.contacts.value.find { it.publicKey == publicKey } ?: state.profile @@ -361,6 +365,7 @@ class ContactDetailViewModel @Inject constructor( return@withLock } pubkyRepo.updateContact( + signIn = signIn, publicKey = publicKey, name = profile.name, bio = profile.bio, @@ -376,6 +381,7 @@ class ContactDetailViewModel @Inject constructor( } onSuccess() }.onFailure { + if (!isTagChangeCurrent(signIn)) return@onFailure Logger.error("Failed to update tags for contact '$redactedPublicKey'", it, context = TAG) ToastEventBus.send( type = Toast.ToastType.ERROR, @@ -386,6 +392,14 @@ class ContactDetailViewModel @Inject constructor( } } } + + private fun isTagChangeCurrent(signIn: PubkySignIn): Boolean { + val isCurrent = pubkyRepo.isCurrent(signIn) + if (!isCurrent) { + Logger.info("Dropped a tag change for '$redactedPublicKey' after the Pubky sign-in ended", context = TAG) + } + return isCurrent + } } @Stable diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt index 63b71068e4..55682c612b 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt @@ -204,11 +204,14 @@ class EditContactViewModel @Inject constructor( _uiState.update { it.copy(showDeleteDialog = false) } } + /** The save belongs to the Pubky sign-in it was made in, and stops quietly once that sign-in has ended. */ fun save() { val state = _uiState.value + val signIn = pubkyRepo.currentSignIn() ?: return viewModelScope.launch { _uiState.update { it.copy(isSaving = true) } pubkyRepo.updateContact( + signIn = signIn, publicKey = publicKey, name = state.name, bio = state.bio, @@ -223,7 +226,11 @@ class EditContactViewModel @Inject constructor( ) _effects.emit(EditContactEffect.SaveSuccess) }.onFailure { - Logger.error("Failed to save contact '$publicKey'", it, context = TAG) + if (pubkyRepo.isCurrent(signIn)) { + Logger.error("Failed to save contact '$publicKey'", it, context = TAG) + } else { + Logger.info("Dropped a contact edit after the Pubky sign-in ended", context = TAG) + } _uiState.update { it.copy(isSaving = false) } } } diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 4d76dedf8f..d54a043956 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -3300,13 +3300,131 @@ class PubkyRepoTest : BaseUnitTest() { } sut.loadContacts() - assertTrue(sut.updateContact(VALID_CONTACT_KEY_A, "Edited", "", null, emptyList(), emptyList()).isSuccess) + val signIn = checkNotNull(sut.currentSignIn()) + assertTrue( + sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Edited", "", null, emptyList(), emptyList()).isSuccess, + ) assertTrue(sut.removeContact(VALID_CONTACT_KEY_B).isSuccess) lookups.values.forEach { it.complete(Unit) } assertEquals(listOf("Edited"), sut.contacts.value.map { it.name }) } + @Test + fun `a contact edit from an ended sign-in saves nothing, also once the same identity signs back in`() = test { + val store = stubGatedPubkyStore() + authenticateForTesting(publicKey = VALID_SELF_KEY) + val signIn = checkNotNull(sut.currentSignIn()) + assertTrue(sut.isCurrent(signIn)) + + assertTrue(sut.signOut().isSuccess) + assertFalse(sut.isCurrent(signIn)) + authenticateForTesting(publicKey = VALID_SELF_KEY) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertFalse(sut.isCurrent(signIn)) + assertEquals(PubkyContactError.SignInChanged, result.exceptionOrNull()) + verify(pubkyService, never()).saveContact(any(), anyOrNull(), anyOrNull(), any(), anyOrNull()) + assertEquals(emptyMap(), store.data.contactProfileOverrides) + assertTrue(sut.isCurrent(checkNotNull(sut.currentSignIn()))) + } + + @Test + fun `a contact edit saves nothing once its identity is adopted again after another identity`() = test { + val store = stubGatedPubkyStore() + val identity = stubRingCredential() + val anotherIdentity = stubRingCredential(VALID_CONTACT_KEY_B, secret = "another_ring_secret") + whenever(pubkyService.signIn(any())).thenReturn(Unit) + assertTrue(sut.adoptRingIdentity(identity).isSuccess) + val signIn = checkNotNull(sut.currentSignIn()) + + assertTrue(sut.adoptRingIdentity(anotherIdentity).isSuccess) + assertTrue(sut.adoptRingIdentity(identity).isSuccess) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertEquals(PubkyContactError.SignInChanged, result.exceptionOrNull()) + assertFalse(sut.isCurrent(signIn)) + verify(pubkyService, never()).saveContact(any(), anyOrNull(), anyOrNull(), any(), anyOrNull()) + assertEquals(emptyMap(), store.data.contactProfileOverrides) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + } + + @Test + fun `a contact edit saves nothing once its Ring identity is adopted again while signed in`() = test { + val store = stubGatedPubkyStore() + val identity = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenReturn(Unit) + assertTrue(sut.adoptRingIdentity(identity).isSuccess) + val signIn = checkNotNull(sut.currentSignIn()) + + assertTrue(sut.adoptRingIdentity(identity).isSuccess) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertEquals(PubkyContactError.SignInChanged, result.exceptionOrNull()) + assertFalse(sut.isCurrent(signIn)) + verify(pubkyService, never()).saveContact(any(), anyOrNull(), anyOrNull(), any(), anyOrNull()) + assertEquals(emptyMap(), store.data.contactProfileOverrides) + assertTrue(sut.isCurrent(checkNotNull(sut.currentSignIn()))) + } + + @Test + fun `a contact edit started before the session of its identity is refreshed still saves`() = test { + val store = stubGatedPubkyStore() + authenticateForTesting(publicKey = VALID_SELF_KEY) + whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn("local_secret") + whenever(pubkyService.signIn("local_secret")).thenReturn(Unit) + whenever(pubkyService.publicKeyFromSecret("local_secret")).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val signIn = checkNotNull(sut.currentSignIn()) + + assertEquals(true, sut.refreshSessionIfPossible().getOrNull()) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertTrue(result.isSuccess) + assertTrue(sut.isCurrent(signIn)) + verify(pubkyService).saveContact(VALID_CONTACT_KEY_A, "Alice", expectedIdentity = VALID_SELF_KEY) + assertEquals(listOf("Friend"), store.data.contactProfileOverrides[VALID_CONTACT_KEY_A]?.tags) + } + + @Test + fun `a sign-in taken while a sign-out resets the store ends once the identity is restored`() = test { + val store = stubGatedPubkyStore() + authenticateForTesting(publicKey = VALID_SELF_KEY) + var signInDuringSignOut: PubkySignIn? = null + whenever(pubkyStore.reset()).thenAnswer { + signInDuringSignOut = sut.currentSignIn() + store.data = PubkyStoreData() + Unit + } + assertTrue(sut.signOut().isSuccess) + val signIn = checkNotNull(signInDuringSignOut) + + authenticateForTesting(publicKey = VALID_SELF_KEY) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertEquals(PubkyContactError.SignInChanged, result.exceptionOrNull()) + verify(pubkyService, never()).saveContact(any(), anyOrNull(), anyOrNull(), any(), anyOrNull()) + assertEquals(emptyMap(), store.data.contactProfileOverrides) + } + + @Test + fun `a contact edit saves through the SDK for its own identity only`() = test { + val store = stubGatedPubkyStore() + authenticateForTesting(publicKey = VALID_SELF_KEY) + whenever(pubkyService.contactRecords()).thenReturn(listOf(createContactRecord(VALID_CONTACT_KEY_A, "Saved"))) + whenever(pubkyService.resolveContactProfile(VALID_CONTACT_KEY_A, true, PaykitReadLane.Bulk)) + .doSuspendableAnswer { awaitCancellation() } + sut.loadContacts() + + val signIn = checkNotNull(sut.currentSignIn()) + val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) + + assertTrue(result.isSuccess) + verify(pubkyService).saveContact(VALID_CONTACT_KEY_A, "Alice", expectedIdentity = VALID_SELF_KEY) + assertEquals(listOf("Friend"), store.data.contactProfileOverrides[VALID_CONTACT_KEY_A]?.tags) + assertEquals(VALID_SELF_KEY, store.data.ownerPublicKey) + assertEquals(listOf("Alice" to listOf("Friend")), sut.contacts.value.map { it.name to it.tags }) + } + @Test fun `resolvePendingContactProfile looks up a label-only contact once on the interactive lane`() = test { authenticateForTesting() @@ -3380,7 +3498,10 @@ class PubkyRepoTest : BaseUnitTest() { assertTrue(resolve.isCompleted) assertEquals(true, queuedLookup?.isCancelled) - assertTrue(sut.updateContact(VALID_CONTACT_KEY_A, "Saved", "", null, emptyList(), listOf("Friend")).isSuccess) + val signIn = checkNotNull(sut.currentSignIn()) + assertTrue( + sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Saved", "", null, emptyList(), listOf("Friend")).isSuccess, + ) otherLookup.complete(createResolution(VALID_CONTACT_KEY_B, paykitProfile = createPaykitProfile("Bob"))) assertEquals( listOf("Bob" to emptyList(), "Saved" to listOf("Friend")), @@ -3865,10 +3986,10 @@ class PubkyRepoTest : BaseUnitTest() { status = status, ) - private suspend fun stubRingCredential(): String { - val ringPubky = VALID_SELF_KEY.removePrefix("pubky") - whenever(sharedPubkyClient.ringCredential(ringPubky)).thenReturn(Result.success("ring_secret")) - whenever(pubkyService.publicKeyFromSecret("ring_secret")).thenReturn(ringPubky) + private suspend fun stubRingCredential(publicKey: String = VALID_SELF_KEY, secret: String = "ring_secret"): String { + val ringPubky = publicKey.removePrefix("pubky") + whenever(sharedPubkyClient.ringCredential(ringPubky)).thenReturn(Result.success(secret)) + whenever(pubkyService.publicKeyFromSecret(secret)).thenReturn(ringPubky) return ringPubky } diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelSignInTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelSignInTest.kt new file mode 100644 index 0000000000..dcd51823a7 --- /dev/null +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelSignInTest.kt @@ -0,0 +1,144 @@ +package to.bitkit.ui.screens.contacts + +import android.content.Context +import androidx.lifecycle.SavedStateHandle +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import to.bitkit.models.PubkyProfile +import to.bitkit.models.Toast +import to.bitkit.repositories.PaykitPaymentRequestRepo +import to.bitkit.repositories.PaykitPaymentRequestTarget +import to.bitkit.repositories.PrivatePaykitRepo +import to.bitkit.repositories.PubkyRepo +import to.bitkit.repositories.PubkySignIn +import to.bitkit.test.BaseUnitTest +import to.bitkit.ui.shared.toast.ToastEventBus +import to.bitkit.utils.AppError +import kotlin.test.assertEquals +import kotlin.time.Clock +import kotlin.time.Instant + +/** A tag change belongs to the Pubky sign-in it was made in, and stops quietly once that sign-in has ended. */ +@OptIn(ExperimentalCoroutinesApi::class) +class ContactDetailViewModelSignInTest : BaseUnitTest() { + companion object { + private const val TEST_PUBLIC_KEY = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" + } + + private val context: Context = mock() + private val pubkyRepo: PubkyRepo = mock() + private val paykitPaymentRequestRepo: PaykitPaymentRequestRepo = mock() + private val clock = object : Clock { + override fun now() = Instant.fromEpochSeconds(1_800_000_000) + } + private val signIn = PubkySignIn(publicKey = "pubkyowner", generation = 0) + + @Test + fun `a tag change whose sign-in ends while the contact loads does not look it up or save`() = test { + whenever(context.getString(any())).thenReturn("") + whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(createContact()))) + val contactLoad = CompletableDeferred() + whenever(pubkyRepo.resolvePendingContactProfile(TEST_PUBLIC_KEY)).doSuspendableAnswer { contactLoad.await() } + val sut = createSut() + val toasts = collectToasts() + advanceUntilIdle() + + sut.addTag("Bitcoin") + whenever(pubkyRepo.isCurrent(signIn)).thenReturn(false) + contactLoad.complete(Unit) + advanceUntilIdle() + + verify(pubkyRepo, times(1)).resolvePendingContactProfile(TEST_PUBLIC_KEY) + verify(pubkyRepo, never()).updateContact(any(), any(), any(), any(), anyOrNull(), any(), any()) + assertEquals(emptyList(), sut.uiState.value.tags) + assertEquals(emptyList(), toasts) + } + + @Test + fun `a tag change whose sign-in ends during the profile lookup saves nothing`() = test { + whenever(context.getString(any())).thenReturn("") + whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(createContact()))) + val lookup = CompletableDeferred() + var lookups = 0 + whenever(pubkyRepo.resolvePendingContactProfile(TEST_PUBLIC_KEY)).doSuspendableAnswer { + lookups++ + if (lookups > 1) lookup.await() + } + val sut = createSut() + val toasts = collectToasts() + advanceUntilIdle() + + sut.addTag("Bitcoin") + advanceUntilIdle() + assertEquals(2, lookups) + whenever(pubkyRepo.isCurrent(signIn)).thenReturn(false) + lookup.complete(Unit) + advanceUntilIdle() + + verify(pubkyRepo, never()).updateContact(any(), any(), any(), any(), anyOrNull(), any(), any()) + assertEquals(emptyList(), sut.uiState.value.tags) + assertEquals(emptyList(), toasts) + } + + @Test + fun `a tag save that fails after its sign-in ended shows no toast`() = test { + whenever(context.getString(any())).thenReturn("") + whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(createContact(tags = listOf("Friend"))))) + val sut = createSut() + var isSignInCurrent = true + whenever(pubkyRepo.isCurrent(signIn)).thenAnswer { isSignInCurrent } + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())).doSuspendableAnswer { + isSignInCurrent = false + Result.failure(SignInTestError("signed out")) + } + val toasts = collectToasts() + advanceUntilIdle() + sut.showAddTagSheet() + + sut.addTag("Bitcoin") + advanceUntilIdle() + + verify(pubkyRepo).updateContact(any(), any(), any(), any(), anyOrNull(), any(), any()) + assertEquals(listOf("Friend"), sut.uiState.value.tags) + assertEquals(emptyList(), toasts) + } + + private fun createSut(): ContactDetailViewModel { + whenever(pubkyRepo.currentSignIn()).thenReturn(signIn) + whenever(pubkyRepo.isCurrent(signIn)).thenReturn(true) + whenever(paykitPaymentRequestRepo.eligibleTargets) + .thenReturn(MutableStateFlow>(emptyList())) + return ContactDetailViewModel( + context = context, + pubkyRepo = pubkyRepo, + privatePaykitRepo = mock(), + paykitPaymentRequestRepo = paykitPaymentRequestRepo, + clock = clock, + savedStateHandle = SavedStateHandle(mapOf("publicKey" to TEST_PUBLIC_KEY)), + ) + } + + private fun TestScope.collectToasts(): List { + val toasts = mutableListOf() + backgroundScope.launch { ToastEventBus.events.collect { toasts += it } } + return toasts + } + + private fun createContact(tags: List = emptyList()) = + PubkyProfile.placeholder(TEST_PUBLIC_KEY).copy(tags = tags) +} + +private class SignInTestError(message: String) : AppError(message) diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelTest.kt index 56a11ff325..7a503e204d 100644 --- a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactDetailViewModelTest.kt @@ -26,6 +26,7 @@ import to.bitkit.repositories.PaykitPaymentRequestTarget import to.bitkit.repositories.PaykitPaymentRequestTargetCheck import to.bitkit.repositories.PrivatePaykitRepo import to.bitkit.repositories.PubkyRepo +import to.bitkit.repositories.PubkySignIn import to.bitkit.repositories.PublicPaykitPaymentResult import to.bitkit.test.BaseUnitTest import to.bitkit.utils.AppError @@ -51,6 +52,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { private val clock = object : Clock { override fun now() = now } + private val signIn = PubkySignIn(publicKey = "pubkyowner", generation = 0) private val eligibleTargets = MutableStateFlow>(emptyList()) private val target = PaykitPaymentRequestTarget(TEST_PUBLIC_KEY, "bitkit/wallet") private val openedPayment = PublicPaykitPaymentResult.Opened( @@ -104,7 +106,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { fun `adding a tag persists the updated contact and closes the sheet`() = test { whenever(context.getString(any())).thenReturn("") whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(createContact(tags = listOf("Friend"))))) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -116,6 +118,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { assertEquals(listOf("Friend", "Bitcoin"), sut.uiState.value.tags) assertFalse(sut.uiState.value.showAddTagSheet) verify(pubkyRepo).updateContact( + signIn = eq(signIn), publicKey = eq(TEST_PUBLIC_KEY), name = any(), bio = any(), @@ -131,7 +134,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { whenever(pubkyRepo.contacts).thenReturn( MutableStateFlow(listOf(createContact(tags = listOf("Friend", "Bitcoin")))), ) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -141,6 +144,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { assertEquals(listOf("Bitcoin"), sut.uiState.value.tags) verify(pubkyRepo).updateContact( + signIn = eq(signIn), publicKey = eq(TEST_PUBLIC_KEY), name = any(), bio = any(), @@ -156,7 +160,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { whenever(pubkyRepo.contacts).thenReturn( MutableStateFlow(listOf(createContact(tags = listOf("Friend", "Bitcoin")))), ) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -166,8 +170,8 @@ class ContactDetailViewModelTest : BaseUnitTest() { advanceUntilIdle() inOrder(pubkyRepo).apply { - verify(pubkyRepo).updateContact(any(), any(), any(), anyOrNull(), any(), eq(listOf("Bitcoin"))) - verify(pubkyRepo).updateContact(any(), any(), any(), anyOrNull(), any(), eq(emptyList())) + verify(pubkyRepo).updateContact(any(), any(), any(), any(), anyOrNull(), any(), eq(listOf("Bitcoin"))) + verify(pubkyRepo).updateContact(any(), any(), any(), any(), anyOrNull(), any(), eq(emptyList())) } assertEquals(emptyList(), sut.uiState.value.tags) } @@ -178,7 +182,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { whenever(pubkyRepo.contacts).thenReturn( MutableStateFlow(listOf(createContact(tags = listOf("Friend", "Bitcoin")))), ) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -191,6 +195,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { any(), any(), any(), + any(), anyOrNull(), any(), eq(listOf("Bitcoin")), @@ -209,18 +214,19 @@ class ContactDetailViewModelTest : BaseUnitTest() { lookup.await() contacts.value = listOf(resolved) } - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() sut.addTag("Bitcoin") advanceUntilIdle() - verify(pubkyRepo, never()).updateContact(any(), any(), any(), anyOrNull(), any(), any()) + verify(pubkyRepo, never()).updateContact(any(), any(), any(), any(), anyOrNull(), any(), any()) lookup.complete(Unit) advanceUntilIdle() verify(pubkyRepo).updateContact( + signIn = signIn, publicKey = TEST_PUBLIC_KEY, name = "Alice", bio = "Hello", @@ -235,7 +241,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { whenever(context.getString(any())).thenReturn("") val labelOnly = PubkyProfile.forDisplay(TEST_PUBLIC_KEY, "Alice", imageUrl = null) whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(labelOnly))) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -244,7 +250,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { sut.addTag("Bitcoin") advanceUntilIdle() - verify(pubkyRepo).updateContact(TEST_PUBLIC_KEY, "Alice", "", null, emptyList(), listOf("Bitcoin")) + verify(pubkyRepo).updateContact(signIn, TEST_PUBLIC_KEY, "Alice", "", null, emptyList(), listOf("Bitcoin")) assertFalse(sut.uiState.value.showAddTagSheet) assertEquals(listOf("Bitcoin"), sut.uiState.value.tags) verify(pubkyRepo, times(2)).resolvePendingContactProfile(TEST_PUBLIC_KEY) @@ -254,7 +260,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { fun `failed tag addition stays open and can be retried`() = test { whenever(context.getString(any())).thenReturn("") whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(createContact(tags = listOf("Friend"))))) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())).thenReturn( + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())).thenReturn( Result.failure(ContactDetailTestError("save failed")), Result.success(Unit), ) @@ -273,7 +279,7 @@ class ContactDetailViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showAddTagSheet) assertEquals(listOf("Friend", "Bitcoin"), sut.uiState.value.tags) - verify(pubkyRepo, times(2)).updateContact(any(), any(), any(), anyOrNull(), any(), any()) + verify(pubkyRepo, times(2)).updateContact(any(), any(), any(), any(), anyOrNull(), any(), any()) } @Test @@ -710,7 +716,10 @@ class ContactDetailViewModelTest : BaseUnitTest() { private fun createSut() = ContactDetailViewModel( context = context, - pubkyRepo = pubkyRepo, + pubkyRepo = pubkyRepo.also { + whenever(it.currentSignIn()).thenReturn(signIn) + whenever(it.isCurrent(signIn)).thenReturn(true) + }, privatePaykitRepo = privatePaykitRepo, paykitPaymentRequestRepo = paykitPaymentRequestRepo.also { whenever(it.eligibleTargets).thenReturn(eligibleTargets) diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactSaveSessionChangeTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactSaveSessionChangeTest.kt new file mode 100644 index 0000000000..7a039d3944 --- /dev/null +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactSaveSessionChangeTest.kt @@ -0,0 +1,372 @@ +package to.bitkit.ui.screens.contacts + +import android.content.Context +import androidx.lifecycle.SavedStateHandle +import coil3.ImageLoader +import com.synonym.paykit.ContactProfileResolution +import com.synonym.paykit.ContactProfileSource +import com.synonym.paykit.ContactRecord +import com.synonym.paykit.PublicationStatus +import io.ktor.client.HttpClient +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.withContext +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import to.bitkit.data.PubkyImageCacheEpoch +import to.bitkit.data.PubkyStore +import to.bitkit.data.PubkyStoreData +import to.bitkit.data.SettingsData +import to.bitkit.data.SettingsStore +import to.bitkit.data.keychain.Keychain +import to.bitkit.data.sharedpubky.SharedPubkyClient +import to.bitkit.models.PubkyProfile +import to.bitkit.models.Toast +import to.bitkit.repositories.PaykitPaymentRequestRepo +import to.bitkit.repositories.PaykitPaymentRequestTargetCheck +import to.bitkit.repositories.PubkyRepo +import to.bitkit.services.PaykitReadLane +import to.bitkit.services.PubkyService +import to.bitkit.test.BaseUnitTest +import to.bitkit.ui.shared.toast.ToastEventBus +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Clock +import kotlin.time.ExperimentalTime +import kotlin.time.Instant +import com.synonym.paykit.PubkyProfile as SdkPubkyProfile + +/** + * A contact save started on a contact's screen must not land on the identity signed in after the one it was started + * in. Runs the real [PubkyRepo] under the contact screens, with the SDK faked: like the SDK, the fake saves a contact + * only for the identity whose session it holds, and only one that identity has saved. + */ +@OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class) +class ContactSaveSessionChangeTest : BaseUnitTest() { + companion object { + private const val OWNER_A = "pubky5rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy" + private const val OWNER_B = "pubky1rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy" + private const val CONTACT = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy" + private const val SIGNED_OUT = "signed out" + } + + private val context = mock() + private val pubkyService = mock() + private val keychain = mock() + private val pubkyStore = mock() + private val settingsStore = mock() + private val paykitPaymentRequestRepo = mock() + private val storeData = MutableStateFlow(PubkyStoreData()) + private val settings = MutableStateFlow(SettingsData()) + private val clock = object : Clock { + override fun now(): Instant = Instant.fromEpochMilliseconds(testDispatcher.scheduler.currentTime) + } + + private var sdkIdentity: String? = null + private val sdkContacts = mutableMapOf>() + private val sdkSaveAttempts = mutableListOf>() + private val sdkSaves = mutableListOf>() + private var heldSave: CompletableDeferred? = null + private var heldSaveSucceeds = true + + private lateinit var repo: PubkyRepo + + @Before + fun setUp() { + whenever(context.getString(any())).thenReturn("") + whenever(pubkyStore.data).thenReturn(storeData) + whenever { pubkyStore.update(any()) }.thenAnswer { + storeData.value = it.getArgument<(PubkyStoreData) -> PubkyStoreData>(0)(storeData.value) + Unit + } + whenever { pubkyStore.reset() }.thenAnswer { + storeData.value = PubkyStoreData() + Unit + } + whenever(settingsStore.data).thenReturn(settings) + whenever(settingsStore.isPubkyProfileSetupPending).thenReturn(MutableStateFlow(false)) + whenever { settingsStore.update(any()) }.thenAnswer { + settings.value = it.getArgument<(SettingsData) -> SettingsData>(0)(settings.value) + Unit + } + whenever { pubkyService.contactRecords() }.thenReturn(emptyList()) + whenever { pubkyService.signOut() }.thenAnswer { + sdkIdentity = null + Unit + } + whenever { pubkyService.saveContact(any(), anyOrNull(), anyOrNull(), any(), anyOrNull()) } + .doSuspendableAnswer { fakeSdkSave(it.getArgument(0), it.getArgument(4)) } + whenever { pubkyService.resolveContactProfile(CONTACT, true, PaykitReadLane.Bulk, null) } + .doSuspendableAnswer { awaitCancellation() } + whenever(paykitPaymentRequestRepo.eligibleTargets).thenReturn(MutableStateFlow(emptyList())) + whenever { paykitPaymentRequestRepo.refreshEligibleTarget(any()) } + .thenReturn(Result.success(PaykitPaymentRequestTargetCheck(target = null, isComplete = true))) + repo = PubkyRepo( + ioDispatcher = testDispatcher, + pubkyService = pubkyService, + keychain = keychain, + sharedPubkyClient = mock(), + imageLoader = mock(), + imageCacheEpoch = PubkyImageCacheEpoch(), + pubkyStore = pubkyStore, + settingsStore = settingsStore, + httpClient = mock(), + clock = clock, + ) + } + + /** + * The reported race: two tag changes wait for the contact's held profile lookup, the user signs out and signs in + * with another identity, and only then does the lookup finish. The next identity has saved the same contact, + * which the SDK accepts a save for. + */ + @Test + fun `tag changes queued before a sign-out save nothing for the next identity's same contact`() = test { + assertQueuedTagChangesSaveNothingAfterSignIn(nextOwner = OWNER_B, nextOwnerHasContact = true) + } + + /** As above, but the next identity has not saved the contact, so the SDK would reject a save. */ + @Test + fun `tag changes queued before a sign-out save nothing when the next identity lacks the contact`() = test { + assertQueuedTagChangesSaveNothingAfterSignIn(nextOwner = OWNER_B, nextOwnerHasContact = false) + } + + /** Signing straight back in starts a new sign-in, so the changes still stop and write back no cleared override. */ + @Test + fun `tag changes queued before a sign-out save nothing when the same identity signs back in`() = test { + assertQueuedTagChangesSaveNothingAfterSignIn(nextOwner = OWNER_A, nextOwnerHasContact = true) + } + + /** Sign-out stops the contact's lookup at once, so the queued changes go on while no identity is signed in. */ + @Test + fun `tag changes waiting for a lookup that sign-out stops save nothing and show no toast`() = test { + val toasts = collectToasts() + whenever(pubkyService.resolveContactProfile(CONTACT, true, PaykitReadLane.Interactive, null)) + .doSuspendableAnswer { awaitCancellation() } + signIn(OWNER_A, listOf(contactRecord(label = "Label only"))) + val screen = contactScreen() + screen.addTag("friend") + screen.addTag("work") + + assertTrue(repo.signOut().isSuccess) + advanceUntilIdle() + + assertEquals(Outcome(rows = emptyList()), observedOutcome(toasts)) + } + + /** + * The first change's save was admitted, so it runs, but a sign-out and another identity's sign-in, which saved + * the same contact, land before it returns. Whether the save then succeeds or fails, nothing reaches the next + * identity, and the second change, queued behind it, never saves. + */ + @Test + fun `a tag save that a session change overtakes leaves the next identity alone`() = test { + listOf(true, false).forEach { saveSucceeds -> + assertTagSaveOvertakenBySessionChange(saveSucceeds) + } + } + + /** An edit-contact save takes the same path as a tag change once its save is in flight. */ + @Test + fun `an edit-contact save that a session change overtakes leaves the next identity alone`() = test { + val toasts = collectToasts() + signIn(OWNER_A, listOf(contactRecord(label = "Alice", name = "Alice"))) + val screen = EditContactViewModel( + context = context, + pubkyRepo = repo, + savedStateHandle = SavedStateHandle(mapOf("publicKey" to CONTACT)), + ) + heldSave = CompletableDeferred() + screen.onNameChange("Alice edited") + screen.save() + + assertTrue(repo.signOut().isSuccess) + signIn(OWNER_B, listOf(contactRecord(label = "Bob's Alice", name = "Bob's Alice"))) + heldSave?.complete(Unit) + advanceUntilIdle() + + assertEquals( + Outcome( + saveAttempts = listOf(OWNER_A to CONTACT), + saves = listOf(OWNER_A to CONTACT), + rows = listOf("Bob's Alice" to emptyList()), + ), + observedOutcome(toasts), + ) + } + + private suspend fun TestScope.assertQueuedTagChangesSaveNothingAfterSignIn( + nextOwner: String, + nextOwnerHasContact: Boolean, + ) { + val toasts = collectToasts() + val lookup = CompletableDeferred() + // Sign-out cancels the lookup, but the read under it still finishes, as one the SDK is running may. + whenever(pubkyService.resolveContactProfile(CONTACT, true, PaykitReadLane.Interactive, null)) + .doSuspendableAnswer { + withContext(NonCancellable) { lookup.await() } + contactResolution(name = "Alice") + } + signIn(OWNER_A, listOf(contactRecord(label = "Label only"))) + val screen = contactScreen() + screen.addTag("friend") + screen.addTag("work") + verify(pubkyService).resolveContactProfile(CONTACT, true, PaykitReadLane.Interactive, null) + + assertTrue(repo.signOut().isSuccess) + signIn(nextOwner, if (nextOwnerHasContact) listOf(contactRecord(label = "Next label")) else emptyList()) + lookup.complete(Unit) + advanceUntilIdle() + + assertEquals( + Outcome(rows = if (nextOwnerHasContact) listOf("Next label" to emptyList()) else emptyList()), + observedOutcome(toasts), + ) + // Neither change looks the next identity's contact up. + verify(pubkyService, times(1)).resolveContactProfile(CONTACT, true, PaykitReadLane.Interactive, null) + } + + private suspend fun TestScope.assertTagSaveOvertakenBySessionChange(saveSucceeds: Boolean) { + val case = if (saveSucceeds) "after a save that succeeds" else "after a save that fails" + sdkSaveAttempts.clear() + sdkSaves.clear() + storeData.value = PubkyStoreData() + heldSaveSucceeds = saveSucceeds + val toasts = collectToasts() + signIn(OWNER_A, listOf(contactRecord(label = "Alice", name = "Alice"))) + val screen = contactScreen() + heldSave = CompletableDeferred() + screen.addTag("friend") + screen.addTag("work") + + assertTrue(repo.signOut().isSuccess) + signIn(OWNER_B, listOf(contactRecord(label = "Bob's Alice", name = "Bob's Alice"))) + heldSave?.complete(Unit) + advanceUntilIdle() + heldSave = null + + assertEquals( + Outcome( + saveAttempts = listOf(OWNER_A to CONTACT), + saves = if (saveSucceeds) listOf(OWNER_A to CONTACT) else emptyList(), + rows = listOf("Bob's Alice" to emptyList()), + ), + observedOutcome(toasts), + case, + ) + assertTrue(repo.signOut().isSuccess) + } + + private suspend fun fakeSdkSave(publicKey: String, expectedIdentity: String?): ContactRecord { + sdkSaveAttempts += (sdkIdentity ?: SIGNED_OUT) to publicKey + val identity = checkNotNull(sdkIdentity) { "No Pubky session" } + check(expectedIdentity == null || expectedIdentity == identity) { + "Paykit identity changed before saving the contact" + } + val hasContact = publicKey in sdkContacts[identity].orEmpty() + heldSave?.let { + it.await() + check(heldSaveSucceeds) { "Save failed" } + } + check(hasContact) { "Contact no longer exists" } + sdkSaves += identity to publicKey + return contactRecord(label = null) + } + + private suspend fun signIn(owner: String, records: List) { + val session = "session-$owner" + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(session) + whenever(pubkyService.importSession(session)).doSuspendableAnswer { + sdkIdentity = owner + owner + } + whenever(pubkyService.resolveContactProfile(owner, true, PaykitReadLane.Interactive, null)) + .thenReturn(ownerResolution(owner)) + whenever(pubkyService.contactRecords()).thenReturn(records) + sdkContacts[owner] = records.map { it.publicKey }.toSet() + repo.initialize() + } + + private fun contactScreen() = ContactDetailViewModel( + context = context, + pubkyRepo = repo, + privatePaykitRepo = mock(), + paykitPaymentRequestRepo = paykitPaymentRequestRepo, + clock = clock, + savedStateHandle = SavedStateHandle(mapOf("publicKey" to CONTACT)), + ) + + private fun TestScope.collectToasts(): List { + val toasts = mutableListOf() + backgroundScope.launch { ToastEventBus.events.collect { toasts += it } } + return toasts + } + + private fun observedOutcome(toasts: List) = Outcome( + saveAttempts = sdkSaveAttempts.toList(), + saves = sdkSaves.toList(), + overrideTags = storeData.value.contactProfileOverrides.mapValues { it.value.tags }, + rows = repo.contacts.value.map { it.name to it.tags }, + toasts = toasts.map { "${it.type}: ${it.description}" }, + ) + + private fun contactRecord(label: String?, name: String? = null) = ContactRecord( + publicKey = CONTACT, + receiverPaths = listOf("bitkit/wallet"), + label = label, + profile = name?.let { + PubkyProfile( + publicKey = CONTACT, + name = it, + bio = "", + imageUrl = null, + links = emptyList(), + tags = emptyList(), + status = null, + ).toProfileData().toPaykitProfile() + }, + profileFetchedAt = null, + createdAt = "2026-01-01T00:00:00Z", + updatedAt = "2026-01-01T00:00:00Z", + publicContactMarkerStatus = PublicationStatus.NOT_PUBLISHED, + publicContactMarkerReceiverPath = null, + publicContactPublishedAt = null, + publicContactRemovedAt = null, + publicContactLastError = null, + ) + + private fun contactResolution(name: String) = pubkyResolution(CONTACT, name) + + private fun ownerResolution(owner: String) = pubkyResolution(owner, "Owner") + + private fun pubkyResolution(publicKey: String, name: String) = ContactProfileResolution( + publicKey = publicKey, + source = ContactProfileSource.PUBKY_PROFILE, + displayName = name, + imageUri = null, + paykitProfile = null, + pubkyProfile = SdkPubkyProfile(name = name, bio = "", image = null, links = emptyList(), status = null), + fetchedAt = "2026-01-01T00:00:00Z", + ) + + private data class Outcome( + val saveAttempts: List> = emptyList(), + val saves: List> = emptyList(), + val overrideTags: Map> = emptyMap(), + val rows: List>>, + val toasts: List = emptyList(), + ) +} diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/EditContactViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/EditContactViewModelTest.kt index 9fce2d751a..5d3609ae5a 100644 --- a/app/src/test/java/to/bitkit/ui/screens/contacts/EditContactViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/EditContactViewModelTest.kt @@ -17,6 +17,7 @@ import org.mockito.kotlin.whenever import to.bitkit.models.PubkyProfile import to.bitkit.models.PubkyProfileLink import to.bitkit.repositories.PubkyRepo +import to.bitkit.repositories.PubkySignIn import to.bitkit.test.BaseUnitTest import to.bitkit.test.forEachCase import kotlin.test.assertEquals @@ -27,6 +28,7 @@ import kotlin.test.assertTrue class EditContactViewModelTest : BaseUnitTest() { private val context: Context = mock() private val pubkyRepo: PubkyRepo = mock() + private val signIn = PubkySignIn(publicKey = "pubkyowner", generation = 0) @Test fun `missing local contact triggers refresh path`() = test { @@ -126,7 +128,7 @@ class EditContactViewModelTest : BaseUnitTest() { whenever(context.getString(any())).thenReturn("") val labelOnly = PubkyProfile.forDisplay(TEST_PUBLIC_KEY, "Alice", imageUrl = null) whenever(pubkyRepo.contacts).thenReturn(MutableStateFlow(listOf(labelOnly))) - whenever(pubkyRepo.updateContact(any(), any(), any(), anyOrNull(), any(), any())) + whenever(pubkyRepo.updateContact(any(), any(), any(), any(), anyOrNull(), any(), any())) .thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -137,7 +139,15 @@ class EditContactViewModelTest : BaseUnitTest() { sut.save() advanceUntilIdle() - verify(pubkyRepo).updateContact(TEST_PUBLIC_KEY, "Alice", "Met at a meetup", null, emptyList(), emptyList()) + verify(pubkyRepo).updateContact( + signIn, + TEST_PUBLIC_KEY, + "Alice", + "Met at a meetup", + null, + emptyList(), + emptyList(), + ) } @Test @@ -156,6 +166,8 @@ class EditContactViewModelTest : BaseUnitTest() { } private fun createSut(publicKey: String = TEST_PUBLIC_KEY): EditContactViewModel { + whenever(pubkyRepo.currentSignIn()).thenReturn(signIn) + whenever(pubkyRepo.isCurrent(signIn)).thenReturn(true) return EditContactViewModel( context = context, pubkyRepo = pubkyRepo, diff --git a/changelog.d/next/1417.fixed.md b/changelog.d/next/1417.fixed.md new file mode 100644 index 0000000000..2457137e95 --- /dev/null +++ b/changelog.d/next/1417.fixed.md @@ -0,0 +1 @@ +Contact tag changes and edits still waiting to save when you sign out of Pubky are now dropped, instead of saving to the profile signed in next. diff --git a/docs/pubky.md b/docs/pubky.md index 3e28828c10..96e20ee186 100644 --- a/docs/pubky.md +++ b/docs/pubky.md @@ -88,6 +88,7 @@ Manages session lifecycle, identity adoption, and profile data. Singleton scoped - Leaving the import screens, by Back on the overview or through the drawer, calls `discardPendingImport()`. It drops the pending import only when no import runs, so a running import still clears it once it succeeds, and a failed one leaves the follows it could not save pending - A contact without a resolved profile still appears in the list under its label, or its truncated public key when it has none - `resolvePendingContactProfile()` looks up a saved contact on the interactive read lane while its row still shows only its label because the background refresh has not finished looking it up. That lookup takes the contact over from the refresh: the refresh cancels its own lookup of the contact, so one still queued behind the bulk read lane never runs, and never applies a result for it. A second caller waits for the same lookup, which a later refresh replacing the first leaves running; only a sign-out or an identity change stops it. When the lookup fails or finds no profile, the row keeps its label and the call returns at once rather than waiting for the refresh. When the refresh has already found the contact's profile but is holding it for its next batch, that batch is applied at once instead of a new lookup. The contact screen calls it when it opens and a tag change waits for it before saving; Edit Contact keeps its form loading until it returns. An edit therefore keeps the contact's avatar, which the edit form cannot set, and starts from its bio and links whenever the lookup finds them; when the lookup fails, the edit saves the row as it shows. Once the user changes a field, Edit Contact stops applying later updates of the contact to the form +- `updateContact()` belongs to a `PubkySignIn`, which the contact screen takes when a tag change is made and Edit Contact when Save is tapped. Clearing the signed-in state, as sign-out and a wipe do, ends it, and so does every sign-in, adopting a Ring identity included, even one of the same identity; restoring or refreshing the session of the identity already signed in keeps it. A tag change checks it before and after its lookup; `updateContact()` checks it before the save, passes its identity to the SDK, which checks it under the save's lock, and checks it again as the override and the contact row are written. A change whose sign-in has ended saves nothing, writes no override and shows no toast: the lookup it waited for can finish after a sign-out, and the next identity may have saved a contact with the same key - `fetchContactProfile()` fetches a single contact's profile on demand (used by the detail screen) ## Contacts Flow diff --git a/journeys/pubky-profile/README.md b/journeys/pubky-profile/README.md index 8209b0c7c0..bb33b4cd9b 100644 --- a/journeys/pubky-profile/README.md +++ b/journeys/pubky-profile/README.md @@ -36,13 +36,12 @@ only where the platform forces it; see [Android vs iOS](#android-vs-ios). Waiting behind other lookups does not count towards those ten seconds. - **Contacts lists saved contacts at once.** Contacts shows every saved contact as soon as the saved records are read, under its saved name or truncated key, and fills in a name and avatar when that - contact's profile lookup finishes; a lookup that fails leaves the row as it is. On Android the - rows fill in a few at a time, at most every 300 ms, rather than the list re-sorting once per - contact. The screen-wide spinner shows only until the saved records first load. Reopening - Contacts in the same session shows the profiles already found at once; on Android it also does - not look up again a profile found less than ten minutes ago, so only contacts still without a - profile get a new lookup. Opening a contact whose profile has not loaded yet looks it up at once, - and its edit form shows the published bio. + contact's profile lookup finishes; a lookup that fails leaves the row as it is. The rows fill in a + few at a time, at most every 300 ms, rather than the list re-sorting once per contact. The + screen-wide spinner shows only until the saved records first load. Reopening Contacts in the same + session shows the profiles already found at once, and does not look up again a profile found less + than ten minutes ago, so only contacts still without a profile get a new lookup. Opening a contact + whose profile has not loaded yet looks it up at once, and its edit form shows the published bio. ## Setup diff --git a/journeys/pubky-profile/contacts-list-loading.xml b/journeys/pubky-profile/contacts-list-loading.xml index 89f6301525..922172b32c 100644 --- a/journeys/pubky-profile/contacts-list-loading.xml +++ b/journeys/pubky-profile/contacts-list-loading.xml @@ -1,12 +1,14 @@ Contacts shows the saved contacts as soon as it opens after a relaunch, under the names they were - saved with, and fills in each contact's profile name and avatar as its lookup finishes. It no - longer shows a spinner over an empty list until every lookup has finished, and a lookup that - fails leaves the row as it is. Within the session, reopening Contacts shows the profiles already - found straight away. Opening a contact whose profile has not loaded yet looks it up at once, and - its edit form shows the contact's published bio rather than an empty one, so saving an edit keeps - it. + saved with, and fills in each contact's profile name and avatar as its lookup finishes, a few rows + at a time at most every 300 ms rather than re-sorting the list once per contact. It no longer + shows a spinner over an empty list until every lookup has finished, and a lookup that fails + leaves the row as it is. Within the session, reopening Contacts shows the profiles already + found straight away, and a profile found less than ten minutes ago is not looked up again, so only + contacts still without a profile get a new lookup. Opening a contact whose profile has not loaded + yet looks it up at once, and its edit form shows the contact's published bio rather than an empty + one, so saving an edit keeps it. Precondition: an onboarded dev wallet with Paykit enabled (the default) and a Pubky identity with at least five saved contacts, at least one of them with a published profile name and bio and one