feat(redis): add initial Redis support (beta) #6

nelihdev · 2025-11-14T00:06:39Z

This pull request introduces Redis integration into the core API, providing a shared Redis connection pool and a new RedisDataService for caching, messaging, and other Redis operations. The changes include configuration support, connection management, health checks, and public API exposure for plugins.

Redis Integration

Added Jedis (Redis client) as a dependency and configuration options for Redis connection pooling in both pom.xml and the default configuration YAML. [1] [2] [3] [4]
Implemented the new RedisManager class to handle Redis pool lifecycle, connection, health checks, and shutdown.
Integrated Redis initialization, health checks, and shutdown logic into CoreManager, along with new getter methods for Redis-related services. [1] [2] [3] [4] [5]

API Exposure

Extended the public API interfaces (NetworkDataAPIProvider and APIRegistry) to expose RedisDataService, direct Jedis pool access, and Redis health/status checks, with detailed Javadoc and usage examples for plugin developers. [1] [2] [3] [4]

General Improvements

Refactored DatabaseManager to provide explicit getter methods for connection status and the database instance, improving clarity and consistency with the new Redis manager. [1] [2]

Copilot

Pull Request Overview

This PR introduces comprehensive Redis support to the NetworkDataAPI, providing a shared connection pool and high-level data service for caching, pub/sub messaging, and various Redis operations. The integration is feature-complete with proper configuration, health checks, and API exposure for plugin developers.

Key Changes:

Added Jedis 5.1.0 as a Redis client dependency with configurable connection pooling
Implemented RedisManager for connection lifecycle management and RedisDataService for high-level Redis operations
Extended public API to expose Redis services with detailed documentation and usage examples

Reviewed Changes

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

Show a summary per file

File	Description
pom.xml	Added Jedis 5.1.0 dependency version property
networkdataapi-core/pom.xml	Added Jedis dependency to core module
ConfigurationManager.java	Added Redis connection and pool configuration with sensible defaults
RedisManager.java	Implements Redis connection pool management with health checks and graceful shutdown
RedisDataService.java	Provides high-level API for Redis operations (strings, hashes, sets, lists, pub/sub, counters)
CoreManager.java	Integrates Redis initialization, health checks, and shutdown; adds getter methods for all managers
DatabaseManager.java	Refactored to use explicit getter methods instead of Lombok annotations for consistency
NetworkDataAPIProvider.java	Extended API interface with Redis service access methods and comprehensive documentation
APIRegistry.java	Implemented new API methods to expose Redis services to plugins

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

Copilot · 2025-11-14T00:11:05Z

