From f92e5ba8251dab49e99288a41cef8081e63cf49a Mon Sep 17 00:00:00 2001 From: Eddie Date: Mon, 24 Aug 2026 09:12:48 -0400 Subject: [PATCH 1/4] fix: refresh Android course structure when requested --- .../java/org/openedx/app/di/ScreenModule.kt | 2 +- .../course/data/repository/CoalescingCache.kt | 111 ++- .../data/repository/CourseRepository.kt | 351 +++++++- .../domain/interactor/CourseInteractor.kt | 14 +- .../data/repository/CoalescingCacheTest.kt | 117 +++ .../repository/CourseRepositoryFreshTest.kt | 775 ++++++++++++++++++ .../interactor/CourseInteractorFreshTest.kt | 102 +++ 7 files changed, 1428 insertions(+), 44 deletions(-) create mode 100644 course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt create mode 100644 course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt create mode 100644 course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt index 1799dafc6..a2b06ac00 100644 --- a/app/src/main/java/org/openedx/app/di/ScreenModule.kt +++ b/app/src/main/java/org/openedx/app/di/ScreenModule.kt @@ -270,7 +270,7 @@ val screenModule = module { factory { CalendarInteractor(get()) } single { CourseRepository(get(), get(), get(), get(), get()) } - factory { CourseInteractor(get()) } + factory { CourseInteractor(get(), get()) } single { get() } viewModel { (pathId: String, infoType: String) -> diff --git a/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt b/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt index 7597e9b50..3c0f0b426 100644 --- a/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt +++ b/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt @@ -13,24 +13,79 @@ import java.util.concurrent.ConcurrentHashMap * @param V the type of cached values * @param fetch the suspend function to fetch data for a given key * @param persist optional callback invoked after successful fetch (e.g., to save to database) + * @param autoCache whether to store a successful fetch result in the cache before [persist] runs + * @param activeGeneration optional function that returns the current cache version; values saved + * under a different version are not returned */ class CoalescingCache( private val fetch: suspend (K) -> V, - private val persist: (suspend (K, V) -> Unit)? = null + private val persist: (suspend (K, V) -> Unit)? = null, + private val autoCache: Boolean = true, + private val activeGeneration: (() -> Long)? = null, ) { - private val cache = ConcurrentHashMap() + private data class CacheEntry( + val value: V, + val generation: Long, + ) + + private val cache = ConcurrentHashMap>() private val pending = ConcurrentHashMap>() /** - * Returns cached value for the key, or null if not cached. + * Returns the cached value for [key], or null when no usable value is cached. + * + * When [activeGeneration] is set, a value saved under a different version is not usable. */ - fun getCached(key: K): V? = cache[key] + fun getCached(key: K): V? { + val entry = cache[key] ?: return null + val generationProvider = activeGeneration + + return when { + generationProvider == null -> { + entry.value + } + + entry.generation == generationProvider() -> { + entry.value + } + + else -> { + cache.remove(key, entry) + null + } + } + } /** - * Manually sets a cached value. + * Stores a cached value. + * + * When [activeGeneration] is set and [writeGeneration] is supplied, stores the value only if + * that version is still current. */ - fun setCached(key: K, value: V) { - cache[key] = value + fun setCached( + key: K, + value: V, + writeGeneration: Long = UNSPECIFIED_GENERATION, + ) { + val generationProvider = activeGeneration + if (generationProvider == null) { + cache[key] = CacheEntry(value, DEFAULT_GENERATION) + return + } + + if (writeGeneration == UNSPECIFIED_GENERATION) { + val currentGeneration = generationProvider() + cache[key] = CacheEntry(value, currentGeneration) + return + } + + cache.compute(key) { _, currentEntry -> + if (generationProvider() == writeGeneration) { + CacheEntry(value, writeGeneration) + } else { + currentEntry + } + } } /** @@ -40,6 +95,23 @@ class CoalescingCache( cache.clear() } + /** + * Stops waiting callers from sharing requests that were pending when this method began. + * + * It removes each recorded request before cancelling callers waiting for it, so a caller that + * resumes can start a new request. A fetch that already started can still finish. When it does, + * it removes a pending request only if that request still belongs to it, not a newer request + * for the same key. + */ + fun cancelPending() { + val pendingSnapshot = HashMap(pending) + for ((key, deferred) in pendingSnapshot) { + if (pending.remove(key, deferred)) { + deferred.cancel() + } + } + } + /** * Gets the value from cache or fetches it. * @@ -49,22 +121,24 @@ class CoalescingCache( */ suspend fun getOrFetch(key: K, forceRefresh: Boolean = false): V { if (!forceRefresh) { - cache[key]?.let { return it } + getCached(key)?.let { return it } } - val (deferred, isOwner) = getOrCreateDeferred(key) - return if (isOwner) { + val (deferred, startsFetch) = getOrCreateDeferred(key) + return if (startsFetch) { try { - val result = fetch(key) - cache[key] = result - persist?.invoke(key, result) - deferred.complete(result) - result + val value = fetch(key) + if (autoCache) { + setCached(key, value) + } + persist?.invoke(key, value) + deferred.complete(value) + value } catch (e: Exception) { deferred.completeExceptionally(e) throw e } finally { - pending.remove(key) + pending.remove(key, deferred) } } else { deferred.await() @@ -77,4 +151,9 @@ class CoalescingCache( val existing = pending.putIfAbsent(key, deferred) return if (existing != null) existing to false else deferred to true } + + private companion object { + const val DEFAULT_GENERATION = 0L + const val UNSPECIFIED_GENERATION = Long.MIN_VALUE + } } diff --git a/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt b/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt index 229963b58..1ca3adafe 100644 --- a/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt +++ b/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt @@ -3,10 +3,13 @@ package org.openedx.course.data.repository import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import okhttp3.MultipartBody import org.openedx.core.ApiConstants import org.openedx.core.data.api.CourseApi import org.openedx.core.data.model.BlocksCompletionBody +import org.openedx.core.data.model.room.CourseStructureEntity import org.openedx.core.data.model.room.OfflineXBlockProgress import org.openedx.core.data.model.room.VideoProgressEntity import org.openedx.core.data.model.room.XBlockProgressData @@ -24,6 +27,7 @@ import org.openedx.core.system.connection.NetworkConnection import java.net.URLDecoder import java.nio.charset.StandardCharsets import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicLong /** * Repository for course data with request coalescing. @@ -39,21 +43,174 @@ class CourseRepository( private val preferencesManager: CorePreferences, private val networkConnection: NetworkConnection, ) { - // Session tracking - when entering a course, mark that data needs refresh - private val needsRefresh = ConcurrentHashMap.newKeySet() + /** + * Combines a course ID with the session number when a request starts. + * + * `endCourseSession()` changes that number. A response that finishes later can update the cache + * and refresh state only for the session in which its request started. + */ + private data class CourseSessionKey( + val sessionGeneration: Long, + val courseId: String, + ) - private val structureCache = CoalescingCache( - fetch = { courseId -> + /** + * Holds the course structure and local Room record returned by one request that does not require + * a refresh. + * + * If an explicit refresh saves newer course structure while this request is running, + * [returnedCourseStructure] is replaced with that newer cached value. The Flow then does not + * emit the older server result. + */ + private data class NonFreshFetchResult( + val capturedSessionGeneration: Long, + val capturedFreshCompletionVersion: Long, + val roomEntity: CourseStructureEntity?, + val fetchedCourseStructure: CourseStructure, + ) { + var returnedCourseStructure: CourseStructure = fetchedCourseStructure + } + + /** + * Holds the course structure and local Room record returned by one explicit refresh request. + * + * They stay together until saved, so a later response cannot pair one response's structure with + * another response's database record. + */ + private data class FreshFetchResult( + val capturedSessionGeneration: Long, + val courseStructure: CourseStructure, + val roomEntity: CourseStructureEntity, + ) + + /** + * Stores the number for the current course session. `endCourseSession()` increases this number. + * + * A request still waiting for its course lock when the number changes does not write. A Room + * insert that already holds the lock may finish, but its old result is not added to the new + * session's memory cache. + */ + private val sessionGeneration = AtomicLong(0) + + /** + * Holds one `Mutex`, a coroutine-safe lock, for each course. + * + * The mutex makes four operations take turns: saving a normal response; saving a refresh + * response; placing a database result in memory cache for a Flow; and placing one in memory + * cache for a cache-only read. + * + * The mutex remains after a session ends because an older database write may already hold it. + * A later session's write waits for that write to finish, then becomes the final stored value. + */ + private val courseWriteMutexes = ConcurrentHashMap() + + /** + * Counts refresh responses saved for each course and session. + * + * A request that does not require a refresh remembers this count before fetching data. If the + * count changes before that request finishes, a refresh saved newer data first, so the older + * result is not stored or returned. + */ + private val freshCompletionVersion = ConcurrentHashMap() + + /** + * Records that a course should request new data during a particular session. + * + * The session number in the key means that an old request removes only its own marker, not a + * marker created for a later session. + */ + private val needsRefresh = ConcurrentHashMap.newKeySet() + + private val structureCache: CoalescingCache = CoalescingCache( + fetch = { courseSessionKey -> + val capturedGeneration = courseSessionKey.sessionGeneration + val courseId = courseSessionKey.courseId + val capturedFreshCompletionVersion = + freshCompletionVersionFor(capturedGeneration, courseId).get() val response = api.getCourseStructure( "stale-if-error=0", "v4", preferencesManager.user?.username, courseId ) - courseDao.insertCourseStructureEntity(response.mapToRoomEntity()) - response.mapToDomain() + NonFreshFetchResult( + capturedSessionGeneration = capturedGeneration, + capturedFreshCompletionVersion = capturedFreshCompletionVersion, + roomEntity = response.mapToRoomEntity(), + fetchedCourseStructure = response.mapToDomain(), + ) + }, + persist = { courseSessionKey, fetchResult -> + val courseId = courseSessionKey.courseId + + runUnderCourseWriteGuard(courseId, fetchResult.capturedSessionGeneration) { + val currentFreshCompletionVersion = freshCompletionVersionFor( + fetchResult.capturedSessionGeneration, + courseId, + ).get() + if (currentFreshCompletionVersion != + fetchResult.capturedFreshCompletionVersion + ) { + getCachedStructure(courseId)?.let { + fetchResult.returnedCourseStructure = it + } + return@runUnderCourseWriteGuard + } + + fetchResult.roomEntity?.let { courseDao.insertCourseStructureEntity(it) } + setCachedStructure( + courseId, + fetchResult.fetchedCourseStructure, + fetchResult.capturedSessionGeneration, + ) + } + needsRefresh.remove(courseSessionKey) }, - persist = { courseId, _ -> needsRefresh.remove(courseId) } + // Do not cache each fetch automatically. First confirm that the request still belongs to + // the current session and that an explicit refresh did not save newer data. + autoCache = false, + activeGeneration = { sessionGeneration.get() }, + ) + + /** + * Keeps explicit refresh requests separate from normal requests. + * + * The two paths send different `Cache-Control` headers and must not share a response, so they + * use separate `CoalescingCache` instances. + */ + private val freshStructureCache = CoalescingCache( + fetch = { courseSessionKey -> + val courseId = courseSessionKey.courseId + val response = api.getCourseStructure( + "no-cache", + "v4", + preferencesManager.user?.username, + courseId, + ) + FreshFetchResult( + capturedSessionGeneration = courseSessionKey.sessionGeneration, + courseStructure = response.mapToDomain(), + roomEntity = response.mapToRoomEntity(), + ) + }, + persist = { courseSessionKey, freshResult -> + val courseId = courseSessionKey.courseId + runUnderCourseWriteGuard(courseId, courseSessionKey.sessionGeneration) { + courseDao.insertCourseStructureEntity(freshResult.roomEntity) + freshCompletionVersionFor( + courseSessionKey.sessionGeneration, + courseId, + ).incrementAndGet() + setCachedStructure( + courseId, + freshResult.courseStructure, + courseSessionKey.sessionGeneration, + ) + } + needsRefresh.remove(courseSessionKey) + }, + autoCache = false, + activeGeneration = { sessionGeneration.get() }, ) private val statusCache = CoalescingCache( @@ -84,48 +241,97 @@ class CourseRepository( * Call when entering a course to mark that data should be refreshed. */ fun startCourseSession(courseId: String) { - needsRefresh.add(courseId) + val courseSessionKey = CourseSessionKey(sessionGeneration.get(), courseId) + needsRefresh.add(courseSessionKey) } fun endCourseSession() { + sessionGeneration.incrementAndGet() + structureCache.cancelPending() + freshStructureCache.cancelPending() structureCache.clear() + freshStructureCache.clear() statusCache.clear() datesCache.clear() progressCache.clear() enrollmentCache.clear() needsRefresh.clear() + freshCompletionVersion.clear() } fun getCourseStructureFlow( courseId: String, forceRefresh: Boolean = false ): Flow = flow { - // Always emit cached data first if available - structureCache.getCached(courseId)?.let { emit(it) } + val flowSessionGeneration = sessionGeneration.get() - if (structureCache.getCached(courseId) == null) { - courseDao.getCourseStructureById(courseId)?.mapToDomain()?.let { - structureCache.setCached(courseId, it) - emit(it) + // Always emit cached data first if available + getCachedStructure(courseId)?.let { emit(it) } + + if (getCachedStructure(courseId) == null) { + val capturedFreshCompletionVersion = + freshCompletionVersionFor(flowSessionGeneration, courseId).get() + val roomStructure = courseDao.getCourseStructureById(courseId)?.mapToDomain() + if (roomStructure != null) { + val structureToEmit = runUnderCourseWriteGuard( + courseId, + flowSessionGeneration, + ) { + resolveStructureFromRoom( + courseId = courseId, + capturedGeneration = flowSessionGeneration, + capturedFreshCompletionVersion = capturedFreshCompletionVersion, + roomStructure = roomStructure, + ) + } + structureToEmit?.let { emit(it) } } } - val shouldRefresh = forceRefresh || needsRefresh.contains(courseId) - if (networkConnection.isOnline() && (structureCache.getCached(courseId) == null || shouldRefresh)) { - emit(structureCache.getOrFetch(courseId, forceRefresh = true)) + val courseSessionKey = CourseSessionKey(flowSessionGeneration, courseId) + val shouldRefresh = forceRefresh || needsRefresh.contains(courseSessionKey) + val hasCachedStructure = getCachedStructure(courseId) != null + val shouldFetch = networkConnection.isOnline() && (!hasCachedStructure || shouldRefresh) + if (shouldFetch) { + val fetchResult = structureCache.getOrFetch(courseSessionKey, forceRefresh = true) + emit(fetchResult.returnedCourseStructure) } - if (structureCache.getCached(courseId) == null) { + if (getCachedStructure(courseId) == null) { throw NoCachedDataException() } } + suspend fun getCourseStructureFresh(courseId: String): CourseStructure { + val courseSessionKey = CourseSessionKey(sessionGeneration.get(), courseId) + return freshStructureCache.getOrFetch(courseSessionKey, forceRefresh = true).courseStructure + } + suspend fun getCourseStructureFromCache(courseId: String): CourseStructure { - return structureCache.getCached(courseId) - ?: courseDao.getCourseStructureById(courseId)?.mapToDomain()?.also { - structureCache.setCached(courseId, it) - } + val cachedStructure = getCachedStructure(courseId) + if (cachedStructure != null) { + return cachedStructure + } + + val capturedGeneration = sessionGeneration.get() + val capturedFreshCompletionVersion = + freshCompletionVersionFor(capturedGeneration, courseId).get() + val roomEntity = courseDao.getCourseStructureById(courseId) ?: throw NoCachedDataException() + val roomStructure = roomEntity.mapToDomain() + + val resolvedStructure = runUnderCourseWriteGuard(courseId, capturedGeneration) { + resolveStructureFromRoom( + courseId = courseId, + capturedGeneration = capturedGeneration, + capturedFreshCompletionVersion = capturedFreshCompletionVersion, + roomStructure = roomStructure, + ) + } + if (resolvedStructure == null) { + throw NoCachedDataException() + } + return resolvedStructure } fun getEnrollmentDetailsFlow( @@ -163,7 +369,9 @@ class CourseRepository( val cached = statusCache.getCached(courseId) emit(cached ?: CourseComponentStatus("")) - val shouldRefresh = forceRefresh || needsRefresh.contains(courseId) + val capturedGeneration = sessionGeneration.get() + val courseSessionKey = CourseSessionKey(capturedGeneration, courseId) + val shouldRefresh = forceRefresh || needsRefresh.contains(courseSessionKey) if (networkConnection.isOnline() && (cached == null || shouldRefresh)) { emit(statusCache.getOrFetch(courseId, forceRefresh = true)) } @@ -184,7 +392,9 @@ class CourseRepository( val cached = datesCache.getCached(courseId) emit(cached ?: emptyCourseDatesResult()) - val shouldRefresh = forceRefresh || needsRefresh.contains(courseId) + val capturedGeneration = sessionGeneration.get() + val courseSessionKey = CourseSessionKey(capturedGeneration, courseId) + val shouldRefresh = forceRefresh || needsRefresh.contains(courseSessionKey) if (networkConnection.isOnline() && (cached == null || shouldRefresh)) { emit(datesCache.getOrFetch(courseId, forceRefresh = true)) } @@ -225,7 +435,9 @@ class CourseRepository( } } - val shouldRefresh = isRefresh || needsRefresh.contains(courseId) + val capturedGeneration = sessionGeneration.get() + val courseSessionKey = CourseSessionKey(capturedGeneration, courseId) + val shouldRefresh = isRefresh || needsRefresh.contains(courseSessionKey) val hasCache = progressCache.getCached(courseId) != null val shouldFetch = shouldRefresh || !hasCache || !getOnlyCacheIfExist @@ -288,6 +500,95 @@ class CourseRepository( submitOfflineXBlockProgress(blockId, courseId, jsonProgressData) } + private fun freshCompletionVersionFor( + generation: Long, + courseId: String, + ): AtomicLong { + val courseSessionKey = CourseSessionKey(generation, courseId) + return freshCompletionVersion.getOrPut(courseSessionKey) { AtomicLong(0) } + } + + private fun getCachedStructure( + courseId: String, + ): CourseStructure? { + return structureCache.getCached( + CourseSessionKey(sessionGeneration.get(), courseId), + )?.returnedCourseStructure + } + + /** + * Adds [courseStructure] to the memory cache only if it belongs to the current session. + */ + private fun setCachedStructure( + courseId: String, + courseStructure: CourseStructure, + generation: Long, + ) { + structureCache.setCached( + CourseSessionKey(generation, courseId), + NonFreshFetchResult( + capturedSessionGeneration = generation, + capturedFreshCompletionVersion = freshCompletionVersionFor(generation, courseId).get(), + roomEntity = null, + fetchedCourseStructure = courseStructure, + ), + generation, + ) + } + + /** + * Runs [block] while holding this course's `Mutex`. + * + * If the course session ends while the call is waiting for the mutex, [block] does not run and + * this function returns null. + */ + private suspend fun runUnderCourseWriteGuard( + courseId: String, + capturedGeneration: Long, + block: suspend () -> R, + ): R? { + val courseWriteMutex = courseWriteMutexes.getOrPut(courseId) { Mutex() } + return courseWriteMutex.withLock { + if (sessionGeneration.get() != capturedGeneration) { + return@withLock null + } + block() + } + } + + /** + * Decides whether course structure read from the local Room database may be added to memory + * cache. + * + * If an explicit refresh saved newer course structure while Room was being read, returns that + * newer cached structure instead. Otherwise, caches and returns the Room result. + */ + private fun resolveStructureFromRoom( + courseId: String, + capturedGeneration: Long, + capturedFreshCompletionVersion: Long, + roomStructure: CourseStructure, + ): CourseStructure? { + val currentCachedStructure = getCachedStructure(courseId) + val currentFreshCompletionVersion = + freshCompletionVersionFor(capturedGeneration, courseId).get() + + return when { + currentFreshCompletionVersion != capturedFreshCompletionVersion -> { + currentCachedStructure + } + + currentCachedStructure != null -> { + currentCachedStructure + } + + else -> { + setCachedStructure(courseId, roomStructure, capturedGeneration) + roomStructure + } + } + } + private suspend fun submitOfflineXBlockProgress( blockId: String, courseId: String, diff --git a/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt b/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt index 543695689..115f02cde 100644 --- a/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt +++ b/course/src/main/java/org/openedx/course/domain/interactor/CourseInteractor.kt @@ -7,11 +7,13 @@ import org.openedx.core.domain.interactor.CourseInteractor import org.openedx.core.domain.model.Block import org.openedx.core.domain.model.CourseEnrollmentDetails import org.openedx.core.domain.model.CourseStructure +import org.openedx.core.system.connection.NetworkConnection import org.openedx.course.data.repository.CourseRepository @Suppress("TooManyFunctions") class CourseInteractor( - private val repository: CourseRepository + private val repository: CourseRepository, + private val networkConnection: NetworkConnection, ) : CourseInteractor { fun startCourseSession(courseId: String) { @@ -33,7 +35,15 @@ class CourseInteractor( courseId: String, isNeedRefresh: Boolean ): CourseStructure { - return repository.getCourseStructureFlow(courseId, isNeedRefresh).first() + return when { + isNeedRefresh && networkConnection.isOnline() -> { + repository.getCourseStructureFresh(courseId) + } + + else -> { + repository.getCourseStructureFlow(courseId, isNeedRefresh).first() + } + } } override suspend fun getCourseStructureFromCache(courseId: String): CourseStructure { diff --git a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt new file mode 100644 index 000000000..1c5b4f77b --- /dev/null +++ b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt @@ -0,0 +1,117 @@ +package org.openedx.course.data.repository + +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class CoalescingCacheTest { + + @Test + fun `old generation write does not replace the current cache entry`() { + var activeGeneration = 1L + val cache = CoalescingCache( + fetch = { "unused" }, + activeGeneration = { activeGeneration }, + ) + + cache.setCached(KEY, "generation-1", writeGeneration = 1L) + activeGeneration = 2L + assertNull(cache.getCached(KEY)) + + cache.setCached(KEY, "generation-2", writeGeneration = 2L) + cache.setCached(KEY, "late-generation-1", writeGeneration = 1L) + assertEquals("generation-2", cache.getCached(KEY)) + } + + @Test + fun `cancelled fetch cannot remove a newer pending request`() = runTest { + val fetchGates = Channel>(Channel.UNLIMITED) + var fetchCount = 0 + val cache = CoalescingCache( + fetch = { + fetchCount += 1 + val gate = CompletableDeferred() + fetchGates.send(gate) + gate.await() + }, + ) + + val oldFetch = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + val oldGate = fetchGates.receive() + cache.cancelPending() + + val newFetch = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + val newGate = fetchGates.receive() + + oldGate.complete("old") + assertEquals("old", oldFetch.await()) + + val newWaiter = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + assertFalse(newWaiter.isCompleted) + assertTrue(fetchGates.tryReceive().isFailure) + + newGate.complete("new") + assertEquals("new", newFetch.await()) + assertEquals("new", newWaiter.await()) + assertEquals(2, fetchCount) + } + + @Test + fun `cancelling pending work removes it before a cancelled waiter starts a new request`() = runTest { + val fetchGates = Channel>(Channel.UNLIMITED) + val cache = CoalescingCache( + fetch = { + val gate = CompletableDeferred() + fetchGates.send(gate) + gate.await() + }, + ) + + val oldFetch = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + val oldGate = fetchGates.receive() + val cancelledWaiter = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + val laterCall = CompletableDeferred>() + cancelledWaiter.invokeOnCompletion { + laterCall.complete( + async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + }, + ) + } + + cache.cancelPending() + + val newFetch = laterCall.await() + assertFalse(newFetch.isCancelled) + val newGate = fetchGates.receive() + + oldGate.complete("old") + assertEquals("old", oldFetch.await()) + newGate.complete("new") + assertEquals("new", newFetch.await()) + } + + private companion object { + const val KEY = "course" + } +} diff --git a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt new file mode 100644 index 000000000..1c0fa3caf --- /dev/null +++ b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt @@ -0,0 +1,775 @@ +package org.openedx.course.data.repository + +import com.google.gson.JsonSyntaxException +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.take +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.supervisorScope +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Before +import org.junit.Test +import org.openedx.core.CoreMocks +import org.openedx.core.data.api.CourseApi +import org.openedx.core.data.model.CourseComponentStatus +import org.openedx.core.data.model.CourseDates +import org.openedx.core.data.model.CourseProgressResponse +import org.openedx.core.data.model.CourseStructureModel +import org.openedx.core.data.model.room.CourseEnrollmentDetailsEntity +import org.openedx.core.data.model.room.CourseProgressEntity +import org.openedx.core.data.model.room.CourseStructureEntity +import org.openedx.core.data.model.room.VideoProgressEntity +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.data.storage.CourseDao +import org.openedx.core.domain.model.CourseStructure +import org.openedx.core.exception.NoCachedDataException +import org.openedx.core.module.db.DownloadDao +import org.openedx.core.system.connection.NetworkConnection +import retrofit2.HttpException +import retrofit2.Response +import java.net.UnknownHostException +import java.util.Collections +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference + +@OptIn(ExperimentalCoroutinesApi::class) +class CourseRepositoryFreshTest { + + private data class CourseStructureFixture( + val domain: CourseStructure, + val roomEntity: CourseStructureEntity, + val response: CourseStructureModel, + ) + + private data class ApiRequest( + val cacheControl: String, + val courseId: String, + val response: CompletableDeferred, + ) + + private lateinit var api: CourseApi + private lateinit var courseDao: GatedCourseDao + private lateinit var preferencesManager: CorePreferences + private lateinit var networkConnection: NetworkConnection + private lateinit var repository: CourseRepository + private lateinit var apiRequests: Channel + + @Before + fun setUp() { + api = mockk() + courseDao = GatedCourseDao() + preferencesManager = mockk() + networkConnection = mockk() + apiRequests = Channel(Channel.UNLIMITED) + + every { preferencesManager.user } returns null + every { networkConnection.isOnline() } returns true + coEvery { + api.getCourseStructure(any(), any(), any(), any()) + } coAnswers { + val response = CompletableDeferred() + val request = ApiRequest( + cacheControl = firstArg(), + courseId = arg(3), + response = response, + ) + apiRequests.send(request) + response.await() + } + + repository = CourseRepository( + api = api, + courseDao = courseDao, + downloadDao = mockk(relaxed = true), + preferencesManager = preferencesManager, + networkConnection = networkConnection, + ) + } + + @Test + fun `fresh fetch returns the server response and updates Room and memory`() = runTest { + val cachedV1 = fixture("v1") + val originV2 = fixture("v2") + courseDao.storedCourseStructure.set(cachedV1.roomEntity) + assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + + val freshCall = startFreshRequest() + val request = apiRequests.receive() + assertEquals(COURSE_ID, request.courseId) + assertEquals("no-cache", request.cacheControl) + request.response.complete(originV2.response) + + assertSame(originV2.domain, freshCall.await()) + assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `fresh fetch failures propagate without returning cached data`() = runTest { + val cachedV1 = fixture("v1") + courseDao.storedCourseStructure.set(cachedV1.roomEntity) + assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + + val failures = listOf( + UnknownHostException("offline"), + httpException(500), + httpException(403), + JsonSyntaxException("invalid response"), + ) + + for (failure in failures) { + val actualFailure = supervisorScope { + val freshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFresh(COURSE_ID) + } + val request = apiRequests.receive() + request.response.completeExceptionally(failure) + failureFrom(freshCall) + } + + assertEquals(failure::class, actualFailure::class) + assertEquals(failure.message, actualFailure.message) + assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + } + + @Test + fun `cache-first Flow emits the memory cache before the server result`() = runTest { + val cachedV1 = fixture("v1") + val serverV2 = fixture("v2") + courseDao.storedCourseStructure.set(cachedV1.roomEntity) + repository.getCourseStructureFromCache(COURSE_ID) + + val collection = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).toList() + } + val request = apiRequests.receive() + assertEquals("stale-if-error=0", request.cacheControl) + request.response.complete(serverV2.response) + + assertEquals(listOf(cachedV1.domain, serverV2.domain), collection.await()) + assertSame(serverV2.roomEntity, courseDao.storedCourseStructure.get()) + } + + @Test + fun `concurrent fresh fetches share one request and persist once`() = runTest { + val originV2 = fixture("v2") + + val firstCall = startFreshRequest() + val secondCall = startFreshRequest() + val request = apiRequests.receive() + + assertTrue(apiRequests.tryReceive().isFailure) + request.response.complete(originV2.response) + + assertSame(originV2.domain, firstCall.await()) + assertSame(originV2.domain, secondCall.await()) + assertEquals(1, courseDao.insertCalls.count { it === originV2.roomEntity }) + } + + @Test + fun `later fresh fetch waits for the first response to persist`() = runTest { + val firstResponse = fixture("first-response") + val insertGate = courseDao.gateInsert(firstResponse.roomEntity) + + val firstCall = startFreshRequest() + apiRequests.receive().response.complete(firstResponse.response) + insertGate.started.await() + + val laterCall = startFreshRequest() + assertTrue(apiRequests.tryReceive().isFailure) + + insertGate.release.complete(Unit) + assertSame(firstResponse.domain, firstCall.await()) + assertSame(firstResponse.domain, laterCall.await()) + assertSame(firstResponse.roomEntity, courseDao.storedCourseStructure.get()) + assertEquals(1, courseDao.insertCalls.count { it === firstResponse.roomEntity }) + } + + @Test + fun `fresh and cache-first fetches send different cache headers`() = runTest { + val nonFreshValue = fixture("non-fresh") + val freshValue = fixture("fresh") + + val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val nonFreshRequest = apiRequests.receive() + assertEquals("stale-if-error=0", nonFreshRequest.cacheControl) + nonFreshRequest.response.complete(nonFreshValue.response) + assertSame(nonFreshValue.domain, nonFreshCall.await()) + + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + assertEquals("no-cache", freshRequest.cacheControl) + freshRequest.response.complete(freshValue.response) + assertSame(freshValue.domain, freshCall.await()) + } + + @Test + fun `fresh fetch never shares a pending cache-first request`() = runTest { + val nonFreshValue = fixture("non-fresh") + val freshValue = fixture("fresh") + + val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val nonFreshRequest = apiRequests.receive() + + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + + assertEquals("stale-if-error=0", nonFreshRequest.cacheControl) + assertEquals("no-cache", freshRequest.cacheControl) + nonFreshRequest.response.complete(nonFreshValue.response) + freshRequest.response.complete(freshValue.response) + + assertSame(nonFreshValue.domain, nonFreshCall.await()) + assertSame(freshValue.domain, freshCall.await()) + } + + @Test + fun `stale cache-first fetch returns the fresh result after fresh completion`() = runTest { + val staleV1 = fixture("v1") + val freshV2 = fixture("v2") + + val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val nonFreshRequest = apiRequests.receive() + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + + freshRequest.response.complete(freshV2.response) + assertSame(freshV2.domain, freshCall.await()) + assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + + nonFreshRequest.response.complete(staleV1.response) + assertSame(freshV2.domain, nonFreshCall.await()) + assertSame(freshV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `fresh fetch replaces an earlier cache-first result`() = runTest { + val nonFreshV1 = fixture("v1") + val freshV2 = fixture("v2") + + val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val nonFreshRequest = apiRequests.receive() + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + + nonFreshRequest.response.complete(nonFreshV1.response) + assertSame(nonFreshV1.domain, nonFreshCall.await()) + assertSame(nonFreshV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + + freshRequest.response.complete(freshV2.response) + assertSame(freshV2.domain, freshCall.await()) + assertSame(freshV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `delayed Room read does not overwrite a completed fresh fetch`() = runTest { + val roomV1 = fixture("v1") + val freshV2 = fixture("v2") + courseDao.storedCourseStructure.set(roomV1.roomEntity) + val readGate = courseDao.gateNextRead() + + every { networkConnection.isOnline() } returns false + val flowCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID).first() + } + readGate.started.await() + + every { networkConnection.isOnline() } returns true + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + freshRequest.response.complete(freshV2.response) + assertSame(freshV2.domain, freshCall.await()) + + readGate.release.complete(Unit) + assertSame(freshV2.domain, flowCall.await()) + assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `delayed cache-only Room read returns the completed fresh result`() = runTest { + val roomV1 = fixture("v1") + val freshV2 = fixture("v2") + courseDao.storedCourseStructure.set(roomV1.roomEntity) + val readGate = courseDao.gateNextRead() + + val cacheCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFromCache(COURSE_ID) + } + readGate.started.await() + + val freshCall = startFreshRequest() + val freshRequest = apiRequests.receive() + freshRequest.response.complete(freshV2.response) + assertSame(freshV2.domain, freshCall.await()) + + readGate.release.complete(Unit) + assertSame(freshV2.domain, cacheCall.await()) + assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `old session does not write after reset when it reaches the write guard`() = runTest { + val mutexHolder = fixture("mutex-holder") + val lateOldValue = fixture("late-old") + val insertGate = courseDao.gateInsert(mutexHolder.roomEntity) + + val holderCall = startFreshRequest() + apiRequests.receive().response.complete(mutexHolder.response) + insertGate.started.await() + + val lateCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + apiRequests.receive().response.complete(lateOldValue.response) + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + insertGate.release.complete(Unit) + + holderCall.await() + lateCall.await() + assertFalse(courseDao.insertCalls.any { it === lateOldValue.roomEntity }) + courseDao.storedCourseStructure.set(null) + assertFailureType { + repository.getCourseStructureFromCache(COURSE_ID) + } + } + + @Test + fun `new session write is final after an old Room insert resumes`() = runTest { + val oldV1 = fixture("old-v1") + val newV2 = fixture("new-v2") + val oldInsertGate = courseDao.gateInsert(oldV1.roomEntity) + + val oldCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + apiRequests.receive().response.complete(oldV1.response) + oldInsertGate.started.await() + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + + val newCall = startFreshRequest() + apiRequests.receive().response.complete(newV2.response) + assertFalse(newCall.isCompleted) + + oldInsertGate.release.complete(Unit) + oldCall.await() + assertSame(newV2.domain, newCall.await()) + + assertTrue(courseDao.insertCalls.any { it === oldV1.roomEntity }) + assertSame(newV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(newV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `fresh fetch does not read an old Room row`() = runTest { + val staleRoomV1 = fixture("stale-room-v1") + val originV2 = fixture("origin-v2") + courseDao.storedCourseStructure.set(staleRoomV1.roomEntity) + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + val readsBeforeFreshCall = courseDao.readCount.get() + + val freshCall = startFreshRequest() + apiRequests.receive().response.complete(originV2.response) + + assertSame(originV2.domain, freshCall.await()) + assertEquals(readsBeforeFreshCall, courseDao.readCount.get()) + assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `cache-first read may return old Room data until a fresh fetch replaces it`() = runTest { + val staleRoomV1 = fixture("stale-room-v1") + val originV2 = fixture("origin-v2") + courseDao.storedCourseStructure.set(staleRoomV1.roomEntity) + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + + every { networkConnection.isOnline() } returns false + val displayedStructure = repository.getCourseStructureFlow(COURSE_ID).first() + assertSame(staleRoomV1.domain, displayedStructure) + + every { networkConnection.isOnline() } returns true + val freshCall = startFreshRequest() + apiRequests.receive().response.complete(originV2.response) + assertSame(originV2.domain, freshCall.await()) + assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `old session completion does not clear a new session refresh marker`() = runTest { + val oldValue = fixture("old") + val cachedV1 = fixture("cached-v1") + val refreshedV2 = fixture("refreshed-v2") + + val oldCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val oldRequest = apiRequests.receive() + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + courseDao.storedCourseStructure.set(cachedV1.roomEntity) + assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + + oldRequest.response.complete(oldValue.response) + assertSame(oldValue.domain, oldCall.await()) + + val newFlow = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID).take(2).toList() + } + val newRequest = apiRequests.receive() + assertEquals("stale-if-error=0", newRequest.cacheControl) + newRequest.response.complete(refreshedV2.response) + + assertEquals(listOf(cachedV1.domain, refreshedV2.domain), newFlow.await()) + assertSame(refreshedV2.roomEntity, courseDao.storedCourseStructure.get()) + } + + @Test + fun `status refresh marker survives an old course structure completion`() = runTest { + val cachedStatus = CourseComponentStatus("cached") + val refreshedStatus = CourseComponentStatus("refreshed") + + completeOldCourseStructureAfterStartingNewSession() + coEvery { api.getCourseStatus(any(), COURSE_ID) } returns cachedStatus + assertEquals(cachedStatus.mapToDomain(), repository.getCourseStatus(COURSE_ID)) + + coEvery { api.getCourseStatus(any(), COURSE_ID) } returns refreshedStatus + assertEquals( + listOf(cachedStatus.mapToDomain(), refreshedStatus.mapToDomain()), + repository.getCourseStatusFlow(COURSE_ID).take(2).toList(), + ) + } + + @Test + fun `dates refresh marker survives an old course structure completion`() = runTest { + val cachedDates = courseDates(hasEnded = false) + val refreshedDates = courseDates(hasEnded = true) + + completeOldCourseStructureAfterStartingNewSession() + coEvery { api.getCourseDates(COURSE_ID, any(), any()) } returns cachedDates + assertEquals( + cachedDates.getCourseDatesResult(), + repository.getCourseDates(COURSE_ID, forceRefresh = true), + ) + + coEvery { api.getCourseDates(COURSE_ID, any(), any()) } returns refreshedDates + assertEquals( + listOf(cachedDates.getCourseDatesResult(), refreshedDates.getCourseDatesResult()), + repository.getCourseDatesFlow(COURSE_ID).take(2).toList(), + ) + } + + @Test + fun `progress refresh marker survives an old course structure completion`() = runTest { + val cachedProgress = courseProgress("cached") + val refreshedProgress = courseProgress("refreshed") + + completeOldCourseStructureAfterStartingNewSession() + coEvery { api.getCourseProgress(COURSE_ID) } returns cachedProgress + assertEquals( + cachedProgress.mapToDomain(), + repository.getCourseProgress( + courseId = COURSE_ID, + isRefresh = true, + getOnlyCacheIfExist = false, + ).first(), + ) + + coEvery { api.getCourseProgress(COURSE_ID) } returns refreshedProgress + assertEquals( + listOf(cachedProgress.mapToDomain(), refreshedProgress.mapToDomain()), + repository.getCourseProgress( + courseId = COURSE_ID, + isRefresh = false, + getOnlyCacheIfExist = true, + ).take(2).toList(), + ) + } + + @Test + fun `cache-first fetch started before reset keeps its original session`() = runTest { + val staleRoomValue = fixture("stale-room") + val oldResponse = fixture("old-response") + val refreshedValue = fixture("refreshed") + courseDao.storedCourseStructure.set(staleRoomValue.roomEntity) + val readGate = courseDao.gateNextRead() + + val oldCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + readGate.started.await() + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + readGate.release.complete(Unit) + + val oldRequest = apiRequests.receive() + oldRequest.response.complete(oldResponse.response) + assertSame(oldResponse.domain, oldCall.await()) + assertFalse(courseDao.insertCalls.any { it === oldResponse.roomEntity }) + + val newFlow = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID).take(2).toList() + } + val newRequest = apiRequests.tryReceive().getOrNull() + ?: error("Expected the new session to issue a refresh request") + newRequest.response.complete(refreshedValue.response) + + assertEquals(listOf(staleRoomValue.domain, refreshedValue.domain), newFlow.await()) + assertSame(refreshedValue.roomEntity, courseDao.storedCourseStructure.get()) + } + + @Test + fun `old fresh completion does not advance the new session completion version`() = runTest { + val oldFreshV1 = fixture("old-fresh-v1") + val newNonFreshV2 = fixture("new-non-fresh-v2") + val oldInsertGate = courseDao.gateInsert(oldFreshV1.roomEntity) + + val oldFreshCall = startFreshRequest() + apiRequests.receive().response.complete(oldFreshV1.response) + oldInsertGate.started.await() + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + + val newNonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val newNonFreshRequest = apiRequests.receive() + + oldInsertGate.release.complete(Unit) + oldFreshCall.await() + newNonFreshRequest.response.complete(newNonFreshV2.response) + assertSame(newNonFreshV2.domain, newNonFreshCall.await()) + + assertSame(newNonFreshV2.roomEntity, courseDao.storedCourseStructure.get()) + assertSame(newNonFreshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) + } + + @Test + fun `fresh request started before reset cannot write to the new session`() = runTest { + val oldFreshValue = fixture("old-fresh") + val newFreshValue = fixture("new-fresh") + + val oldFreshCall = startFreshRequest() + val oldFreshRequest = apiRequests.receive() + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + + oldFreshRequest.response.complete(oldFreshValue.response) + assertSame(oldFreshValue.domain, oldFreshCall.await()) + assertFalse(courseDao.insertCalls.any { it === oldFreshValue.roomEntity }) + + val newFreshCall = startFreshRequest() + val newFreshRequest = apiRequests.receive() + newFreshRequest.response.complete(newFreshValue.response) + + assertSame(newFreshValue.domain, newFreshCall.await()) + assertSame(newFreshValue.roomEntity, courseDao.storedCourseStructure.get()) + } + + private fun TestScope.startFreshRequest(): Deferred { + return async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFresh(COURSE_ID) + } + } + + private suspend fun TestScope.completeOldCourseStructureAfterStartingNewSession() { + val oldStructure = fixture("old") + val oldCall = async(start = CoroutineStart.UNDISPATCHED) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() + } + val oldRequest = apiRequests.receive() + + repository.endCourseSession() + repository.startCourseSession(COURSE_ID) + + oldRequest.response.complete(oldStructure.response) + assertSame(oldStructure.domain, oldCall.await()) + } + + private fun courseDates(hasEnded: Boolean) = CourseDates( + courseDateBlocks = emptyList(), + datesBannerInfo = null, + hasEnded = hasEnded, + ) + + private fun courseProgress(verifiedMode: String) = CourseProgressResponse( + verifiedMode = verifiedMode, + accessExpiration = null, + certificateData = null, + completionSummary = null, + courseGrade = null, + creditCourseRequirements = null, + end = null, + enrollmentMode = null, + gradingPolicy = null, + hasScheduledContent = null, + sectionScores = null, + studioUrl = null, + username = null, + userHasPassingGrade = null, + verificationData = null, + disableProgressGraph = null, + ) + + private fun fixture(version: String): CourseStructureFixture { + val domain = CoreMocks.mockCourseStructure.copy( + id = COURSE_ID, + name = version, + ) + val roomEntity = mockk() + val response = mockk() + every { roomEntity.mapToDomain() } returns domain + every { response.mapToDomain() } returns domain + every { response.mapToRoomEntity() } returns roomEntity + return CourseStructureFixture(domain, roomEntity, response) + } + + private suspend fun failureFrom(call: Deferred<*>): Throwable { + return try { + call.await() + fail("Expected the request to fail") + error("unreachable") + } catch (throwable: Throwable) { + throwable + } + } + + private suspend inline fun assertFailureType( + crossinline block: suspend () -> Unit, + ) { + val failure = try { + block() + fail("Expected ${T::class.simpleName}") + error("unreachable") + } catch (throwable: Throwable) { + throwable + } + assertTrue(failure is T) + } + + private fun httpException(statusCode: Int): HttpException { + val body = "{}".toResponseBody("application/json".toMediaType()) + return HttpException(Response.error(statusCode, body)) + } + + private class GatedCourseDao : CourseDao { + data class Gate( + val started: CompletableDeferred = CompletableDeferred(), + val release: CompletableDeferred = CompletableDeferred(), + ) + + val storedCourseStructure = AtomicReference() + val insertCalls = Collections.synchronizedList(mutableListOf()) + val readCount = AtomicInteger(0) + + private var nextReadGate: Gate? = null + private var gatedInsertEntity: CourseStructureEntity? = null + private var insertGate: Gate? = null + + fun gateNextRead(): Gate { + return Gate().also { nextReadGate = it } + } + + fun gateInsert(roomEntity: CourseStructureEntity): Gate { + gatedInsertEntity = roomEntity + return Gate().also { insertGate = it } + } + + override suspend fun getCourseStructureById(id: String): CourseStructureEntity? { + readCount.incrementAndGet() + val structureAtReadStart = storedCourseStructure.get() + val gate = nextReadGate + nextReadGate = null + if (gate != null) { + gate.started.complete(Unit) + gate.release.await() + } + return structureAtReadStart + } + + override suspend fun insertCourseStructureEntity( + vararg courseStructureEntity: CourseStructureEntity, + ) { + for (roomEntity in courseStructureEntity) { + insertCalls.add(roomEntity) + if (roomEntity === gatedInsertEntity) { + val gate = insertGate + gate?.started?.complete(Unit) + gate?.release?.await() + } + storedCourseStructure.set(roomEntity) + } + } + + override suspend fun clearCourseStructure() { + storedCourseStructure.set(null) + } + + override suspend fun clearVideoProgress() = Unit + + override suspend fun clearEnrollmentCachedData() = Unit + + override suspend fun clearCourseProgressData() = Unit + + override suspend fun insertCourseEnrollmentDetailsEntity( + vararg courseEnrollmentDetailsEntity: CourseEnrollmentDetailsEntity, + ) = Unit + + override suspend fun getCourseEnrollmentDetailsById( + id: String, + ): CourseEnrollmentDetailsEntity? = null + + override suspend fun insertVideoProgressEntity( + vararg videoProgressEntity: VideoProgressEntity, + ) = Unit + + override suspend fun getVideoProgressByBlockId(blockId: String): VideoProgressEntity? = null + + override suspend fun insertCourseProgressEntity( + vararg courseProgressEntity: CourseProgressEntity, + ) = Unit + + override suspend fun getCourseProgressById(id: String): CourseProgressEntity? = null + } + + private companion object { + const val COURSE_ID = "course-v1:TestX+Freshness+2026" + } +} diff --git a/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt b/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt new file mode 100644 index 000000000..89b1657e1 --- /dev/null +++ b/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt @@ -0,0 +1,102 @@ +package org.openedx.course.domain.interactor + +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Before +import org.junit.Test +import org.openedx.core.CoreMocks +import org.openedx.core.exception.NoCachedDataException +import org.openedx.core.system.connection.NetworkConnection +import org.openedx.course.data.repository.CourseRepository + +@OptIn(ExperimentalCoroutinesApi::class) +class CourseInteractorFreshTest { + + private lateinit var repository: CourseRepository + private lateinit var networkConnection: NetworkConnection + private lateinit var interactor: CourseInteractor + + @Before + fun setUp() { + repository = mockk() + networkConnection = mockk() + interactor = CourseInteractor(repository, networkConnection) + } + + @Test + fun `offline refresh returns cached data without using the fresh path`() = runTest { + val cachedStructure = CoreMocks.mockCourseStructure.copy(name = "cached") + every { networkConnection.isOnline() } returns false + every { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true) + } returns flowOf(cachedStructure) + + val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) + + assertSame(cachedStructure, result) + coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } + } + + @Test + fun `offline refresh without cached data throws the cache exception`() = runTest { + every { networkConnection.isOnline() } returns false + every { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true) + } returns flow { throw NoCachedDataException() } + + val failure = try { + interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) + fail("Expected NoCachedDataException") + error("unreachable") + } catch (throwable: Throwable) { + throwable + } + + assertTrue(failure is NoCachedDataException) + coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } + } + + @Test + fun `online refresh uses the fresh repository path`() = runTest { + val freshStructure = CoreMocks.mockCourseStructure.copy(name = "fresh") + every { networkConnection.isOnline() } returns true + coEvery { repository.getCourseStructureFresh(COURSE_ID) } returns freshStructure + + val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) + + assertSame(freshStructure, result) + coVerify(exactly = 1) { repository.getCourseStructureFresh(COURSE_ID) } + verify(exactly = 0) { repository.getCourseStructureFlow(any(), any()) } + } + + @Test + fun `cache-first request keeps Flow behavior`() = runTest { + val cachedStructure = CoreMocks.mockCourseStructure.copy(name = "cached") + every { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = false) + } returns flowOf(cachedStructure) + + val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = false) + + assertSame(cachedStructure, result) + verify(exactly = 1) { + repository.getCourseStructureFlow(COURSE_ID, forceRefresh = false) + } + coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } + verify(exactly = 0) { networkConnection.isOnline() } + } + + private companion object { + const val COURSE_ID = "course-v1:TestX+Freshness+2026" + } +} From b49f95d27fba8815dba2fe92f7920c4f75070c22 Mon Sep 17 00:00:00 2001 From: Eddie Date: Tue, 8 Sep 2026 16:18:42 -0400 Subject: [PATCH 2/4] docs: clarify course freshness test behavior --- .../org/openedx/course/data/repository/CoalescingCacheTest.kt | 2 +- .../course/data/repository/CourseRepositoryFreshTest.kt | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt index 1c5b4f77b..0fb96dfbf 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt @@ -34,7 +34,7 @@ class CoalescingCacheTest { } @Test - fun `cancelled fetch cannot remove a newer pending request`() = runTest { + fun `old fetch completion cannot remove a newer pending request`() = runTest { val fetchGates = Channel>(Channel.UNLIMITED) var fetchCount = 0 val cache = CoalescingCache( diff --git a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt index 1c0fa3caf..36980ab51 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt @@ -689,6 +689,10 @@ class CourseRepositoryFreshTest { return HttpException(Response.error(statusCode, body)) } + /** + * Snapshots data when a read starts and uses CompletableDeferred gates to pause reads or inserts. + * Models operation ordering, not real Room transactions or HTTP behavior. + */ private class GatedCourseDao : CourseDao { data class Gate( val started: CompletableDeferred = CompletableDeferred(), From 9282e9aa1f27b0d9a627b40f26999330c582b520 Mon Sep 17 00:00:00 2001 From: Eddie Date: Tue, 8 Sep 2026 16:25:31 -0400 Subject: [PATCH 3/4] test: consolidate course freshness regression coverage --- .../data/repository/CoalescingCacheTest.kt | 52 +-- .../repository/CourseRepositoryFreshTest.kt | 401 +++--------------- .../interactor/CourseInteractorFreshTest.kt | 102 ----- 3 files changed, 64 insertions(+), 491 deletions(-) delete mode 100644 course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt diff --git a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt index 0fb96dfbf..cdf9a2520 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt @@ -6,6 +6,7 @@ import kotlinx.coroutines.Deferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -34,7 +35,7 @@ class CoalescingCacheTest { } @Test - fun `old fetch completion cannot remove a newer pending request`() = runTest { + fun `cancellation starts a replacement request that survives old fetch completion`() = runTest { val fetchGates = Channel>(Channel.UNLIMITED) var fetchCount = 0 val cache = CoalescingCache( @@ -50,44 +51,7 @@ class CoalescingCacheTest { cache.getOrFetch(KEY, forceRefresh = true) } val oldGate = fetchGates.receive() - cache.cancelPending() - - val newFetch = async(start = CoroutineStart.UNDISPATCHED) { - cache.getOrFetch(KEY, forceRefresh = true) - } - val newGate = fetchGates.receive() - - oldGate.complete("old") - assertEquals("old", oldFetch.await()) - - val newWaiter = async(start = CoroutineStart.UNDISPATCHED) { - cache.getOrFetch(KEY, forceRefresh = true) - } - assertFalse(newWaiter.isCompleted) - assertTrue(fetchGates.tryReceive().isFailure) - - newGate.complete("new") - assertEquals("new", newFetch.await()) - assertEquals("new", newWaiter.await()) - assertEquals(2, fetchCount) - } - - @Test - fun `cancelling pending work removes it before a cancelled waiter starts a new request`() = runTest { - val fetchGates = Channel>(Channel.UNLIMITED) - val cache = CoalescingCache( - fetch = { - val gate = CompletableDeferred() - fetchGates.send(gate) - gate.await() - }, - ) - - val oldFetch = async(start = CoroutineStart.UNDISPATCHED) { - cache.getOrFetch(KEY, forceRefresh = true) - } - val oldGate = fetchGates.receive() - val cancelledWaiter = async(start = CoroutineStart.UNDISPATCHED) { + val cancelledWaiter = async(UnconfinedTestDispatcher(testScheduler)) { cache.getOrFetch(KEY, forceRefresh = true) } val laterCall = CompletableDeferred>() @@ -101,14 +65,24 @@ class CoalescingCacheTest { cache.cancelPending() + assertTrue(cancelledWaiter.isCancelled) val newFetch = laterCall.await() assertFalse(newFetch.isCancelled) val newGate = fetchGates.receive() oldGate.complete("old") assertEquals("old", oldFetch.await()) + + val newWaiter = async(start = CoroutineStart.UNDISPATCHED) { + cache.getOrFetch(KEY, forceRefresh = true) + } + assertFalse(newWaiter.isCompleted) + assertTrue(fetchGates.tryReceive().isFailure) + newGate.complete("new") assertEquals("new", newFetch.await()) + assertEquals("new", newWaiter.await()) + assertEquals(2, fetchCount) } private companion object { diff --git a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt index 36980ab51..81620881b 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt @@ -27,9 +27,6 @@ import org.junit.Before import org.junit.Test import org.openedx.core.CoreMocks import org.openedx.core.data.api.CourseApi -import org.openedx.core.data.model.CourseComponentStatus -import org.openedx.core.data.model.CourseDates -import org.openedx.core.data.model.CourseProgressResponse import org.openedx.core.data.model.CourseStructureModel import org.openedx.core.data.model.room.CourseEnrollmentDetailsEntity import org.openedx.core.data.model.room.CourseProgressEntity @@ -38,9 +35,9 @@ import org.openedx.core.data.model.room.VideoProgressEntity import org.openedx.core.data.storage.CorePreferences import org.openedx.core.data.storage.CourseDao import org.openedx.core.domain.model.CourseStructure -import org.openedx.core.exception.NoCachedDataException import org.openedx.core.module.db.DownloadDao import org.openedx.core.system.connection.NetworkConnection +import org.openedx.course.domain.interactor.CourseInteractor import retrofit2.HttpException import retrofit2.Response import java.net.UnknownHostException @@ -68,6 +65,7 @@ class CourseRepositoryFreshTest { private lateinit var preferencesManager: CorePreferences private lateinit var networkConnection: NetworkConnection private lateinit var repository: CourseRepository + private lateinit var interactor: CourseInteractor private lateinit var apiRequests: Channel @Before @@ -100,28 +98,47 @@ class CourseRepositoryFreshTest { preferencesManager = preferencesManager, networkConnection = networkConnection, ) + interactor = CourseInteractor(repository, networkConnection) } @Test - fun `fresh fetch returns the server response and updates Room and memory`() = runTest { + fun `online refresh returns the server response and updates Room and memory`() = runTest { val cachedV1 = fixture("v1") val originV2 = fixture("v2") courseDao.storedCourseStructure.set(cachedV1.roomEntity) assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) - val freshCall = startFreshRequest() + val readsBeforeRefresh = courseDao.readCount.get() + val freshCall = async(start = CoroutineStart.UNDISPATCHED) { + interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) + } val request = apiRequests.receive() assertEquals(COURSE_ID, request.courseId) assertEquals("no-cache", request.cacheControl) request.response.complete(originV2.response) assertSame(originV2.domain, freshCall.await()) + assertEquals(readsBeforeRefresh, courseDao.readCount.get()) assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) } @Test - fun `fresh fetch failures propagate without returning cached data`() = runTest { + fun `offline refresh returns saved course data without a network request`() = runTest { + val cachedV1 = fixture("v1") + courseDao.storedCourseStructure.set(cachedV1.roomEntity) + every { networkConnection.isOnline() } returns false + + val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) + + assertSame(cachedV1.domain, result) + assertEquals(1, courseDao.readCount.get()) + assertTrue(apiRequests.tryReceive().isFailure) + assertTrue(courseDao.insertCalls.isEmpty()) + } + + @Test + fun `online refresh failures propagate without returning cached data`() = runTest { val cachedV1 = fixture("v1") courseDao.storedCourseStructure.set(cachedV1.roomEntity) assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) @@ -136,7 +153,7 @@ class CourseRepositoryFreshTest { for (failure in failures) { val actualFailure = supervisorScope { val freshCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFresh(COURSE_ID) + interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) } val request = apiRequests.receive() request.response.completeExceptionally(failure) @@ -146,18 +163,26 @@ class CourseRepositoryFreshTest { assertEquals(failure::class, actualFailure::class) assertEquals(failure.message, actualFailure.message) assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) + assertSame(cachedV1.roomEntity, courseDao.storedCourseStructure.get()) + assertTrue(courseDao.insertCalls.isEmpty()) } } @Test - fun `cache-first Flow emits the memory cache before the server result`() = runTest { + fun `ordinary reads remain cache-first and the Flow delivers the server update`() = runTest { val cachedV1 = fixture("v1") val serverV2 = fixture("v2") courseDao.storedCourseStructure.set(cachedV1.roomEntity) repository.getCourseStructureFromCache(COURSE_ID) + assertSame( + cachedV1.domain, + interactor.getCourseStructure(COURSE_ID, isNeedRefresh = false), + ) + assertTrue(apiRequests.tryReceive().isFailure) + val collection = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).toList() + interactor.getCourseStructureFlow(COURSE_ID, forceRefresh = true).toList() } val request = apiRequests.receive() assertEquals("stale-if-error=0", request.cacheControl) @@ -168,82 +193,34 @@ class CourseRepositoryFreshTest { } @Test - fun `concurrent fresh fetches share one request and persist once`() = runTest { - val originV2 = fixture("v2") + fun `concurrent fresh fetches share one request until the response is persisted`() = runTest { + val firstResponse = fixture("first-response") + val insertGate = courseDao.gateInsert(firstResponse.roomEntity) val firstCall = startFreshRequest() val secondCall = startFreshRequest() val request = apiRequests.receive() - assertTrue(apiRequests.tryReceive().isFailure) - request.response.complete(originV2.response) - - assertSame(originV2.domain, firstCall.await()) - assertSame(originV2.domain, secondCall.await()) - assertEquals(1, courseDao.insertCalls.count { it === originV2.roomEntity }) - } - - @Test - fun `later fresh fetch waits for the first response to persist`() = runTest { - val firstResponse = fixture("first-response") - val insertGate = courseDao.gateInsert(firstResponse.roomEntity) + assertFalse(firstCall.isCompleted) + assertFalse(secondCall.isCompleted) - val firstCall = startFreshRequest() - apiRequests.receive().response.complete(firstResponse.response) + request.response.complete(firstResponse.response) insertGate.started.await() val laterCall = startFreshRequest() assertTrue(apiRequests.tryReceive().isFailure) + assertFalse(firstCall.isCompleted) + assertFalse(secondCall.isCompleted) + assertFalse(laterCall.isCompleted) insertGate.release.complete(Unit) assertSame(firstResponse.domain, firstCall.await()) + assertSame(firstResponse.domain, secondCall.await()) assertSame(firstResponse.domain, laterCall.await()) assertSame(firstResponse.roomEntity, courseDao.storedCourseStructure.get()) assertEquals(1, courseDao.insertCalls.count { it === firstResponse.roomEntity }) } - @Test - fun `fresh and cache-first fetches send different cache headers`() = runTest { - val nonFreshValue = fixture("non-fresh") - val freshValue = fixture("fresh") - - val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val nonFreshRequest = apiRequests.receive() - assertEquals("stale-if-error=0", nonFreshRequest.cacheControl) - nonFreshRequest.response.complete(nonFreshValue.response) - assertSame(nonFreshValue.domain, nonFreshCall.await()) - - val freshCall = startFreshRequest() - val freshRequest = apiRequests.receive() - assertEquals("no-cache", freshRequest.cacheControl) - freshRequest.response.complete(freshValue.response) - assertSame(freshValue.domain, freshCall.await()) - } - - @Test - fun `fresh fetch never shares a pending cache-first request`() = runTest { - val nonFreshValue = fixture("non-fresh") - val freshValue = fixture("fresh") - - val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val nonFreshRequest = apiRequests.receive() - - val freshCall = startFreshRequest() - val freshRequest = apiRequests.receive() - - assertEquals("stale-if-error=0", nonFreshRequest.cacheControl) - assertEquals("no-cache", freshRequest.cacheControl) - nonFreshRequest.response.complete(nonFreshValue.response) - freshRequest.response.complete(freshValue.response) - - assertSame(nonFreshValue.domain, nonFreshCall.await()) - assertSame(freshValue.domain, freshCall.await()) - } - @Test fun `stale cache-first fetch returns the fresh result after fresh completion`() = runTest { val staleV1 = fixture("v1") @@ -256,6 +233,12 @@ class CourseRepositoryFreshTest { val freshCall = startFreshRequest() val freshRequest = apiRequests.receive() + assertEquals("stale-if-error=0", nonFreshRequest.cacheControl) + assertEquals("no-cache", freshRequest.cacheControl) + assertEquals(COURSE_ID, nonFreshRequest.courseId) + assertEquals(COURSE_ID, freshRequest.courseId) + assertTrue(apiRequests.tryReceive().isFailure) + freshRequest.response.complete(freshV2.response) assertSame(freshV2.domain, freshCall.await()) assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) @@ -266,28 +249,6 @@ class CourseRepositoryFreshTest { assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) } - @Test - fun `fresh fetch replaces an earlier cache-first result`() = runTest { - val nonFreshV1 = fixture("v1") - val freshV2 = fixture("v2") - - val nonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val nonFreshRequest = apiRequests.receive() - val freshCall = startFreshRequest() - val freshRequest = apiRequests.receive() - - nonFreshRequest.response.complete(nonFreshV1.response) - assertSame(nonFreshV1.domain, nonFreshCall.await()) - assertSame(nonFreshV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) - - freshRequest.response.complete(freshV2.response) - assertSame(freshV2.domain, freshCall.await()) - assertSame(freshV2.roomEntity, courseDao.storedCourseStructure.get()) - assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) - } - @Test fun `delayed Room read does not overwrite a completed fresh fetch`() = runTest { val roomV1 = fixture("v1") @@ -334,34 +295,6 @@ class CourseRepositoryFreshTest { assertSame(freshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) } - @Test - fun `old session does not write after reset when it reaches the write guard`() = runTest { - val mutexHolder = fixture("mutex-holder") - val lateOldValue = fixture("late-old") - val insertGate = courseDao.gateInsert(mutexHolder.roomEntity) - - val holderCall = startFreshRequest() - apiRequests.receive().response.complete(mutexHolder.response) - insertGate.started.await() - - val lateCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - apiRequests.receive().response.complete(lateOldValue.response) - - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - insertGate.release.complete(Unit) - - holderCall.await() - lateCall.await() - assertFalse(courseDao.insertCalls.any { it === lateOldValue.roomEntity }) - courseDao.storedCourseStructure.set(null) - assertFailureType { - repository.getCourseStructureFromCache(COURSE_ID) - } - } - @Test fun `new session write is final after an old Room insert resumes`() = runTest { val oldV1 = fixture("old-v1") @@ -390,137 +323,6 @@ class CourseRepositoryFreshTest { assertSame(newV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) } - @Test - fun `fresh fetch does not read an old Room row`() = runTest { - val staleRoomV1 = fixture("stale-room-v1") - val originV2 = fixture("origin-v2") - courseDao.storedCourseStructure.set(staleRoomV1.roomEntity) - - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - val readsBeforeFreshCall = courseDao.readCount.get() - - val freshCall = startFreshRequest() - apiRequests.receive().response.complete(originV2.response) - - assertSame(originV2.domain, freshCall.await()) - assertEquals(readsBeforeFreshCall, courseDao.readCount.get()) - assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) - assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) - } - - @Test - fun `cache-first read may return old Room data until a fresh fetch replaces it`() = runTest { - val staleRoomV1 = fixture("stale-room-v1") - val originV2 = fixture("origin-v2") - courseDao.storedCourseStructure.set(staleRoomV1.roomEntity) - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - - every { networkConnection.isOnline() } returns false - val displayedStructure = repository.getCourseStructureFlow(COURSE_ID).first() - assertSame(staleRoomV1.domain, displayedStructure) - - every { networkConnection.isOnline() } returns true - val freshCall = startFreshRequest() - apiRequests.receive().response.complete(originV2.response) - assertSame(originV2.domain, freshCall.await()) - assertSame(originV2.roomEntity, courseDao.storedCourseStructure.get()) - assertSame(originV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) - } - - @Test - fun `old session completion does not clear a new session refresh marker`() = runTest { - val oldValue = fixture("old") - val cachedV1 = fixture("cached-v1") - val refreshedV2 = fixture("refreshed-v2") - - val oldCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val oldRequest = apiRequests.receive() - - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - courseDao.storedCourseStructure.set(cachedV1.roomEntity) - assertSame(cachedV1.domain, repository.getCourseStructureFromCache(COURSE_ID)) - - oldRequest.response.complete(oldValue.response) - assertSame(oldValue.domain, oldCall.await()) - - val newFlow = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID).take(2).toList() - } - val newRequest = apiRequests.receive() - assertEquals("stale-if-error=0", newRequest.cacheControl) - newRequest.response.complete(refreshedV2.response) - - assertEquals(listOf(cachedV1.domain, refreshedV2.domain), newFlow.await()) - assertSame(refreshedV2.roomEntity, courseDao.storedCourseStructure.get()) - } - - @Test - fun `status refresh marker survives an old course structure completion`() = runTest { - val cachedStatus = CourseComponentStatus("cached") - val refreshedStatus = CourseComponentStatus("refreshed") - - completeOldCourseStructureAfterStartingNewSession() - coEvery { api.getCourseStatus(any(), COURSE_ID) } returns cachedStatus - assertEquals(cachedStatus.mapToDomain(), repository.getCourseStatus(COURSE_ID)) - - coEvery { api.getCourseStatus(any(), COURSE_ID) } returns refreshedStatus - assertEquals( - listOf(cachedStatus.mapToDomain(), refreshedStatus.mapToDomain()), - repository.getCourseStatusFlow(COURSE_ID).take(2).toList(), - ) - } - - @Test - fun `dates refresh marker survives an old course structure completion`() = runTest { - val cachedDates = courseDates(hasEnded = false) - val refreshedDates = courseDates(hasEnded = true) - - completeOldCourseStructureAfterStartingNewSession() - coEvery { api.getCourseDates(COURSE_ID, any(), any()) } returns cachedDates - assertEquals( - cachedDates.getCourseDatesResult(), - repository.getCourseDates(COURSE_ID, forceRefresh = true), - ) - - coEvery { api.getCourseDates(COURSE_ID, any(), any()) } returns refreshedDates - assertEquals( - listOf(cachedDates.getCourseDatesResult(), refreshedDates.getCourseDatesResult()), - repository.getCourseDatesFlow(COURSE_ID).take(2).toList(), - ) - } - - @Test - fun `progress refresh marker survives an old course structure completion`() = runTest { - val cachedProgress = courseProgress("cached") - val refreshedProgress = courseProgress("refreshed") - - completeOldCourseStructureAfterStartingNewSession() - coEvery { api.getCourseProgress(COURSE_ID) } returns cachedProgress - assertEquals( - cachedProgress.mapToDomain(), - repository.getCourseProgress( - courseId = COURSE_ID, - isRefresh = true, - getOnlyCacheIfExist = false, - ).first(), - ) - - coEvery { api.getCourseProgress(COURSE_ID) } returns refreshedProgress - assertEquals( - listOf(cachedProgress.mapToDomain(), refreshedProgress.mapToDomain()), - repository.getCourseProgress( - courseId = COURSE_ID, - isRefresh = false, - getOnlyCacheIfExist = true, - ).take(2).toList(), - ) - } - @Test fun `cache-first fetch started before reset keeps its original session`() = runTest { val staleRoomValue = fixture("stale-room") @@ -554,100 +356,12 @@ class CourseRepositoryFreshTest { assertSame(refreshedValue.roomEntity, courseDao.storedCourseStructure.get()) } - @Test - fun `old fresh completion does not advance the new session completion version`() = runTest { - val oldFreshV1 = fixture("old-fresh-v1") - val newNonFreshV2 = fixture("new-non-fresh-v2") - val oldInsertGate = courseDao.gateInsert(oldFreshV1.roomEntity) - - val oldFreshCall = startFreshRequest() - apiRequests.receive().response.complete(oldFreshV1.response) - oldInsertGate.started.await() - - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - - val newNonFreshCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val newNonFreshRequest = apiRequests.receive() - - oldInsertGate.release.complete(Unit) - oldFreshCall.await() - newNonFreshRequest.response.complete(newNonFreshV2.response) - assertSame(newNonFreshV2.domain, newNonFreshCall.await()) - - assertSame(newNonFreshV2.roomEntity, courseDao.storedCourseStructure.get()) - assertSame(newNonFreshV2.domain, repository.getCourseStructureFromCache(COURSE_ID)) - } - - @Test - fun `fresh request started before reset cannot write to the new session`() = runTest { - val oldFreshValue = fixture("old-fresh") - val newFreshValue = fixture("new-fresh") - - val oldFreshCall = startFreshRequest() - val oldFreshRequest = apiRequests.receive() - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - - oldFreshRequest.response.complete(oldFreshValue.response) - assertSame(oldFreshValue.domain, oldFreshCall.await()) - assertFalse(courseDao.insertCalls.any { it === oldFreshValue.roomEntity }) - - val newFreshCall = startFreshRequest() - val newFreshRequest = apiRequests.receive() - newFreshRequest.response.complete(newFreshValue.response) - - assertSame(newFreshValue.domain, newFreshCall.await()) - assertSame(newFreshValue.roomEntity, courseDao.storedCourseStructure.get()) - } - private fun TestScope.startFreshRequest(): Deferred { return async(start = CoroutineStart.UNDISPATCHED) { repository.getCourseStructureFresh(COURSE_ID) } } - private suspend fun TestScope.completeOldCourseStructureAfterStartingNewSession() { - val oldStructure = fixture("old") - val oldCall = async(start = CoroutineStart.UNDISPATCHED) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true).first() - } - val oldRequest = apiRequests.receive() - - repository.endCourseSession() - repository.startCourseSession(COURSE_ID) - - oldRequest.response.complete(oldStructure.response) - assertSame(oldStructure.domain, oldCall.await()) - } - - private fun courseDates(hasEnded: Boolean) = CourseDates( - courseDateBlocks = emptyList(), - datesBannerInfo = null, - hasEnded = hasEnded, - ) - - private fun courseProgress(verifiedMode: String) = CourseProgressResponse( - verifiedMode = verifiedMode, - accessExpiration = null, - certificateData = null, - completionSummary = null, - courseGrade = null, - creditCourseRequirements = null, - end = null, - enrollmentMode = null, - gradingPolicy = null, - hasScheduledContent = null, - sectionScores = null, - studioUrl = null, - username = null, - userHasPassingGrade = null, - verificationData = null, - disableProgressGraph = null, - ) - private fun fixture(version: String): CourseStructureFixture { val domain = CoreMocks.mockCourseStructure.copy( id = COURSE_ID, @@ -671,19 +385,6 @@ class CourseRepositoryFreshTest { } } - private suspend inline fun assertFailureType( - crossinline block: suspend () -> Unit, - ) { - val failure = try { - block() - fail("Expected ${T::class.simpleName}") - error("unreachable") - } catch (throwable: Throwable) { - throwable - } - assertTrue(failure is T) - } - private fun httpException(statusCode: Int): HttpException { val body = "{}".toResponseBody("application/json".toMediaType()) return HttpException(Response.error(statusCode, body)) diff --git a/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt b/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt deleted file mode 100644 index 89b1657e1..000000000 --- a/course/src/test/java/org/openedx/course/domain/interactor/CourseInteractorFreshTest.kt +++ /dev/null @@ -1,102 +0,0 @@ -package org.openedx.course.domain.interactor - -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.every -import io.mockk.mockk -import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertSame -import org.junit.Assert.assertTrue -import org.junit.Assert.fail -import org.junit.Before -import org.junit.Test -import org.openedx.core.CoreMocks -import org.openedx.core.exception.NoCachedDataException -import org.openedx.core.system.connection.NetworkConnection -import org.openedx.course.data.repository.CourseRepository - -@OptIn(ExperimentalCoroutinesApi::class) -class CourseInteractorFreshTest { - - private lateinit var repository: CourseRepository - private lateinit var networkConnection: NetworkConnection - private lateinit var interactor: CourseInteractor - - @Before - fun setUp() { - repository = mockk() - networkConnection = mockk() - interactor = CourseInteractor(repository, networkConnection) - } - - @Test - fun `offline refresh returns cached data without using the fresh path`() = runTest { - val cachedStructure = CoreMocks.mockCourseStructure.copy(name = "cached") - every { networkConnection.isOnline() } returns false - every { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true) - } returns flowOf(cachedStructure) - - val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) - - assertSame(cachedStructure, result) - coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } - } - - @Test - fun `offline refresh without cached data throws the cache exception`() = runTest { - every { networkConnection.isOnline() } returns false - every { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = true) - } returns flow { throw NoCachedDataException() } - - val failure = try { - interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) - fail("Expected NoCachedDataException") - error("unreachable") - } catch (throwable: Throwable) { - throwable - } - - assertTrue(failure is NoCachedDataException) - coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } - } - - @Test - fun `online refresh uses the fresh repository path`() = runTest { - val freshStructure = CoreMocks.mockCourseStructure.copy(name = "fresh") - every { networkConnection.isOnline() } returns true - coEvery { repository.getCourseStructureFresh(COURSE_ID) } returns freshStructure - - val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = true) - - assertSame(freshStructure, result) - coVerify(exactly = 1) { repository.getCourseStructureFresh(COURSE_ID) } - verify(exactly = 0) { repository.getCourseStructureFlow(any(), any()) } - } - - @Test - fun `cache-first request keeps Flow behavior`() = runTest { - val cachedStructure = CoreMocks.mockCourseStructure.copy(name = "cached") - every { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = false) - } returns flowOf(cachedStructure) - - val result = interactor.getCourseStructure(COURSE_ID, isNeedRefresh = false) - - assertSame(cachedStructure, result) - verify(exactly = 1) { - repository.getCourseStructureFlow(COURSE_ID, forceRefresh = false) - } - coVerify(exactly = 0) { repository.getCourseStructureFresh(any()) } - verify(exactly = 0) { networkConnection.isOnline() } - } - - private companion object { - const val COURSE_ID = "course-v1:TestX+Freshness+2026" - } -} From 4575c09b360146bcef3c2112d45e446b4b5cac52 Mon Sep 17 00:00:00 2001 From: Eddie Date: Wed, 9 Sep 2026 13:58:34 -0400 Subject: [PATCH 4/4] refactor: remove unused freshness state and clarify comments --- .../course/data/repository/CoalescingCache.kt | 19 ++++---- .../data/repository/CourseRepository.kt | 46 +++++++++---------- .../data/repository/CoalescingCacheTest.kt | 1 + .../repository/CourseRepositoryFreshTest.kt | 4 +- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt b/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt index 3c0f0b426..3270f7498 100644 --- a/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt +++ b/course/src/main/java/org/openedx/course/data/repository/CoalescingCache.kt @@ -4,10 +4,10 @@ import kotlinx.coroutines.CompletableDeferred import java.util.concurrent.ConcurrentHashMap /** - * A cache with request coalescing support. + * Stores cached values and shares fetches for the same key. * - * When multiple callers request the same data simultaneously, - * only one fetch operation is performed and all callers receive the same result. + * If a caller needs to fetch a value and another fetch for that key is already running, + * it waits for that result instead of starting a duplicate request. * * @param K the type of cache keys * @param V the type of cached values @@ -32,9 +32,9 @@ class CoalescingCache( private val pending = ConcurrentHashMap>() /** - * Returns the cached value for [key], or null when no usable value is cached. + * Returns the cached value for [key], or null if no value is stored. * - * When [activeGeneration] is set, a value saved under a different version is not usable. + * When [activeGeneration] is set, also returns null if the stored version is no longer current. */ fun getCached(key: K): V? { val entry = cache[key] ?: return null @@ -96,12 +96,11 @@ class CoalescingCache( } /** - * Stops waiting callers from sharing requests that were pending when this method began. + * Removes pending request entries and cancels callers waiting for their results. * - * It removes each recorded request before cancelling callers waiting for it, so a caller that - * resumes can start a new request. A fetch that already started can still finish. When it does, - * it removes a pending request only if that request still belongs to it, not a newer request - * for the same key. + * Remove each entry before cancellation so a waiting caller can start a replacement request + * from its cancellation callback. Fetches already running can still finish. Their cleanup + * removes only their own entry, preserving any replacement request for the same key. */ fun cancelPending() { val pendingSnapshot = HashMap(pending) diff --git a/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt b/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt index 1ca3adafe..a93d8cf37 100644 --- a/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt +++ b/course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt @@ -32,8 +32,9 @@ import java.util.concurrent.atomic.AtomicLong /** * Repository for course data with request coalescing. * - * When multiple callers request the same data simultaneously, - * only one network request is made and all callers receive the same result. + * Concurrent requests share work within each cache. Course-structure requests are grouped by + * course and session, with explicit refreshes kept separate from requests through + * [getCourseStructureFlow]. */ @Suppress("TooManyFunctions") class CourseRepository( @@ -55,8 +56,8 @@ class CourseRepository( ) /** - * Holds the course structure and local Room record returned by one request that does not require - * a refresh. + * Holds the course structure and local Room record returned by a request through + * [getCourseStructureFlow]. * * If an explicit refresh saves newer course structure while this request is running, * [returnedCourseStructure] is replaced with that newer cached value. The Flow then does not @@ -74,11 +75,10 @@ class CourseRepository( /** * Holds the course structure and local Room record returned by one explicit refresh request. * - * They stay together until saved, so a later response cannot pair one response's structure with - * another response's database record. + * Keeping both representations together prevents saving a database record from a different + * response. */ private data class FreshFetchResult( - val capturedSessionGeneration: Long, val courseStructure: CourseStructure, val roomEntity: CourseStructureEntity, ) @@ -93,23 +93,22 @@ class CourseRepository( private val sessionGeneration = AtomicLong(0) /** - * Holds one `Mutex`, a coroutine-safe lock, for each course. + * Holds one `Mutex` per course so the following operations run one at a time. * - * The mutex makes four operations take turns: saving a normal response; saving a refresh - * response; placing a database result in memory cache for a Flow; and placing one in memory - * cache for a cache-only read. + * This covers saving responses from [getCourseStructureFlow], saving explicit refresh + * responses, and adding Room results to memory from either a Flow or a cache-only read. * - * The mutex remains after a session ends because an older database write may already hold it. - * A later session's write waits for that write to finish, then becomes the final stored value. + * Keep the same mutex after a session ends: an old write may still hold it. A new session's + * write must wait for that write to finish so the old value cannot overwrite the new value. */ private val courseWriteMutexes = ConcurrentHashMap() /** * Counts refresh responses saved for each course and session. * - * A request that does not require a refresh remembers this count before fetching data. If the - * count changes before that request finishes, a refresh saved newer data first, so the older - * result is not stored or returned. + * A request through [getCourseStructureFlow] captures this count before fetching data. If the + * count changes before that request finishes, an explicit refresh saved newer data first, so + * the older result is not stored or returned. */ private val freshCompletionVersion = ConcurrentHashMap() @@ -173,7 +172,7 @@ class CourseRepository( ) /** - * Keeps explicit refresh requests separate from normal requests. + * Keeps explicit refresh requests separate from requests through [getCourseStructureFlow]. * * The two paths send different `Cache-Control` headers and must not share a response, so they * use separate `CoalescingCache` instances. @@ -188,7 +187,6 @@ class CourseRepository( courseId, ) FreshFetchResult( - capturedSessionGeneration = courseSessionKey.sessionGeneration, courseStructure = response.mapToDomain(), roomEntity = response.mapToRoomEntity(), ) @@ -210,7 +208,6 @@ class CourseRepository( needsRefresh.remove(courseSessionKey) }, autoCache = false, - activeGeneration = { sessionGeneration.get() }, ) private val statusCache = CoalescingCache( @@ -250,7 +247,6 @@ class CourseRepository( structureCache.cancelPending() freshStructureCache.cancelPending() structureCache.clear() - freshStructureCache.clear() statusCache.clear() datesCache.clear() progressCache.clear() @@ -537,10 +533,10 @@ class CourseRepository( } /** - * Runs [block] while holding this course's `Mutex`. + * Runs [block] while holding this course's `Mutex`, provided the session is still current. * - * If the course session ends while the call is waiting for the mutex, [block] does not run and - * this function returns null. + * After acquiring the mutex, returns null without running [block] if [capturedGeneration] + * no longer matches the current session. */ private suspend fun runUnderCourseWriteGuard( courseId: String, @@ -560,8 +556,8 @@ class CourseRepository( * Decides whether course structure read from the local Room database may be added to memory * cache. * - * If an explicit refresh saved newer course structure while Room was being read, returns that - * newer cached structure instead. Otherwise, caches and returns the Room result. + * Returns an existing memory value when available. Otherwise, caches and returns the Room + * result only if no explicit refresh completed while Room was being read. */ private fun resolveStructureFromRoom( courseId: String, diff --git a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt index cdf9a2520..ec33ce787 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CoalescingCacheTest.kt @@ -51,6 +51,7 @@ class CoalescingCacheTest { cache.getOrFetch(KEY, forceRefresh = true) } val oldGate = fetchGates.receive() + // Start a new request while the waiting caller is being cancelled to verify it survives cancellation. val cancelledWaiter = async(UnconfinedTestDispatcher(testScheduler)) { cache.getOrFetch(KEY, forceRefresh = true) } diff --git a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt index 81620881b..197eba3e6 100644 --- a/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt +++ b/course/src/test/java/org/openedx/course/data/repository/CourseRepositoryFreshTest.kt @@ -391,8 +391,8 @@ class CourseRepositoryFreshTest { } /** - * Snapshots data when a read starts and uses CompletableDeferred gates to pause reads or inserts. - * Models operation ordering, not real Room transactions or HTTP behavior. + * Captures the stored value before pausing a read. Tests use [CompletableDeferred] to control + * when reads and inserts finish. This fake does not simulate Room transactions or HTTP behavior. */ private class GatedCourseDao : CourseDao { data class Gate(