Remove suspend cache (#52)

* remove suspend cache

This PR replaces suspend cahce usages w/ direct guava cache usage.
I think we should also get rid of guava cache.
First of all it is in java so needs to be moved to kotlin at least for
future multi-platform support.
Second, it would be nicer to have something that uses kotlin's time so
that people can provide a scope w/ a delay functionality (like the
TestCoroutineScope) and also test time related stuff. An alternative might
be letting people pass a Timer in the builder

Fixes #41

* remove commented code :/
This commit is contained in:
Yigit Boyar 2019-11-14 04:31:07 -08:00 committed by Mike Nakhimovich
parent b7a4c89cc0
commit 8347de79aa
17 changed files with 35 additions and 562 deletions

View file

@ -63,7 +63,6 @@ dependencies {
implementation project(':store')
implementation project(':cache')
implementation project(':filesystem')
implementation project(':suspendCache')
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$versions.kotlin"
implementation libraries.coroutinesCore

View file

@ -4,8 +4,8 @@ import android.os.Bundle
import android.view.View
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import com.nytimes.android.external.store4.FlowStoreBuilder
import com.nytimes.android.external.store4.MemoryPolicy
import com.nytimes.android.external.store4.StoreRequest
import com.nytimes.android.external.store4.fresh
import com.nytimes.android.external.store4.get

View file

@ -1 +1 @@
include ':app', ':store', ':cache', ':filesystem', ':suspendCache'
include ':app', ':store', ':cache', ':filesystem'

View file

@ -24,7 +24,6 @@ version = VERSION_NAME
dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
implementation project(path: ':cache')
implementation project(path: ':suspendCache')
implementation libraries.coroutinesCore

View file

@ -1,11 +1,11 @@
package com.nytimes.android.external.store4
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import com.nytimes.android.external.store4.impl.PersistentNonFlowingSourceOfTruth
import com.nytimes.android.external.store4.impl.PersistentSourceOfTruth
import com.nytimes.android.external.store4.impl.RealStore
import com.nytimes.android.external.store4.impl.SourceOfTruth
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flow
@ -79,6 +79,7 @@ class Builder<Key, Input, Output>(
return this
}
@ExperimentalCoroutinesApi
fun build(): Store<Key, Output> {
@Suppress("UNCHECKED_CAST")
return RealStore(

View file

@ -1,4 +1,4 @@
package com.nytimes.android.external.store3.base.impl
package com.nytimes.android.external.store4
import java.util.concurrent.TimeUnit

View file

@ -1,6 +1,5 @@
package com.nytimes.android.external.store4
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import java.util.concurrent.TimeUnit
internal object StoreDefaults {

View file

@ -1,15 +1,24 @@
package com.nytimes.android.external.store4.impl
import com.com.nytimes.suspendCache.StoreCache
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import com.nytimes.android.external.store4.*
import com.nytimes.android.external.cache3.CacheBuilder
import com.nytimes.android.external.store4.CacheType
import com.nytimes.android.external.store4.MemoryPolicy
import com.nytimes.android.external.store4.ResponseOrigin
import com.nytimes.android.external.store4.Store
import com.nytimes.android.external.store4.StoreRequest
import com.nytimes.android.external.store4.StoreResponse
import com.nytimes.android.external.store4.impl.operators.Either
import com.nytimes.android.external.store4.impl.operators.merge
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.*
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.transform
import kotlinx.coroutines.flow.withIndex
@ExperimentalCoroutinesApi
@FlowPreview
@ -31,18 +40,20 @@ class RealStore<Key, Input, Output>(
SourceOfTruthWithBarrier(it)
}
private val memCache = memoryPolicy?.let {
StoreCache.fromRequest<Key, Output?, StoreRequest<Key>>(
loader = {
TODO(
"""
This should've never been called. We don't need this anymore, should remove
loader after we clean old Store ?
""".trimIndent()
)
},
memoryPolicy = memoryPolicy
)
CacheBuilder.newBuilder()
.also {
if (memoryPolicy.hasAccessPolicy()) {
it.expireAfterAccess(memoryPolicy.expireAfterAccess, memoryPolicy.expireAfterTimeUnit)
}
if (memoryPolicy.hasWritePolicy()) {
it.expireAfterWrite(memoryPolicy.expireAfterWrite, memoryPolicy.expireAfterTimeUnit)
}
if (memoryPolicy.hasMaxSize()) {
it.maximumSize(memoryPolicy.maxSize)
}
}.build<Key, Output>()
}
/**
* Fetcher controller maintains 1 and only 1 `Multiplexer` for a given key to ensure network
* requests are shared.

View file

@ -1,11 +1,11 @@
package com.nytimes.android.external.store3
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import com.nytimes.android.external.store3.util.KeyParser
import com.nytimes.android.external.store4.Fetcher
import com.nytimes.android.external.store4.Store
import com.nytimes.android.external.store4.FlowStoreBuilder
import com.nytimes.android.external.store4.MemoryPolicy
import com.nytimes.android.external.store4.Persister
import com.nytimes.android.external.store4.Store
import com.nytimes.android.external.store4.impl.SourceOfTruth
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.flow

View file

@ -1,5 +1,7 @@
package com.nytimes.android.external.store3.base.impl;
import com.nytimes.android.external.store4.MemoryPolicy;
import org.junit.Test;
import java.util.concurrent.TimeUnit;

View file

@ -1,2 +0,0 @@
re-implementation for:
https://github.com/nytimes/Store/compare/tech/removeCache

View file

@ -1,18 +0,0 @@
plugins {
id 'org.jetbrains.kotlin.jvm'
}
group = GROUP
version = VERSION_NAME
sourceCompatibility = "8"
targetCompatibility = "8"
dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
implementation project(path: ":cache")
implementation libraries.coroutinesCore
testImplementation libraries.junit
testImplementation libraries.coroutinesTest
testImplementation libraries.assertJ
}

View file

@ -1,58 +0,0 @@
package com.com.nytimes.suspendCache
import com.nytimes.android.external.cache3.CacheBuilder
import com.nytimes.android.external.cache3.CacheLoader
import com.nytimes.android.external.cache3.Ticker
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
internal class RealStoreCache<K, V, Request>(
private val loader: suspend (Request) -> V,
private val memoryPolicy: MemoryPolicy,
ticker: Ticker = Ticker.systemTicker()
) : StoreCache<K, V, Request> {
private val realCache = CacheBuilder.newBuilder()
.ticker(ticker)
.also {
if (memoryPolicy.hasAccessPolicy()) {
it.expireAfterAccess(memoryPolicy.expireAfterAccess, memoryPolicy.expireAfterTimeUnit)
}
if (memoryPolicy.hasWritePolicy()) {
it.expireAfterWrite(memoryPolicy.expireAfterWrite, memoryPolicy.expireAfterTimeUnit)
}
if (memoryPolicy.hasMaxSize()) {
it.maximumSize(memoryPolicy.maxSize)
}
}
.build(object : CacheLoader<K, StoreRecord<V, Request>>() {
override fun load(key: K): StoreRecord<V, Request>? {
return StoreRecord(
loader = loader)
}
})
override suspend fun fresh(key: K, request: Request): V {
return realCache.get(key)!!.freshValue(request)
}
override suspend fun get(key: K, request: Request): V {
return realCache.get(key)!!.value(request)
}
override suspend fun put(key: K, value: V) {
realCache.put(key, StoreRecord(
loader = loader,
precomputedValue = value))
}
override suspend fun invalidate(key: K) {
realCache.invalidate(key)
}
override suspend fun clearAll() {
realCache.cleanUp()
}
override suspend fun getIfPresent(key: K): V? {
return realCache.getIfPresent(key)?.cachedValue()
}
}

View file

@ -1,43 +0,0 @@
package com.com.nytimes.suspendCache
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
typealias Loader<K, V> = suspend (K) -> V
/**
* Cache definition used by Store internally.
*/
// TODO this API is sub-optimal because it supports both Store & Pipeline where requirements
// mismatch. We can simplify it after we get rid of one of them.
interface StoreCache<K, V, Request> {
suspend fun get(key: K, request : Request): V
suspend fun fresh(key: K, request : Request): V
suspend fun put(key: K, value: V)
suspend fun invalidate(key: K)
suspend fun clearAll()
suspend fun getIfPresent(key: K): V?
companion object {
fun <K, V> from(
loader: suspend (K) -> V,
memoryPolicy: MemoryPolicy
): StoreCache<K, V, K> {
return RealStoreCache(
loader = loader,
memoryPolicy = memoryPolicy
)
}
// TODO rename to from after cleanup. Has a different name to avoid conflict w/ the old
// `from`.
fun <K, V, Request> fromRequest(
loader: suspend (Request) -> V,
memoryPolicy: MemoryPolicy
): StoreCache<K, V, Request> {
return RealStoreCache(
loader = loader,
memoryPolicy = memoryPolicy
)
}
}
}

View file

@ -1,61 +0,0 @@
package com.com.nytimes.suspendCache
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
/**
* The value we keep in guava's cache and it handles custom logic for store
* * not caching failures
* * not having concurrent fetches for the same key
* * maybe fresh?
* * deduplication
*/
internal class StoreRecord<V, Request>(
precomputedValue : V? = null,
private val loader: Loader<Request, V>
) {
private var inFlight = Mutex(false)
@Volatile
private var _value: V? = precomputedValue
fun cachedValue(): V? = _value
suspend fun freshValue(request: Request): V {
// first try to lock inflight request so that we can avoid get() from making a call
// but if we failed to lock, just request w/o a lock.
// we want fresh to be really fresh and we don't want it to wait for another request
if (inFlight.tryLock()) {
try {
return internalDoLoadAndCache(request)
} finally {
inFlight.unlock()
}
} else {
return inFlight.withLock {
return internalDoLoadAndCache(request)
}
}
}
private inline suspend fun internalDoLoadAndCache(request: Request): V {
return runCatching {
loader(request)
}.also {
it.getOrNull()?.let {
_value = it
}
}.getOrThrow()
}
suspend fun value(request: Request): V {
val cached = _value
if (cached != null) {
return cached
}
return inFlight.withLock {
_value?.let {
return it
} ?: internalDoLoadAndCache(request)
}
}
}

View file

@ -1,169 +0,0 @@
package com.nytimes.suspendCache
import com.com.nytimes.suspendCache.RealStoreCache
import com.nytimes.android.external.cache3.Ticker
import com.nytimes.android.external.store3.base.impl.MemoryPolicy
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.test.TestCoroutineScope
import kotlinx.coroutines.test.runBlockingTest
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.fail
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import java.util.*
import java.util.concurrent.TimeUnit
@Suppress("UsePropertyAccessSyntax") // for isTrue() / isFalse()
@ExperimentalCoroutinesApi
@RunWith(JUnit4::class)
class RealStoreCacheTest {
private val testScope = TestCoroutineScope()
private val loader = TestLoader()
private val ticker = object : Ticker() {
override fun read(): Long {
return TimeUnit.MILLISECONDS.toNanos(testScope.currentTime)
}
}
@Test
fun sanity_notEnquedShouldThrow() = testScope.runBlockingTest {
val cache = createCache()
try {
cache.get("unused key")
fail("should've failed")
} catch (assertionError: AssertionError) {
assertThat(assertionError.localizedMessage).isEqualTo("nothing enqueued")
}
}
@Test
fun cache() = testScope.runBlockingTest {
val cache = createCache()
loader.enqueueResponse("foo", "bar")
assertThat(cache.get("foo")).isEqualTo("bar")
// get again, is cached
assertThat(cache.get("foo")).isEqualTo("bar")
}
@Test
fun cache_expired() = testScope.runBlockingTest {
val cache = createCache(
MemoryPolicy.builder()
.setExpireAfterAccess(10)
.setExpireAfterTimeUnit(TimeUnit.MILLISECONDS)
.build())
loader.enqueueResponse("foo", "bar")
loader.enqueueResponse("foo", "bar_updated")
assertThat(cache.get("foo")).isEqualTo("bar")
// get again, is cached
assertThat(cache.get("foo")).isEqualTo("bar")
testScope.advanceTimeBy(11)
assertThat(cache.get("foo")).isEqualTo("bar_updated")
}
@Test
fun getIfPresent() = testScope.runBlockingTest {
val cache = createCache()
assertThat(cache.getIfPresent("foo")).isNull()
loader.enqueueResponse("foo", "bar")
assertThat(cache.getIfPresent("foo")).isNull()
assertThat(cache.get("foo")).isEqualTo("bar")
assertThat(cache.getIfPresent("foo")).isEqualTo("bar")
}
@Test
fun getIfPresent_pendingFetch() = testScope.runBlockingTest {
val cache = createCache()
val deferredResult = CompletableDeferred<String>()
loader.enqueueResponse("foo", deferredResult)
val asyncGet = async {
cache.get("foo")
}
testScope.advanceUntilIdle()
assertThat(asyncGet.isCompleted).isFalse()
assertThat(cache.getIfPresent("foo")).isNull()
deferredResult.complete("bar")
testScope.advanceUntilIdle()
assertThat(asyncGet.isCompleted).isTrue()
assertThat(cache.getIfPresent("foo")).isEqualTo("bar")
}
@Test
fun invalidate() = testScope.runBlockingTest {
val cache = createCache()
loader.enqueueResponse("foo", "bar")
loader.enqueueResponse("foo", "bar_updated")
assertThat(cache.get("foo")).isEqualTo("bar")
cache.invalidate("foo")
assertThat(cache.get("foo")).isEqualTo("bar_updated")
}
@Test
fun clearAll() = testScope.runBlockingTest {
val cache = createCache()
loader.enqueueResponse("foo", "bar")
loader.enqueueResponse("foo", "bar_updated")
loader.enqueueResponse("baz", "bat")
loader.enqueueResponse("baz", "bat_updated")
assertThat(cache.get("foo")).isEqualTo("bar")
assertThat(cache.get("baz")).isEqualTo("bat")
cache.clearAll()
assertThat(cache.get("foo")).isEqualTo("bar_updated")
assertThat(cache.get("baz")).isEqualTo("bat_updated")
}
@Test
fun put() = testScope.runBlockingTest {
val cache = createCache()
cache.put("foo", "bar")
assertThat(cache.get("foo")).isEqualTo("bar")
cache.put("foo", "bar_updated")
assertThat(cache.get("foo")).isEqualTo("bar_updated")
}
@Test
fun fresh() = testScope.runBlockingTest {
val cache = createCache()
cache.put("foo", "bar")
loader.enqueueResponse("foo", "bar_updated")
assertThat(cache.fresh("foo")).isEqualTo("bar_updated")
assertThat(cache.get("foo")).isEqualTo("bar_updated")
}
private fun createCache(
memoryPolicy: MemoryPolicy = MemoryPolicy.builder().build()
): RealStoreCache<String, String, String> {
return RealStoreCache(
loader = loader::invoke,
ticker = ticker,
memoryPolicy = memoryPolicy
)
}
private suspend fun <K, V> RealStoreCache<K, V, K>.get(key : K) = get(key, key)
private suspend fun <K, V> RealStoreCache<K, V, K>.fresh(key : K) = fresh(key, key)
}
private class TestLoader {
private val enqueued = mutableMapOf<String, LinkedList<Deferred<String>>>()
fun enqueueResponse(key: String, value: String) {
enqueueResponse(key, CompletableDeferred(value))
}
fun enqueueResponse(key: String, deferred: Deferred<String>) {
enqueued.getOrPut(key) {
LinkedList()
}.add(deferred)
}
suspend fun invoke(key: String): String {
val response = enqueued[key]?.pop() ?: throw AssertionError("nothing enqueued")
return response.await()
}
}

View file

@ -1,187 +0,0 @@
package com.nytimes.suspendCache
import com.com.nytimes.suspendCache.StoreRecord
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.test.TestCoroutineScope
import kotlinx.coroutines.test.runBlockingTest
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
@Suppress("UsePropertyAccessSyntax") // for isTrue()/isFalse()
@ExperimentalCoroutinesApi
@RunWith(JUnit4::class)
class StoreRecordTest {
private val testScope = TestCoroutineScope()
@Test
fun precomputed() = testScope.runBlockingTest {
val record = StoreRecord(
loader = { _: String -> TODO() },
precomputedValue = "bar")
assertThat(record.cachedValue()).isEqualTo("bar")
assertThat(record.value("foo")).isEqualTo("bar")
}
@Test
fun fetched() = testScope.runBlockingTest {
val record = StoreRecord { request: String ->
assertThat(request).isEqualTo("foo")
"bar"
}
assertThat(record.value("foo")).isEqualTo("bar")
}
@Test
fun fetched_multipleValueGet() = testScope.runBlockingTest {
var runCount = 0
val record = StoreRecord { _: String ->
runCount++
"bar"
}
assertThat(record.value("foo")).isEqualTo("bar")
assertThat(record.value("foo")).isEqualTo("bar")
assertThat(runCount).isEqualTo(1)
}
@Test
fun fetched_multipleValueGet_firstError() = testScope.runBlockingTest {
var runCount = 0
val errorMsg = "i'd like to fail"
val record = StoreRecord { _: String ->
runCount++
if (runCount == 1) {
throw RuntimeException(errorMsg)
} else {
"bar"
}
}
val first = runCatching {
record.value("foo")
}
assertThat(first.isFailure).isTrue()
assertThat(first.exceptionOrNull()?.localizedMessage).isEqualTo(errorMsg)
assertThat(record.value("foo")).isEqualTo("bar")
assertThat(runCount).isEqualTo(2)
}
@Test
fun fetched_multipleValueGet_firstOneFails_delayed() = testScope.runBlockingTest {
var runCount = 0
val firstResponse = CompletableDeferred<String>()
val secondResponse = CompletableDeferred<String>()
val errorMsg = "i'd like to fail"
val record = StoreRecord { _: String ->
runCount++
if (runCount == 1) {
return@StoreRecord firstResponse.await()
} else {
return@StoreRecord secondResponse.await()
}
}
val first = async {
record.value("foo")
}
val second = async {
record.value("foo")
}
testScope.advanceUntilIdle()
assertThat(first.isCompleted).isFalse()
assertThat(second.isCompleted).isFalse()
firstResponse.completeExceptionally(RuntimeException(errorMsg))
assertThat(first.isCompleted).isTrue()
assertThat(second.isCompleted).isFalse()
assertThat(first.getCompletionExceptionOrNull()?.localizedMessage).isEqualTo(errorMsg)
secondResponse.complete("bar")
assertThat(second.await()).isEqualTo("bar")
assertThat(runCount).isEqualTo(2)
}
@Test
fun freshSimple_alreadyCached() = testScope.runBlockingTest {
var runCount = 0
val responses = listOf(
"bar",
"bar2"
)
val record = StoreRecord { _: String ->
val index = runCount
runCount++
return@StoreRecord responses[index]
}
assertThat(record.value("foo")).isEqualTo("bar")
assertThat(record.freshValue("foo")).isEqualTo("bar2")
assertThat(record.value("foo")).isEqualTo("bar2")
}
@Test
fun freshSimple_notCached() = testScope.runBlockingTest {
val record = StoreRecord { _: String ->
"bar"
}
assertThat(record.freshValue("foo")).isEqualTo("bar")
}
@Test
fun fresh_multipleParallel() = testScope.runBlockingTest {
val responses = listOf<CompletableDeferred<String>>(
CompletableDeferred(),
CompletableDeferred()
)
var runCount = 0
val record = StoreRecord { _: String ->
val index = runCount
runCount++
responses[index].await()
}
val first = async {
record.freshValue("foo")
}
val second = async {
record.freshValue("foo")
}
assertThat(first.isActive).isTrue()
assertThat(second.isActive).isTrue()
responses[0].complete("bar")
assertThat(first.await()).isEqualTo("bar")
assertThat(second.isActive).isTrue()
responses[1].complete("bar2")
assertThat(second.await()).isEqualTo("bar2")
assertThat(runCount).isEqualTo(2)
}
@Test
fun fresh_multipleParallel_firstOneFails() = testScope.runBlockingTest {
val responses = listOf(
CompletableDeferred<String>(),
CompletableDeferred()
)
var runCount = 0
val record = StoreRecord { _: String ->
val index = runCount
runCount++
responses[index].await()
}
val first = async {
record.freshValue("foo")
}
val second = async {
record.freshValue("foo")
}
val errorMsg = "i'd like to fail"
assertThat(first.isActive).isTrue()
assertThat(second.isActive).isTrue()
responses[0].completeExceptionally(RuntimeException(errorMsg))
assertThat(first.getCompletionExceptionOrNull()?.localizedMessage).isEqualTo(errorMsg)
assertThat(second.isActive).isTrue()
responses[1].complete("bar")
assertThat(second.await()).isEqualTo("bar")
assertThat(runCount).isEqualTo(2)
}
}