fix(kanban): preserve transfer cleanup and screen lifecycle (#1137)
This commit is contained in:
1 parent
748f7854d2
commit
6f076a79d3
6 files changed
+295
-36
No files matched your search
@@ -0,0 +1,81 @@
|
||||
package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.runtime.MutableState
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
import androidx.compose.ui.test.junit4.StateRestorationTester
|
||||
import androidx.compose.ui.test.junit4.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.LifecycleRegistry
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class KanbanLifecycleTest {
|
||||
@get:Rule
|
||||
val compose = createComposeRule()
|
||||
|
||||
@Test
|
||||
fun pendingExportSurvivesSavedStateRestoration() {
|
||||
val restoration = StateRestorationTester(compose)
|
||||
lateinit var path: MutableState<String?>
|
||||
restoration.setContent {
|
||||
path = rememberPendingKanbanExportPath()
|
||||
}
|
||||
compose.runOnIdle { path.value = "/cache/board.tar.gz" }
|
||||
restoration.emulateSavedInstanceStateRestore()
|
||||
compose.runOnIdle { assertEquals("/cache/board.tar.gz", path.value) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tickerStopsInBackgroundRefreshesOnResumeAndStopsOnDisposal() {
|
||||
lateinit var registry: LifecycleRegistry
|
||||
val owner =
|
||||
object : LifecycleOwner {
|
||||
override val lifecycle: Lifecycle get() = registry
|
||||
}
|
||||
compose.runOnUiThread {
|
||||
registry = LifecycleRegistry(owner)
|
||||
registry.currentState = Lifecycle.State.STARTED
|
||||
}
|
||||
var now = 100L
|
||||
val visible = mutableStateOf(true)
|
||||
var displayed = 0L
|
||||
compose.mainClock.autoAdvance = false
|
||||
compose.setContent {
|
||||
CompositionLocalProvider(LocalLifecycleOwner provides owner) {
|
||||
if (visible.value) {
|
||||
displayed = rememberKanbanNowSeconds(true) { now }
|
||||
Text("$displayed")
|
||||
}
|
||||
}
|
||||
}
|
||||
compose.mainClock.advanceTimeBy(32)
|
||||
compose.onNodeWithText("100").assertTextEquals("100")
|
||||
compose.runOnUiThread { now = 105L }
|
||||
compose.mainClock.advanceTimeBy(5_100)
|
||||
compose.onNodeWithText("105").assertTextEquals("105")
|
||||
compose.runOnUiThread {
|
||||
registry.currentState = Lifecycle.State.CREATED
|
||||
now = 200L
|
||||
}
|
||||
compose.mainClock.advanceTimeBy(10_100)
|
||||
compose.onNodeWithText("105").assertTextEquals("105")
|
||||
compose.runOnUiThread { registry.currentState = Lifecycle.State.STARTED }
|
||||
compose.mainClock.advanceTimeBy(64)
|
||||
compose.onNodeWithText("200").assertTextEquals("200")
|
||||
compose.runOnUiThread { visible.value = false }
|
||||
compose.mainClock.advanceTimeBy(64)
|
||||
compose.runOnUiThread { now = 300L }
|
||||
compose.mainClock.advanceTimeBy(10_100)
|
||||
compose.runOnIdle { assertEquals(200L, displayed) }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,10 @@
|
||||
package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.MutableState
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.saveable.rememberSaveable
|
||||
|
||||
/** Keep the cache path while the external document picker recreates the Activity (#1137). */
|
||||
@Composable
|
||||
internal fun rememberPendingKanbanExportPath(): MutableState<String?> = rememberSaveable { mutableStateOf(null) }
|
||||
@@ -0,0 +1,26 @@
|
||||
package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import kotlinx.coroutines.CoroutineDispatcher
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.NonCancellable
|
||||
import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.withTimeout
|
||||
|
||||
/** Own only confirmed uploads; cancellation must not strand their temporary archive (#1137). */
|
||||
internal suspend fun <T> withKanbanImportCleanup(
|
||||
cleanup: suspend () -> Unit,
|
||||
onCleanupFailure: (Exception) -> Unit,
|
||||
dispatcher: CoroutineDispatcher = Dispatchers.IO,
|
||||
operation: suspend () -> T,
|
||||
): T =
|
||||
try {
|
||||
operation()
|
||||
} finally {
|
||||
withContext(NonCancellable + dispatcher) {
|
||||
try {
|
||||
withTimeout(10_000L) { cleanup() }
|
||||
} catch (e: Exception) {
|
||||
onCleanupFailure(e)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2,8 +2,12 @@ package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.produceState
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import androidx.lifecycle.repeatOnLifecycle
|
||||
import com.m57.hermescontrol.data.model.KanbanTask
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.isActive
|
||||
|
||||
enum class KanbanArcState {
|
||||
QUEUED,
|
||||
@@ -54,13 +58,23 @@ object KanbanRuntimeHelper {
|
||||
|
||||
/** One coarse clock per board screen, rather than one coroutine per task card. */
|
||||
@Composable
|
||||
fun rememberKanbanNowSeconds(enabled: Boolean): Long =
|
||||
produceState(
|
||||
initialValue = System.currentTimeMillis() / 1000L,
|
||||
fun rememberKanbanNowSeconds(
|
||||
enabled: Boolean,
|
||||
clock: () -> Long = { System.currentTimeMillis() / 1000L },
|
||||
): Long {
|
||||
val lifecycle = LocalLifecycleOwner.current.lifecycle
|
||||
return produceState(
|
||||
initialValue = clock(),
|
||||
key1 = enabled,
|
||||
key2 = lifecycle,
|
||||
) {
|
||||
while (enabled) {
|
||||
delay(5_000L)
|
||||
value = System.currentTimeMillis() / 1000L
|
||||
if (enabled) {
|
||||
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
|
||||
while (isActive) {
|
||||
value = clock()
|
||||
delay(5_000L)
|
||||
}
|
||||
}
|
||||
}
|
||||
}.value
|
||||
}
|
||||
@@ -90,13 +90,16 @@ import com.m57.hermescontrol.ui.kanban.components.KanbanFilterSheet
|
||||
import com.m57.hermescontrol.ui.kanban.components.KanbanOrchestrationDialog
|
||||
import com.m57.hermescontrol.ui.kanban.components.KanbanTaskCard
|
||||
import com.m57.hermescontrol.util.StreamingUriRequestBody
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ensureActive
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import okhttp3.MediaType.Companion.toMediaTypeOrNull
|
||||
import okhttp3.MultipartBody
|
||||
import okhttp3.RequestBody.Companion.toRequestBody
|
||||
import java.io.File
|
||||
import kotlin.coroutines.coroutineContext
|
||||
|
||||
private const val DEFAULT_COLUMN = "todo"
|
||||
|
||||
@@ -159,26 +162,27 @@ fun KanbanScreen(
|
||||
var showOrchestrationDialog by remember { mutableStateOf(false) }
|
||||
|
||||
// Board Transfer (Export / Import)
|
||||
var pendingExportPath by remember { mutableStateOf<String?>(null) }
|
||||
var pendingExportPath by rememberPendingKanbanExportPath()
|
||||
|
||||
val exportLauncher =
|
||||
rememberLauncherForActivityResult(
|
||||
contract = ActivityResultContracts.CreateDocument("application/gzip"),
|
||||
) { uri ->
|
||||
if (uri != null && pendingExportPath != null) {
|
||||
if (uri != null) {
|
||||
coroutineScope.launch {
|
||||
try {
|
||||
val cachedFile = File(pendingExportPath!!)
|
||||
if (cachedFile.exists()) {
|
||||
withContext(Dispatchers.IO) {
|
||||
context.contentResolver.openOutputStream(uri)?.use { output ->
|
||||
cachedFile.inputStream().use { input ->
|
||||
input.copyTo(output)
|
||||
}
|
||||
} ?: error("Unable to open selected destination")
|
||||
}
|
||||
viewModel.showToast("Board exported successfully")
|
||||
val cachedFile = File(checkNotNull(pendingExportPath) { "No pending board export" })
|
||||
withContext(Dispatchers.IO) {
|
||||
check(cachedFile.isFile) { "Cached board export is missing; export the board again" }
|
||||
context.contentResolver.openOutputStream(uri)?.use { output ->
|
||||
cachedFile.inputStream().use { input ->
|
||||
input.copyTo(output)
|
||||
}
|
||||
} ?: error("Unable to open selected destination")
|
||||
}
|
||||
viewModel.showToast("Board exported successfully")
|
||||
} catch (e: CancellationException) {
|
||||
throw e
|
||||
} catch (e: Exception) {
|
||||
viewModel.showToast("Failed to save export: ${e.message}")
|
||||
} finally {
|
||||
@@ -230,39 +234,55 @@ fun KanbanScreen(
|
||||
contentLength = contentLength,
|
||||
)
|
||||
val filePart = MultipartBody.Part.createFormData("file", fileName, requestBody)
|
||||
// Retrofit streams the body on OkHttp's worker; avoid losing a confirmed
|
||||
// response to withContext's prompt cancellation on the return hop (#1137).
|
||||
val uploadResult =
|
||||
withContext(Dispatchers.IO) {
|
||||
safeApiCall {
|
||||
ApiClient.hermesApi.uploadManagedFileStream(pathBody, overwriteBody, filePart)
|
||||
}
|
||||
safeApiCall {
|
||||
ApiClient.hermesApi.uploadManagedFileStream(pathBody, overwriteBody, filePart)
|
||||
}
|
||||
when (uploadResult) {
|
||||
is NetworkResult.Success -> {
|
||||
val serverPath =
|
||||
uploadResult.data.entry?.path ?: uploadResult.data.path ?: targetPath
|
||||
val importResult = viewModel.importBoard(serverPath)
|
||||
when (importResult) {
|
||||
is NetworkResult.Success -> {
|
||||
val res = importResult.data
|
||||
var msg = "Board '${res.name}' imported"
|
||||
if (res.renamed) msg += " (as ${res.board})"
|
||||
viewModel.showToast(msg)
|
||||
viewModel.loadBoards()
|
||||
}
|
||||
withKanbanImportCleanup(
|
||||
cleanup = {
|
||||
val response =
|
||||
ApiClient.hermesApi.deleteManagedFile(
|
||||
ManagedFileDelete(serverPath),
|
||||
)
|
||||
check(
|
||||
response.isSuccessful,
|
||||
) { "Archive cleanup failed (HTTP ${response.code()})" }
|
||||
},
|
||||
onCleanupFailure = {
|
||||
viewModel.showToast("Temporary import archive cleanup failed: ${it.message}")
|
||||
},
|
||||
) {
|
||||
val importResult = viewModel.importBoard(serverPath)
|
||||
coroutineContext.ensureActive()
|
||||
when (importResult) {
|
||||
is NetworkResult.Success -> {
|
||||
val res = importResult.data
|
||||
var msg = "Board '${res.name}' imported"
|
||||
if (res.renamed) msg += " (as ${res.board})"
|
||||
viewModel.showToast(msg)
|
||||
viewModel.loadBoards()
|
||||
}
|
||||
|
||||
is NetworkResult.Failure -> {
|
||||
viewModel.showToast("Import failed: ${importResult.error.message}")
|
||||
is NetworkResult.Failure -> {
|
||||
viewModel.showToast("Import failed: ${importResult.error.message}")
|
||||
}
|
||||
}
|
||||
}
|
||||
runCatching {
|
||||
ApiClient.hermesApi.deleteManagedFile(ManagedFileDelete(serverPath))
|
||||
}
|
||||
}
|
||||
|
||||
is NetworkResult.Failure -> {
|
||||
coroutineContext.ensureActive()
|
||||
viewModel.showToast("Failed to upload archive: ${uploadResult.error.message}")
|
||||
}
|
||||
}
|
||||
} catch (e: CancellationException) {
|
||||
throw e
|
||||
} catch (e: Exception) {
|
||||
viewModel.showToast("Import failed: ${e.message}")
|
||||
}
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.awaitCancellation
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.StandardTestDispatcher
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertSame
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class KanbanImportCleanupTest {
|
||||
@Test
|
||||
fun cleansAfterSuccess() =
|
||||
runTest {
|
||||
var cleaned = false
|
||||
val result =
|
||||
withKanbanImportCleanup(
|
||||
cleanup = { cleaned = true },
|
||||
onCleanupFailure = { throw AssertionError(it) },
|
||||
dispatcher = StandardTestDispatcher(testScheduler),
|
||||
) { "imported" }
|
||||
assertEquals("imported", result)
|
||||
assertTrue(cleaned)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cleansAfterImportFailureWithoutMaskingIt() =
|
||||
runTest {
|
||||
val failure = IllegalStateException("import failed")
|
||||
var cleaned = false
|
||||
try {
|
||||
withKanbanImportCleanup(
|
||||
cleanup = { cleaned = true },
|
||||
onCleanupFailure = { throw AssertionError(it) },
|
||||
dispatcher = StandardTestDispatcher(testScheduler),
|
||||
) { throw failure }
|
||||
} catch (e: IllegalStateException) {
|
||||
assertSame(failure, e)
|
||||
}
|
||||
assertTrue(cleaned)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cancellationWaitsForSuspendingCleanupAndRemainsCancelled() =
|
||||
runTest {
|
||||
var cleaned = false
|
||||
var cancellationPropagated = false
|
||||
val job =
|
||||
launch {
|
||||
try {
|
||||
withKanbanImportCleanup(
|
||||
cleanup = {
|
||||
delay(100)
|
||||
cleaned = true
|
||||
},
|
||||
onCleanupFailure = { throw AssertionError(it) },
|
||||
dispatcher = StandardTestDispatcher(testScheduler),
|
||||
) { awaitCancellation() }
|
||||
} catch (e: CancellationException) {
|
||||
cancellationPropagated = true
|
||||
throw e
|
||||
}
|
||||
}
|
||||
runCurrent()
|
||||
job.cancel()
|
||||
advanceUntilIdle()
|
||||
assertTrue(cleaned)
|
||||
assertTrue(cancellationPropagated)
|
||||
assertTrue(job.isCancelled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cleanupFailureIsReportedWithoutMaskingImportResult() =
|
||||
runTest {
|
||||
val failure = IllegalStateException("HTTP 500")
|
||||
var reported: Exception? = null
|
||||
val result =
|
||||
withKanbanImportCleanup(
|
||||
cleanup = { throw failure },
|
||||
onCleanupFailure = { reported = it },
|
||||
dispatcher = StandardTestDispatcher(testScheduler),
|
||||
) { "imported" }
|
||||
assertEquals("imported", result)
|
||||
// Coroutine stack-trace recovery can copy exceptions across dispatcher boundaries.
|
||||
assertEquals(failure.message, reported?.message)
|
||||
assertTrue(reported is IllegalStateException)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cleanupIsBounded() =
|
||||
runTest {
|
||||
var reported = false
|
||||
withKanbanImportCleanup(
|
||||
cleanup = { awaitCancellation() },
|
||||
onCleanupFailure = { reported = true },
|
||||
dispatcher = StandardTestDispatcher(testScheduler),
|
||||
) { Unit }
|
||||
assertTrue(reported)
|
||||
assertEquals(10_000L, testScheduler.currentTime)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user