Skip to content

Conversation

@sksamuel
Copy link
Owner

Closes #38

@sksamuel sksamuel requested a review from Copilot April 18, 2025 03:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for the "refresh" and "refresh all" functionality in the LoadingCache and its tests. Key changes include:

  • Adding two new methods (refresh and refreshAll) in LoadingCache.kt that delegate to the underlying Caffeine cache.
  • Creating corresponding tests in AsLoadingCacheTest.kt to verify refresh behavior for single and multiple keys.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
aedile-core/src/test/kotlin/com/sksamuel/aedile/core/AsLoadingCacheTest.kt Added tests to verify refresh and refreshAll functionality.
aedile-core/src/main/kotlin/com/sksamuel/aedile/core/LoadingCache.kt Introduced refresh and refreshAll methods for cache refresh operations.

@sksamuel sksamuel merged commit 0b049c0 into main Apr 18, 2025
1 check passed
@sksamuel sksamuel deleted the sks/refresh branch April 18, 2025 03:23
Comment on lines +184 to +194
fun refresh(key: K) {
cache.synchronous().refresh(key)
}

/**
* Loads a new value for each key, asynchronously. While the new value is loading the
* previous value (if any) will continue to be returned by get(key) unless it is evicted.
*
* See full docs at [com.github.benmanes.caffeine.cache.LoadingCache.refreshAll].
*/
fun refreshAll(keys: Collection<K>) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the new methods be suspending, calling await and returning the value?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't know what I was thinking?!
2.1.1 coming out

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... sorry to bug again, but what about the return value? :-)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what happens when I do things after drinking :)
Try 2.1.2

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refresh and refreshAll for LoadingCache

3 participants