https://kotlinlang.org logo
#codereview
Title
# codereview
d

dave08

02/21/2024, 11:52 AM
Specific question: what would be a better way to reference a cache in my https://github.com/dave08/kacheable library? For now, I'm using a String key, but maybe I should limit it to using a specified Enum, or the cache configuration itself, or maybe just leave it generic for the user to choose, or is that all just overkill and just leave the user to define a const for themselves? If the user were only using one particular cache in one place, it wouldn't matter, but there's an option to invalidate specific keys in a particular cache... also, I'd appreciate any comments on the library, it's my first one, so I might not have taken all the regular things into consideration for publishing a library (but the code itself is tested and stable, and being used in production internally)
c

CLOVIS

02/22/2024, 9:02 AM
Hey! You may want to give more context and code examples of what you mean in the question itself; most people here haven't read the entire library nor have the time for it. This would increase the likeliness of getting useful answers by a lot.
👍🏼 1
d

dave08

02/22/2024, 10:36 AM
For now, I have a string key (here "user-cache"):
Copy code
suspend fun getUser(userId: Int) = cache("user-cache", userId) {
        // some db query to retrieve user... this will be cached the first
        // time for any unique userId and will be retrieved from the cache
        // if without re-running the query if it's still there.
    }
but when invalidating you need to re-use it:
Copy code
suspend fun someOperationThatNeedsToRefreshTheUserInTheCacheFirst(userId: Int) = cache.invalidate("user-cache", userId) {
        ...
    }
I could rely on the user to define a constant for
user-cache
, or force them to use an Enum instead, or maybe instead of initializing the caches when constructing the Kacheable instance:
Copy code
val cache = Kacheable(RedisKacheableStore(conn),mapOf("user-cache", CacheConfig(...)))
maybe use the
CacheConfig
instance as the key itself... or maybe just make it generic, and let the user define (while I call toString() for the name...). There's pros and cons for each one of these possibilities...
Also, it bothers me a bit that I don't really limit a particular cache to be called with any parameters a user may want... it would have been nice to have a way that each cache should have a required set of parameters with their types... but I'm not sure how I'd do that... in the previous example, the best thing would have been that cache(userCache: ???, param1: Int) would be the way to call it... not cache(userCache: ???, vararg params: Any)...
If the CacheConfig (which could have a few impls with varying number of type params) would somehow contain the param info and be passed to a function that corresponds to the number of generic params... that could be done.. but maybe that's overkill... and would force maintaining something like Arrow's TupleX...
c

CLOVIS

02/22/2024, 10:50 AM
If I understand correctly, the string key here is used to distinguish between multiple caches, then the second argument is used as the identifier for that particular value?
d

dave08

02/22/2024, 10:51 AM
yes, but there can be multiple arguments after the first:
Copy code
interface Kacheable {
    suspend fun <R> invalidate(vararg keys: Pair<String, List<Any>>, block: suspend () -> R): R

    suspend fun <R> invoke(
        name: String,
        type: KSerializer<R>,
        vararg params: Any,
        saveResultIf: (R) -> Boolean = { true },
        block: suspend () -> R
    ): R
}

suspend inline operator fun <reified R> Kacheable.invoke(
    name: String,
    vararg params: Any,
    noinline saveResultIf: (R) -> Boolean = { true },
    noinline block: suspend () -> R
): R =
    invoke(name, serializer<R>(), *params, saveResultIf = saveResultIf, block = block)

suspend inline fun <reified R> Kacheable.cache(
    name: String,
    vararg params: Any,
    noinline shouldSaveResult: (R) -> Boolean = { true },
    noinline block: suspend () -> R
): R =
    invoke(name, serializer(), *params, saveResultIf = shouldSaveResult, block = block)
c

CLOVIS

02/22/2024, 10:51 AM
Ah, I see, so there is one massive cache, and each user is responsible for declaring which subset of the operations that are interested in, as part of the key?
d

dave08

02/22/2024, 10:52 AM
There's one object being created to manage many caches...
The key is composed of the "cache"'s name and it's parameters... the point is to cache the result of a computation, keeping the cache name distinct from the parameters being cached which are entries in that cache, this would all be managed in one Redis (or any store) instance, in Redis what distinguishes the "caches" is the name and the fixed param types that you'll always use for that computation. But depending on the computation's input params, there'll be distinct entries in that "cache"
👍 1
c

CLOVIS

02/22/2024, 10:56 AM
Does the user ever use the specified string other than during that call?
d

dave08

