redis/go-redis
1
0
Fork 0
mirror of https://github.com/redis/go-redis.git synced 2025-07-18 00:20:57 +03:00
Code Activity

refactor: simplify push notification interface

Browse Source 
- Remove RegisterPushNotificationHandlerFunc methods from all types
- Remove PushNotificationHandlerFunc type adapter
- Keep only RegisterPushNotificationHandler method for cleaner interface
- Remove unnecessary push notification constants (keep only Redis Cluster ones)
- Update all tests to use simplified interface with direct handler implementations

Benefits:
- Cleaner, simpler API with single registration method
- Reduced code complexity and maintenance burden
- Focus on essential Redis Cluster push notifications only
- Users implement PushNotificationHandler interface directly
- No functional changes, just interface simplification
This commit is contained in:
Nedyalko Dyakov
2025-06-27 00:17:47 +03:00
parent 4747610d01
commit 70231ae4e9
4 changed files with 89 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
push_notification_coverage_test.go
View File

@ -11,6 +11,20 @@ import (
"github.com/redis/go-redis/v9/internal/proto"
)
// testHandler is a simple implementation of PushNotificationHandler for testing
type testHandler struct {
handlerFunc func(ctx context.Context, notification []interface{}) bool
}
func (h *testHandler) HandlePushNotification(ctx context.Context, notification []interface{}) bool {
return h.handlerFunc(ctx, notification)
}
// newTestHandler creates a test handler from a function
func newTestHandler(f func(ctx context.Context, notification []interface{}) bool) *testHandler {
return &testHandler{handlerFunc: f}
}
// TestConnectionPoolPushNotificationIntegration tests the connection pool's
// integration with push notifications for 100% coverage.
func TestConnectionPoolPushNotificationIntegration(t *testing.T) {
@ -102,9 +116,9 @@ func TestConnectionHealthCheckWithPushNotifications(t *testing.T) {
defer client.Close()
// Register a handler to ensure processor is active
err := client.RegisterPushNotificationHandlerFunc("TEST_HEALTH", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST_HEALTH", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -147,7 +161,7 @@ func TestConnPushNotificationMethods(t *testing.T) {
}
// Test RegisterPushNotificationHandler
handler := PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
@ -156,10 +170,10 @@ func TestConnPushNotificationMethods(t *testing.T) {
t.Errorf("Failed to register handler on Conn: %v", err)
}
// Test RegisterPushNotificationHandlerFunc
err = conn.RegisterPushNotificationHandlerFunc("TEST_CONN_FUNC", func(ctx context.Context, notification []interface{}) bool {
// Test RegisterPushNotificationHandler with function wrapper
err = conn.RegisterPushNotificationHandler("TEST_CONN_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Errorf("Failed to register handler func on Conn: %v", err)
}
@ -206,17 +220,17 @@ func TestConnWithoutPushNotifications(t *testing.T) {
}
// Test RegisterPushNotificationHandler returns nil (no error)
err := conn.RegisterPushNotificationHandler("TEST", PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
err := conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
}))
if err != nil {
t.Errorf("Should return nil error when no processor: %v", err)
}
// Test RegisterPushNotificationHandlerFunc returns nil (no error)
err = conn.RegisterPushNotificationHandlerFunc("TEST", func(ctx context.Context, notification []interface{}) bool {
// Test RegisterPushNotificationHandler returns nil (no error)
err = conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Errorf("Should return nil error when no processor: %v", err)
}
@ -263,9 +277,9 @@ func TestClonedClientPushNotifications(t *testing.T) {
}
// Register handler on original
err := client.RegisterPushNotificationHandlerFunc("TEST_CLONE", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST_CLONE", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -289,9 +303,9 @@ func TestClonedClientPushNotifications(t *testing.T) {
}
// Test registering new handler on cloned client
err = clonedClient.RegisterPushNotificationHandlerFunc("TEST_CLONE_NEW", func(ctx context.Context, notification []interface{}) bool {
err = clonedClient.RegisterPushNotificationHandler("TEST_CLONE_NEW", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Errorf("Failed to register handler on cloned client: %v", err)
}

39
push_notifications.go
View File

@ -16,14 +16,6 @@ type PushNotificationHandler interface {
HandlePushNotification(ctx context.Context, notification []interface{}) bool
}
// PushNotificationHandlerFunc is a function adapter for PushNotificationHandler.
type PushNotificationHandlerFunc func(ctx context.Context, notification []interface{}) bool
// HandlePushNotification implements PushNotificationHandler.
func (f PushNotificationHandlerFunc) HandlePushNotification(ctx context.Context, notification []interface{}) bool {
return f(ctx, notification)
}
// PushNotificationRegistry manages handlers for different types of push notifications.
type PushNotificationRegistry struct {
mu sync.RWMutex
@ -185,42 +177,13 @@ func (p *PushNotificationProcessor) RegisterHandler(command string, handler Push
return p.registry.RegisterHandler(command, handler)
}
// RegisterHandlerFunc is a convenience method to register a function as a handler.
// Returns an error if a handler is already registered for this command.
func (p *PushNotificationProcessor) RegisterHandlerFunc(command string, handlerFunc func(ctx context.Context, notification []interface{}) bool) error {
return p.registry.RegisterHandler(command, PushNotificationHandlerFunc(handlerFunc))
}
// Common push notification commands
// Redis Cluster push notification commands
const (
// Redis Cluster notifications
PushNotificationMoving = "MOVING"
PushNotificationMigrating = "MIGRATING"
PushNotificationMigrated = "MIGRATED"
PushNotificationFailingOver = "FAILING_OVER"
PushNotificationFailedOver = "FAILED_OVER"
// Redis Pub/Sub notifications
PushNotificationPubSubMessage = "message"
PushNotificationPMessage = "pmessage"
PushNotificationSubscribe = "subscribe"
PushNotificationUnsubscribe = "unsubscribe"
PushNotificationPSubscribe = "psubscribe"
PushNotificationPUnsubscribe = "punsubscribe"
// Redis Stream notifications
PushNotificationXRead = "xread"
PushNotificationXReadGroup = "xreadgroup"
// Redis Keyspace notifications
PushNotificationKeyspace = "keyspace"
PushNotificationKeyevent = "keyevent"
// Redis Module notifications
PushNotificationModule = "module"
// Custom application notifications
PushNotificationCustom = "custom"
)
// PushNotificationInfo contains metadata about a push notification.

132
push_notifications_test.go
View File

@ -9,6 +9,20 @@ import (
"github.com/redis/go-redis/v9/internal/pool"
)
// testHandler is a simple implementation of PushNotificationHandler for testing
type testHandler struct {
handlerFunc func(ctx context.Context, notification []interface{}) bool
}
func (h *testHandler) HandlePushNotification(ctx context.Context, notification []interface{}) bool {
return h.handlerFunc(ctx, notification)
}
// newTestHandler creates a test handler from a function
func newTestHandler(f func(ctx context.Context, notification []interface{}) bool) *testHandler {
return &testHandler{handlerFunc: f}
}
func TestPushNotificationRegistry(t *testing.T) {
// Test the push notification registry functionality
registry := redis.NewPushNotificationRegistry()
@ -25,7 +39,7 @@ func TestPushNotificationRegistry(t *testing.T) {
// Test registering a specific handler
handlerCalled := false
handler := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
@ -58,7 +72,7 @@ func TestPushNotificationRegistry(t *testing.T) {
}
// Test duplicate handler registration error
duplicateHandler := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
duplicateHandler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
err = registry.RegisterHandler("TEST_COMMAND", duplicateHandler)
@ -81,7 +95,7 @@ func TestPushNotificationProcessor(t *testing.T) {
// Test registering handlers
handlerCalled := false
err := processor.RegisterHandlerFunc("CUSTOM_NOTIFICATION", func(ctx context.Context, notification []interface{}) bool {
err := processor.RegisterHandler("CUSTOM_NOTIFICATION", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
if len(notification) < 2 {
t.Error("Expected at least 2 elements in notification")
@ -92,7 +106,7 @@ func TestPushNotificationProcessor(t *testing.T) {
return false
}
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -138,10 +152,10 @@ func TestClientPushNotificationIntegration(t *testing.T) {
// Test registering handlers through client
handlerCalled := false
err := client.RegisterPushNotificationHandlerFunc("CUSTOM_EVENT", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("CUSTOM_EVENT", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -175,9 +189,9 @@ func TestClientWithoutPushNotifications(t *testing.T) {
}
// Registering handlers should not panic
err := client.RegisterPushNotificationHandlerFunc("TEST", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Errorf("Expected nil error when processor is nil, got: %v", err)
}
@ -204,10 +218,10 @@ func TestPushNotificationEnabledClient(t *testing.T) {
// Test registering a handler
handlerCalled := false
err := client.RegisterPushNotificationHandlerFunc("TEST_NOTIFICATION", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST_NOTIFICATION", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -228,17 +242,13 @@ func TestPushNotificationEnabledClient(t *testing.T) {
}
func TestPushNotificationConstants(t *testing.T) {
// Test that push notification constants are defined correctly
// Test that Redis Cluster push notification constants are defined correctly
constants := map[string]string{
redis.PushNotificationMoving: "MOVING",
redis.PushNotificationMigrating: "MIGRATING",
redis.PushNotificationMigrated: "MIGRATED",
redis.PushNotificationPubSubMessage: "message",
redis.PushNotificationPMessage: "pmessage",
redis.PushNotificationSubscribe: "subscribe",
redis.PushNotificationUnsubscribe: "unsubscribe",
redis.PushNotificationKeyspace: "keyspace",
redis.PushNotificationKeyevent: "keyevent",
redis.PushNotificationMoving: "MOVING",
redis.PushNotificationMigrating: "MIGRATING",
redis.PushNotificationMigrated: "MIGRATED",
redis.PushNotificationFailingOver: "FAILING_OVER",
redis.PushNotificationFailedOver: "FAILED_OVER",
}
for constant, expected := range constants {
@ -293,11 +303,11 @@ func TestPubSubWithGenericPushNotifications(t *testing.T) {
// Register a handler for custom push notifications
customNotificationReceived := false
err := client.RegisterPushNotificationHandlerFunc("CUSTOM_PUBSUB_EVENT", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("CUSTOM_PUBSUB_EVENT", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
customNotificationReceived = true
t.Logf("Received custom push notification in PubSub context: %v", notification)
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}
@ -353,7 +363,7 @@ func TestPushNotificationRegistryUnregisterHandler(t *testing.T) {
// Register a handler
handlerCalled := false
handler := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
@ -440,11 +450,11 @@ func TestPushNotificationRegistryDuplicateHandlerError(t *testing.T) {
registry := redis.NewPushNotificationRegistry()
// Test that registering duplicate handlers returns an error
handler1 := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler1 := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
handler2 := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler2 := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return false
})
@ -478,7 +488,7 @@ func TestPushNotificationRegistrySpecificHandlerOnly(t *testing.T) {
specificCalled := false
// Register specific handler
err := registry.RegisterHandler("SPECIFIC_CMD", redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
err := registry.RegisterHandler("SPECIFIC_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
specificCalled = true
return true
}))
@ -525,10 +535,10 @@ func TestPushNotificationProcessorEdgeCases(t *testing.T) {
// Test that disabled processor doesn't process notifications
handlerCalled := false
processor.RegisterHandlerFunc("TEST_CMD", func(ctx context.Context, notification []interface{}) bool {
processor.RegisterHandler("TEST_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
}))
// Even with handlers registered, disabled processor shouldn't process
ctx := context.Background()
@ -555,7 +565,7 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) {
// Test RegisterHandler convenience method
handlerCalled := false
handler := redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
handler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
handlerCalled = true
return true
})
@ -565,12 +575,12 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) {
t.Fatalf("Failed to register handler: %v", err)
}
// Test RegisterHandlerFunc convenience method
// Test RegisterHandler convenience method with function
funcHandlerCalled := false
err = processor.RegisterHandlerFunc("FUNC_CMD", func(ctx context.Context, notification []interface{}) bool {
err = processor.RegisterHandler("FUNC_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
funcHandlerCalled = true
return true
})
}))
if err != nil {
t.Fatalf("Failed to register func handler: %v", err)
}
@ -616,16 +626,16 @@ func TestClientPushNotificationEdgeCases(t *testing.T) {
defer client.Close()
// These should not panic even when processor is nil and should return nil error
err := client.RegisterPushNotificationHandler("TEST", redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
}))
if err != nil {
t.Errorf("Expected nil error when processor is nil, got: %v", err)
}
err = client.RegisterPushNotificationHandlerFunc("TEST_FUNC", func(ctx context.Context, notification []interface{}) bool {
err = client.RegisterPushNotificationHandler("TEST_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
if err != nil {
t.Errorf("Expected nil error when processor is nil, got: %v", err)
}
@ -650,7 +660,7 @@ func TestPushNotificationHandlerFunc(t *testing.T) {
return true
}
handler := redis.PushNotificationHandlerFunc(handlerFunc)
handler := newTestHandler(handlerFunc)
// Test that the adapter works correctly
ctx := context.Background()
@ -709,36 +719,14 @@ func TestPushNotificationInfoEdgeCases(t *testing.T) {
}
func TestPushNotificationConstantsCompleteness(t *testing.T) {
// Test that all expected constants are defined
// Test that all Redis Cluster push notification constants are defined
expectedConstants := map[string]string{
// Cluster notifications
redis.PushNotificationMoving: "MOVING",
redis.PushNotificationMigrating: "MIGRATING",
redis.PushNotificationMigrated: "MIGRATED",
redis.PushNotificationFailingOver: "FAILING_OVER",
redis.PushNotificationFailedOver: "FAILED_OVER",
// Pub/Sub notifications
redis.PushNotificationPubSubMessage: "message",
redis.PushNotificationPMessage: "pmessage",
redis.PushNotificationSubscribe: "subscribe",
redis.PushNotificationUnsubscribe: "unsubscribe",
redis.PushNotificationPSubscribe: "psubscribe",
redis.PushNotificationPUnsubscribe: "punsubscribe",
// Stream notifications
redis.PushNotificationXRead: "xread",
redis.PushNotificationXReadGroup: "xreadgroup",
// Keyspace notifications
redis.PushNotificationKeyspace: "keyspace",
redis.PushNotificationKeyevent: "keyevent",
// Module notifications
redis.PushNotificationModule: "module",
// Custom notifications
redis.PushNotificationCustom: "custom",
// Cluster notifications only (other types removed for simplicity)
redis.PushNotificationMoving: "MOVING",
redis.PushNotificationMigrating: "MIGRATING",
redis.PushNotificationMigrated: "MIGRATED",
redis.PushNotificationFailingOver: "FAILING_OVER",
redis.PushNotificationFailedOver: "FAILED_OVER",
}
for constant, expected := range expectedConstants {
@ -767,7 +755,7 @@ func TestPushNotificationRegistryConcurrency(t *testing.T) {
for j := 0; j < numOperations; j++ {
// Register handler (ignore errors in concurrency test)
command := fmt.Sprintf("CMD_%d_%d", id, j)
registry.RegisterHandler(command, redis.PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
registry.RegisterHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
}))
@ -815,9 +803,9 @@ func TestPushNotificationProcessorConcurrency(t *testing.T) {
for j := 0; j < numOperations; j++ {
// Register handlers (ignore errors in concurrency test)
command := fmt.Sprintf("PROC_CMD_%d_%d", id, j)
processor.RegisterHandlerFunc(command, func(ctx context.Context, notification []interface{}) bool {
processor.RegisterHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
// Handle notifications
notification := []interface{}{command, "data"}
@ -869,9 +857,9 @@ func TestPushNotificationClientConcurrency(t *testing.T) {
for j := 0; j < numOperations; j++ {
// Register handlers concurrently (ignore errors in concurrency test)
command := fmt.Sprintf("CLIENT_CMD_%d_%d", id, j)
client.RegisterPushNotificationHandlerFunc(command, func(ctx context.Context, notification []interface{}) bool {
client.RegisterPushNotificationHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool {
return true
})
}))
// Access processor
processor := client.GetPushNotificationProcessor()
@ -912,10 +900,10 @@ func TestPushNotificationConnectionHealthCheck(t *testing.T) {
}
// Register a handler for testing
err := client.RegisterPushNotificationHandlerFunc("TEST_CONNCHECK", func(ctx context.Context, notification []interface{}) bool {
err := client.RegisterPushNotificationHandler("TEST_CONNCHECK", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
t.Logf("Received test notification: %v", notification)
return true
})
}))
if err != nil {
t.Fatalf("Failed to register handler: %v", err)
}

18
redis.go
View File

@ -833,15 +833,6 @@ func (c *Client) RegisterPushNotificationHandler(command string, handler PushNot
return nil
}
// RegisterPushNotificationHandlerFunc registers a function as a handler for a specific push notification command.
// Returns an error if a handler is already registered for this command.
func (c *Client) RegisterPushNotificationHandlerFunc(command string, handlerFunc func(ctx context.Context, notification []interface{}) bool) error {
if c.pushProcessor != nil {
return c.pushProcessor.RegisterHandlerFunc(command, handlerFunc)
}
return nil
}
// GetPushNotificationProcessor returns the push notification processor.
func (c *Client) GetPushNotificationProcessor() *PushNotificationProcessor {
return c.pushProcessor
@ -1014,15 +1005,6 @@ func (c *Conn) RegisterPushNotificationHandler(command string, handler PushNotif
return nil
}
// RegisterPushNotificationHandlerFunc registers a function as a handler for a specific push notification command.
// Returns an error if a handler is already registered for this command.
func (c *Conn) RegisterPushNotificationHandlerFunc(command string, handlerFunc func(ctx context.Context, notification []interface{}) bool) error {
if c.pushProcessor != nil {
return c.pushProcessor.RegisterHandlerFunc(command, handlerFunc)
}
return nil
}
// GetPushNotificationProcessor returns the push notification processor.
func (c *Conn) GetPushNotificationProcessor() *PushNotificationProcessor {
return c.pushProcessor