diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 6825a97e47..8539afd619 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -931,35 +931,31 @@ class PubkyRepo @Inject constructor( throw it } - suspend fun importContacts(publicKeys: List): Result = runSuspendCatching { + suspend fun importContacts(profiles: List): Result = runSuspendCatching { withContext(ioDispatcher) { - val imported = coroutineScope { - publicKeys.map { contactPk -> - val prefixedKey = contactPk.ensurePubkyPrefix() - async { - runSuspendCatching { - val profile = resolveContactProfile(prefixedKey).getOrThrow() - ?: PubkyProfile.placeholder(prefixedKey) - pubkyService.saveContact( - prefixedKey, - profile.name, - relevantReceiverPaths(prefixedKey), - restorePrivateConnection = true, - ) - profile - }.onFailure { - Logger.warn("Failed to import contact '${redacted(prefixedKey)}'", it, context = TAG) - }.getOrNull() - } - }.awaitAll().filterNotNull() + val imported = mutableListOf() + val existing = _contacts.value.map { it.publicKey }.toMutableSet() + var firstError: Throwable? = null + for (profile in profiles.distinctBy { it.publicKey }) { + if (profile.publicKey in existing) continue + runSuspendCatching { + // The preview already resolved this profile. Receiver discovery runs during contact refresh. + pubkyService.saveContact(profile.publicKey, profile.name, restorePrivateConnection = true) + imported.add(profile) + existing.add(profile.publicKey) + }.onFailure { + firstError = firstError ?: it + Logger.warn("Failed to import contact '${redacted(profile.publicKey)}'", it, context = TAG) + } } updateContacts { current -> - val existing = current.map { it.publicKey }.toSet() - (current + imported.filter { it.publicKey !in existing }) + val currentKeys = current.map { it.publicKey }.toSet() + (current + imported.filter { it.publicKey !in currentKeys }) .sortedBy { it.name.lowercase() } } markContactsLoaded() Logger.info("Imported '${imported.size}' contacts", context = TAG) + firstError?.let { throw it } } } @@ -967,7 +963,10 @@ class PubkyRepo @Inject constructor( clearPendingImport() val pk = requireNotNull(_publicKey.value) { "Not authenticated" } withContext(ioDispatcher) { - val contactKeys = pubkyService.getContacts(pk) + val canonicalOwnKey = PubkyPublicKeyFormat.canonicalized(pk) + val contactKeys = pubkyService.getContacts(pk).filterNot { + canonicalOwnKey != null && PubkyPublicKeyFormat.canonicalized(it) == canonicalOwnKey + } Logger.debug("Discovered '${contactKeys.size}' contacts for import", context = TAG) val contacts = coroutineScope { diff --git a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt index 342ab08650..8c309ae7b5 100644 --- a/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PublicPaykitRepo.kt @@ -22,6 +22,7 @@ import to.bitkit.services.CoreService import to.bitkit.services.PaykitReceiverPaths import to.bitkit.services.PaykitSdkService import to.bitkit.utils.AppError +import to.bitkit.utils.Logger import to.bitkit.utils.NetworkValidationHelper import to.bitkit.utils.encodeToUrl import java.util.Locale @@ -294,6 +295,12 @@ class PublicPaykitRepo @Inject constructor( publishMutex.withLock { requireCurrentPublicKey() val report = paykitSdkService.syncPublicEndpoints(desiredEndpoints) + report.failed.forEach { failure -> + Logger.warn( + "Failed to sync public Paykit endpoint '${failure.identifier}': ${failure.error}", + context = "PublicPaykitRepo", + ) + } if (report.failed.isNotEmpty()) throw PublicPaykitError.PublicationFailed } } diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewScreen.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewScreen.kt index 35aebff72c..7b6c941a8f 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewScreen.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewScreen.kt @@ -156,6 +156,7 @@ private fun Content( SecondaryButton( text = stringResource(R.string.contacts__import_select), onClick = onClickSelect, + enabled = !uiState.isImporting, modifier = Modifier.weight(1f), ) PrimaryButton( diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModel.kt index d101dbef10..75a012497c 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModel.kt @@ -58,26 +58,29 @@ class ContactImportOverviewViewModel @Inject constructor( } fun importAll() { + if (_uiState.value.isImporting) return val contacts = _uiState.value.contacts if (contacts.isEmpty()) return + _uiState.update { it.copy(isImporting = true) } viewModelScope.launch { - _uiState.update { it.copy(isImporting = true) } - pubkyRepo.importContacts(contacts.map { it.publicKey }) - .onSuccess { - pubkyRepo.clearPendingImport() - _uiState.update { it.copy(isImporting = false) } - _effects.emit(ContactImportOverviewEffect.ImportComplete) - } - .onFailure { - Logger.error("Failed to import all contacts", it, context = TAG) - _uiState.update { it.copy(isImporting = false) } - ToastEventBus.send( - type = Toast.ToastType.ERROR, - title = context.getString(R.string.common__error), - description = it.message, - ) - } + try { + pubkyRepo.importContacts(contacts) + .onSuccess { + pubkyRepo.clearPendingImport() + _effects.emit(ContactImportOverviewEffect.ImportComplete) + } + .onFailure { + Logger.error("Failed to import all contacts", it, context = TAG) + ToastEventBus.send( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.common__error), + description = it.message, + ) + } + } finally { + _uiState.update { it.copy(isImporting = false) } + } } } diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModel.kt index c2dd44acb7..32bba0bda0 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModel.kt @@ -85,30 +85,33 @@ class ContactImportSelectViewModel @Inject constructor( } fun importSelected() { + if (_uiState.value.isImporting) return val selected = _uiState.value.contacts.filter { it.isSelected } + _uiState.update { it.copy(isImporting = true) } viewModelScope.launch { - if (selected.isEmpty()) { - pubkyRepo.clearPendingImport() - _effects.emit(ContactImportSelectEffect.ImportComplete) - return@launch - } - - _uiState.update { it.copy(isImporting = true) } - pubkyRepo.importContacts(selected.map { it.profile.publicKey }) - .onSuccess { + try { + if (selected.isEmpty()) { pubkyRepo.clearPendingImport() - _uiState.update { it.copy(isImporting = false) } _effects.emit(ContactImportSelectEffect.ImportComplete) + return@launch } - .onFailure { - Logger.error("Failed to import selected contacts", it, context = TAG) - _uiState.update { it.copy(isImporting = false) } - ToastEventBus.send( - type = Toast.ToastType.ERROR, - title = context.getString(R.string.common__error), - description = it.message, - ) - } + + pubkyRepo.importContacts(selected.map { it.profile }) + .onSuccess { + pubkyRepo.clearPendingImport() + _effects.emit(ContactImportSelectEffect.ImportComplete) + } + .onFailure { + Logger.error("Failed to import selected contacts", it, context = TAG) + ToastEventBus.send( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.common__error), + description = it.message, + ) + } + } finally { + _uiState.update { it.copy(isImporting = false) } + } } } diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/PayContactsViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/PayContactsViewModel.kt index 7b9edc0b3f..4f446968ab 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/PayContactsViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/PayContactsViewModel.kt @@ -58,12 +58,14 @@ class PayContactsViewModel @Inject constructor( } } - private fun syncErrorMessage(error: Throwable): String = when (error) { + private fun syncErrorMessage(error: Throwable): String = when ( + generateSequence(error) { it.cause }.filterIsInstance().firstOrNull() + ) { PublicPaykitError.InvalidPayload -> context.getString(R.string.profile__pay_contacts_error_invalid_payload) PublicPaykitError.NoSupportedEndpoint -> context.getString(R.string.profile__pay_contacts_error_no_endpoint) PublicPaykitError.SessionNotActive -> context.getString(R.string.profile__pay_contacts_error_session) PublicPaykitError.WalletNotReady -> context.getString(R.string.profile__pay_contacts_error_wallet) - else -> context.getString(R.string.common__error_body) + else -> context.getString(R.string.profile__pay_contacts_error_retry) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 17d09e7795..bc65f16c45 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -695,6 +695,7 @@ Use Bitkit with your contacts to send payments directly, anytime, anywhere. Payment endpoint data could not be prepared. No supported payment endpoint is available. + Could not enable contact payments. Your saved contacts are unchanged. Please try again. Reconnect your Pubky profile to share payment data. Wallet is still starting. Try again in a moment. Let your\ncontacts\n<accent>pay you</accent> diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 3e8c61c949..c67da2889a 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -137,6 +137,79 @@ class PubkyRepoTest : BaseUnitTest() { adoptedSource = SharedPubkyContract.RING_SOURCE_PREFIX + VALID_SELF_KEY.removePrefix("pubky") } + @Test + fun `import saves prepared profiles without network lookups and ignores duplicates`() = test { + val profiles = listOf( + PubkyProfile.placeholder(VALID_CONTACT_KEY_A).copy(name = "Alice"), + PubkyProfile.placeholder(VALID_CONTACT_KEY_B).copy(name = "Bob"), + ) + for (profile in profiles) { + whenever(pubkyService.saveContact(profile.publicKey, profile.name, restorePrivateConnection = true)) + .thenReturn(mock()) + } + whenever(pubkyService.resolveContactProfile(any(), any())).thenAnswer { throw TestAppError("Offline") } + whenever(pubkyService.discoverRelevantReceiverPaths(any())).thenAnswer { throw TestAppError("Offline") } + + val result = sut.importContacts(profiles + profiles) + + assertTrue(result.isSuccess) + assertEquals(profiles, sut.contacts.value) + for (profile in profiles) { + verify(pubkyService).saveContact(profile.publicKey, profile.name, restorePrivateConnection = true) + } + verify(pubkyService, never()).resolveContactProfile(any(), any()) + verify(pubkyService, never()).discoverRelevantReceiverPaths(any()) + } + + @Test + fun `failed import keeps successful contacts and retry saves only missing contacts`() = test { + val alice = PubkyProfile.placeholder(VALID_CONTACT_KEY_A).copy(name = "Alice") + val bob = PubkyProfile.placeholder(VALID_CONTACT_KEY_B).copy(name = "Bob") + whenever(pubkyService.saveContact(alice.publicKey, alice.name, restorePrivateConnection = true)) + .thenReturn(mock()) + whenever(pubkyService.saveContact(bob.publicKey, bob.name, restorePrivateConnection = true)) + .thenAnswer { throw TestAppError("Storage unavailable") }.thenReturn(mock()) + + assertTrue(sut.importContacts(listOf(alice, bob)).isFailure) + assertEquals(listOf(alice), sut.contacts.value) + assertTrue(sut.importContacts(listOf(alice, bob)).isSuccess) + assertEquals(listOf(alice, bob), sut.contacts.value) + verify(pubkyService).saveContact(alice.publicKey, alice.name, restorePrivateConnection = true) + verify(pubkyService, times(2)).saveContact(bob.publicKey, bob.name, restorePrivateConnection = true) + } + + @Test + fun `prepareImport excludes own key and imports remaining follows`() = test { + authenticateForTesting(publicKey = VALID_CONTACT_KEY_A) + val ownProfile = checkNotNull(sut.profile.value) + val ownKeys = listOf( + VALID_CONTACT_KEY_A, + VALID_CONTACT_KEY_A.removePrefix("pubky"), + NON_CANONICAL_CONTACT_KEY_A, + NON_CANONICAL_CONTACT_KEY_A.removePrefix("pubky"), + ) + val alice = PubkyProfile.placeholder(VALID_CONTACT_KEY_B).copy(name = "Alice") + whenever(pubkyService.getContacts(VALID_CONTACT_KEY_A)).thenReturn(ownKeys + VALID_CONTACT_KEY_B) + whenever(pubkyService.resolveContactProfile(VALID_CONTACT_KEY_B, true)) + .thenReturn(createResolution(VALID_CONTACT_KEY_B, paykitProfile = createPaykitProfile("Alice"))) + whenever(pubkyService.saveContact(alice.publicKey, alice.name, restorePrivateConnection = true)) + .thenReturn(mock()) + whenever(pubkyService.saveContact(VALID_CONTACT_KEY_A, ownProfile.name, restorePrivateConnection = true)) + .thenAnswer { throw TestAppError("Cannot save own identity") } + + assertTrue(sut.prepareImport().isSuccess) + assertEquals(ownProfile, sut.pendingImportProfile.value) + assertEquals(listOf(alice), sut.pendingImportContacts.value) + assertTrue(sut.importContacts(sut.pendingImportContacts.value).isSuccess) + assertEquals(listOf(alice), sut.contacts.value) + + whenever(pubkyService.getContacts(VALID_CONTACT_KEY_A)).thenReturn(ownKeys) + + assertTrue(sut.prepareImport().isSuccess) + assertEquals(ownProfile, sut.pendingImportProfile.value) + assertTrue(sut.pendingImportContacts.value.isEmpty()) + } + @Test fun `initial state should have no public key`() = test { assertNull(sut.publicKey.value) diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModelTest.kt index e3c578b55d..65ef000353 100644 --- a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportOverviewViewModelTest.kt @@ -1,11 +1,13 @@ package to.bitkit.ui.screens.contacts import android.content.Context +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Test +import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -13,6 +15,7 @@ import to.bitkit.models.PubkyProfile import to.bitkit.repositories.PubkyRepo import to.bitkit.test.BaseUnitTest import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue @OptIn(ExperimentalCoroutinesApi::class) @@ -20,6 +23,26 @@ class ContactImportOverviewViewModelTest : BaseUnitTest() { private val context: Context = mock() private val pubkyRepo: PubkyRepo = mock() + @Test + fun `pending import blocks duplicate requests and clears progress after cancellation`() = test { + val contacts = listOf(createProfile(publicKey = "pubkyalice")) + stubPendingImport(createProfile(publicKey = "pubkyself"), contacts) + val pending = CompletableDeferred>() + whenever(pubkyRepo.importContacts(contacts)).doSuspendableAnswer { pending.await() } + val sut = createSut() + advanceUntilIdle() + + sut.importAll() + sut.importAll() + advanceUntilIdle() + assertTrue(sut.uiState.value.isImporting) + verify(pubkyRepo).importContacts(contacts) + + pending.cancel() + advanceUntilIdle() + assertFalse(sut.uiState.value.isImporting) + } + @Test fun `missing pending import redirects to pay contacts`() = test { stubPendingImport(profile = null, contacts = emptyList()) @@ -34,7 +57,7 @@ class ContactImportOverviewViewModelTest : BaseUnitTest() { fun `importAll clears pending import and completes`() = test { val contacts = listOf(createProfile(publicKey = "pubkyalice"), createProfile(publicKey = "pubkybob")) stubPendingImport(profile = createProfile(publicKey = "pubkyself"), contacts = contacts) - whenever(pubkyRepo.importContacts(contacts.map { it.publicKey })).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.importContacts(contacts)).thenReturn(Result.success(Unit)) val sut = createSut() val effects = mutableListOf() diff --git a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModelTest.kt index 7d0f55395c..f177b61af2 100644 --- a/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportSelectViewModelTest.kt @@ -56,7 +56,7 @@ class ContactImportSelectViewModelTest : BaseUnitTest() { fun `importSelected success clears pending import and completes`() = test { val contacts = listOf(createProfile(publicKey = "pubkyalice"), createProfile(publicKey = "pubkybob")) stubPendingImport(profile = createProfile(publicKey = "pubkyself"), contacts = contacts) - whenever(pubkyRepo.importContacts(contacts.map { it.publicKey })).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.importContacts(contacts)).thenReturn(Result.success(Unit)) val sut = createSut() val effects = mutableListOf() diff --git a/app/src/test/java/to/bitkit/ui/screens/profile/PayContactsViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/profile/PayContactsViewModelTest.kt index e0aa67f34c..6f80212be8 100644 --- a/app/src/test/java/to/bitkit/ui/screens/profile/PayContactsViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/profile/PayContactsViewModelTest.kt @@ -10,8 +10,11 @@ import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import to.bitkit.R import to.bitkit.repositories.ContactPaymentSettingsRepo +import to.bitkit.repositories.PublicPaykitError import to.bitkit.test.BaseUnitTest +import to.bitkit.ui.shared.toast.ToastEventBus import to.bitkit.utils.AppError import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -58,6 +61,27 @@ class PayContactsViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.isLoading) } + @Test + fun `continue explains wrapped session errors and gives retry guidance for other failures`() = test { + val cases = listOf( + AppError(PublicPaykitError.SessionNotActive) to R.string.profile__pay_contacts_error_session, + PayContactsTestAppError("sync failed") to R.string.profile__pay_contacts_error_retry, + ) + for ((error, messageId) in cases) { + val message = "message-$messageId" + whenever(context.getString(messageId)).thenReturn(message) + whenever(contactPaymentSettingsRepo.setEnabled(true)).thenReturn(Result.failure(error)) + val sut = createSut() + + ToastEventBus.events.test { + sut.continueToProfile() + advanceUntilIdle() + assertEquals(message, awaitItem().description) + } + assertFalse(sut.uiState.value.isLoading) + } + } + private fun createSut() = PayContactsViewModel( context = context, contactPaymentSettingsRepo = contactPaymentSettingsRepo, diff --git a/changelog.d/next/1395.fixed.md b/changelog.d/next/1395.fixed.md new file mode 100644 index 0000000000..a5f758ee61 --- /dev/null +++ b/changelog.d/next/1395.fixed.md @@ -0,0 +1 @@ +Fixed slow contact imports by saving prepared contacts without repeating remote lookups, excluding your own profile and keeping failed imports available for retry. diff --git a/journeys/contacts/README.md b/journeys/contacts/README.md new file mode 100644 index 0000000000..3e7abb48c3 --- /dev/null +++ b/journeys/contacts/README.md @@ -0,0 +1,7 @@ +# Contacts + +Import journeys require a disposable identity with a known following list. They save local Bitkit contacts; payment sharing remains a separate step. + +For Import All, include the identity's own key in the following list. Repeat with a spelling that changes only the final z-base32 padding bits, which still represents the same 32-byte key. The preview friend count and saved contacts exclude that identity, while the other follows import normally. Carry the same self-follow checks and padding-alias repeat in the matching iOS journey. + +Network and storage fault injection are outside journey-runner capabilities. Manually disable connectivity after the preview has loaded: importing the prepared contacts must still finish. Simulate a failed local save: stay on import, preserve successful saves, and retry only missing contacts without claiming complete success. On Android, a failed Continue on the payment-sharing screen should offer recovery guidance and leave saved contacts intact. diff --git a/journeys/contacts/import-all-contacts.xml b/journeys/contacts/import-all-contacts.xml new file mode 100644 index 0000000000..aedbcadb9d --- /dev/null +++ b/journeys/contacts/import-all-contacts.xml @@ -0,0 +1,18 @@ + + + Precondition: the import preview is open for a disposable Ring identity with known follows + that are not yet saved in this Bitkit wallet. Include Pubky-only profiles without Paykit receivers. + Include the identity's own key in the following list. + Repeat with a spelling of that key that changes only the final z-base32 padding bits. + Repeat with a large list (for example 62 contacts). + + + Note the friend count shown in the import preview + Verify the friend count excludes your own profile + Tap Import All + Verify the import finishes and the Let Your Contacts Pay You screen appears + Return Home and open Contacts from the menu + Verify the prepared contacts, including Pubky-only profiles, are present with their preview names + Verify your own profile is absent from Contacts + + diff --git a/journeys/contacts/import-selected-contacts.xml b/journeys/contacts/import-selected-contacts.xml new file mode 100644 index 0000000000..8d5421a6fd --- /dev/null +++ b/journeys/contacts/import-selected-contacts.xml @@ -0,0 +1,14 @@ + + + Precondition: the import preview is open for a disposable Ring identity with at least three + known follows that are not yet saved in this Bitkit wallet. + + + Tap Select + Deselect one contact and note its name + Tap the import button + Verify the import finishes and the Let Your Contacts Pay You screen appears + Return Home and open Contacts from the menu + Verify the selected contacts are present and the deselected contact is absent + +