Specific question: what would be a better way to r...

# codereview

[dave08]

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)

[CLOVIS]

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.

[dave08]

For now, I have a string key (here "user-cache"):

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:

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:

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...

[dave08]

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)...

[dave08]

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...

[CLOVIS]

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?

[dave08]

yes, but there can be multiple arguments after the first:

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)

[CLOVIS]

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?

[dave08]

There's one object being created to manage many caches...

[dave08]

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"

[CLOVIS]

Does the user ever use the specified string other than during that call?

[dave08]

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) .

[dave08]

The config class tells the cache about ttls and what to do with null results, etc..

[dave08]

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,
)

[CLOVIS]

You could have a

@JvmInline value class CacheKey(val name: String)

this way users would write

val userCache = CacheKey("user-cache")

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

cache.invalidate(userCache, "123")

[dave08]

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?

[CLOVIS]

If you go that way, I would probably split it into:

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).

[dave08]

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?

// I mean (but then I couldn't use that for invalidate...):
fun <P1 : Any, R> Kacheable.withKeyAndP1(name: String): (P1) -> R ...

[dave08]

I could always do this:

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)...

[CLOVIS]

What type parameters do you have that cause issues?

[dave08]

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

[dave08]

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

[dave08]

Any given cache has a certain fixed amount of parameters with fixed types.

[CLOVIS]

With the solution I was alluding to:

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.

[CLOVIS]

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

[dave08]

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.

[dave08]

The only little other issue is this:

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!