Topic/ggivo/cae 2680 redis server stub for push notifications - #4487
Open
ggivo wants to merge 56 commits into
Open
Topic/ggivo/cae 2680 redis server stub for push notifications#4487ggivo wants to merge 56 commits into
ggivo wants to merge 56 commits into
Conversation
- Preparation step for processing custom push notifications - Push notification can appear out-of band in-between executed commands - Current Connection implementation does not support out of band Push notifications - Meaning it will crash if "CLIENT TRACKING ON is enabled" on regular Jedis Connection and "invalidation" push event is triggered This commit provides a way to register push handler for the connection which process incoming push messages, before actual command is executed. To preserve backward compatibility unprocessed push messages are forward to application logic as before. - By default Connection will start with NOOP push handler which marks any incoming push event as processed and skips it - On subcsribe/psubscribe a dedicated push handler is registered which propagates to the app only supported push vents such as (message, subscribe, unsubscribe ...) - CacheConection is refactored to use a push handler handling "invalidate" push events only, and skipping any other # Conflicts: # src/main/java/redis/clients/jedis/Connection.java # src/test/java/redis/clients/jedis/commands/jedis/PublishSubscribeCommandsTest.java
This commit adds a new PushHandlerChain class that implements the Chain of
Responsibility pattern for Redis RESP3 push message handling. Key features:
- Allows composing multiple PushHandlers in a processing chain
- Push events propagate through the complete chain in sequence
- Events marked as not processed are propagated to the client application
- Provides both constructor-based and fluent builder API for chain creation
- Includes predefined handlers for common use cases (CONSUME_ALL, PROPAGATE_ALL)
- Supports immutable chain transformations via methods like then(),
The chain approach provides a flexible way to handle different types of push
messages (invalidations, pub/sub, etc.) with specialized handlers while
maintaining a clean separation of concerns.
Example usage:
PushHandlerChain chain = PushHandlerChain.of(loggingHandler)
.then(invalidationHandler)
.then(PushHandlerChain.PROPAGATE_PUB_SUB_PUSH_HANDLER);
- code clean up - added relaxed timeout configuration - fix unit tests
Register PushInvalidateConsumer after cache is initialised
ConenctionFactory should be rebound before triggering the disposal of Idle connection, so that any newly creaetd are using the proper hostname
Issue : If Maintenace notifications are received during blocking command, relaxTimeout is enforced instead of infinit timeout. Fix: Introduce dedicated relax timeout setting for blocking commands. It will fall back to infinit timeout if not set
- Mark all pushes by default as processed - Remove CONSUME_ALL_HANDLER
- Use existing ReflectionTestUtil instead of ReflectionTestUtils.java
- address connection pool now uses builder - socketFactory not accessible
- test should now use Endpoints
- ListenerNotificationConsumer from Jedis
moved to Connection
- fix PushMessageNotificationTest
- fix pom.xml missing includes tag
- ConnectionTestHelper is obsolete after rebase
- fix A MIGRATING event on connection A triggers AdaptiveTimeoutHandler for all connections
… for relaxed timeouts
- redis-stub-server core infrastructure and command framework files # Conflicts: # src/test/java/redis/clients/jedis/upgrade/ConnectionAdaptiveTimeoutTest.java # src/test/java/redis/clients/jedis/util/server/TcpMockServer.java
- prepare infra to run basic pub/sub tests agains RedisServerStub
- add pub/sub tests against RedisServerStub verifying arbitrary push messages are handled
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9b790db. Configure here.
| "Socket timeout should be relaxed blocking timeout during blocking command"); | ||
| // await command to be executed | ||
| Awaitility.await().atMost(Duration.ofSeconds(100)) | ||
| .until(() -> connection.isRelaxedTimeoutActive()); |
There was a problem hiding this comment.
Awaitility timeout vastly exceeds JUnit test timeout
Low Severity
The test has @Timeout(value = 1, unit = TimeUnit.SECONDS) but both Awaitility.await().atMost(Duration.ofSeconds(100)) calls use a 100-second max wait. Awaitility's timeout can never trigger because JUnit kills the test at 1 second first, producing a generic timeout error instead of Awaitility's descriptive condition-not-met failure. The 100 likely should be 1 or the @Timeout value needs increasing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9b790db. Configure here.
Base automatically changed from
feature/hu-notifications-rebased-wo-generic-push-listeners
to
feature/sch-1
July 9, 2026 13:39
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
1 participant
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.
Note
Medium Risk
Moderate risk because it replaces the custom test server infrastructure and rewrites several timing-sensitive tests; failures are likely to surface as flaky/changed test behavior even though production code is largely untouched.
Overview
Introduces a new in-test RESP3
RedisServerStub(with an in-memory datastore, command registry, push injection, and a pub/sub manager) and removes the previousredis.clients.jedis.util.server.TcpMockServer/RespResponse/CommandHandlerutilities.Updates existing mock-based tests (
ConnectionMockTest,CacheConnectionMockTest,UnifiedJedisProactiveRebindTest,ConnectionAdaptiveTimeoutTest) to useRedisServerStuband explicitMaintenanceEventpush messages; the adaptive-timeout test is reworked to use a command interceptor + async blocking to simulate blocking behavior.Adds new RedisClient pub/sub test suite: a shared
RedisClientPubSubTestBase, a real-server integration test (RedisClientPubSubIT) for RESP2/RESP3, and a RESP3-only mock test (RedisClientPubSubMockTest) plusPubSubTestHelper. Also expands the formatter plugin includes to cover newserverandpubsubtest packages.Reviewed by Cursor Bugbot for commit 9b790db. Bugbot is set up for automated code reviews on this repo. Configure here.