Skip to content

CommandObjects refactoring Part 2: remove protocol setter and make protocol ummutable#4514

Merged
uglide merged 3 commits into
masterfrom
im/refactor-command-objects-p2
May 20, 2026
Merged

CommandObjects refactoring Part 2: remove protocol setter and make protocol ummutable#4514
uglide merged 3 commits into
masterfrom
im/refactor-command-objects-p2

Conversation

@uglide
Copy link
Copy Markdown
Contributor

@uglide uglide commented May 8, 2026

Note

Medium Risk
Touches core client command construction by making RedisProtocol immutable and changing how CommandObjects is instantiated across Jedis/cluster/multi-db flows, which could affect protocol selection at runtime. Risk is moderated by mostly-constructor-level refactoring plus updated tests, but mis-defaulting to RESP2/RESP3 could cause subtle behavior differences.

Overview
Refactors CommandObjects to require a RedisProtocol at construction time (protocol is now final and setProtocol/no-arg ctor are removed), and deletes several deprecated constructors that previously created a protocol-less instance and later mutated it.

Updates Jedis and related cluster/multi-db transaction/pipeline entry points to always build CommandObjects with an explicit protocol (defaulting to RESP2 when unspecified), removes a test-only UnifiedJedis constructor that probed connections to override protocol, and adjusts unit tests to pass explicit RedisProtocol values (including RESP3 where cache keys require it).

Reviewed by Cursor Bugbot for commit 5bf34e0. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Test Results

  195 files  ±0    195 suites  ±0   9m 38s ⏱️ -13s
7 766 tests ±0  7 714 ✅ +183  52 💤  - 183  0 ❌ ±0 
7 786 runs  ±0  7 734 ✅ +183  52 💤  - 183  0 ❌ ±0 

Results for commit 5bf34e0. ± Comparison against base commit 76be487.

♻️ This comment has been updated with latest results.

@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented May 8, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@uglide uglide added the breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. label May 8, 2026
@uglide uglide force-pushed the im/refactor-command-objects-p2 branch from 9c3f4de to ebf4b67 Compare May 8, 2026 08:28
@uglide uglide added the techdebt Things that can be improved or refactored label May 8, 2026
@uglide uglide requested review from atakavci, ggivo and tishun May 8, 2026 08:59
Copy link
Copy Markdown
Collaborator

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

left some comment on PART one,
Considering they are addressed separately
LGTM

Comment thread src/main/java/redis/clients/jedis/Jedis.java Outdated
Copy link
Copy Markdown
Contributor

@atakavci atakavci left a comment

Choose a reason for hiding this comment

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

LGTM
(went through only the changes against part1)

@uglide uglide force-pushed the im/refactor-command-objects-p2 branch from 08fd10b to 4923232 Compare May 20, 2026 09:04
@uglide uglide merged commit 49ac82c into master May 20, 2026
18 checks passed
@uglide uglide deleted the im/refactor-command-objects-p2 branch May 20, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. techdebt Things that can be improved or refactored

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants