mayakaapps / kache Goto Github PK

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): update dependency com.vanniktech.maven.publish to v0.29.0
• chore(deps): update dependency org.jetbrains.kotlin.multiplatform to v2

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

I just came across this library, and while it looks interesting, I'm hesitating to take it into use in ORT to replace DiskLruCache, mostly because of a complete lack of tests.

So, would it be possible to add tests to get some confidence into the implementation?

I am wondering if we would be able to receive a stable version of Kache. Right now, I believe the only available versions of Kache are beta versions. When do you think we would be able to see version 2.1.0?

The FileKache invoke() functions take the directory and maxSize in addition to Configuration as parameters. However, directory and maxSize can also be passed via Configuration itself. While this probably was done this way for user convenience (to configure most common parameters more easily), I believe this makes things more confusing, as the semantics are unclear if e.g. the directory is provided both as a direct argument and via Configuration (precedence is unclear then). To guard against unintended misuse, I'd recommend to drop the direct function parameters and rely solely on the Configuration.

Some cache entities should not be cached forever but expire after a certain amount of time, e.g. to force doing an expensive network request to fetch the original data to ensure it's still valid. For this, I'd propose to add e.g. a var maxAge: Long property to Configuration.

FYI, in our DiskLru wrapper we implemented this like so:

https://github.com/oss-review-toolkit/ort/blob/e484f98f6375d78ce6f7934edf9728863c36ed40/utils/common/src/main/kotlin/DiskCache.kt#L115-L123

Is your feature request related to a problem? Please describe.
The current API documentation may be outdated or missing important notes. Also, some important notes about the cache behavior need to be clarified.

Describe the solution you'd like
Current API documentation need to be updated and simple documentation needs to written using Mkdocs.

Additional context
This request depends on #55 and involves two important steps:

• #92
• #97

Describe the bug
The multiplatform version (v2.x) is using Kotlin's built-in LinkedHashMap which is missing the accessOrder property from the JVM version. When I was migrating the library to KMP, I thought that the Kotlin version already has this behavior (actually, I cannot remember what made me think that this is the expected behavior) which is not the case.

To Reproduce
Steps to reproduce the behavior:

1. Add two entries to LruCache
2. Access the first one
3. Trim to size 1

Expected behavior
The second entry should be evicted but the first one is evicted instead.

System (please complete the following information):
Any system

Additional context
Not applicable

Describe the bug
When getOrPut() is executed it locks the creation mutex till it returns, even if it is waiting for an on-going creation which may be expensive.

To Reproduce
Steps to reproduce the behavior:

1. Create a Kache and call putAsync("KEY1") { /* Expensive Task */ }
2. Launch a coroutine that is calling getOrPut("KEY1") { ... }
3. Call delay(1) for suspension point
4. Call putAsync, getOrPut, or put with lambda for KEY2

Expected behavior
The call for KEY2 shouldn't wait for the call for KEY1

System (please complete the following information):
Any system

Additional context
Not applicable

Is your feature request related to a problem? Please describe.
If getOrPut() is called with a lambda that is never returning null and V type isn't nullable, the nullability-check operator is redundant.

Describe the solution you'd like
Get rid of unnecessary null checks when using getOrPut with a lamda that is always non-nullable

Describe alternatives you've considered
Not applicable

Additional context
Not applicable

com.cybernetics.cyrclo:test: Could not find Kache:kache-common:unspecified.
Required by:
project : > com.mayakapps.kache:kache-jvm:2.0.0-SNAPSHOT:20230429.030344-34

Describe the bug
The expected behavior for the cache when writeFunction fails and the key has an existing value is to keep this value. The reason behind this behavior will be documented. But, the journal v1 represents this by a dirty operation followed by remove operation. This is the same behavior when deleting an existing key.
This results in missing entries when reopening the journal with possible orphan files.

To Reproduce
Steps to reproduce the behavior:

1. Put an entry into DiskLruCache
2. Re-put the same key asynchronously, throwing a cancellation exception for example
3. Close and re-open the cache

Expected behavior
The old value should be existing for this key.

System (please complete the following information):
Any system

Additional context
Not applicable

While migrating from DiskLru, it occurred to me that there's an API inconsistency in the directory parameters: The FileKache class / invoke function takes directoryPath as a String, whereas everywhere else it's directory as a Path. Is it worth to align that on Path for the 2.0.0 final release?

The community needs a good multiplatform LRU implementation. This library seems like it's got a good base for it. I actually noticed in the source that you were playing around with multiplatform. I thought I'd add this ticket to track the work.

This is a meta-issue for all changes that are expected to be included in the second version of the journal.

Issues to be included

• #44
• #45
• #46

Other changes

• Change the default key transformer to produce lowercase hex characters

Is your feature request related to a problem? Please describe.
When the application changes the way it is generating the cached values, or transforming the keys, it will be required to clear the cache and starts a new one.

