From cbb46cfb0dcb8854066569611791acb73f49599b Mon Sep 17 00:00:00 2001 From: William Brawner Date: Tue, 14 Nov 2023 09:57:24 -0700 Subject: [PATCH 1/5] Add cacheOnly option to StoreReadRequest Signed-off-by: William Brawner --- .../store/store5/StoreReadRequest.kt | 17 ++++- .../store/store5/impl/RealStore.kt | 18 +++++- .../store5/StoreWithInMemoryCacheTests.kt | 62 +++++++++++++++++++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt index a10c3afcc..166834bba 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt @@ -21,13 +21,14 @@ package org.mobilenativefoundation.store.store5 * @param skippedCaches List of cache types that should be skipped when retuning the response see [CacheType] * @param refresh If set to true [Store] will always get fresh value from fetcher while also * starting the stream from the local [com.dropbox.android.external.store4.impl.SourceOfTruth] and memory cache - * + * @param fetch If set to false, then fetcher will not be used */ data class StoreReadRequest private constructor( val key: Key, private val skippedCaches: Int, val refresh: Boolean = false, - val fallBackToSourceOfTruth: Boolean = false + val fallBackToSourceOfTruth: Boolean = false, + val fetch: Boolean = true ) { internal fun shouldSkipCache(type: CacheType) = skippedCaches.and(type.flag) != 0 @@ -57,7 +58,8 @@ data class StoreReadRequest private constructor( ) /** - * Create a [StoreReadRequest] which will return data from memory/disk caches + * Create a [StoreReadRequest] which will return data from memory/disk caches if present, + * otherwise will hit your fetcher (filling your caches). * @param refresh if true then return fetcher (new) data as well (updating your caches) */ fun cached(key: Key, refresh: Boolean) = StoreReadRequest( @@ -66,6 +68,15 @@ data class StoreReadRequest private constructor( refresh = refresh ) + /** + * Create a [StoreReadRequest] which will return data from memory/disk caches + */ + fun cacheOnly(key: Key) = StoreReadRequest( + key = key, + skippedCaches = 0, + fetch = false + ) + /** * Create a [StoreReadRequest] which will return data from disk cache * @param refresh if true then return fetcher (new) data as well (updating your caches) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt index 2a7c30140..fdce62795 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt @@ -85,10 +85,24 @@ internal class RealStore( } } - cachedToEmit?.let { it: Output -> + if (cachedToEmit != null) { // if we read a value from cache, dispatch it first - emit(StoreReadResponse.Data(value = it, origin = StoreReadResponseOrigin.Cache)) + emit( + StoreReadResponse.Data( + value = cachedToEmit, + origin = StoreReadResponseOrigin.Cache + ) + ) + if (!request.fetch) { + // This request was only for cached items, so we stop here + return@flow + } + } else if (!request.fetch) { + // The cache is empty or invalid but we don't want to hit the fetcher + emit(StoreReadResponse.NoNewData(origin = StoreReadResponseOrigin.Cache)) + return@flow } + val stream: Flow> = if (sourceOfTruth == null) { // piggypack only if not specified fresh data AND we emitted a value from the cache val piggybackOnly = !request.refresh && cachedToEmit != null diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt index f0cd08ca1..66ace5796 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt @@ -1,12 +1,15 @@ package org.mobilenativefoundation.store.store5 +import kotlinx.atomicfu.atomic import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.mobilenativefoundation.store.store5.impl.extensions.get import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.time.Duration import kotlin.time.Duration.Companion.hours @FlowPreview @@ -37,4 +40,63 @@ class StoreWithInMemoryCacheTests { assertEquals("result", c) assertEquals("result", d) } + + @Test + fun givenEmptyCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val store = StoreBuilder + .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) + .cachePolicy( + MemoryPolicy + .builder() + .build() + ) + .build() + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) + } + + @Test + fun givenPrimedCacheThenCacheOnlyRequestReturnsData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val store = StoreBuilder + .from(Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }) + .cachePolicy( + MemoryPolicy + .builder() + .build() + ) + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals("result", response.requireData()) + assertEquals(1, fetcherHitCounter.value) + } + + @Test + fun givenInvalidCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val store = StoreBuilder + .from(Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }) + .cachePolicy( + MemoryPolicy + .builder() + .setExpireAfterWrite(Duration.ZERO) + .build() + ) + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) + assertEquals(1, fetcherHitCounter.value) + } } From b71736b86cf6a34fed422c5bc4f7a51a647e198e Mon Sep 17 00:00:00 2001 From: William Brawner Date: Tue, 14 Nov 2023 10:26:01 -0700 Subject: [PATCH 2/5] Fix doc on StoreReadRequest.cacheOnly Signed-off-by: William Brawner --- .../org/mobilenativefoundation/store/store5/StoreReadRequest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt index 166834bba..7dc0b2b57 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt @@ -69,7 +69,7 @@ data class StoreReadRequest private constructor( ) /** - * Create a [StoreReadRequest] which will return data from memory/disk caches + * Create a [StoreReadRequest] which will return data from memory caches */ fun cacheOnly(key: Key) = StoreReadRequest( key = key, From 5159609506c7c39060b339c07ebff4595a9f738c Mon Sep 17 00:00:00 2001 From: William Brawner Date: Wed, 29 Nov 2023 11:08:29 -0700 Subject: [PATCH 3/5] Hit disk caches for cacheOnly requests Signed-off-by: William Brawner --- .../store/store5/StoreReadRequest.kt | 3 +- .../store/store5/impl/RealStore.kt | 36 +++-- .../store/store5/CacheOnlyTests.kt | 149 ++++++++++++++++++ .../store5/StoreWithInMemoryCacheTests.kt | 62 -------- 4 files changed, 174 insertions(+), 76 deletions(-) create mode 100644 store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt index 7dc0b2b57..84dc0ee4f 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt @@ -69,7 +69,8 @@ data class StoreReadRequest private constructor( ) /** - * Create a [StoreReadRequest] which will return data from memory caches + * Create a [StoreReadRequest] which will return data from memory/disk caches if present, + * otherwise will return [StoreReadResponse.NoNewData] */ fun cacheOnly(key: Key) = StoreReadRequest( key = key, diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt index fdce62795..4d9803160 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt @@ -85,21 +85,20 @@ internal class RealStore( } } - if (cachedToEmit != null) { + cachedToEmit?.let { it: Output -> // if we read a value from cache, dispatch it first - emit( - StoreReadResponse.Data( - value = cachedToEmit, + emit(StoreReadResponse.Data(value = it, origin = StoreReadResponseOrigin.Cache)) + } + + if (sourceOfTruth == null && !request.fetch) { + if (memCache == null) { + emit(StoreReadResponse.Error.Exception( + error = IllegalStateException("Cache-only request made with no cache configured"), origin = StoreReadResponseOrigin.Cache - ) - ) - if (!request.fetch) { - // This request was only for cached items, so we stop here - return@flow + )) + } else if (cachedToEmit == null) { + emit(StoreReadResponse.NoNewData(origin = StoreReadResponseOrigin.Cache)) } - } else if (!request.fetch) { - // The cache is empty or invalid but we don't want to hit the fetcher - emit(StoreReadResponse.NoNewData(origin = StoreReadResponseOrigin.Cache)) return@flow } @@ -113,8 +112,19 @@ internal class RealStore( networkLock = null, piggybackOnly = piggybackOnly ) as Flow> // when no source of truth Input == Output - } else { + } else if (request.fetch) { diskNetworkCombined(request, sourceOfTruth) + } else { + val diskLock = CompletableDeferred() + diskLock.complete(Unit) + sourceOfTruth.reader(request.key, diskLock).transform { response -> + val data = response.dataOrNull() + if (data == null || validator?.isValid(data) == false) { + emit(StoreReadResponse.NoNewData(origin = response.origin)) + } else { + emit(StoreReadResponse.Data(value = data, origin = response.origin)) + } + } } emitAll( stream.transform { output: StoreReadResponse -> diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt new file mode 100644 index 000000000..12bccfe25 --- /dev/null +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt @@ -0,0 +1,149 @@ +package org.mobilenativefoundation.store.store5 + +import kotlinx.atomicfu.atomic +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.mobilenativefoundation.store.store5.impl.extensions.get +import org.mobilenativefoundation.store.store5.util.InMemoryPersister +import org.mobilenativefoundation.store.store5.util.asSourceOfTruth +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Duration + +class CacheOnlyTests { + private val testScope = TestScope() + + @Test + fun givenEmptyMemoryCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val store = StoreBuilder + .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) + .cachePolicy( + MemoryPolicy + .builder() + .build() + ) + .build() + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) + } + + @Test + fun givenPrimedMemoryCacheThenCacheOnlyRequestReturnsData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val store = StoreBuilder + .from(Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }) + .cachePolicy( + MemoryPolicy + .builder() + .build() + ) + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals("result", response.requireData()) + assertEquals(1, fetcherHitCounter.value) + } + + @Test + fun givenInvalidMemoryCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val store = StoreBuilder + .from(Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }) + .cachePolicy( + MemoryPolicy + .builder() + .setExpireAfterWrite(Duration.ZERO) + .build() + ) + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) + assertEquals(1, fetcherHitCounter.value) + } + + @Test + fun givenEmptyDiskCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val persister = InMemoryPersister() + val store = StoreBuilder + .from( + fetcher = Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }, + sourceOfTruth = persister.asSourceOfTruth() + ) + .disableCache() + .build() + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.SourceOfTruth), response) + } + + @Test + fun givenPrimedDiskCacheThenCacheOnlyRequestReturnsData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val persister = InMemoryPersister() + val store = StoreBuilder + .from( + fetcher = Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }, + sourceOfTruth = persister.asSourceOfTruth() + ) + .disableCache() + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals("result", response.requireData()) + assertEquals(StoreReadResponseOrigin.SourceOfTruth, response.origin) + assertEquals(1, fetcherHitCounter.value) + } + + @Test + fun givenInvalidDiskCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { + val fetcherHitCounter = atomic(0) + val persister = InMemoryPersister() + persister.write(0, "result") + val store = StoreBuilder + .from( + fetcher = Fetcher.of { _: Int -> + fetcherHitCounter += 1 + "result" + }, + sourceOfTruth = persister.asSourceOfTruth() + ) + .disableCache() + .validator(Validator.by { false }) + .build() + val a = store.get(0) + assertEquals("result", a) + assertEquals(1, fetcherHitCounter.value) + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.SourceOfTruth), response) + assertEquals(1, fetcherHitCounter.value) + } + + @Test + fun givenNoCacheThenCacheOnlyRequestReturnsError() = testScope.runTest { + val store = StoreBuilder + .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) + .disableCache() + .build() + val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + assertTrue(response is StoreReadResponse.Error.Exception) + assertTrue(response.error is IllegalStateException) + assertEquals(StoreReadResponseOrigin.Cache, response.origin) + } +} \ No newline at end of file diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt index 66ace5796..f0cd08ca1 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt @@ -1,15 +1,12 @@ package org.mobilenativefoundation.store.store5 -import kotlinx.atomicfu.atomic import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview -import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.mobilenativefoundation.store.store5.impl.extensions.get import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.time.Duration import kotlin.time.Duration.Companion.hours @FlowPreview @@ -40,63 +37,4 @@ class StoreWithInMemoryCacheTests { assertEquals("result", c) assertEquals("result", d) } - - @Test - fun givenEmptyCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { - val store = StoreBuilder - .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) - .cachePolicy( - MemoryPolicy - .builder() - .build() - ) - .build() - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() - assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) - } - - @Test - fun givenPrimedCacheThenCacheOnlyRequestReturnsData() = testScope.runTest { - val fetcherHitCounter = atomic(0) - val store = StoreBuilder - .from(Fetcher.of { _: Int -> - fetcherHitCounter += 1 - "result" - }) - .cachePolicy( - MemoryPolicy - .builder() - .build() - ) - .build() - val a = store.get(0) - assertEquals("result", a) - assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() - assertEquals("result", response.requireData()) - assertEquals(1, fetcherHitCounter.value) - } - - @Test - fun givenInvalidCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { - val fetcherHitCounter = atomic(0) - val store = StoreBuilder - .from(Fetcher.of { _: Int -> - fetcherHitCounter += 1 - "result" - }) - .cachePolicy( - MemoryPolicy - .builder() - .setExpireAfterWrite(Duration.ZERO) - .build() - ) - .build() - val a = store.get(0) - assertEquals("result", a) - assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() - assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) - assertEquals(1, fetcherHitCounter.value) - } } From 15c6f6327c52ec668d6cf009ae3b58192fa21a8d Mon Sep 17 00:00:00 2001 From: William Brawner Date: Fri, 1 Dec 2023 11:38:08 -0700 Subject: [PATCH 4/5] Rename cacheOnly to localOnly Signed-off-by: William Brawner --- .../store/store5/StoreReadRequest.kt | 2 +- .../{CacheOnlyTests.kt => LocalOnlyTests.kt} | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) rename store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/{CacheOnlyTests.kt => LocalOnlyTests.kt} (91%) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt index 84dc0ee4f..8cf581348 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/StoreReadRequest.kt @@ -72,7 +72,7 @@ data class StoreReadRequest private constructor( * Create a [StoreReadRequest] which will return data from memory/disk caches if present, * otherwise will return [StoreReadResponse.NoNewData] */ - fun cacheOnly(key: Key) = StoreReadRequest( + fun localOnly(key: Key) = StoreReadRequest( key = key, skippedCaches = 0, fetch = false diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt similarity index 91% rename from store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt rename to store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt index 12bccfe25..39e19ae91 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/CacheOnlyTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt @@ -12,7 +12,7 @@ import kotlin.test.assertEquals import kotlin.test.assertTrue import kotlin.time.Duration -class CacheOnlyTests { +class LocalOnlyTests { private val testScope = TestScope() @Test @@ -25,7 +25,7 @@ class CacheOnlyTests { .build() ) .build() - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) } @@ -46,7 +46,7 @@ class CacheOnlyTests { val a = store.get(0) assertEquals("result", a) assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals("result", response.requireData()) assertEquals(1, fetcherHitCounter.value) } @@ -69,7 +69,7 @@ class CacheOnlyTests { val a = store.get(0) assertEquals("result", a) assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.Cache), response) assertEquals(1, fetcherHitCounter.value) } @@ -84,7 +84,7 @@ class CacheOnlyTests { ) .disableCache() .build() - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.SourceOfTruth), response) } @@ -105,7 +105,7 @@ class CacheOnlyTests { val a = store.get(0) assertEquals("result", a) assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals("result", response.requireData()) assertEquals(StoreReadResponseOrigin.SourceOfTruth, response.origin) assertEquals(1, fetcherHitCounter.value) @@ -130,7 +130,7 @@ class CacheOnlyTests { val a = store.get(0) assertEquals("result", a) assertEquals(1, fetcherHitCounter.value) - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertEquals(StoreReadResponse.NoNewData(StoreReadResponseOrigin.SourceOfTruth), response) assertEquals(1, fetcherHitCounter.value) } @@ -141,7 +141,7 @@ class CacheOnlyTests { .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) .disableCache() .build() - val response = store.stream(StoreReadRequest.cacheOnly(0)).first() + val response = store.stream(StoreReadRequest.localOnly(0)).first() assertTrue(response is StoreReadResponse.Error.Exception) assertTrue(response.error is IllegalStateException) assertEquals(StoreReadResponseOrigin.Cache, response.origin) From 123d18178b3ca224e92f700e4cac7b110248a2a5 Mon Sep 17 00:00:00 2001 From: William Brawner Date: Fri, 1 Dec 2023 11:42:17 -0700 Subject: [PATCH 5/5] Send NoNewData and log warning for localOnly requests with no local data sources configured Signed-off-by: William Brawner --- .../store/store5/impl/RealStore.kt | 17 +++++++++++------ .../store/store5/LocalOnlyTests.kt | 6 +++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt index 4d9803160..878d22c72 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealStore.kt @@ -15,6 +15,8 @@ */ package org.mobilenativefoundation.store.store5.impl +import co.touchlab.kermit.CommonWriter +import co.touchlab.kermit.Logger import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow @@ -92,13 +94,9 @@ internal class RealStore( if (sourceOfTruth == null && !request.fetch) { if (memCache == null) { - emit(StoreReadResponse.Error.Exception( - error = IllegalStateException("Cache-only request made with no cache configured"), - origin = StoreReadResponseOrigin.Cache - )) - } else if (cachedToEmit == null) { - emit(StoreReadResponse.NoNewData(origin = StoreReadResponseOrigin.Cache)) + logger.w("Local-only request made with no cache or source of truth configured") } + emit(StoreReadResponse.NoNewData(origin = StoreReadResponseOrigin.Cache)) return@flow } @@ -336,4 +334,11 @@ internal class RealStore( sourceOfTruth?.reader(key, CompletableDeferred(Unit))?.map { it.dataOrNull() }?.first() private fun fromMemCache(key: Key) = memCache?.getIfPresent(key) + + companion object { + private val logger = Logger.apply { + setLogWriters(listOf(CommonWriter())) + setTag("Store") + } + } } diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt index 39e19ae91..b32044e65 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/LocalOnlyTests.kt @@ -1,5 +1,6 @@ package org.mobilenativefoundation.store.store5 +import co.touchlab.kermit.Logger import kotlinx.atomicfu.atomic import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.TestScope @@ -136,14 +137,13 @@ class LocalOnlyTests { } @Test - fun givenNoCacheThenCacheOnlyRequestReturnsError() = testScope.runTest { + fun givenNoCacheThenCacheOnlyRequestReturnsNoNewData() = testScope.runTest { val store = StoreBuilder .from(Fetcher.of { _: Int -> throw RuntimeException("Fetcher shouldn't be hit") }) .disableCache() .build() val response = store.stream(StoreReadRequest.localOnly(0)).first() - assertTrue(response is StoreReadResponse.Error.Exception) - assertTrue(response.error is IllegalStateException) + assertTrue(response is StoreReadResponse.NoNewData) assertEquals(StoreReadResponseOrigin.Cache, response.origin) } } \ No newline at end of file