Skip to content

WIP: Make Cache Configurable#75

Open
DanielDango wants to merge 25 commits into
ardoco:mainfrom
DanielDango:feature/add-cache-configurations
Open

WIP: Make Cache Configurable#75
DanielDango wants to merge 25 commits into
ardoco:mainfrom
DanielDango:feature/add-cache-configurations

Conversation

@DanielDango
Copy link
Copy Markdown
Collaborator

@DanielDango DanielDango commented Apr 23, 2026

This PR aims to make caches in LiSSA configurable.
Caching with REST-Redis as in #59 can be added afterwards.

The current Redis and Local Cache implementations are split into independent implementations of the Cache interface. Instead, a new Cache, the hierarchical Cache, is added to define how two caches can be coupled. Hierarchical caches can also be coupled with another hierarchical cache. If values are missing in any level of the cache hierarchy, they will be propagated.

The CacheReplacementStrategy Enum describes different conflict resolution strategies (None / Error / Overwrite).
The strategy and general cache hierarchy are defined in environment variables with the defaults of redis with local cache, and no consequences on cache conflicts.

The internal local cache implementation has not been altered to keep support for existing local cache files. Their location continues to be specified by the configuration file to enable effortless replication.

Copilot AI review requested due to automatic review settings April 23, 2026 14:52
@DanielDango DanielDango requested a review from dfuchss as a code owner April 23, 2026 14:52
Copy link
Copy Markdown
Contributor

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…e/add-cache-configurations

# Conflicts:
#	src/main/java/edu/kit/kastel/sdq/lissa/ratlr/configuration/EvaluationConfiguration.java
@dfuchss dfuchss marked this pull request as draft April 29, 2026 19:13
@DanielDango DanielDango marked this pull request as ready for review May 4, 2026 11:41
Copilot AI review requested due to automatic review settings May 4, 2026 11:41
Copy link
Copy Markdown
Contributor

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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/HierarchicalCache.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheManager.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/HierarchicalCache.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Copilot AI review requested due to automatic review settings May 5, 2026 12:43
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/test/java/edu/kit/kastel/sdq/lissa/ratlr/ArchitectureTest.java
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java Outdated
Copilot AI review requested due to automatic review settings May 5, 2026 16:30
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheReplacementStrategy.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java
Copy link
Copy Markdown
Member

@dfuchss dfuchss left a comment

Choose a reason for hiding this comment

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

Some initial comments

* @throws IllegalArgumentException If the type is not recognized or the cache cannot be created
*/
static <K extends CacheKey> Cache<K> createByType(
String type, CacheParameter<K> parameters, @Nullable String cacheDir, @Nullable ObjectMapper mapper) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do I need a string to create a cache. Either this should be an enum or not dynamic at all :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Using a string here instead of an enum is consistent with other component implementations that can be configured with Configuration entries

Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java Outdated
Comment thread src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java Outdated
}
yield new RedisCache<>(parameters, mapper);
}
default ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

with enum, no default will be needed

@sonarqubecloud
Copy link
Copy Markdown

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.

3 participants