Describe the solution you'd like
Adding a cache version parameter to the journaled cache and writing it to the journal header will help the library to track these changes and clear the cache directory if the version is changed than the passed one.

Describe alternatives you've considered
Leaving this to application developers to handle but it is easier to handle internally.

Additional context
This behavior is already seen in Jake Wharton's DiskLruCache for example.

Is your feature request related to a problem? Please describe.
I want to create a bitmap cache (ie val bitmapCache = InMemoryKache<Int, Bitmap>).
If I run bitmapCache.get(1) I want to get the data. If it doesn't exist, it should automatically run a suspending function first, then return the value.

getOrPut theoretically does this, but I would have to tediously copy over the creationFunction function to all places where I want to get values.

Describe the solution you'd like
The Configuration could accept a factory lambda.

Describe alternatives you've considered
Define a lambda like this once, and reference it in all getOrPut calls:

    val bitmapConstruction = { resId: Int ->
        bitmapHelper.getBitmap(resId, context)
    }
    bitmapCache.getOrPut(R.drawable.xyz, bitmapConstruction)

Additional context
LruCache has a way to override the create method to do just this. But it's written in Java and doesn't support coroutines etc.

Is your feature request related to a problem? Please describe.
For now, Okio is considered the standard KMP I/O library. This makes exposing Okio-based API favorable in multiplatform projects. Yet, in simple projects, or JVM/Android-only projects, it may be inconvenient to force application developers to use it.

Describe the solution you'd like
It seems to be better to expose Okio from the main implementation, letting developers to pass custom FileSystem or get Path, for example, in addition to adding a default artifact that wraps around the Okio-based one and exposes paths as strings in the common source set, File in JVM source set, and maybe NSURL in Apple OSes source set.

Describe alternatives you've considered
Not applicable

Additional context
This will allow to keep the JVM API as similar as possible to v1.

Describe the bug
This library defines several androidx classes that make the task :checkDebugDuplicateClasses fail with the following error:

Duplicate class androidx.annotation.IntRange found in modules annotation-jvm-1.7.0 (androidx.annotation:annotation-jvm:1.7.0) and kache-jvm (com.mayakapps.kache:kache-jvm:2.1.0-beta01)
Duplicate class androidx.collection.MapEntry found in modules collection-jvm-1.4.0-rc01 (androidx.collection:collection-jvm:1.4.0-rc01) and kache-jvm (com.mayakapps.kache:kache-jvm:2.1.0-beta01)
[...]

I see some minor edits have been applied to the original classes; I guess this shouldn't (and cannot) be resolved with Gradle.
Is it on purpose?

Describe the bug
Journal doesn't keep the access operations, so when closing and reopening the cache, it behaves as an FIFO cache for older entries.

To Reproduce
Steps to reproduce the behavior:

1. Create a DiskLruCache and add two entries to it.
2. Access the first one
3. Close and re-open it
4. Trim the cache size to 1

Expected behavior
The second entry should be removed but the first one is removed instead.

System (please complete the following information):
Any System

Additional context
Though this issue is related to #43, it is a completely different one but the other issue should be fixed first.

Describe the solution you'd like
It will be nice to add a caching strategy parameter when building new caches. Its initial possible values are LRU and FIFO with possible addition of more strategies in future versions.

Describe alternatives you've considered
Not applicable

Additional context
Not applicable

All the Kache-class constructors are private, to they can only be instantiated via the invoke() operators. These function, however, are suspend function. That makes migration from DiskLru not very straight-forward, as you cannot easily swap out a top-level class property which used to be a DiskLru to e.g. a FileKache.

What's the recommendation here? Also, I'm curious about the design choice to disallow creating a FileKache / OkioFileKache directory, which would have avoided this pitfall.

Describe the issue
Currently, test coverage is present for in-memory Kache and the journaling for FileKache, but not for the logic of FileKache itself. I currently don't have enough time to implement it myself, so I released preview version without them. Anyone who want to contribute is welcome.

Suggestions
You can check out the tests for Kache for a sample. Okio's FakeFileSystem is, of course, necessary for the required tests.

This is a continuation of #5

Describe the solution you'd like
Add Mkdocs and workflows for automatically deploying documentation

Describe alternatives you've considered
Continuing to use README.md and APIs only will make it harder to get started and learn about the behavior of the library.

Additional context
Not applicable.

Hi 😸
I noticed I was getting NullPointerExceptions when using SHA256KeyHasher.
I could narrow it down to InMemoryKache - the following test fails:

    @Test
    fun concurrency() = runTest {
        val cache = InMemoryKache<String, String>(maxSize = 1)

        (0..100000).map(Int::toString).forEach {
            launch { cache.getOrPut(it) { it }!! }
        }
    }

Stack trace:

