Skip to content

Commit 3dbc299

Browse files
ben-kaufmanben-kaufman
committed
fix: address claude pr feedback
1 parent d325bc3 commit 3dbc299

10 files changed

Lines changed: 189 additions & 125 deletions

File tree

‎app/src/main/java/to/bitkit/repositories/BackupRepo.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ class BackupRepo @Inject constructor(
505505
BackupCategory.LIGHTNING_CONNECTIONS -> throw NotImplementedError("LIGHTNING backup is managed by ldk-node")
506506
}
507507

508-
private suspend fun getMetadataBackupDataBytes(): ByteArray {
508+
private suspend fun getMetadataBackupDataBytes(): ByteArray = withContext(ioDispatcher) {
509509
val preActivityMetadata = preActivityMetadataRepo.getAllPreActivityMetadata().getOrDefault(emptyList())
510510
val cacheData = cacheStore.data.first()
511511
val pubkySession = pubkyRepo.snapshotSessionBackupState().getOrDefault(null)
@@ -517,7 +517,7 @@ class BackupRepo @Inject constructor(
517517
pubkySession = pubkySession,
518518
)
519519

520-
return json.encodeToString(payload).toByteArray()
520+
json.encodeToString(payload).toByteArray()
521521
}
522522

523523
suspend fun performFullRestoreFromLatestBackup(

‎app/src/main/java/to/bitkit/repositories/PubkyRepo.kt‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -171,45 +171,45 @@ class PubkyRepo @Inject constructor(
171171
private suspend fun resolveSessionInitialization(
172172
savedSessionSecret: String?,
173173
storedSecretKeyHex: String?,
174-
): InitResult {
174+
): InitResult = withContext(ioDispatcher) {
175175
if (!savedSessionSecret.isNullOrEmpty()) {
176-
return runCatching {
176+
runCatching {
177177
val publicKey = pubkyService.importSession(savedSessionSecret)
178178
InitResult.Restored(publicKey)
179179
}.getOrElse {
180180
Logger.warn("Failed to restore paykit session, attempting re-sign-in", it, context = TAG)
181181
resolveSignedInSession(savedSessionSecret, storedSecretKeyHex)
182182
}
183+
} else {
184+
resolveSignedInSession(savedSessionSecret, storedSecretKeyHex)
183185
}
184-
185-
return resolveSignedInSession(savedSessionSecret, storedSecretKeyHex)
186186
}
187187

188188
private suspend fun resolveSignedInSession(
189189
savedSessionSecret: String?,
190190
storedSecretKeyHex: String?,
191-
): InitResult {
191+
): InitResult = withContext(ioDispatcher) {
192192
if (storedSecretKeyHex.isNullOrEmpty()) {
193193
if (!savedSessionSecret.isNullOrEmpty()) {
194194
Logger.warn("Skipped re-sign-in recovery, no secret key available", context = TAG)
195-
return InitResult.RestorationFailed
195+
InitResult.RestorationFailed
196+
} else {
197+
InitResult.NoSession
198+
}
199+
} else {
200+
runCatching {
201+
val newSession = pubkyService.signIn(storedSecretKeyHex)
202+
keychain.upsertString(Keychain.Key.PAYKIT_SESSION.name, newSession)
203+
notifyBackupStateChanged()
204+
val publicKey = pubkyService.importSession(newSession)
205+
Logger.info("Re-signed in and restored session for '$publicKey'", context = TAG)
206+
InitResult.Restored(publicKey)
207+
}.getOrElse {
208+
Logger.error("Failed re-sign-in recovery", it, context = TAG)
209+
runCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) }
210+
notifyBackupStateChanged()
211+
InitResult.RestorationFailed
196212
}
197-
198-
return InitResult.NoSession
199-
}
200-
201-
return runCatching {
202-
val newSession = pubkyService.signIn(storedSecretKeyHex)
203-
keychain.upsertString(Keychain.Key.PAYKIT_SESSION.name, newSession)
204-
notifyBackupStateChanged()
205-
val publicKey = pubkyService.importSession(newSession)
206-
Logger.info("Re-signed in and restored session for '$publicKey'", context = TAG)
207-
InitResult.Restored(publicKey)
208-
}.getOrElse {
209-
Logger.error("Failed re-sign-in recovery", it, context = TAG)
210-
runCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) }
211-
notifyBackupStateChanged()
212-
InitResult.RestorationFailed
213213
}
214214
}
215215

@@ -422,6 +422,16 @@ class PubkyRepo @Inject constructor(
422422
}
423423
}
424424

425+
suspend fun deleteProfileWithSessionRetry(): Result<Unit> = withContext(ioDispatcher) {
426+
val initialResult = deleteProfile()
427+
if (initialResult.isSuccess) return@withContext initialResult
428+
429+
val refreshedSession = refreshSessionIfPossible().getOrDefault(false)
430+
if (!refreshedSession) return@withContext initialResult
431+
432+
deleteProfile()
433+
}
434+
425435
suspend fun deleteProfile(): Result<Unit> = runCatching {
426436
withContext(ioDispatcher) {
427437
val session = requireNotNull(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)) {

‎app/src/main/java/to/bitkit/ui/screens/contacts/ContactImportFlow.kt‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,6 @@ internal fun resolveAddContactValidation(
3636
return AddContactValidationResult.Valid(normalizedKey = normalizedKey)
3737
}
3838

39-
internal fun resolvePastedPubkyRoute(
40-
input: String,
41-
ownPublicKey: String?,
42-
contacts: List<PubkyProfile>,
43-
): Routes? {
44-
val normalizedKey = PubkyPublicKeyFormat.normalized(input) ?: return null
45-
46-
if (PubkyPublicKeyFormat.matches(normalizedKey, ownPublicKey)) {
47-
return Routes.Profile
48-
}
49-
50-
if (contacts.any { PubkyPublicKeyFormat.matches(it.publicKey, normalizedKey) }) {
51-
return Routes.ContactDetail(normalizedKey)
52-
}
53-
54-
return Routes.AddContact(normalizedKey)
55-
}
56-
5739
internal fun shouldDiscardPendingImport(currentDestination: NavDestination?, destination: Routes?): Boolean {
5840
if (!currentDestination.isContactImportRoute()) {
5941
return false

‎app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,13 @@ class EditProfileViewModel @Inject constructor(
208208

209209
fun deleteProfile() {
210210
viewModelScope.launch {
211-
attemptDeleteProfile(allowSessionRefresh = true)
211+
attemptDeleteProfile()
212212
}
213213
}
214214

215215
fun retryDeleteProfile() {
216216
viewModelScope.launch {
217-
attemptDeleteProfile(allowSessionRefresh = false)
217+
attemptDeleteProfile()
218218
}
219219
}
220220

@@ -242,15 +242,15 @@ class EditProfileViewModel @Inject constructor(
242242
}
243243
}
244244

245-
private suspend fun attemptDeleteProfile(allowSessionRefresh: Boolean) {
245+
private suspend fun attemptDeleteProfile() {
246246
_uiState.update {
247247
it.copy(
248248
showDeleteDialog = false,
249249
showDeleteFailureDialog = false,
250250
isSaving = true,
251251
)
252252
}
253-
pubkyRepo.deleteProfile()
253+
pubkyRepo.deleteProfileWithSessionRetry()
254254
.onSuccess {
255255
_uiState.update { it.copy(isSaving = false) }
256256
ToastEventBus.send(
@@ -261,20 +261,6 @@ class EditProfileViewModel @Inject constructor(
261261
}
262262
.onFailure {
263263
Logger.error("Failed to delete profile", it, context = TAG)
264-
265-
if (allowSessionRefresh) {
266-
val refreshedSession = pubkyRepo.refreshSessionIfPossible()
267-
.onFailure {
268-
Logger.error("Failed to refresh pubky session", it, context = TAG)
269-
}
270-
.getOrDefault(false)
271-
272-
if (refreshedSession) {
273-
attemptDeleteProfile(allowSessionRefresh = false)
274-
return
275-
}
276-
}
277-
278264
_uiState.update {
279265
it.copy(
280266
isSaving = false,

‎app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ import to.bitkit.services.CoreService
126126
import to.bitkit.services.MigrationService
127127
import to.bitkit.ui.Routes
128128
import to.bitkit.ui.components.Sheet
129-
import to.bitkit.ui.screens.contacts.resolvePastedPubkyRoute
130129
import to.bitkit.ui.shared.toast.ToastEventBus
131130
import to.bitkit.ui.shared.toast.ToastQueueManager
132131
import to.bitkit.ui.sheets.SendRoute

‎app/src/main/java/to/bitkit/viewmodels/PubkyRouteResolver.kt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package to.bitkit.viewmodels
2+
3+
import to.bitkit.models.PubkyProfile
4+
import to.bitkit.models.PubkyPublicKeyFormat
5+
import to.bitkit.ui.Routes
6+
7+
internal fun resolvePastedPubkyRoute(
8+
input: String,
9+
ownPublicKey: String?,
10+
contacts: List<PubkyProfile>,
11+
): Routes? {
12+
val normalizedKey = PubkyPublicKeyFormat.normalized(input) ?: return null
13+
14+
if (PubkyPublicKeyFormat.matches(normalizedKey, ownPublicKey)) {
15+
return Routes.Profile
16+
}
17+
18+
if (contacts.any { PubkyPublicKeyFormat.matches(it.publicKey, normalizedKey) }) {
19+
return Routes.ContactDetail(normalizedKey)
20+
}
21+
22+
return Routes.AddContact(normalizedKey)
23+
}

‎app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,44 @@ class PubkyRepoTest : BaseUnitTest() {
274274
assertTrue(result.isFailure)
275275
}
276276

277+
@Test
278+
fun `deleteProfileWithSessionRetry should refresh session and retry delete`() = test {
279+
val expiredSession = "expired_session"
280+
val newSession = "new_session"
281+
val secretKey = "local_secret"
282+
authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession)
283+
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession, newSession)
284+
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey)
285+
whenever(pubkyService.sessionList(expiredSession, Env.contactsBasePath)).thenReturn(emptyList())
286+
whenever(pubkyService.sessionList(newSession, Env.contactsBasePath)).thenReturn(emptyList())
287+
whenever(pubkyService.sessionDelete(expiredSession, Env.profilePath)).thenThrow(RuntimeException("Expired"))
288+
whenever(pubkyService.signIn(secretKey)).thenReturn(newSession)
289+
whenever(pubkyService.importSession(newSession)).thenReturn(VALID_SELF_KEY)
290+
291+
val result = sut.deleteProfileWithSessionRetry()
292+
293+
assertTrue(result.isSuccess)
294+
verifyBlocking(pubkyService) { sessionDelete(expiredSession, Env.profilePath) }
295+
verifyBlocking(pubkyService) { sessionDelete(newSession, Env.profilePath) }
296+
verifyBlocking(keychain) { upsertString(Keychain.Key.PAYKIT_SESSION.name, newSession) }
297+
}
298+
299+
@Test
300+
fun `deleteProfileWithSessionRetry should return failure when session cannot refresh`() = test {
301+
val expiredSession = "expired_session"
302+
authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession)
303+
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession)
304+
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null)
305+
whenever(pubkyService.sessionList(expiredSession, Env.contactsBasePath)).thenReturn(emptyList())
306+
whenever(pubkyService.sessionDelete(expiredSession, Env.profilePath)).thenThrow(RuntimeException("Expired"))
307+
308+
val result = sut.deleteProfileWithSessionRetry()
309+
310+
assertTrue(result.isFailure)
311+
verifyBlocking(pubkyService) { sessionDelete(expiredSession, Env.profilePath) }
312+
verifyBlocking(pubkyService, never()) { signIn(any()) }
313+
}
314+
277315
@Test
278316
fun `signOut should force sign out when server sign out fails`() = test {
279317
authenticateForTesting()

‎app/src/test/java/to/bitkit/ui/screens/contacts/ContactImportFlowTest.kt‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
package to.bitkit.ui.screens.contacts
22

33
import org.junit.Test
4-
import to.bitkit.models.PubkyProfile
5-
import to.bitkit.ui.Routes
64
import kotlin.test.assertEquals
7-
import kotlin.test.assertNull
85

96
class ContactImportFlowTest {
107
private companion object {
118
const val VALID_PUBLIC_KEY = "pubkyybndrfg8ejkmcpqxot1uwisza345h769ybndrfg8ejkmcpqxot1u"
12-
const val OTHER_VALID_PUBLIC_KEY = "pubkya345h769ybndrfg8ejkmcpqxot1uwiszybndrfg8ejkmcpqxot1u"
139
}
1410

1511
@Test
@@ -45,51 +41,4 @@ class ContactImportFlowTest {
4541
resolveAddContactValidation(input = rawKey, ownPublicKey = null),
4642
)
4743
}
48-
49-
@Test
50-
fun `resolvePastedPubkyRoute returns profile for own key`() {
51-
assertEquals(
52-
Routes.Profile,
53-
resolvePastedPubkyRoute(
54-
input = VALID_PUBLIC_KEY,
55-
ownPublicKey = VALID_PUBLIC_KEY,
56-
contacts = emptyList(),
57-
),
58-
)
59-
}
60-
61-
@Test
62-
fun `resolvePastedPubkyRoute returns contact detail for existing contact`() {
63-
assertEquals(
64-
Routes.ContactDetail(VALID_PUBLIC_KEY),
65-
resolvePastedPubkyRoute(
66-
input = VALID_PUBLIC_KEY,
67-
ownPublicKey = OTHER_VALID_PUBLIC_KEY,
68-
contacts = listOf(PubkyProfile.placeholder(VALID_PUBLIC_KEY)),
69-
),
70-
)
71-
}
72-
73-
@Test
74-
fun `resolvePastedPubkyRoute returns add contact for unknown key`() {
75-
assertEquals(
76-
Routes.AddContact(VALID_PUBLIC_KEY),
77-
resolvePastedPubkyRoute(
78-
input = VALID_PUBLIC_KEY,
79-
ownPublicKey = OTHER_VALID_PUBLIC_KEY,
80-
contacts = emptyList(),
81-
),
82-
)
83-
}
84-
85-
@Test
86-
fun `resolvePastedPubkyRoute returns null for invalid input`() {
87-
assertNull(
88-
resolvePastedPubkyRoute(
89-
input = "not-a-pubky",
90-
ownPublicKey = null,
91-
contacts = emptyList(),
92-
),
93-
)
94-
}
9544
}

0 commit comments

Comments
 (0)