Give each codec its own staging file

kstore-file/api/desktop/kstore-file.api

@@ -4,6 +4,10 @@ public final class io/github/xxfast/kstore/file/FileCodec : io/github/xxfast/kst
 	public fun encode (Ljava/lang/Object;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 }
 
+public final class io/github/xxfast/kstore/file/FileCodecKt {
+	public static final fun uniqueTempFile (Lkotlinx/io/files/Path;)Lkotlinx/io/files/Path;
+}
+
 public final class io/github/xxfast/kstore/file/extensions/KVersionedStoreKt {
 	public static final fun DefaultMigration (Ljava/lang/Object;)Lkotlin/jvm/functions/Function2;
 }

kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt

@@ -3,6 +3,7 @@ package io.github.xxfast.kstore.file
 
 import io.github.xxfast.kstore.Codec
 import io.github.xxfast.kstore.DefaultJson
+import kotlin.uuid.Uuid
 import kotlinx.io.buffered
 import kotlinx.io.files.FileNotFoundException
 import kotlinx.io.files.Path
@@ -19,12 +20,14 @@ import kotlinx.serialization.json.io.encodeToSink as encode
 /**
  * Creates a store with [FileCodec] with json serializer
  * @param file path to the file that is managed by this store
+ * @param tempFile staging file this codec writes through. Defaults to a path unique to this codec,
+ * so that codecs sharing a [file] never stage into the same buffer - see [uniqueTempFile]
  * @param json JSON Serializer to use. defaults to [DefaultJson]
  * @return store that contains a value of type [T]
  */
 public inline fun <reified T : @Serializable Any> FileCodec(
   file: Path,
-  tempFile: Path = Path("$file.temp"),
+  tempFile: Path = uniqueTempFile(file),
   json: Json = DefaultJson,
 ): FileCodec<T> = FileCodec(
   file = file,
@@ -79,6 +82,30 @@ public class FileCodec<T : @Serializable Any>(
   }
 }
 
+/**
+ * A staging path unique to a single codec, of the form `<file>.<random>.temp`.
+ *
+ * Stores pointing at the same [file] each get their own staging file. Sharing one lets concurrent
+ * writes interleave into a single buffer before either is moved into place, and whichever moves
+ * last publishes the mixture, which is how a store ends up corrupt on disk (see issue #85). With a
+ * staging file each, the move stays atomic and the worst case is last write wins.
+ *
+ * Note this is one staging file per codec, not per write, so a process killed mid-write leaves at
+ * most one behind per store. The next write is unaffected either way.
+ *
+ * The suffix comes from [Uuid.random] rather than [kotlin.random.Random] on purpose. Random.Default
+ * is a PRNG seeded from the clock on some targets, so two processes cold starting in the same tick
+ * can draw the same sequence and land on the same staging path, which is the one case this is meant
+ * to prevent. Uuid.random draws from the platform's secure source and has no shared seed.
+ *
+ * @param file path to the file being staged for
+ * @return a staging path that no other codec will pick
+ */
+public fun uniqueTempFile(file: Path): Path {
+  val suffix: String = Uuid.random().toHexString()
+  return Path("$file.$suffix.temp")
+}
+
 /**
  * Moves [source] onto [destination] atomically via [atomicMove].
  *

kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt

@@ -4,6 +4,7 @@ import io.github.xxfast.kstore.Codec
 import io.github.xxfast.kstore.DefaultJson
 import io.github.xxfast.kstore.KStore
 import io.github.xxfast.kstore.file.moveOrCopy
+import io.github.xxfast.kstore.file.uniqueTempFile
 import io.github.xxfast.kstore.storeOf
 import kotlinx.io.buffered
 import kotlinx.io.files.FileNotFoundException
@@ -61,8 +62,8 @@ public class VersionedCodec<T : @Serializable Any>(
   private val serializer: KSerializer<T>,
   private val migration: Migration<T>,
   private val versionPath: Path = Path("$file.version"), // TODO: Save to file metadata instead
-  private val tempPath: Path = Path("$file.temp"),
-  private val tempVersionPath: Path = Path("$versionPath.temp"),
+  private val tempPath: Path = uniqueTempFile(file),
+  private val tempVersionPath: Path = uniqueTempFile(versionPath),
 ) : Codec<T> {
 
   /**

kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt

@@ -35,10 +35,16 @@ class FileCodecTests {
         .use { DefaultJson.encodeToSink(value, it) }
     }
 
+  /** Staging files left beside the store, which should be none once a write settles */
+  private fun stagingFiles(): List<Path> =
+    SystemFileSystem.list(Path(FILE_PATH).parent ?: Path("."))
+      .filter { it.name.startsWith(Path(FILE_PATH).name) && it.name.endsWith(".temp") }
+
   @AfterTest
   fun cleanUp() {
     SystemFileSystem.delete(Path(FILE_PATH), false)
     SystemFileSystem.delete(Path("$FILE_PATH.temp"), false)
+    stagingFiles().forEach { SystemFileSystem.delete(it, false) }
   }
 
   @Test
