diff --git a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt index b5c0bfa..3aff99e 100644 --- a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt +++ b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt @@ -57,6 +57,8 @@ public class FileCodec( * If the value is null, the file is deleted. * If the encoding fails, the temp file is deleted. * If the encoding succeeds, the temp file is atomically moved to the target file - completing the transaction. + * On platforms where atomic move is not supported (e.g., Android 7 and below) this falls back to a + * non-atomic copy-and-delete; the transactional guarantee does not hold for that fallback path. * @param value optional value to encode */ @OptIn(ExperimentalSerializationApi::class) @@ -73,6 +75,38 @@ public class FileCodec( throw e } - SystemFileSystem.atomicMove(source = tempFile, destination = file) + moveOrCopy(source = tempFile, destination = file) + } +} + +/** + * Moves [source] onto [destination] atomically via [atomicMove]. + * + * On platforms where atomic move is unsupported (e.g., Android 7 and below - see issue #137) this falls + * back to a non-atomic copy-and-delete. The fallback is not transactional: a crash mid-copy can leave the + * destination partially written. On a failed copy the (possibly corrupt) destination is removed. + * + * The [atomicMove] parameter is a seam over SystemFileSystem.atomicMove so the fallback path can be exercised in tests. + */ +internal fun moveOrCopy( + source: Path, + destination: Path, + atomicMove: (source: Path, destination: Path) -> Unit = SystemFileSystem::atomicMove, +) { + try { + atomicMove(source, destination) + } catch (_: UnsupportedOperationException) { + try { + SystemFileSystem.source(source).buffered().use { from -> + SystemFileSystem.sink(destination).buffered().use { to -> + from.transferTo(to) + } + } + } catch (e: Throwable) { + SystemFileSystem.delete(destination, mustExist = false) + throw e + } finally { + SystemFileSystem.delete(source, mustExist = false) + } } } diff --git a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt index c96a31c..e376b33 100644 --- a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt +++ b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt @@ -3,6 +3,7 @@ package io.github.xxfast.kstore.file import io.github.xxfast.kstore.DefaultJson import kotlinx.coroutines.test.runTest import kotlinx.io.buffered +import kotlinx.io.files.FileNotFoundException import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem import kotlinx.io.writeString @@ -37,6 +38,7 @@ class FileCodecTests { @AfterTest fun cleanUp() { SystemFileSystem.delete(Path(FILE_PATH), false) + SystemFileSystem.delete(Path("$FILE_PATH.temp"), false) } @Test @@ -80,4 +82,39 @@ class FileCodecTests { SystemFileSystem.sink(Path(FILE_PATH)).buffered().use { it.writeString("💩") } assertFailsWith { codec.decode() } } + + @Test + fun testMoveFallsBackToCopyWhenAtomicMoveUnsupported() = runTest { + val tempFile = Path("$FILE_PATH.temp") + SystemFileSystem.sink(tempFile).buffered().use { DefaultJson.encodeToSink>(listOf(MYLO), it) } + + // Simulate platforms (e.g. Android 7 and below) where atomicMove throws - see issue #137 + moveOrCopy( + source = tempFile, + destination = Path(FILE_PATH), + atomicMove = { _, _ -> throw UnsupportedOperationException("Atomic move not supported") }, + ) + + // The value is persisted via the copy-and-delete fallback ... + assertEquals(listOf(MYLO), stored) + // ... and the temp file is cleaned up afterwards. + assertEquals(false, SystemFileSystem.exists(tempFile)) + } + + @Test + fun testMoveDeletesCorruptDestinationWhenCopyFails() = runTest { + val tempFile = Path("$FILE_PATH.temp") // intentionally never created, so the fallback copy fails + SystemFileSystem.sink(Path(FILE_PATH)).buffered().use { it.writeString("stale") } // pre-existing destination + + assertFailsWith { + moveOrCopy( + source = tempFile, + destination = Path(FILE_PATH), + atomicMove = { _, _ -> throw UnsupportedOperationException("Atomic move not supported") }, + ) + } + + // The possibly-corrupt destination is removed rather than left half-written. + assertEquals(false, SystemFileSystem.exists(Path(FILE_PATH))) + } }