Feature/add rest redis cache#88
Merged
Merged
Conversation
- add new redis cache replacement strategy
…pdate CacheManager to support initialization from ModuleConfiguration
…tegy to be more generic
…RedisCache to simplify as a regular cache
…e/add-cache-configurations # Conflicts: # src/main/java/edu/kit/kastel/sdq/lissa/ratlr/configuration/EvaluationConfiguration.java
…ature/add-cache-configurations # Conflicts: # src/test/java/edu/kit/kastel/sdq/lissa/ratlr/ArchitectureTest.java
…ions - aside from the local cache dir for replication - from individual environment variables.
…ion, and backward compatibility checks
…rwriting restrictions
…ve obsolete cache type description
…propriate methods to avoid serialization conflicts
…e non-empty list. Also fail fast if configured caches are unavailable
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a REST-Redis-based caching option to LiSSA by introducing a unified Redis client abstraction and wiring a new REST_REDIS cache type into the existing cache factory/hierarchy.
Changes:
- Introduce
UnifiedRedisClientplus adapters for Jedis (RedisAdapter) and REST-Redis (RestRedisAdapter). - Add
RestRedisCacheandCacheType.REST_REDIS, update cache factory/docs/templates accordingly. - Add a Testcontainers-backed integration test for REST-Redis and adjust
.envoverwrite loading behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RestRedisTest.java | New integration test spinning up Redis + REST-Redis server and exercising cache + hierarchy behavior. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/utils/Environment.java | Adjust .env override loading to set directory + filename explicitly. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/UnifiedRedisClient.java | New minimal interface to abstract Redis operations used by caches. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RestRedisCache.java | New cache implementation backed by REST-Redis client. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RestRedisAdapter.java | Adapter exposing REST-Redis client through UnifiedRedisClient. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RedisCache.java | Refactor to depend on UnifiedRedisClient and support injected client. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RedisAdapter.java | Adapter exposing Jedis client through UnifiedRedisClient. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheType.java | Add REST_REDIS cache type. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheManager.java | Minor cleanup in cache hierarchy building loop. |
| src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java | Extend factory to create RestRedisCache for REST_REDIS. |
| pom.xml | Add rest-redis and Testcontainers dependencies. |
| env-template | Document REST_REDIS as a valid cache hierarchy entry. |
| docs/caching.md | Document REST-Redis cache option + related env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
dfuchss
reviewed
Jun 24, 2026
dfuchss
approved these changes
Jun 24, 2026
dfuchss
left a comment
dfuchss
left a comment
Member
There was a problem hiding this comment.
Check the copilot comments once more :)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…d classes and update exception types in RedisCache
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
DanielDango
changed the base branch from
feature/add-cache-configurations
to
main
July 15, 2026 14:12
…tions' into feature/add-rest-redis-cache # Conflicts: # docs/caching.md
# Conflicts: # docs/caching.md # env-template # src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/Cache.java # src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/CacheType.java # src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RedisCache.java # src/main/java/edu/kit/kastel/sdq/lissa/ratlr/utils/Environment.java # src/test/java/edu/kit/kastel/sdq/lissa/ratlr/ArchitectureTest.java
|
dfuchss
approved these changes
Jul 15, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/main/java/edu/kit/kastel/sdq/lissa/ratlr/cache/RedisCache.java:36
- UnifiedRedisClient is closeable, but RedisCache never closes the underlying client, and CacheManager only calls flush() on reset/teardown. This can leak connections/sockets over a long-running process or in repeated tests. Consider adding a close lifecycle (e.g., make Cache extend AutoCloseable and have CacheManager close caches on reset/shutdown, and/or add a close() method to RedisCache/HierarchicalCache that delegates to underlying clients).
private final UnifiedRedisClient redis;
/**
* Creates a new Redis cache instance.
* This constructor will throw an exception if Redis is unavailable.
|
|
||
| @Container | ||
| private static final GenericContainer<?> REDIS = | ||
| new GenericContainer<>(DockerImageName.parse("redis:latest")).withExposedPorts(6379); |
Comment on lines
+228
to
+231
| assertTrue(Assertions.assertThrows( | ||
| IllegalStateException.class, () -> hierarchicalCacheError.get(testKey, String.class)) | ||
| .getMessage() | ||
| .contains("Cache inconsistency")); |
Comment on lines
+267
to
+270
| private static int findFreePort() throws IOException { | ||
| try (ServerSocket socket = new ServerSocket(0)) { | ||
| return socket.getLocalPort(); | ||
| } |
Comment on lines
+29
to
+32
| @Override | ||
| public boolean ping() { | ||
| return jedis.ping().equals("PONG"); | ||
| } |
| jedis.ping(); | ||
| if (!redis.ping()) { | ||
| redis.close(); | ||
| throw new IllegalStateException("Could not connect to Redis. Make sure the container is up and running."); |
Comment on lines
138
to
142
| static <K extends CacheKey> Cache<K> createByType( | ||
| CacheType type, CacheParameter<K> parameters, @Nullable String cacheDir, @Nullable ObjectMapper mapper) { | ||
| return switch (type) { | ||
| case LOCAL -> new LocalCache<>(cacheDir, parameters); | ||
| case REDIS -> new RedisCache<>(parameters, mapper); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #59
Adds a rest-redis-based cache to LiSSA.
This PR establishes a Unified Redis Client Interface to bundle access via the Java Redis Client "Jedis" (RedisCache) and the new REST Redis client.