From 6b711ea983c5029e4a9df5fcd44fa42580d26479 Mon Sep 17 00:00:00 2001 From: Matt Ramotar Date: Sat, 10 Jan 2026 20:36:01 -0500 Subject: [PATCH 1/4] Fix MutableStore.write() ignoring SourceOfTruth write failures Signed-off-by: Matt Ramotar --- .../store/store5/impl/RealMutableStore.kt | 40 +++++++++++-------- .../store/store5/impl/RealStore.kt | 8 +++- .../store5/impl/SourceOfTruthWithBarrier.kt | 37 ++++++++++------- .../mutablestore/RealMutableStoreTest.kt | 28 +++++++++++++ 4 files changed, 80 insertions(+), 33 deletions(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt index 39c2ac833..6e74462c7 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt @@ -24,6 +24,7 @@ import org.mobilenativefoundation.store.store5.impl.extensions.now import org.mobilenativefoundation.store.store5.internal.concurrent.ThreadSafety import org.mobilenativefoundation.store.store5.internal.definition.WriteRequestQueue import org.mobilenativefoundation.store.store5.internal.result.EagerConflictResolutionResult +import org.mobilenativefoundation.store.store5.internal.result.StoreDelegateWriteResult @OptIn(ExperimentalStoreApi::class) internal class RealMutableStore( @@ -85,25 +86,30 @@ internal class RealMutableStore StoreWriteResponse. - when (updaterResult) { - is UpdaterResult.Error.Exception -> StoreWriteResponse.Error.Exception(updaterResult.error) - is UpdaterResult.Error.Message -> StoreWriteResponse.Error.Message(updaterResult.message) - is UpdaterResult.Success.Typed<*> -> { - val typedValue = updaterResult.value as? Response - if (typedValue == null) { - StoreWriteResponse.Success.Untyped(updaterResult.value) - } else { - StoreWriteResponse.Success.Typed(updaterResult.value) + // Only proceed to network if local write succeeded. + when (val delegateWriteResult = delegate.write(writeRequest.key, writeRequest.value)) { + is StoreDelegateWriteResult.Error.Exception -> { + StoreWriteResponse.Error.Exception(delegateWriteResult.error) + } + is StoreDelegateWriteResult.Error.Message -> { + StoreWriteResponse.Error.Message(delegateWriteResult.error) + } + is StoreDelegateWriteResult.Success -> { + // Try to sync to network. + when (val updaterResult = tryUpdateServer(writeRequest)) { + is UpdaterResult.Error.Exception -> StoreWriteResponse.Error.Exception(updaterResult.error) + is UpdaterResult.Error.Message -> StoreWriteResponse.Error.Message(updaterResult.message) + is UpdaterResult.Success.Typed<*> -> { + val typedValue = updaterResult.value as? Response + if (typedValue == null) { + StoreWriteResponse.Success.Untyped(updaterResult.value) + } else { + StoreWriteResponse.Success.Typed(updaterResult.value) + } + } + is UpdaterResult.Success.Untyped -> StoreWriteResponse.Success.Untyped(updaterResult.value) } } - - is UpdaterResult.Success.Untyped -> StoreWriteResponse.Success.Untyped(updaterResult.value) } } catch (throwable: Throwable) { StoreWriteResponse.Error.Exception(throwable) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt index d0198297c..9f70c5c8e 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt @@ -333,8 +333,12 @@ internal class RealStore( ): StoreDelegateWriteResult = try { memCache?.put(key, value) - sourceOfTruth?.write(key, converter.fromOutputToLocal(value)) - StoreDelegateWriteResult.Success + val writeException = sourceOfTruth?.write(key, converter.fromOutputToLocal(value)) + if (writeException != null) { + StoreDelegateWriteResult.Error.Exception(writeException) + } else { + StoreDelegateWriteResult.Success + } } catch (error: Throwable) { StoreDelegateWriteResult.Error.Exception(error) } diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt index a0a0953b9..de949f0e5 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt @@ -144,11 +144,18 @@ internal class SourceOfTruthWithBarrier( + key = key, + value = Note(key, "content"), + created = 3333L, + onCompletions = null, + ) + + // When + val response = mutableStore.write(request) + + // Then + val errorResponse = assertIs(response) + val writeException = assertIs(errorResponse.error) + val cause = assertIs(writeException.cause) + assertEquals(errorMessage, cause.message) + } + @Test fun clearAll_givenSomeKeys_whenCalled_thenDelegateIsCleared() = runTest { From ef1cda3c067e2ed4347e1b2ab7810c2d32d53340 Mon Sep 17 00:00:00 2001 From: Matt Ramotar Date: Sat, 10 Jan 2026 21:11:12 -0500 Subject: [PATCH 2/4] Run ktlintFormat Signed-off-by: Matt Ramotar --- .../store5/impl/SourceOfTruthWithBarrier.kt | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt index de949f0e5..d791757a7 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/SourceOfTruthWithBarrier.kt @@ -172,14 +172,15 @@ internal class SourceOfTruthWithBarrier Date: Sat, 10 Jan 2026 21:51:11 -0500 Subject: [PATCH 3/4] Add coverage Signed-off-by: Matt Ramotar --- .../mutablestore/RealMutableStoreTest.kt | 59 +++++++++++++++++++ .../store5/mutablestore/util/TestStore.kt | 2 +- .../store5/mutablestore/util/TestUpdater.kt | 2 + 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt index 2418d30fe..4cb660086 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt @@ -325,6 +325,65 @@ class RealMutableStoreTest { assertEquals(errorMessage, cause.message) } + @Test + fun write_givenSourceOfTruthFailure_whenCalled_thenNetworkSyncNotAttempted() = + runTest { + // Given + val key = "key" + testUpdater.postCallCount = 0 + testSourceOfTruth.throwOnWrite(key) { IllegalStateException("SOT failure") } + + val request = + StoreWriteRequest.of( + key = key, + value = Note(key, "content"), + created = 4444L, + onCompletions = null, + ) + + // When + val response = mutableStore.write(request) + + // Then + assertIs(response) + assertEquals(0, testUpdater.postCallCount, "Network updater should not be called when SOT write fails") + } + + @Test + fun write_givenNoSourceOfTruth_whenCalled_thenSucceeds() = + runTest { + // Given + val storeWithoutSot = + testStore( + fetcher = testFetcher, + sourceOfTruth = null, + converter = testConverter, + validator = testValidator, + memoryCache = testCache, + ) + val mutableStoreWithoutSot = + RealMutableStore( + delegate = storeWithoutSot, + updater = testUpdater, + bookkeeper = testBookkeeper, + logger = testLogger, + ) + + val request = + StoreWriteRequest.of( + key = "noSotKey", + value = Note("id", "content"), + created = 5555L, + onCompletions = null, + ) + + // When + val response = mutableStoreWithoutSot.write(request) + + // Then + assertIs(response) + } + @Test fun clearAll_givenSomeKeys_whenCalled_thenDelegateIsCleared() = runTest { diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestStore.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestStore.kt index 1a2fb1f5a..62504297c 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestStore.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestStore.kt @@ -14,7 +14,7 @@ internal fun testStore( dispatcher: CoroutineDispatcher = Dispatchers.Default, scope: CoroutineScope = CoroutineScope(dispatcher), fetcher: Fetcher = TestFetcher(), - sourceOfTruth: SourceOfTruth = TestSourceOfTruth(), + sourceOfTruth: SourceOfTruth? = TestSourceOfTruth(), converter: Converter = TestConverter(), validator: Validator = TestValidator(), memoryCache: Cache = TestCache(), diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestUpdater.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestUpdater.kt index 802faa34d..f83dc8936 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestUpdater.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/util/TestUpdater.kt @@ -8,11 +8,13 @@ class TestUpdater : Updater Date: Sat, 10 Jan 2026 22:18:48 -0500 Subject: [PATCH 4/4] Update memory cache after writing to SOT Signed-off-by: Matt Ramotar --- .../store/store5/impl/RealStore.kt | 2 +- .../mutablestore/RealMutableStoreTest.kt | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt index 9f70c5c8e..20f7fe8b9 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt @@ -332,11 +332,11 @@ internal class RealStore( value: Output, ): StoreDelegateWriteResult = try { - memCache?.put(key, value) val writeException = sourceOfTruth?.write(key, converter.fromOutputToLocal(value)) if (writeException != null) { StoreDelegateWriteResult.Error.Exception(writeException) } else { + memCache?.put(key, value) StoreDelegateWriteResult.Success } } catch (error: Throwable) { diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt index 4cb660086..734193b74 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/RealMutableStoreTest.kt @@ -349,6 +349,29 @@ class RealMutableStoreTest { assertEquals(0, testUpdater.postCallCount, "Network updater should not be called when SOT write fails") } + @Test + fun write_givenSourceOfTruthFailure_whenCalled_thenMemCacheNotUpdated() = + runTest { + // Given + val key = "key" + testSourceOfTruth.throwOnWrite(key) { IllegalStateException("SOT failure") } + + val request = + StoreWriteRequest.of( + key = key, + value = Note(key, "content"), + created = 6666L, + onCompletions = null, + ) + + // When + val response = mutableStore.write(request) + + // Then + assertIs(response) + assertNull(delegateStore.latestOrNull(key), "Value should not be in cache after SOT write failure") + } + @Test fun write_givenNoSourceOfTruth_whenCalled_thenSucceeds() = runTest {