[Commands API interface consistency #3] Fix node-selection aggregate interface wiring - #3867
Open
uglide wants to merge 5 commits into
Open
[Commands API interface consistency #3] Fix node-selection aggregate interface wiring#3867uglide wants to merge 5 commits into
uglide wants to merge 5 commits into
Conversation
4 tasks
uglide
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c9d0e672e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| NodeSelectionArrayAsyncCommands<K, V>, NodeSelectionFunctionAsyncCommands<K, V>, NodeSelectionGeoAsyncCommands<K, V>, | ||
| NodeSelectionHashAsyncCommands<K, V>, NodeSelectionHLLAsyncCommands<K, V>, NodeSelectionKeyAsyncCommands<K, V>, | ||
| NodeSelectionListAsyncCommands<K, V>, NodeSelectionScriptingAsyncCommands<K, V>, NodeSelectionServerAsyncCommands<K, V>, | ||
| NodeSelectionSetAsyncCommands<K, V>, NodeSelectionSortedSetAsyncCommands<K, V>, NodeSelectionStreamAsyncCommands<K, V>, |
There was a problem hiding this comment.
On the current 7.7.0-SNAPSHOT line (pom.xml:6), replacing NodeSelectionStreamCommands with this async interface removes every inherited x* method descriptor returning Executions<T> and replaces it with an unrelated AsyncExecutions<T> return type. Applications compiled against an earlier 7.x release will resolve the old descriptor and fail with NoSuchMethodError, while source code assigning these results to Executions<T> stops compiling; this major-version-only API break should therefore be applied on the 8.0 line rather than released in 7.7.
Useful? React with 👍 / 👎.
uglide
force-pushed
the
im/node-selection-aggregate-wiring
branch
from
July 28, 2026 16:33
0c9d0e6 to
ae9138c
Compare
Introduce reflection-based unit tests that verify the sync, async, reactive, Kotlin coroutine and cluster node-selection command interfaces stay in lockstep, replacing the consistency-by-regeneration guarantee of the template-based API generators: - CommandInterfaces: catalog of all command groups and their flavors - KnownApiDeviations: single registry of intentional deviations, ported from the generator rule tables - TypeSignatures: shared reflection and type-normalization helpers - SyncAsync/SyncReactive/NodeSelection/AggregateInterface/ CommandBuilderCoverage/KotlinCoroutines consistency tests The suite checks method parity in both directions, the per-flavor return-type mapping (RedisFuture, Mono/Flux, suspend/Flow, Executions), aggregate interface wiring and interface-to-command- builder coverage. Sync/async parity is load-bearing at runtime: the sync API is a dynamic proxy over async (FutureSyncInvocationHandler). Supersedes SyncAsyncApiConvergenceUnitTests. Adds kotlin-reflect as a test dependency. Note: at this commit the suite documents real drift in the committed interfaces — several tests fail intentionally; the deviations are fixed in the follow-up commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uglide
force-pushed
the
im/api-consistency-tests
branch
from
July 28, 2026 16:38
7d09bed to
fddac9d
Compare
uglide
force-pushed
the
im/node-selection-aggregate-wiring
branch
2 times, most recently
from
July 28, 2026 16:54
3d48911 to
71d31e6
Compare
Real drift found by the new consistency tests, accumulated while the API generators were out of use: - RedisCoroutinesCommands did not extend RediSearchCoroutinesCommands (the implementation already delegated to it) - Kotlin coroutine API was missing bitopDiff/bitopDiff1/bitopAndor/ bitopOne, hgetex(key, fields), hsetex(key, map) and the xackdel/xdelex stream commands - reactive hgetdel/hgetex streaming-channel variants were missing their @deprecated marker The node-selection aggregate wiring drift (missing ACL/Array groups, async aggregate extending the sync Stream interface) is exempted via KnownApiDeviations.NODE_SELECTION_AGGREGATE_PENDING — fixing it is a breaking change scheduled for the 8.0 release. Makes the API consistency test suite pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- verify the methods declared directly on the aggregate interfaces (auth, select, the CLUSTER commands, ...) against their async and reactive counterparts, restoring coverage the removed SyncAsyncApiConvergenceUnitTests had via RedisCommands.getMethods() - include RedisClusterCoroutinesCommands in the aggregate wiring check and add its missing FUNCTION/JSON/VECTOR_SET/ARRAY/SEARCH groups (experimental API; the impl already delegates per group) - keep the Kotlin generator's FLOW_METHODS in sync with the hand-added Flow methods for as long as the generators still exist - add behavioral coroutine integration tests for the new hash-field-expiry, BITOP and stream-deletion commands Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uglide
force-pushed
the
im/api-consistency-tests
branch
from
July 28, 2026 16:57
16eebdb to
622d675
Compare
uglide
force-pushed
the
im/node-selection-aggregate-wiring
branch
from
July 28, 2026 16:57
71d31e6 to
2597756
Compare
Erased-parameter matching alone would accept flavors whose generic parameter signatures diverge (e.g. Map<K, V> vs Map<V, K>), and nothing compared @deprecated across the Java flavors. The suite now verifies both for sync/async, sync/reactive, node-selection and the aggregate-declared methods. The reactive dispatch parameter mutation (CommandOutput<K, V, ?>) and the intentional deprecation of the CommandOutput-based node-selection dispatch overloads are recorded in KnownApiDeviations. Fixes surfaced by the new checks: RedisAdvancedClusterReactiveCommands .keysLegacy and RedisAdvancedClusterAsyncCommands.masters carried a @deprecated tag without the matching @deprecated annotation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NodeSelectionCommands and NodeSelectionAsyncCommands did not extend the ACL and Array node-selection groups, and NodeSelectionAsyncCommands extended the sync NodeSelectionStreamCommands instead of NodeSelectionStreamAsyncCommands. Breaking change: stream commands on AsyncNodeSelection now return AsyncExecutions<T> instead of Executions<T>. Removes the NODE_SELECTION_AGGREGATE_PENDING exemption so the consistency test suite enforces the aggregate wiring again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uglide
force-pushed
the
im/node-selection-aggregate-wiring
branch
from
July 28, 2026 17:12
2597756 to
f5f86b3
Compare
uglide
force-pushed
the
im/api-consistency-tests
branch
3 times, most recently
from
August 6, 2026 13:40
463a98b to
8354828
Compare
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
2 participants
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.
Fixes the cluster node-selection aggregate interfaces (extracted from #3865 because part of it is a breaking change; stacked on #3865 — rebase onto the 8.0 line once it merges):
NodeSelectionCommandsandNodeSelectionAsyncCommandsdid not extend the ACL and Array node-selection groups, soacl*/ar*commands were unreachable through node selections (additive, non-breaking)NodeSelectionAsyncCommandsextended the syncNodeSelectionStreamCommandsinstead ofNodeSelectionStreamAsyncCommandsStream commands invoked on an
AsyncNodeSelection(e.g.xadd,xread,xrange) now returnAsyncExecutions<T>instead of the sync-shapedExecutions<T>. Source-incompatible for code that assigned those results toExecutions<T>. Targeted for the 8.0.0 release.Also removes the
NODE_SELECTION_AGGREGATE_PENDINGexemption fromKnownApiDeviationsso the consistency test suite from #3865 enforces the aggregate wiring again.Make sure that:
mvn formatter:formattarget. Don't submit any formatting related changes.🤖 Generated with Claude Code
Note
Medium Risk
Breaking API change on async node-selection stream return types for 8.0; otherwise additive interface extensions with low runtime risk.
Overview
Fixes cluster node-selection aggregate interfaces so they expose the same command groups as the consistency suite expects.
NodeSelectionCommandsandNodeSelectionAsyncCommandsnow extend the ACL and Array node-selection groups, makingacl*/ar*commands reachable through node selections (additive).NodeSelectionAsyncCommandsnow extendsNodeSelectionStreamAsyncCommandsinstead of the syncNodeSelectionStreamCommands. Stream calls on an async node selection (e.g.xadd,xread) returnAsyncExecutions<T>instead ofExecutions<T>— source-incompatible for code that assumed sync-shaped results.The
NODE_SELECTION_AGGREGATE_PENDINGskip list is removed sonodeSelectionAggregatesCoverAllNodeSelectionGroupsenforces full aggregate wiring again.Reviewed by Cursor Bugbot for commit f5f86b3. Bugbot is set up for automated code reviews on this repo. Configure here.