@@ -83,6 +89,61 @@ class FileCodecTests {
     assertFailsWith<SerializationException> { codec.decode() }
   }
 
+  // Codecs sharing a file must not share a staging file, or concurrent writes interleave into one
+  // buffer and whichever moves last publishes the mixture - see issue #85
+
+  @Test
+  fun testUniqueTempFileDiffersEveryCall() {
+    val file = Path(FILE_PATH)
+    val staging: Set<Path> = List(100) { uniqueTempFile(file) }.toSet()
+    assertEquals(100, staging.size)
+  }
+
+  @Test
+  fun testUniqueTempFileStagesBesideTheTargetFile() {
+    // atomicMove is only atomic within one filesystem, so staging has to sit next to the target
+    val file = Path("some/dir/pets.json")
+    assertEquals(Path("some/dir").toString(), uniqueTempFile(file).parent.toString())
+  }
+
+  @Test
+  fun testCodecDoesNotStageThroughTheSharedPath() = runTest {
+    // The path every codec used to stage through. Stand in for another codec's in-flight write.
+    val shared = Path("$FILE_PATH.temp")
+    SystemFileSystem.sink(shared).buffered().use { it.writeString("another codec's staging") }
+
+    codec.encode(listOf(MYLO))
+
+    // This codec staged somewhere of its own, so it neither read nor consumed the other write
+    assertEquals(listOf(MYLO), codec.decode())
+    assertEquals(true, SystemFileSystem.exists(shared))
+  }
+
+  @Test
+  fun testSeparateCodecsOnSameFileDoNotShareStaging() = runTest {
+    val other: FileCodec<List<Pet>> = FileCodec(file = Path(FILE_PATH))
+
+    codec.encode(listOf(MYLO))
+    other.encode(listOf(OREO))
+
+    // Last write wins, and it is a whole document rather than a mixture of both
+    assertEquals(listOf(OREO), codec.decode())
+    assertEquals(emptyList(), stagingFiles())
+  }
+
+  @Test
+  fun testFailedEncodeLeavesNoStagingBehind() = runTest {
+    val other: FileCodec<List<Pet>> = FileCodec(file = Path(FILE_PATH))
+    codec.encode(listOf(MYLO))
+
+    // KAT's serializer throws part way through, so this codec fails mid-write
+    assertFailsWith<NotImplementedError> { other.encode(listOf(MYLO, KAT)) }
+
+    // The failure cleans up after itself and leaves the other codec's file intact
+    assertEquals(listOf(MYLO), codec.decode())
+    assertEquals(emptyList(), stagingFiles())
+  }
+
   @Test
   fun testMoveFallsBackToCopyWhenAtomicMoveUnsupported() = runTest {
     val tempFile = Path("$FILE_PATH.temp")

kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt

@@ -210,6 +210,21 @@ class KVersionedStoreTests {
     assertEquals(null, storeV2.get())
   }
 
+  @Test
+  fun testCodecDoesNotStageThroughTheSharedPaths() = runTest {
+    // The paths every versioned codec used to stage through - see issue #85
+    val sharedData = Path("$file.temp")
+    val sharedVersion = Path("$versionFile.temp")
+    write(sharedData, "another codec's staging")
+    write(sharedVersion, "another codec's staging")
+
+    storeV2.set(MYLO_V2)
+
+    assertEquals(MYLO_V2, storeV2.get())
+    assertEquals(true, SystemFileSystem.exists(sharedData))
+    assertEquals(true, SystemFileSystem.exists(sharedVersion))
+  }
+
   @Test
   fun testTransactionalEncode() = runTest {
     assertFailsWith<NotImplementedError> { storeV41.set(MYLO_V41) }