diff --git a/store/api/android/store.api b/store/api/android/store.api index 99c14d7fe..0c84c4f9b 100644 --- a/store/api/android/store.api +++ b/store/api/android/store.api @@ -123,6 +123,15 @@ public final class org/mobilenativefoundation/store/store5/FetcherResult$Error$M public fun toString ()Ljava/lang/String; } +public abstract interface class org/mobilenativefoundation/store/store5/Logger { + public abstract fun debug (Ljava/lang/String;)V + public abstract fun error (Ljava/lang/String;Ljava/lang/Throwable;)V +} + +public final class org/mobilenativefoundation/store/store5/Logger$DefaultImpls { + public static synthetic fun error$default (Lorg/mobilenativefoundation/store/store5/Logger;Ljava/lang/String;Ljava/lang/Throwable;ILjava/lang/Object;)V +} + public final class org/mobilenativefoundation/store/store5/MemoryPolicy { public static final field Companion Lorg/mobilenativefoundation/store/store5/MemoryPolicy$Companion; public static final field DEFAULT_SIZE_POLICY J diff --git a/store/api/jvm/store.api b/store/api/jvm/store.api index b992e2c43..61d4c12fc 100644 --- a/store/api/jvm/store.api +++ b/store/api/jvm/store.api @@ -116,6 +116,15 @@ public final class org/mobilenativefoundation/store/store5/FetcherResult$Error$M public fun toString ()Ljava/lang/String; } +public abstract interface class org/mobilenativefoundation/store/store5/Logger { + public abstract fun debug (Ljava/lang/String;)V + public abstract fun error (Ljava/lang/String;Ljava/lang/Throwable;)V +} + +public final class org/mobilenativefoundation/store/store5/Logger$DefaultImpls { + public static synthetic fun error$default (Lorg/mobilenativefoundation/store/store5/Logger;Ljava/lang/String;Ljava/lang/Throwable;ILjava/lang/Object;)V +} + public final class org/mobilenativefoundation/store/store5/MemoryPolicy { public static final field Companion Lorg/mobilenativefoundation/store/store5/MemoryPolicy$Companion; public static final field DEFAULT_SIZE_POLICY J diff --git a/store/build.gradle.kts b/store/build.gradle.kts index 3b928ce89..7ce9ce34c 100644 --- a/store/build.gradle.kts +++ b/store/build.gradle.kts @@ -1,9 +1,7 @@ -import org.gradle.internal.impldep.org.testng.reporters.XMLUtils.xml - - plugins { id("org.mobilenativefoundation.store.multiplatform") alias(libs.plugins.kover) + id("dev.mokkery") version "2.5.1" } kotlin { diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/Logger.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/Logger.kt new file mode 100644 index 000000000..d7318b9af --- /dev/null +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/Logger.kt @@ -0,0 +1,24 @@ +package org.mobilenativefoundation.store.store5 + +/** + * A simple logging interface for logging error and debug messages. + */ +interface Logger { + /** + * Logs an error message, optionally with a throwable. + * + * @param message The error message to log. + * @param throwable An optional [Throwable] associated with the error. + */ + fun error( + message: String, + throwable: Throwable? = null, + ) + + /** + * Logs a debug message. + * + * @param message The debug message to log. + */ + fun debug(message: String) +} diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/DefaultLogger.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/DefaultLogger.kt new file mode 100644 index 000000000..9219683c3 --- /dev/null +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/DefaultLogger.kt @@ -0,0 +1,26 @@ +package org.mobilenativefoundation.store.store5.impl + +import co.touchlab.kermit.CommonWriter +import org.mobilenativefoundation.store.store5.Logger + +/** + * Default implementation of [Logger] using the Kermit logging library. + */ +internal class DefaultLogger : Logger { + private val delegate = + co.touchlab.kermit.Logger.apply { + setLogWriters(listOf(CommonWriter())) + setTag("Store") + } + + override fun debug(message: String) { + delegate.d(message) + } + + override fun error( + message: String, + throwable: Throwable?, + ) { + delegate.e(message, throwable) + } +} 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 d798d57fe..0804c5cdb 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 @@ -2,8 +2,6 @@ package org.mobilenativefoundation.store.store5.impl -import co.touchlab.kermit.CommonWriter -import co.touchlab.kermit.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow @@ -14,6 +12,7 @@ import kotlinx.coroutines.sync.withLock import org.mobilenativefoundation.store.core5.ExperimentalStoreApi import org.mobilenativefoundation.store.store5.Bookkeeper import org.mobilenativefoundation.store.store5.Clear +import org.mobilenativefoundation.store.store5.Logger import org.mobilenativefoundation.store.store5.MutableStore import org.mobilenativefoundation.store.store5.StoreReadRequest import org.mobilenativefoundation.store.store5.StoreReadResponse @@ -27,11 +26,12 @@ import org.mobilenativefoundation.store.store5.internal.concurrent.ThreadSafety import org.mobilenativefoundation.store.store5.internal.definition.WriteRequestQueue import org.mobilenativefoundation.store.store5.internal.result.EagerConflictResolutionResult -@ExperimentalStoreApi +@OptIn(ExperimentalStoreApi::class) internal class RealMutableStore( private val delegate: RealStore, private val updater: Updater, private val bookkeeper: Bookkeeper?, + private val logger: Logger = DefaultLogger(), ) : MutableStore, Clear.Key by delegate, Clear.All by delegate { private val storeLock = Mutex() private val keyToWriteRequestQueue = mutableMapOf>() @@ -42,20 +42,29 @@ internal class RealMutableStore(request.key)) { + // TODO(matt-ramotar): Many use cases will not want to pull immediately after failing + // to push local changes. We should enable configuration of conflict resolution strategies, + // such as logging, retrying, canceling. + is EagerConflictResolutionResult.Error.Exception -> { - logger.e(eagerConflictResolutionResult.error.toString()) + logger.error(eagerConflictResolutionResult.error.toString()) } is EagerConflictResolutionResult.Error.Message -> { - logger.e(eagerConflictResolutionResult.message) + logger.error(eagerConflictResolutionResult.message) } is EagerConflictResolutionResult.Success.ConflictsResolved -> { - logger.d(eagerConflictResolutionResult.value.toString()) + val message = + when (val result = eagerConflictResolutionResult.value) { + is UpdaterResult.Success.Typed<*> -> result.value.toString() + is UpdaterResult.Success.Untyped -> result.value.toString() + } + logger.debug(message) } EagerConflictResolutionResult.Success.NoConflicts -> { - logger.d(eagerConflictResolutionResult.toString()) + logger.debug("No conflicts.") } } @@ -220,57 +229,45 @@ internal class RealMutableStore tryEagerlyResolveConflicts(key: Key): EagerConflictResolutionResult = - withThreadSafety(key) { - val latest = delegate.latestOrNull(key) - when { - latest == null || bookkeeper == null || conflictsMightExist(key).not() -> EagerConflictResolutionResult.Success.NoConflicts - else -> { - try { - val updaterResult = - updater.post(key, latest).also { updaterResult -> - if (updaterResult is UpdaterResult.Success) { - updateWriteRequestQueue(key = key, created = now(), updaterResult = updaterResult) - } - } + private suspend fun tryEagerlyResolveConflicts(key: Key): EagerConflictResolutionResult { + val (latest, conflictsExist) = + withThreadSafety(key) { + val latest = delegate.latestOrNull(key) + val conflictsExist = latest != null && bookkeeper != null && conflictsMightExist(key) + latest to conflictsExist + } - when (updaterResult) { - is UpdaterResult.Error.Exception -> EagerConflictResolutionResult.Error.Exception(updaterResult.error) - is UpdaterResult.Error.Message -> EagerConflictResolutionResult.Error.Message(updaterResult.message) - is UpdaterResult.Success -> EagerConflictResolutionResult.Success.ConflictsResolved(updaterResult) - } - } catch (throwable: Throwable) { - EagerConflictResolutionResult.Error.Exception(throwable) + if (!conflictsExist || latest == null) { + return EagerConflictResolutionResult.Success.NoConflicts + } + + return try { + val updaterResult = + updater.post(key, latest).also { updaterResult -> + if (updaterResult is UpdaterResult.Success) { + updateWriteRequestQueue(key = key, created = now(), updaterResult = updaterResult) + bookkeeper?.clear(key) } } - } - } - private suspend fun safeInitWriteRequestQueue(key: Key) = - withThreadSafety(key) { - if (keyToWriteRequestQueue[key] == null) { - keyToWriteRequestQueue[key] = ArrayDeque() + when (updaterResult) { + is UpdaterResult.Error.Exception -> EagerConflictResolutionResult.Error.Exception(updaterResult.error) + is UpdaterResult.Error.Message -> EagerConflictResolutionResult.Error.Message(updaterResult.message) + is UpdaterResult.Success -> EagerConflictResolutionResult.Success.ConflictsResolved(updaterResult) } + } catch (throwable: Throwable) { + EagerConflictResolutionResult.Error.Exception(throwable) } + } - private suspend fun safeInitThreadSafety(key: Key) = + private suspend fun safeInitStore(key: Key) { storeLock.withLock { if (keyToThreadSafety[key] == null) { keyToThreadSafety[key] = ThreadSafety() } - } - - private suspend fun safeInitStore(key: Key) { - safeInitThreadSafety(key) - safeInitWriteRequestQueue(key) - } - - companion object { - private val logger = - Logger.apply { - setLogWriters(listOf(CommonWriter())) - setTag("Store") + if (keyToWriteRequestQueue[key] == null) { + keyToWriteRequestQueue[key] = ArrayDeque() } - private const val UNKNOWN_ERROR = "Unknown error occurred" + } } } 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..c6ab782cb 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 @@ -42,7 +42,7 @@ import org.mobilenativefoundation.store.store5.impl.operators.Either import org.mobilenativefoundation.store.store5.impl.operators.merge import org.mobilenativefoundation.store.store5.internal.result.StoreDelegateWriteResult -internal class RealStore( +internal open class RealStore( scope: CoroutineScope, fetcher: Fetcher, sourceOfTruth: SourceOfTruth? = null, @@ -327,7 +327,7 @@ internal class RealStore( } } - internal suspend fun write( + internal open suspend fun write( key: Key, value: Output, ): StoreDelegateWriteResult = @@ -339,7 +339,7 @@ internal class RealStore( StoreDelegateWriteResult.Error.Exception(error) } - internal suspend fun latestOrNull(key: Key): Output? = fromMemCache(key) ?: fromSourceOfTruth(key) + internal open suspend fun latestOrNull(key: Key): Output? = fromMemCache(key) ?: fromSourceOfTruth(key) private suspend fun fromSourceOfTruth(key: Key) = sourceOfTruth?.reader(key, CompletableDeferred(Unit))?.map { it.dataOrNull() }?.first() diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/EagerConflictResolutionTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/EagerConflictResolutionTests.kt new file mode 100644 index 000000000..051a3bafa --- /dev/null +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/EagerConflictResolutionTests.kt @@ -0,0 +1,288 @@ +package org.mobilenativefoundation.store.store5.mutablestore + +import app.cash.turbine.test +import dev.mokkery.MockMode.autoUnit +import dev.mokkery.answering.returns +import dev.mokkery.every +import dev.mokkery.everySuspend +import dev.mokkery.matcher.any +import dev.mokkery.matcher.eq +import dev.mokkery.mock +import dev.mokkery.verify +import dev.mokkery.verify.VerifyMode.Companion.exactly +import dev.mokkery.verify.VerifyMode.Companion.not +import dev.mokkery.verifySuspend +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.mobilenativefoundation.store.store5.Bookkeeper +import org.mobilenativefoundation.store.store5.Logger +import org.mobilenativefoundation.store.store5.StoreReadRequest +import org.mobilenativefoundation.store.store5.StoreReadResponse +import org.mobilenativefoundation.store.store5.StoreReadResponseOrigin +import org.mobilenativefoundation.store.store5.Updater +import org.mobilenativefoundation.store.store5.UpdaterResult +import org.mobilenativefoundation.store.store5.impl.RealMutableStore +import org.mobilenativefoundation.store.store5.impl.RealStore +import org.mobilenativefoundation.store.store5.util.model.Note +import kotlin.test.Test +import kotlin.test.assertEquals + +class EagerConflictResolutionTests { + private val testScope = TestScope() + + private val delegate = mock>(autoUnit) + private val updater = mock>(autoUnit) + private val bookkeeper = mock>(autoUnit) + private val logger = mock(autoUnit) + + private val mutableStore = + RealMutableStore( + delegate, + updater, + bookkeeper, + logger, + ) + + @Test + fun stream_givenConflicts_whenExceptionResolvingConflicts_thenShouldLog() = + testScope.runTest { + // Given + val latestNote = Note("id", "Title", "Content") + val readResponse = StoreReadResponse.Data(latestNote, StoreReadResponseOrigin.Cache) + val delegateFlow = flowOf(readResponse) + val exception = Exception("Error updating network.") + val readRequest = StoreReadRequest.fresh("id") + + every { + delegate.stream(any()) + } returns delegateFlow + + everySuspend { + delegate.latestOrNull(any()) + } returns latestNote + + everySuspend { + bookkeeper.getLastFailedSync(any()) + } returns 1L + + everySuspend { + updater.post(any(), any()) + } returns UpdaterResult.Error.Exception(exception) + + // When + val stream = + mutableStore.stream( + readRequest, + ) + + // Then + + stream.test { + verifySuspend(exactly(1)) { + updater.post(eq("id"), eq(latestNote)) + } + + verifySuspend(not) { + bookkeeper.clear(eq("id")) + } + + verify(exactly(1)) { + logger.error(eq(exception.toString())) + } + + verify(exactly(1)) { + delegate.stream(eq(readRequest)) + } + + assertEquals(readResponse, awaitItem()) + + awaitComplete() + } + } + + @Test + fun stream_givenConflicts_whenErrorMessageResolvingConflicts_thenShouldLog() = + testScope.runTest { + // Given + val latestNote = Note("id", "Title", "Content") + val readResponse = StoreReadResponse.Data(latestNote, StoreReadResponseOrigin.Cache) + val delegateFlow = flowOf(readResponse) + val errorMessage = "Error updating network." + val readRequest = StoreReadRequest.fresh("id") + + every { + delegate.stream(any()) + } returns delegateFlow + + everySuspend { + delegate.latestOrNull(any()) + } returns latestNote + + everySuspend { + bookkeeper.getLastFailedSync(any()) + } returns 1L + + everySuspend { + updater.post(any(), any()) + } returns UpdaterResult.Error.Message(errorMessage) + + // When + val stream = + mutableStore.stream( + readRequest, + ) + + // Then + + stream.test { + verifySuspend(exactly(1)) { + updater.post(eq("id"), eq(latestNote)) + } + + verifySuspend(not) { + bookkeeper.clear(eq("id")) + } + + verify(exactly(1)) { + logger.error(eq(errorMessage)) + } + + verify(exactly(1)) { + delegate.stream(eq(readRequest)) + } + + assertEquals(readResponse, awaitItem()) + + awaitComplete() + } + } + + @Test + fun stream_givenNoConflicts_whenCalled_thenShouldLog() = + testScope.runTest { + // Given + val latestNote = Note("id", "Title", "Content") + val readResponse = StoreReadResponse.Data(latestNote, StoreReadResponseOrigin.Cache) + val delegateFlow = flowOf(readResponse) + val readRequest = StoreReadRequest.fresh("id") + + every { + delegate.stream(any()) + } returns delegateFlow + + everySuspend { + delegate.latestOrNull(any()) + } returns latestNote + + everySuspend { + bookkeeper.getLastFailedSync(any()) + } returns null + + everySuspend { + updater.post(any(), any()) + } returns UpdaterResult.Success.Typed(true) + + every { + updater.onCompletion + } returns null + + everySuspend { + bookkeeper.clear(any()) + } returns true + + // When + val stream = + mutableStore.stream( + readRequest, + ) + + // Then + + stream.test { + verifySuspend(not) { + updater.post(eq("id"), eq(latestNote)) + } + + verify(exactly(1)) { + logger.debug(eq("No conflicts.")) + } + + verifySuspend(not) { + bookkeeper.clear(eq("id")) + } + + verify(exactly(1)) { + delegate.stream(eq(readRequest)) + } + + assertEquals(readResponse, awaitItem()) + + awaitComplete() + } + } + + @Test + fun stream_givenConflicts_whenSuccessResolvingConflicts_thenShouldLog() = + testScope.runTest { + // Given + val latestNote = Note("id", "Title", "Content") + val readResponse = StoreReadResponse.Data(latestNote, StoreReadResponseOrigin.Cache) + val delegateFlow = flowOf(readResponse) + val readRequest = StoreReadRequest.fresh("id") + + every { + delegate.stream(any()) + } returns delegateFlow + + everySuspend { + delegate.latestOrNull(any()) + } returns latestNote + + everySuspend { + bookkeeper.getLastFailedSync(any()) + } returns 1L + + everySuspend { + updater.post(any(), any()) + } returns UpdaterResult.Success.Typed(true) + + every { + updater.onCompletion + } returns null + + everySuspend { + bookkeeper.clear(any()) + } returns true + + // When + val stream = + mutableStore.stream( + readRequest, + ) + + // Then + + stream.test { + verifySuspend(exactly(1)) { + updater.post(eq("id"), eq(latestNote)) + } + + verify(exactly(1)) { + logger.debug(eq("true")) + } + + verifySuspend(exactly(1)) { + bookkeeper.clear(eq("id")) + } + + verify(exactly(1)) { + delegate.stream(eq(readRequest)) + } + + assertEquals(readResponse, awaitItem()) + + awaitComplete() + } + } +}