networkdataapi-core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/CoreManager.java

                databaseManager.shutdown();
            } catch (Exception e) {
-                logger.error("Error closing database connection", e);
+                logger.error("Error shutting down database", e);


[nitpick] The error message was changed from "Error closing database connection" to "Error shutting down database". While this is more consistent with the new Redis shutdown message, this is a pre-existing line that was modified. Consider whether this change should be in a separate commit focused on consistency improvements, as it's not directly related to Redis integration.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-11-14T00:11:05Z

...core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/service/RedisDataService.java

+     */
+    public void hmset(String key, Map<String, String> hash) {
+        try (Jedis jedis = redisManager.getResource()) {
+            jedis.hmset(key, hash);


The hmset method is deprecated in Jedis 4.0+ and removed in some newer versions. Consider using hset(String key, Map<String, String> hash) instead, which is the recommended approach in modern Jedis versions and provides the same functionality.

Suggested change

jedis.hmset(key, hash);

jedis.hset(key, hash);

Copilot · 2025-11-14T00:11:05Z

...core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/service/RedisDataService.java

+    public long hincrBy(String key, String field, long increment) {
+        try (Jedis jedis = redisManager.getResource()) {
+            return jedis.hincrBy(key, field, increment);
+        } catch (Exception e) {
+            logger.error("Failed to increment hash field: {} -> {}", key, field, e);
+            return 0;
+        }


[nitpick] When incrementing a hash field fails (lines 286-291), the method returns 0, which is indistinguishable from a successful increment that happens to result in 0. This makes it impossible for callers to detect errors. Consider throwing a RuntimeException, using Optional, or documenting this behavior clearly so callers know that 0 might indicate an error.

Copilot · 2025-11-14T00:11:06Z

pom.xml


        <!-- Dependency versions -->
        <mongodb.version>4.11.1</mongodb.version>
+        <redis.version>5.1.0</redis.version>


Jedis 5.1.0 requires Java 11 or higher. Ensure the project's Java version is compatible with this dependency. If the project targets Java 8, this will cause compatibility issues.

Copilot · 2025-11-14T00:11:06Z

...core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/service/RedisDataService.java

+     */
+    public boolean expire(String key, long seconds) {
+        try (Jedis jedis = redisManager.getResource()) {
+            return jedis.expire(key, seconds) == 1;


The comparison jedis.expire(key, seconds) == 1 assumes expire returns an int or long. In Jedis 5.x, expire returns a Long object. While autoboxing handles this, it would be safer and clearer to use .equals(1L) or check for null first to avoid potential NullPointerException if the operation returns null.

Suggested change

return jedis.expire(key, seconds) == 1;

Long result = jedis.expire(key, seconds);

return result != null && result.equals(1L);

Copilot · 2025-11-14T00:11:06Z

...core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/service/RedisDataService.java

+
+import com.astroid.stijnjakobs.networkdataapi.core.async.AsyncExecutor;
+import com.astroid.stijnjakobs.networkdataapi.core.redis.RedisManager;
+import lombok.RequiredArgsConstructor;


The @RequiredArgsConstructor annotation is present but an explicit constructor is also defined. This is redundant - either remove the Lombok annotation or remove the explicit constructor. Since the constructor includes Javadoc, it's recommended to remove the @RequiredArgsConstructor annotation.

Suggested change

import lombok.RequiredArgsConstructor;

Copilot · 2025-11-14T00:11:06Z

networkdataapi-core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/CoreManager.java

+        if (redisManager != null) {
+            asyncExecutor.schedule(() -> {
+                if (!redisManager.isAlive()) {
+                    logger.warn("Redis health check failed");
+                }
+            }, 1, TimeUnit.MINUTES);


The Redis health check is scheduled but won't repeat automatically. The schedule method schedules a one-time execution after 1 minute. To make this a recurring health check like the database health check, use scheduleAtFixedRate or scheduleWithFixedDelay instead. For example: asyncExecutor.scheduleAtFixedRate(() -> { ... }, 1, 1, TimeUnit.MINUTES);

Copilot · 2025-11-14T00:11:07Z

...e/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/config/ConfigurationManager.java

+                redis:
+                  enabled: false
+                  host: "localhost"
+                  port: 6379


[nitpick] The password configuration uses an empty string "" as the default value. This might be interpreted differently than null. In RedisManager line 108, there's a check for password != null && !password.isEmpty(), which correctly handles this case. However, for clarity, consider using null or adding a comment explaining that an empty string means "no password".

Suggested change

port: 6379

port: 6379

# Leave empty ("") for no password

Copilot · 2025-11-14T00:11:07Z

...taapi-core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/redis/RedisManager.java

+            try (Jedis jedis = jedisPool.getResource()) {
+                String pong = jedis.ping();
+                if (!"PONG".equals(pong)) {
+                    throw new Exception("Failed to ping Redis server");
+                }


If the ping check fails during connection initialization (lines 116-119), an exception is thrown and caught, but the jedisPool might already be created. Consider explicitly closing the pool before throwing the exception to avoid resource leaks: if (jedisPool != null) { jedisPool.close(); }

Copilot · 2025-11-14T00:11:07Z

...taapi-core/src/main/java/com/astroid/stijnjakobs/networkdataapi/core/redis/RedisManager.java

+package com.astroid.stijnjakobs.networkdataapi.core.redis;
+
+import com.astroid.stijnjakobs.networkdataapi.core.config.ConfigurationManager;
+import lombok.Getter;


The class-level @Getter annotation generates getters for all fields, but explicit getter methods are already defined for jedisPool (lines 59-61) and connected (lines 68-70). This will cause compilation errors due to duplicate method definitions. Either remove the @Getter annotation or remove the explicit getter methods. Since the explicit getters include Javadoc, it's recommended to remove the @Getter annotation.

Suggested change

import lombok.Getter;

Copilot · 2025-11-14T00:12:31Z

@Cynive I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you.

feat(redis): add initial Redis support (beta)

832b266

nelihdev requested a review from Copilot November 14, 2025 00:07

Copilot started reviewing on behalf of nelihdev November 14, 2025 00:07 View session

Copilot finished reviewing on behalf of nelihdev November 14, 2025 00:09

Copilot AI reviewed Nov 14, 2025

View reviewed changes

Copilot AI mentioned this pull request Nov 14, 2025

[WIP] Update Redis support implementation based on feedback from PR #6 #7

Closed

nelihdev closed this Nov 14, 2025

nelihdev deleted the feature/redis-support branch November 14, 2025 00:14

	return jedis.expire(key, seconds) == 1;
	Long result = jedis.expire(key, seconds);
	return result != null && result.equals(1L);

feat(redis): add initial Redis support (beta) #6

feat(redis): add initial Redis support (beta) #6

Uh oh!

Conversation

nelihdev commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

nelihdev Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants