Skip to content

Conversation

uglide
Copy link
Contributor

@uglide uglide commented Jul 15, 2025

This PR introduces three new Redis client classes (RedisClient, RedisClusterClient, RedisSentinelClient) that provide a clean, developer-friendly API while addressing critical developer experience issues identified in the current Jedis architecture.

Problem Statement

The current Jedis library suffers from significant developer experience issues:

  • Confusing Entry Points: Multiple client classes (Jedis, JedisPooled, JedisCluster, UnifiedJedis) with unclear distinctions
  • Resource Management Complexity in Jedis class: Users end up using Jedis, where it's easy to create connection leaks with manual resource management
  • Constructor Explosion: Classes like JedisPool have 56+ constructors, JedisPooled has 65+ constructors, creating analysis paralysis
  • Poor maintainability of the code

Current Architecture Problems

jedis-architecture-simplified

The diagram above shows the current problematic architecture with:

  • Multiple confusing entry points (red diamond)
  • Constructor explosion across all client classes (red boxes showing constructor counts)
  • No clear guidance for developers on which client class to choose

Solution

This PR introduces a new client architecture that addresses these issues through:

  1. Clean API Design: Maximum 2-3 constructors per client class
  2. Builder Pattern: Fluent configuration API with sensible defaults
  3. Inheritance Architecture: Single implementation of all Redis commands in BaseRedisClient
  4. Clear Separation: Distinct clients for different deployment scenarios

New Architecture

jedis-architecture-with-new-clients

The new architecture (green boxes) provides:

  • Clear inheritance hierarchy with BaseRedisClient containing all command implementations
  • Builder pattern for all new clients (eliminating constructor explosion)
  • Focused client classes for specific use cases

BaseRedisClient (Abstract)

  • Implements ALL Redis command interfaces (~500+ methods)
  • Uses existing infrastructure classes
  • Eliminates code duplication across client implementations
  • Single point of implementation for all Redis commands

RedisClient (Standalone Redis)

  • Extends BaseRedisClient for command inheritance
  • Uses PooledConnectionProvider for automatic connection management
  • Builder pattern for configuration

RedisClusterClient (Redis Cluster)

  • Extends BaseRedisClient with cluster-specific functionality
  • Uses ClusterConnectionProvider for slot-based routing
  • Automatic cluster topology management
  • Cluster-aware pipeline support

RedisSentinelClient (Redis Sentinel)

  • Extends BaseRedisClient for Sentinel support
  • Uses SentineledConnectionProvider for automatic failover
  • Master discovery and monitoring
  • Sentinel-specific configuration options

Builder Pattern Implementation

All new clients use a consistent builder pattern with shared configuration through AbstractRedisClientBuilder

Future Work

  • Deprecation timeline for legacy client classes
  • Additional builder convenience methods based on user feedback

Copy link

github-actions bot commented Jul 15, 2025

Test Results

  253 files  + 4    253 suites  +4   10m 2s ⏱️ + 3m 3s
4 697 tests +77  4 608 ✅ +182  89 💤  - 105  0 ❌ ±0 
2 537 runs  +81  2 537 ✅ +129   0 💤  -  48  0 ❌ ±0 

Results for commit 92bd295. ± Comparison against base commit 8e60c1b.

This pull request removes 4 and adds 81 tests. Note that renamed tests count towards both.
redis.clients.jedis.commands.unified.cluster.ClusterBinaryValuesCommandsTest ‑ getSet
redis.clients.jedis.commands.unified.cluster.ClusterStringValuesCommandsTest ‑ getSet
redis.clients.jedis.commands.unified.pooled.PooledBinaryValuesCommandsTest ‑ getSet
redis.clients.jedis.commands.unified.pooled.PooledStringValuesCommandsTest ‑ getSet
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testAbstractBuilderGenerics
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testBuilderSpecificMethods
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testCommonMethodsAvailableOnBothBuilders
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testConnectionProviderOverride
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testRedisClientBuilderInheritance
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testRedisSentinelClientBuilderInheritance
redis.clients.jedis.AbstractRedisClientBuilderTest ‑ testSearchDialectValidation
redis.clients.jedis.RedisClientTest ‑ testAbstractMethodImplementations
redis.clients.jedis.RedisClientTest ‑ testAutoCloseable
redis.clients.jedis.RedisClientTest ‑ testBasicRedisOperations
…