02/22/2024, 10:56 AM
Then invalidating would either be passing the same cache name, with the same inputs to invalidate that entry, or use * for one of those params to invalidate multiple entries (with any value for the parameter that * was passed in) .
The config class tells the cache about ttls and what to do with null results, etc..
Copy code
data class CacheConfig(
    val name: String,
    val expiryType: ExpiryType = ExpiryType.none,
    val expiry: Duration = Duration.INFINITE,
    /**
    If this is a real null, the cache entry will not be saved at all.
    This should ONLY be set if the function's return type is nullable!
     */
    val nullPlaceholder: String? = null,
)
c

CLOVIS

02/22/2024, 10:59 AM
You could have a
Copy code
@JvmInline value class CacheKey(val name: String)
this way users would write
Copy code
val userCache = CacheKey("user-cache")

suspend fun getUser(id: String) = cache(userCache, id) { … }

cache.invalidate(userCache, "123")
d

dave08

02/22/2024, 11:01 AM
That's actually a great idea! It would force users to create that instance... and then encourage them to save it if it needs to be re-used... Any ideas for the type safe params?
c

CLOVIS

02/22/2024, 11:01 AM
If you go that way, I would probably split it into:
Copy code
val mainCache = // your existing cache

val userCache = mainCache.withKey("user-cache")

suspend fun getUser(id: String) = userCache(id) { … }

userCache.invalide("123")
I think it's easier to understand, and it looks as if the cache instances are nicely segregated by operation (even if they're all stored together for real).
2
d

dave08

02/22/2024, 11:24 AM
That looks very nice! But I'd need to manage another interface without the name parameter? Or maybe you're thinking of something else? Maybe this could have helped me with typing the parameters... if it was only creating lambdas, but managing an X amount of interfaces would maybe be too much?
Copy code
// I mean (but then I couldn't use that for invalidate...):
fun <P1 : Any, R> Kacheable.withKeyAndP1(name: String): (P1) -> R ...
I could always do this:
Copy code
fun <P1 : Any, R> Kacheable.withCacheAndInvalidate1(name: String): Pair<(P1) -> R, (P1) -> R) ...

val (userCache, userCacheInvalidate) = mainCache.withCacheAndInvalidate1<Int>("user-cache")
but would that be something that a user could understand on the first look? Is that a good practice (there is such a thing in React, and some Android Compose libraries)...
c

CLOVIS

02/22/2024, 11:41 AM
What type parameters do you have that cause issues?
d

dave08

02/22/2024, 11:42 AM
Copy code
suspend inline operator fun <reified R> Kacheable.invoke(
    name: String,
    vararg params: Any, // <--- I'd be nice if this could be type safe per cache name...
    noinline saveResultIf: (R) -> Boolean = { true },
    noinline block: suspend () -> R
): R
Also in
suspend fun <R> invalidate(vararg keys: Pair<String, List<Any>>, block: suspend () -> R): R
there's those keys (in this implementation a user could invalidate entries in multiple caches -- Pair<CacheName>, List<Any>> where List<Any> are the params
Any given cache has a certain fixed amount of parameters with fixed types.
c

CLOVIS

02/22/2024, 11:45 AM
With the solution I was alluding to:
Copy code
class KeyedKachable<R>(
    val cache: Kacheable,
    val key: String,
) {
    operator fun invoke(…) = cache(key, …)
    fun invalidate(…) = cache.invalidate(key, …)
}

fun <R> Kacheable.withKey(key: String) = KeyedKacheable<R>(ths, key)
so the result
R
type parameter is ok. Indeed, if you want to have typesafe varargs, you will need to copy-paste
KeyedKacheable
for each number of argument you accept, but the result will be typesafe. At the very least,
KeyedCacheable
is very small, since all it does is store the key and delegate to
Kacheable
, so it's not a big problem if it's duplicated.
1
Also in general, always have an overload that accept a
List
or
Collection
whenever you have a
vararg
: https://kotlinlang.org/docs/jvm-api-guidelines-predictability.html#avoid-varargs
1
d

dave08

02/22/2024, 11:53 AM
Wow! That's a very nice solution... I wonder if
withKey
would still be a good name here then? I'd have to find a way to name these things clearly enough for the type params too.
The only little other issue is this:
Copy code
suspend inline operator fun <reified R> Kacheable.invoke(
    name: String,
    vararg params: Any,
    noinline saveResultIf: (R) -> Boolean = { true },
    noinline block: suspend () -> R
): R =
    invoke(name, serializer<R>(), *params, saveResultIf = saveResultIf, block = block)

suspend inline fun <reified R> Kacheable.cache(
    name: String,
    vararg params: Any,
    noinline shouldSaveResult: (R) -> Boolean = { true },
    noinline block: suspend () -> R
): R =
    invoke(name, serializer(), *params, saveResultIf = shouldSaveResult, block = block)
the receiver is on the original Kacheable interface... so I guess I'd have to add a KSerializer parameter to withKey... or maybe a type parameter for it. But all in all this seems to solve the problem very nicely, thanks a lot!
3 Views