Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package dev.obiente.nextcloudnative

import android.content.Context
import dev.obiente.nextcloudnative.app.DynamicApiRequestCoalescer
import dev.obiente.nextcloudnative.app.AccountPrivateMemoryCleanup
import dev.obiente.nextcloudnative.app.DynamicApiRequestCoalescer
import dev.obiente.nextcloudnative.app.DynamicNativeMemoryAccountLifecycle
import dev.obiente.nextcloudnative.app.NextcloudSession
import dev.obiente.nextcloudnative.app.durableMutationAccountScope
import dev.obiente.nextcloudnative.app.removeAndroidHomeWorkspaceAccountPreferences
Expand Down Expand Up @@ -43,7 +44,14 @@ internal class AndroidAccountOwnedStateCleanup(
cacheIdentity,
clearPreviewAccount,
listOf(
{ fenceAndroidDynamicApiStateForRemoval(cacheIdentity, dynamicApiState.coalescer, dynamicApiState.cache) },
{
fenceAndroidDynamicApiStateForRemoval(
cacheIdentity,
dynamicApiState.coalescer,
dynamicApiState.cache,
session.accountId.storageKey,
)
},
{ dynamicDiscoveryCache.retireAccount(session.accountId.storageKey, cacheIdentity) },
{ removeSupportAccount(accountIdentity) },
{
Expand Down Expand Up @@ -83,8 +91,15 @@ internal class AndroidAccountOwnedStateCleanup(
clearPreviewAccount,
listOf(
{
previewCacheIdentity?.let { identity ->
fenceAndroidDynamicApiStateForRemoval(identity, dynamicApiState.coalescer, dynamicApiState.cache)
if (previewCacheIdentity == null) {
DynamicNativeMemoryAccountLifecycle.retireAccount(session.accountId.storageKey)
Comment on lines +94 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fence legacy dynamic API state using the available session

When retrying a two-field legacy removal journal, previewCacheIdentity is null even though this overload has the exact session; this branch therefore retires only process memory and skips both the request-coalescer fence and dynamicApiState.cache.invalidateAccount(). saveSession() can then clear the cleanup journal and reactivate the account while its persisted dynamic responses under NextcloudDocumentIds.cacheAccountId(session) remain available, allowing private data from the prior account incarnation to be shown after the same server/login identity is added again. Derive the full cache identity from session and run the normal fenced cleanup for this legacy case.

AGENTS.md reference: AGENTS.md:L410-L413

Useful? React with 👍 / 👎.

} else {
fenceAndroidDynamicApiStateForRemoval(
previewCacheIdentity,
dynamicApiState.coalescer,
dynamicApiState.cache,
session.accountId.storageKey,
)
}
},
{ dynamicDiscoveryCache.retireAccount(session.accountId.storageKey, previewCacheIdentity) },
Expand Down Expand Up @@ -126,8 +141,15 @@ internal class AndroidAccountOwnedStateCleanup(
clearPreviewAccount,
listOf(
{
previewCacheIdentity?.let { identity ->
fenceAndroidDynamicApiStateForRemoval(identity, dynamicApiState.coalescer, dynamicApiState.cache)
if (previewCacheIdentity == null) {
DynamicNativeMemoryAccountLifecycle.retireAccount(accountStorageKey)
} else {
fenceAndroidDynamicApiStateForRemoval(
previewCacheIdentity,
dynamicApiState.coalescer,
dynamicApiState.cache,
accountStorageKey,
)
}
},
{ dynamicDiscoveryCache.retireAccount(accountStorageKey, previewCacheIdentity) },
Expand Down Expand Up @@ -156,20 +178,28 @@ internal class AndroidAccountOwnedStateCleanup(
),
)
}

}

internal suspend fun <T> clearAndroidDynamicApiState(
accountIdentity: String,
coalescer: DynamicApiRequestCoalescer<T>,
cache: DynamicApiResponseCache,
) = coalescer.fenceAccount(accountIdentity) { cache.invalidateAccount(accountIdentity) }
accountStorageKey: String? = null,
retireMemoryAccount: (String) -> Unit = DynamicNativeMemoryAccountLifecycle::retireAccount,
) = coalescer.fenceAccount(accountIdentity) {
accountStorageKey?.let(retireMemoryAccount)
cache.invalidateAccount(accountIdentity)
}

internal suspend fun <T> fenceAndroidDynamicApiStateForRemoval(
accountIdentity: String,
coalescer: DynamicApiRequestCoalescer<T>,
cache: DynamicApiResponseCache,
accountStorageKey: String? = null,
retireMemoryAccount: (String) -> Unit = DynamicNativeMemoryAccountLifecycle::retireAccount,
) = withContext(NonCancellable) {
clearAndroidDynamicApiState(accountIdentity, coalescer, cache)
clearAndroidDynamicApiState(accountIdentity, coalescer, cache, accountStorageKey, retireMemoryAccount)
}

internal suspend fun runAndroidAccountOwnedStateCleanups(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package dev.obiente.nextcloudnative

import dev.obiente.nextcloudnative.app.DynamicApiRequestCoalescer
import dev.obiente.nextcloudnative.app.DynamicNativeMemoryAccountLifecycle
import dev.obiente.nextcloudnative.app.NextcloudApiResponse
import dev.obiente.nextcloudnative.app.NextcloudSession

internal class AndroidDynamicAccountActivation(
private val coalescer: DynamicApiRequestCoalescer<NextcloudApiResponse>,
private val activateMemory: (String) -> Unit = DynamicNativeMemoryAccountLifecycle::activateAccount,
) {
suspend fun afterCredentialSave(persistedSession: NextcloudSession) {
activateMemory(persistedSession.accountId.storageKey)
coalescer.activateAccount(NextcloudDocumentIds.cacheAccountId(persistedSession))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import android.provider.Settings
import android.util.Base64
import android.util.Log
import dev.obiente.nextcloudnative.app.AcquiredOpenApiContract
import dev.obiente.nextcloudnative.app.AccountPrivateMemoryLifecycle
import dev.obiente.nextcloudnative.app.AcquiredOpenApiContractSourceKind
import dev.obiente.nextcloudnative.app.AcquiredContractKind
import dev.obiente.nextcloudnative.app.DeckAttachment
Expand Down Expand Up @@ -464,6 +463,7 @@ internal class AndroidNextcloudServices(
diagnostics = supportDiagnostics,
client = httpClient,
)
private val dynamicAccountActivation = AndroidDynamicAccountActivation(dynamicApiRequestCoalescer)
private val accountCredentials = AndroidAccountCredentialController(
context = appContext,
preferences = preferences,
Expand All @@ -482,8 +482,7 @@ internal class AndroidNextcloudServices(
retryQueuedUploadsCleanup = accountOwnedStateCleanup::retry,
retryQueuedUploadsCleanupWithoutCredentials = accountOwnedStateCleanup::retryWithoutCredentials,
activatePersistedAccount = { session ->
dynamicApiRequestCoalescer.activateAccount(NextcloudDocumentIds.cacheAccountId(session))
AccountPrivateMemoryLifecycle.activateAccount(session.accountId.storageKey)
dynamicAccountActivation.afterCredentialSave(session)
dynamicDiscoveryCache.activateAccount(session.accountId.storageKey)
},
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package dev.obiente.nextcloudnative

import dev.obiente.nextcloudnative.app.DynamicApiRequestCoalescer
import dev.obiente.nextcloudnative.app.NextcloudApiResponse
import dev.obiente.nextcloudnative.app.NextcloudSession
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlinx.coroutines.runBlocking

class AndroidDynamicAccountActivationTest {
@Test
fun currentCredentialSaveReopensBothDynamicCaches() = runBlocking {
val session = NextcloudSession("https://cloud.example.test", "alice", "password")
val cacheAccountId = NextcloudDocumentIds.cacheAccountId(session)
val coalescer = DynamicApiRequestCoalescer<NextcloudApiResponse>()
coalescer.fenceAccount(cacheAccountId) {}
var activatedMemoryAccount: String? = null
val activation = AndroidDynamicAccountActivation(
coalescer = coalescer,
activateMemory = { activatedMemoryAccount = it },
)

activation.afterCredentialSave(session)

assertEquals(session.accountId.storageKey, activatedMemoryAccount)
assertEquals(
200,
coalescer.execute(cacheAccountId, "GET /status", load = {
NextcloudApiResponse(200, byteArrayOf(), null, null)
}).status,
)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dev.obiente.nextcloudnative
import dev.obiente.nextcloudnative.app.DynamicApiRequestCoalescer
import dev.obiente.nextcloudnative.app.NextcloudApiCachePolicy
import dev.obiente.nextcloudnative.app.NextcloudApiResponse
import dev.obiente.nextcloudnative.app.NextcloudSession
import dev.obiente.nextcloudnative.contracts.CachedDynamicApiResponse
import dev.obiente.nextcloudnative.contracts.DynamicApiResponseCache
import java.nio.file.Files
Expand All @@ -16,6 +17,7 @@ import kotlinx.coroutines.supervisorScope
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFails
import kotlin.test.assertFailsWith
import kotlin.test.assertNull
import kotlin.test.assertSame

Expand Down Expand Up @@ -103,6 +105,8 @@ class AndroidDynamicApiCachePolicyTest {
val requestIdentity = "GET /dashboard/widgets"
val cache = DynamicApiResponseCache(root)
val coalescer = DynamicApiRequestCoalescer<CachedDynamicApiResponse>()
val session = NextcloudSession("https://cloud.example.test", "alice", "password")
var retiredStorageKey: String? = null
val started = CompletableDeferred<Unit>()
val release = CompletableDeferred<Unit>()
val response = CachedDynamicApiResponse(200, "private".encodeToByteArray(), null, null)
Expand All @@ -116,11 +120,18 @@ class AndroidDynamicApiCachePolicyTest {
}
started.await()

clearAndroidDynamicApiState(accountId, coalescer, cache)
clearAndroidDynamicApiState(
accountId,
coalescer,
cache,
session.accountId.storageKey,
{ retiredStorageKey = it },
)
release.complete(Unit)

assertFails { read.await() }
assertNull(cache.load(accountId, requestIdentity, 1_024))
assertEquals(session.accountId.storageKey, retiredStorageKey)
} finally {
root.deleteRecursively()
}
Expand Down Expand Up @@ -156,6 +167,37 @@ class AndroidDynamicApiCachePolicyTest {
}
}

@Test
fun `Android memory retirement survives a rejected disk cache purge`(): Unit = runBlocking {
val root = Files.createTempDirectory("android-dynamic-cache-rejected-cleanup-").toFile()
try {
val accountId = "e".repeat(64)
val accountDirectory = root.resolve(accountId).apply { mkdirs() }
accountDirectory.resolve("unsafe-entry").mkdir()
val coalescer = DynamicApiRequestCoalescer<CachedDynamicApiResponse>()
val cache = DynamicApiResponseCache(root)
val accountStorageKey = "f".repeat(64)
var retiredStorageKey: String? = null

assertFailsWith<IllegalStateException> {
clearAndroidDynamicApiState(
accountId,
coalescer,
cache,
accountStorageKey,
{ retiredStorageKey = it },
)
}

assertEquals(accountStorageKey, retiredStorageKey)
assertFailsWith<Exception> {
coalescer.execute(accountId, "GET /dashboard/widgets", load = { error("must remain fenced") })
}
} finally {
root.deleteRecursively()
}
}

@Test
fun `second Android service cannot commit a GET that crossed account removal`() = runBlocking {
supervisorScope {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ class AndroidDynamicDiscoveryCacheRetirementTest {
val removedCacheId = "1".repeat(64)
val retainedStorageKey = "b".repeat(64)
val retainedCacheId = "2".repeat(64)
val removedProducer = DynamicNativeMemoryCacheProducer(removedStorageKey, 0L)
val retainedProducer = DynamicNativeMemoryCacheProducer(retainedStorageKey, 0L)
val removedProducer = producerForTest(removedStorageKey, 0L)
val retainedProducer = producerForTest(retainedStorageKey, 0L)
try {
cache.save(removedStorageKey, removedCacheId, "deck", "removed", removedProducer)
cache.save(retainedStorageKey, retainedCacheId, "deck", "retained", retainedProducer)
Expand All @@ -34,7 +34,7 @@ class AndroidDynamicDiscoveryCacheRetirementTest {

cache.save(
removedStorageKey, removedCacheId, "deck", "current",
DynamicNativeMemoryCacheProducer(removedStorageKey, 1L),
producerForTest(removedStorageKey, 1L),
)
assertEquals("current", cache.load(removedStorageKey, removedCacheId, "deck"))
} finally {
Expand All @@ -49,11 +49,11 @@ class AndroidDynamicDiscoveryCacheRetirementTest {
try {
cache.save(
"a".repeat(64), "1".repeat(64), "deck", "first",
DynamicNativeMemoryCacheProducer("a".repeat(64), 0L),
producerForTest("a".repeat(64), 0L),
)
cache.save(
"b".repeat(64), "2".repeat(64), "talk", "second",
DynamicNativeMemoryCacheProducer("b".repeat(64), 0L),
producerForTest("b".repeat(64), 0L),
)

cache.retireAccount("a".repeat(64), null)
Expand All @@ -63,4 +63,9 @@ class AndroidDynamicDiscoveryCacheRetirementTest {
root.deleteRecursively()
}
}

private fun producerForTest(accountStorageKey: String, incarnation: Long): DynamicNativeMemoryCacheProducer =
DynamicNativeMemoryCacheProducer::class.java
.getDeclaredConstructor(String::class.java, java.lang.Long.TYPE)
.newInstance(accountStorageKey, incarnation)
}
7 changes: 7 additions & 0 deletions changes/unreleased/dynamic-memory-account-retirement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
category: fix
issue: 172
pull: none
platforms: android, desktop
user-facing: yes

Account removal clears dynamic app memory and rejects late responses even after the account is added again. Pending cleanup from older desktop versions no longer prevents unrelated accounts from loading.
6 changes: 3 additions & 3 deletions tools/kotlin-file-size-baseline.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt|851
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt|4230
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt|4229
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidProjectContentClient.kt|985
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt|995
contractAcquisition/src/main/kotlin/dev/obiente/nextcloudnative/contracts/SignedAppStoreContractAcquirer.kt|1224
Expand All @@ -25,7 +25,7 @@ ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckBoardSurface.
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckRelationDialogs.kt|1234
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckScreen.kt|1883
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt|1331
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|12348
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|12332
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt|1693
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt|808
ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt|1717
Expand Down Expand Up @@ -53,7 +53,7 @@ ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileReadCache.k
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncEngine.kt|884
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncRemoteTree.kt|883
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncStore.kt|808
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6236
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6216
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopVirtualRangeCache.kt|2759
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/LinuxVirtualFileSystem.kt|1697
ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt|1085
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ internal class DynamicNativeMemoryCache(
}
}

data class DynamicNativeMemoryCacheProducer(
class DynamicNativeMemoryCacheProducer internal constructor(
val accountStorageKey: String,
val incarnation: Long,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,24 @@ class AccountPrivateMemoryCleanupTest {
AccountPrivateMemoryLifecycle.activateAccount(retainedKey)
val removedPreview = PreviewCacheKey(removedKey, "core", 1L, "etag", 64, 64)
val retainedPreview = PreviewCacheKey(retainedKey, "core", 2L, "etag", 64, 64)
val removedDynamicKey = dynamicKey(removed)
val retainedDynamicKey = dynamicKey(retained)
val removedPhotoState = PhotoTimelineUiStateRepository.stateFor(removed)
val retainedPhotoState = PhotoTimelineUiStateRepository.stateFor(retained)
val removedProducer = sharedAccountPrivateMemoryGate.producer(removedKey)
val retainedProducer = sharedAccountPrivateMemoryGate.producer(retainedKey)
val removedDynamicProducer = sharedDynamicNativeMemoryCache.producer(removed)
val retainedDynamicProducer = sharedDynamicNativeMemoryCache.producer(retained)
val removedDynamicProducer = requireNotNull(sharedDynamicNativeMemoryCache.producer(removedDynamicKey))
val retainedDynamicProducer = requireNotNull(sharedDynamicNativeMemoryCache.producer(retainedDynamicKey))
try {
PreviewMemoryCache.put(removedPreview, byteArrayOf(1), removedProducer)
PreviewMemoryCache.put(retainedPreview, byteArrayOf(2), retainedProducer)
sharedNextcloudNotesCache.storeDetail(removed, note(1L, "Removed"), removedProducer)
sharedNextcloudNotesCache.storeDetail(retained, note(2L, "Retained"), retainedProducer)
sharedDynamicNativeMemoryCache.storeScreen(
dynamicKey(removed), dynamicSnapshot(1), removedDynamicProducer,
removedDynamicKey, dynamicSnapshot(1), removedDynamicProducer,
)
sharedDynamicNativeMemoryCache.storeScreen(
dynamicKey(retained), dynamicSnapshot(2), retainedDynamicProducer,
retainedDynamicKey, dynamicSnapshot(2), retainedDynamicProducer,
)
sharedDashboardStatusMemoryCache.store(
removed, NativeDashboardSnapshot(emptyList(), emptyMap()), null, 1L, removedProducer,
Expand Down Expand Up @@ -76,8 +78,8 @@ class AccountPrivateMemoryCleanupTest {
assertContentEquals(byteArrayOf(2), PreviewMemoryCache.get(retainedPreview))
assertNull(sharedNextcloudNotesCache.detail(removed, 1L))
assertEquals("Retained", sharedNextcloudNotesCache.detail(retained, 2L)?.title)
assertNull(sharedDynamicNativeMemoryCache.screen(dynamicKey(removed)))
assertNotNull(sharedDynamicNativeMemoryCache.screen(dynamicKey(retained)))
assertNull(sharedDynamicNativeMemoryCache.screen(removedDynamicKey))
assertNotNull(sharedDynamicNativeMemoryCache.screen(retainedDynamicKey))
assertNull(sharedDashboardStatusMemoryCache.get(removed, 1L))
assertNotNull(sharedDashboardStatusMemoryCache.get(retained, 1L))
assertNull(ContactsWorkspaceMemoryCache.get(removed, "removed"))
Expand Down
Loading