♻️ This comment has been updated with latest results.

@uglide uglide marked this pull request as draft July 15, 2025 11:16
@uglide uglide requested a review from Copilot July 16, 2025 14:10
Copy link
Contributor

@Copilot 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 introduces three new Redis client classes with a unified builder pattern to simplify configuration and improve developer experience. Key changes include:

  • Addition of RedisClient, RedisClusterClient, and RedisSentinelClient with builder-based APIs
  • Implementation of a shared AbstractRedisClientBuilder and removal of constructor explosion
  • Removal of deprecated command methods and definition of a DEFAULT_DIALECT constant

Reviewed Changes

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

Show a summary per file
File Description
src/main/java/redis/clients/jedis/RedisClient.java New standalone Redis client using builder pattern
src/main/java/redis/clients/jedis/RedisClusterClient.java New Redis Cluster client using builder pattern
src/main/java/redis/clients/jedis/RedisSentinelClient.java New Redis Sentinel client using builder pattern
src/main/java/redis/clients/jedis/AbstractRedisClientBuilder.java Shared base class for client builders
src/main/java/redis/clients/jedis/CommandObjects.java Updated default search dialect initialization
src/main/java/redis/clients/jedis/search/SearchProtocol.java Added DEFAULT_DIALECT constant
src/main/java/redis/clients/jedis/search/RediSearchCommands.java Removed deprecated search command overloads
src/main/java/redis/clients/jedis/commands/StringCommands.java Removed deprecated getSet methods
src/main/java/redis/clients/jedis/commands/StringBinaryCommands.java Removed deprecated getSet binary overload
src/main/java/redis/clients/jedis/commands/StreamBinaryCommands.java Removed deprecated xread and xreadGroup methods
src/main/java/redis/clients/jedis/commands/SortedSetCommands.java Removed deprecated zdiffStore methods
src/main/java/redis/clients/jedis/commands/SortedSetBinaryCommands.java Removed deprecated binary zdiffStore methods
src/main/java/redis/clients/jedis/commands/ClusterCommands.java Removed deprecated cluster command methods
src/test/java/redis/clients/jedis/RedisClientTest.java Added comprehensive tests for RedisClient
src/test/java/redis/clients/jedis/RedisClusterClientTest.java Added unit tests for RedisClusterClient
src/test/java/redis/clients/jedis/RedisSentinelClientTest.java Added unit and integration tests for RedisSentinelClient
src/test/java/redis/clients/jedis/AbstractRedisClientBuilderTest.java Added tests for shared builder behavior
Comments suppressed due to low confidence (2)

src/main/java/redis/clients/jedis/search/SearchProtocol.java:9

  • [nitpick] Consider adding a JavaDoc comment for DEFAULT_DIALECT to explain its purpose and default value.
  public static final int DEFAULT_DIALECT = 2;

src/main/java/redis/clients/jedis/AbstractRedisClientBuilder.java:147

  • Currently only searchDialect == 0 is disallowed; consider extending validation to reject negative or otherwise out-of-range dialect values (e.g., searchDialect <= 0).
  public T searchDialect(int searchDialect) {

@uglide uglide changed the title Introduce new RedisClient classes Introduce Refined Redis Client Classes Jul 16, 2025
uglide added 3 commits July 17, 2025 18:20
- Implement autocloseable interface in BaseRedisClient
to use it in existing tests
- Add MULTI methods in BaseRedisClient
- Clean up builders
- Add missing constructors
@uglide uglide marked this pull request as ready for review July 18, 2025 09:53
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.

1 participant