diff --git a/core/data/src/androidUnitTest/kotlin/com/garfiec/librechat/core/data/repository/RepositoryCacheMutexTest.kt b/core/data/src/androidUnitTest/kotlin/com/garfiec/librechat/core/data/repository/RepositoryCacheMutexTest.kt new file mode 100644 index 0000000..90a3833 --- /dev/null +++ b/core/data/src/androidUnitTest/kotlin/com/garfiec/librechat/core/data/repository/RepositoryCacheMutexTest.kt @@ -0,0 +1,130 @@ +package com.garfiec.librechat.core.data.repository + +import com.garfiec.librechat.core.common.result.Result +import com.garfiec.librechat.core.model.Agent +import com.garfiec.librechat.core.model.Preset +import com.garfiec.librechat.core.model.response.AgentListResponse +import com.garfiec.librechat.core.network.api.AgentsApi +import com.garfiec.librechat.core.network.api.PresetsApi +import com.google.common.truth.Truth.assertThat +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class RepositoryCacheMutexTest { + + @Test + fun `AgentRepositoryImpl - two concurrent getAgents on cold cache hit the API exactly once`() = + runTest { + val agentsApi = mockk() + val responseAgents = listOf(Agent(id = "a1"), Agent(id = "a2")) + coEvery { agentsApi.getAgents(null) } coAnswers { + delay(50) + AgentListResponse(data = responseAgents) + } + + val repository = AgentRepositoryImpl(agentsApi = agentsApi) + + val first = async { repository.getAgents(category = null) } + val second = async { repository.getAgents(category = null) } + advanceUntilIdle() + + val firstResult = first.await() + val secondResult = second.await() + + assertThat(firstResult).isInstanceOf(Result.Success::class.java) + assertThat(secondResult).isInstanceOf(Result.Success::class.java) + assertThat((firstResult as Result.Success).data).isEqualTo(responseAgents) + assertThat((secondResult as Result.Success).data).isEqualTo(responseAgents) + coVerify(exactly = 1) { agentsApi.getAgents(null) } + } + + @Test + fun `AgentRepositoryImpl - second call after cache hydrated does not re-fetch`() = runTest { + val agentsApi = mockk() + val responseAgents = listOf(Agent(id = "a1")) + coEvery { agentsApi.getAgents(null) } returns AgentListResponse(data = responseAgents) + + val repository = AgentRepositoryImpl(agentsApi = agentsApi) + repository.getAgents(category = null) + repository.getAgents(category = null) + repository.getAgents(category = null) + + coVerify(exactly = 1) { agentsApi.getAgents(null) } + } + + @Test + fun `AgentRepositoryImpl - categorised getAgents bypasses cache and does not populate it`() = + runTest { + val agentsApi = mockk() + val filteredAgents = listOf(Agent(id = "cat1")) + val allAgents = listOf(Agent(id = "a1"), Agent(id = "a2")) + coEvery { agentsApi.getAgents("writing") } returns AgentListResponse(data = filteredAgents) + coEvery { agentsApi.getAgents(null) } returns AgentListResponse(data = allAgents) + + val repository = AgentRepositoryImpl(agentsApi = agentsApi) + repository.getAgents(category = "writing") + // Cache must still be cold — next call with category=null must hit the API. + repository.getAgents(category = null) + repository.getAgents(category = null) + + coVerify(exactly = 1) { agentsApi.getAgents("writing") } + coVerify(exactly = 1) { agentsApi.getAgents(null) } + } + + @Test + fun `PresetRepositoryImpl - two concurrent getAll on cold cache hit the API exactly once`() = + runTest { + val presetsApi = mockk() + val responsePresets = listOf( + Preset(presetId = "p1", title = "One"), + Preset(presetId = "p2", title = "Two"), + ) + coEvery { presetsApi.getPresets() } coAnswers { + delay(50) + responsePresets + } + + val repository = PresetRepositoryImpl(presetsApi = presetsApi) + + val first = async { repository.getAll() } + val second = async { repository.getAll() } + advanceUntilIdle() + + val firstResult = first.await() + val secondResult = second.await() + + assertThat(firstResult).isInstanceOf(Result.Success::class.java) + assertThat(secondResult).isInstanceOf(Result.Success::class.java) + assertThat((firstResult as Result.Success).data).isEqualTo(responsePresets) + assertThat((secondResult as Result.Success).data).isEqualTo(responsePresets) + coVerify(exactly = 1) { presetsApi.getPresets() } + } + + @Test + fun `PresetRepositoryImpl - mutation invalidates cache so next getAll re-fetches`() = runTest { + val presetsApi = mockk() + val first = listOf(Preset(presetId = "p1", title = "One")) + val second = listOf( + Preset(presetId = "p1", title = "One"), + Preset(presetId = "p2", title = "Two"), + ) + coEvery { presetsApi.getPresets() } returnsMany listOf(first, second) + coEvery { presetsApi.createPreset(any()) } returns Preset(presetId = "p2", title = "Two") + + val repository = PresetRepositoryImpl(presetsApi = presetsApi) + repository.getAll() + repository.create(Preset(presetId = "p2", title = "Two")) + val result = repository.getAll() + + assertThat((result as Result.Success).data).isEqualTo(second) + coVerify(exactly = 2) { presetsApi.getPresets() } + } +} diff --git a/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/AgentRepositoryImpl.kt b/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/AgentRepositoryImpl.kt index dfbd4e8..6fd038a 100644 --- a/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/AgentRepositoryImpl.kt +++ b/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/AgentRepositoryImpl.kt @@ -17,28 +17,30 @@ import com.garfiec.librechat.core.model.request.RevertAgentRequest import com.garfiec.librechat.core.model.request.ToolCallRequest import com.garfiec.librechat.core.model.request.UpdateAgentRequest import com.garfiec.librechat.core.network.api.AgentsApi +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.serialization.json.Json -import kotlin.concurrent.Volatile class AgentRepositoryImpl( private val agentsApi: AgentsApi, ) : AgentRepository { - @Volatile + private val cacheMutex = Mutex() private var cachedAgents: List? = null // --- Agent CRUD --- override suspend fun getAgents(category: String?): Result> { return safeApiCall { - if (category == null && cachedAgents != null) { - return@safeApiCall cachedAgents!! + if (category != null) { + return@safeApiCall agentsApi.getAgents(category).data } - val agents = agentsApi.getAgents(category).data - if (category == null) { + cacheMutex.withLock { + cachedAgents?.let { return@withLock it } + val agents = agentsApi.getAgents(null).data cachedAgents = agents + agents } - agents } } @@ -65,7 +67,8 @@ class AgentRepositoryImpl( override suspend fun getAgent(id: String): Result { return safeApiCall { - cachedAgents?.find { it.id == id }?.let { return@safeApiCall it } + cacheMutex.withLock { cachedAgents?.find { it.id == id } } + ?.let { return@safeApiCall it } agentsApi.getAgent(id) } } @@ -201,7 +204,7 @@ class AgentRepositoryImpl( } } - private fun invalidateCache() { - cachedAgents = null + private suspend fun invalidateCache() { + cacheMutex.withLock { cachedAgents = null } } } diff --git a/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/PresetRepositoryImpl.kt b/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/PresetRepositoryImpl.kt index 907605e..cf728d6 100644 --- a/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/PresetRepositoryImpl.kt +++ b/core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/PresetRepositoryImpl.kt @@ -4,28 +4,31 @@ import com.garfiec.librechat.core.common.result.Result import com.garfiec.librechat.core.common.result.safeApiCall import com.garfiec.librechat.core.model.Preset import com.garfiec.librechat.core.network.api.PresetsApi -import kotlin.concurrent.Volatile +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock class PresetRepositoryImpl( private val presetsApi: PresetsApi, ) : PresetRepository { - @Volatile + private val cacheMutex = Mutex() private var cachedPresets: List? = null override suspend fun getAll(): Result> { return safeApiCall { - if (cachedPresets != null) return@safeApiCall cachedPresets!! - val presets = presetsApi.getPresets() - cachedPresets = presets - presets + cacheMutex.withLock { + cachedPresets?.let { return@withLock it } + val presets = presetsApi.getPresets() + cachedPresets = presets + presets + } } } override suspend fun create(preset: Preset): Result { return safeApiCall { val created = presetsApi.createPreset(preset) - cachedPresets = null + cacheMutex.withLock { cachedPresets = null } created } } @@ -33,7 +36,7 @@ class PresetRepositoryImpl( override suspend fun update(preset: Preset): Result { return safeApiCall { val updated = presetsApi.updatePreset(preset) - cachedPresets = null + cacheMutex.withLock { cachedPresets = null } updated } } @@ -41,7 +44,7 @@ class PresetRepositoryImpl( override suspend fun delete(presetId: String): Result { return safeApiCall { presetsApi.deletePreset(presetId) - cachedPresets = null + cacheMutex.withLock { cachedPresets = null } } } }