java.lang.NullPointerException
	at com.mayakapps.kache.InMemoryKacheTest$concurrency$1$2$1.invokeSuspend(InMemoryKacheTest.kt:36)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
	at kotlinx.coroutines.test.TestDispatcher.processEvent$kotlinx_coroutines_test(TestDispatcher.kt:24)
	at kotlinx.coroutines.test.TestCoroutineScheduler.tryRunNextTaskUnless$kotlinx_coroutines_test(TestCoroutineScheduler.kt:99)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$2$1$workRunner$1.invokeSuspend(TestBuilders.kt:322)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:277)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:95)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:69)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:48)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersJvmKt.createTestResult(TestBuildersJvm.kt:10)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:310)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:168)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:160)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0$default(Unknown Source)
	at com.mayakapps.kache.InMemoryKacheTest.concurrency(InMemoryKacheTest.kt:32)

Describe the bug
If you use the Java 9+ modular system, in my case 17, then a packaging error occurs

To Reproduce
Steps to reproduce the behavior:

1. Add file-kache-jvm.jar and kache-jvm.jar in ur classpath
2. Run ur app
3. See the error

Expected behavior
Running app without errors

System (please complete the following information):

• Device: Desktop
• OS: Windows 11

Stacktrace

Exception in thread "main" java.lang.module.ResolutionException: Modules file.kache.jvm and kache.jvm export package com.mayakapps.kache to module org.jsoup
at java.base/java.lang.module.Resolver.resolveFail(Resolver.java:901)
at java.base/java.lang.module.Resolver.failTwoSuppliers(Resolver.java:815)
at java.base/java.lang.module.Resolver.checkExportSuppliers(Resolver.java:736)
at java.base/java.lang.module.Resolver.finish(Resolver.java:380)
at java.base/java.lang.module.Configuration.<init>(Configuration.java:140)
at java.base/java.lang.module.Configuration.resolveAndBind(Configuration.java:494)

Describe the solution you'd like
Add testing workflows to GitHub actions.

Describe alternatives you've considered
Manually running tests is inefficient.

Additional context
Not applicable

After loading the dependency cannot find Kache?

Is your feature request related to a problem? Please describe.
It is not convenient to use different style for creating different types of cache. Also, with increasing configuration variables, it may be better to use another style.

Describe the solution you'd like
Luckily, Kotlin allows using different styles for this. The one I'm preferring is inspired by kotlinx-serialization. It looks like this:

Kache {
    maxSize = 1_000_000
}

All configuration variables are expected to have default values unless necessary, so the application developer keeps his code as concise as possible.

Describe alternatives you've considered
Other options include using builder pattern with sequential configuration calls that return this.

Additional context
Not applicable

Is your feature request related to a problem? Please describe.
More functions are required for better performance. An example that is requiring this is DiskLruCache that is built around in-memory cache. While reading the journal, it replicates the journaled operations one by one which means it is executing fine-grained mutual exclusion affecting the performance.

Describe the solution you'd like
Adding more functions to in-memory cache will help in this case and other cases. These functions are:

• putAll
• evictAll

Describe alternatives you've considered
Not applicable

Additional context
Not applicable

Currently, the library saves journal entries using transformed/hashed keys. This makes it impossible to implement getKeys() or similar functions as the original key was lost.

Changing the journal shouldn't be breaking much as journal version is already written to the header and if it is incorrect, the cache will be invalidated.

Describe the bug
It looks like the cache is getting trimmed without a reason:

To Reproduce
Consider this test:

    @Test
    fun issue173() = runTest {
        val fileSystem = FakeFileSystem()

        val kache = testOkioFileKache(fileSystem)

        for (i in 0 until 16) {
            kache.put(i.toString()) { cachePath ->
                fileSystem.sink(cachePath).buffer().use { it.writeByte(0) }
                true
            }
        }

        assertEquals(16, kache.size)
        assertEquals(16, kache.getKeys().size)
    }

It writes 16 keys for a total of 16 bytes (the cache max size is 1024).

However, the last test fails: but there should be 16 keys!

Is your feature request related to a problem? Please describe.
I'm planning to support more cache strategies in addition to LRU. So, the name may become obsolete soon.

Describe the solution you'd like
The new suggested name is just Kache. There is already a library with the same name for Kotlin/JVM but seems to be abandoned as the last code commit is 4 years old. So, it may be OK to use it.

This requires the following changes:

• Renaming the Git repo. GitHub will allow this to happen easily as it redirects any request to the old name.
• Updating GitHub Actions to use the new repository name
• Rename the artifact group to com.mayakapps.kache and rename the artifacts to kache and file-kache-okio
• Update the package name to com.mayakapps.kache and rename LruCache to InMemoryKache and DiskLruCache to FileKache
• Update README.md to use the new library name

Describe alternatives you've considered
Keeping the name as is to help developers who are just looking for LRU cache. This can be addressed by mentioning LRU in the repository description.

Additional context
Not applicable

Describe the solution you'd like
This will make the code more testable and executable. It will also make implementing #52 easier. These abstracts are suggested to be interfaces named ObjectKache which is the parent of InMemoryKache and ContainerKache which is the parent of FileKache.

Describe alternatives you've considered
Not applicable

Additional context
Not applicable