diff --git a/push_notification_coverage_test.go b/push_notification_coverage_test.go index f163b13c..eee48216 100644 --- a/push_notification_coverage_test.go +++ b/push_notification_coverage_test.go @@ -118,7 +118,7 @@ func TestConnectionHealthCheckWithPushNotifications(t *testing.T) { // Register a handler to ensure processor is active err := client.RegisterPushNotificationHandler("TEST_HEALTH", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -165,7 +165,7 @@ func TestConnPushNotificationMethods(t *testing.T) { return true }) - err := conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler) + err := conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler, false) if err != nil { t.Errorf("Failed to register handler on Conn: %v", err) } @@ -173,13 +173,13 @@ func TestConnPushNotificationMethods(t *testing.T) { // Test RegisterPushNotificationHandler with function wrapper err = conn.RegisterPushNotificationHandler("TEST_CONN_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Failed to register handler func on Conn: %v", err) } // Test duplicate handler error - err = conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler) + err = conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler, false) if err == nil { t.Error("Should get error when registering duplicate handler") } @@ -222,7 +222,7 @@ func TestConnWithoutPushNotifications(t *testing.T) { // Test RegisterPushNotificationHandler returns nil (no error) err := conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Should return nil error when no processor: %v", err) } @@ -230,7 +230,7 @@ func TestConnWithoutPushNotifications(t *testing.T) { // Test RegisterPushNotificationHandler returns nil (no error) err = conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Should return nil error when no processor: %v", err) } @@ -279,7 +279,7 @@ func TestClonedClientPushNotifications(t *testing.T) { // Register handler on original err := client.RegisterPushNotificationHandler("TEST_CLONE", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -305,7 +305,7 @@ func TestClonedClientPushNotifications(t *testing.T) { // Test registering new handler on cloned client err = clonedClient.RegisterPushNotificationHandler("TEST_CLONE_NEW", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Failed to register handler on cloned client: %v", err) } @@ -350,22 +350,22 @@ func TestPushNotificationInfoStructure(t *testing.T) { t.Run(tc.name, func(t *testing.T) { info := ParsePushNotificationInfo(tc.notification) - if info.Command != tc.expectedCmd { - t.Errorf("Expected command %s, got %s", tc.expectedCmd, info.Command) + if info.Name != tc.expectedCmd { + t.Errorf("Expected name %s, got %s", tc.expectedCmd, info.Name) } if len(info.Args) != tc.expectedArgs { t.Errorf("Expected %d args, got %d", tc.expectedArgs, len(info.Args)) } - // Verify no unused fields exist by checking the struct only has Command and Args + // Verify no unused fields exist by checking the struct only has Name and Args // This is a compile-time check - if unused fields were added back, this would fail _ = struct { - Command string - Args []interface{} + Name string + Args []interface{} }{ - Command: info.Command, - Args: info.Args, + Name: info.Name, + Args: info.Args, } }) } diff --git a/push_notifications.go b/push_notifications.go index b1c89ca3..e6c749ab 100644 --- a/push_notifications.go +++ b/push_notifications.go @@ -18,36 +18,47 @@ type PushNotificationHandler interface { // PushNotificationRegistry manages handlers for different types of push notifications. type PushNotificationRegistry struct { - mu sync.RWMutex - handlers map[string]PushNotificationHandler // command -> single handler + mu sync.RWMutex + handlers map[string]PushNotificationHandler // pushNotificationName -> single handler + protected map[string]bool // pushNotificationName -> protected flag } // NewPushNotificationRegistry creates a new push notification registry. func NewPushNotificationRegistry() *PushNotificationRegistry { return &PushNotificationRegistry{ - handlers: make(map[string]PushNotificationHandler), + handlers: make(map[string]PushNotificationHandler), + protected: make(map[string]bool), } } -// RegisterHandler registers a handler for a specific push notification command. -// Returns an error if a handler is already registered for this command. -func (r *PushNotificationRegistry) RegisterHandler(command string, handler PushNotificationHandler) error { +// RegisterHandler registers a handler for a specific push notification name. +// Returns an error if a handler is already registered for this push notification name. +// If protected is true, the handler cannot be unregistered. +func (r *PushNotificationRegistry) RegisterHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error { r.mu.Lock() defer r.mu.Unlock() - if _, exists := r.handlers[command]; exists { - return fmt.Errorf("handler already registered for command: %s", command) + if _, exists := r.handlers[pushNotificationName]; exists { + return fmt.Errorf("handler already registered for push notification: %s", pushNotificationName) } - r.handlers[command] = handler + r.handlers[pushNotificationName] = handler + r.protected[pushNotificationName] = protected return nil } -// UnregisterHandler removes the handler for a specific push notification command. -func (r *PushNotificationRegistry) UnregisterHandler(command string) { +// UnregisterHandler removes the handler for a specific push notification name. +// Returns an error if the handler is protected. +func (r *PushNotificationRegistry) UnregisterHandler(pushNotificationName string) error { r.mu.Lock() defer r.mu.Unlock() - delete(r.handlers, command) + if r.protected[pushNotificationName] { + return fmt.Errorf("cannot unregister protected handler for push notification: %s", pushNotificationName) + } + + delete(r.handlers, pushNotificationName) + delete(r.protected, pushNotificationName) + return nil } // HandleNotification processes a push notification by calling the registered handler. @@ -56,8 +67,8 @@ func (r *PushNotificationRegistry) HandleNotification(ctx context.Context, notif return false } - // Extract command from notification - command, ok := notification[0].(string) + // Extract push notification name from notification + pushNotificationName, ok := notification[0].(string) if !ok { return false } @@ -66,23 +77,23 @@ func (r *PushNotificationRegistry) HandleNotification(ctx context.Context, notif defer r.mu.RUnlock() // Call specific handler - if handler, exists := r.handlers[command]; exists { + if handler, exists := r.handlers[pushNotificationName]; exists { return handler.HandlePushNotification(ctx, notification) } return false } -// GetRegisteredCommands returns a list of commands that have registered handlers. -func (r *PushNotificationRegistry) GetRegisteredCommands() []string { +// GetRegisteredPushNotificationNames returns a list of push notification names that have registered handlers. +func (r *PushNotificationRegistry) GetRegisteredPushNotificationNames() []string { r.mu.RLock() defer r.mu.RUnlock() - commands := make([]string, 0, len(r.handlers)) - for command := range r.handlers { - commands = append(commands, command) + names := make([]string, 0, len(r.handlers)) + for name := range r.handlers { + names = append(names, name) } - return commands + return names } // HasHandlers returns true if there are any handlers registered. @@ -176,13 +187,14 @@ func (p *PushNotificationProcessor) ProcessPendingNotifications(ctx context.Cont return nil } -// RegisterHandler is a convenience method to register a handler for a specific command. -// Returns an error if a handler is already registered for this command. -func (p *PushNotificationProcessor) RegisterHandler(command string, handler PushNotificationHandler) error { - return p.registry.RegisterHandler(command, handler) +// RegisterHandler is a convenience method to register a handler for a specific push notification name. +// Returns an error if a handler is already registered for this push notification name. +// If protected is true, the handler cannot be unregistered. +func (p *PushNotificationProcessor) RegisterHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error { + return p.registry.RegisterHandler(pushNotificationName, handler, protected) } -// Redis Cluster push notification commands +// Redis Cluster push notification names const ( PushNotificationMoving = "MOVING" PushNotificationMigrating = "MIGRATING" @@ -193,8 +205,8 @@ const ( // PushNotificationInfo contains metadata about a push notification. type PushNotificationInfo struct { - Command string - Args []interface{} + Name string + Args []interface{} } // ParsePushNotificationInfo extracts information from a push notification. @@ -203,14 +215,14 @@ func ParsePushNotificationInfo(notification []interface{}) *PushNotificationInfo return nil } - command, ok := notification[0].(string) + name, ok := notification[0].(string) if !ok { return nil } return &PushNotificationInfo{ - Command: command, - Args: notification[1:], + Name: name, + Args: notification[1:], } } @@ -219,5 +231,5 @@ func (info *PushNotificationInfo) String() string { if info == nil { return "" } - return info.Command + return info.Name } diff --git a/push_notifications_test.go b/push_notifications_test.go index 963958c0..88d676bf 100644 --- a/push_notifications_test.go +++ b/push_notifications_test.go @@ -32,7 +32,7 @@ func TestPushNotificationRegistry(t *testing.T) { t.Error("Registry should not have handlers initially") } - commands := registry.GetRegisteredCommands() + commands := registry.GetRegisteredPushNotificationNames() if len(commands) != 0 { t.Errorf("Expected 0 registered commands, got %d", len(commands)) } @@ -44,7 +44,7 @@ func TestPushNotificationRegistry(t *testing.T) { return true }) - err := registry.RegisterHandler("TEST_COMMAND", handler) + err := registry.RegisterHandler("TEST_COMMAND", handler, false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -53,7 +53,7 @@ func TestPushNotificationRegistry(t *testing.T) { t.Error("Registry should have handlers after registration") } - commands = registry.GetRegisteredCommands() + commands = registry.GetRegisteredPushNotificationNames() if len(commands) != 1 || commands[0] != "TEST_COMMAND" { t.Errorf("Expected ['TEST_COMMAND'], got %v", commands) } @@ -75,11 +75,11 @@ func TestPushNotificationRegistry(t *testing.T) { duplicateHandler := newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true }) - err = registry.RegisterHandler("TEST_COMMAND", duplicateHandler) + err = registry.RegisterHandler("TEST_COMMAND", duplicateHandler, false) if err == nil { t.Error("Expected error when registering duplicate handler") } - expectedError := "handler already registered for command: TEST_COMMAND" + expectedError := "handler already registered for push notification: TEST_COMMAND" if err.Error() != expectedError { t.Errorf("Expected error '%s', got '%s'", expectedError, err.Error()) } @@ -106,7 +106,7 @@ func TestPushNotificationProcessor(t *testing.T) { return false } return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -155,7 +155,7 @@ func TestClientPushNotificationIntegration(t *testing.T) { err := client.RegisterPushNotificationHandler("CUSTOM_EVENT", newTestHandler(func(ctx context.Context, notification []interface{}) bool { handlerCalled = true return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -191,7 +191,7 @@ func TestClientWithoutPushNotifications(t *testing.T) { // Registering handlers should not panic err := client.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Expected nil error when processor is nil, got: %v", err) } @@ -221,7 +221,7 @@ func TestPushNotificationEnabledClient(t *testing.T) { err := client.RegisterPushNotificationHandler("TEST_NOTIFICATION", newTestHandler(func(ctx context.Context, notification []interface{}) bool { handlerCalled = true return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -241,6 +241,58 @@ func TestPushNotificationEnabledClient(t *testing.T) { } } +func TestPushNotificationProtectedHandlers(t *testing.T) { + registry := redis.NewPushNotificationRegistry() + + // Register a protected handler + protectedHandler := newTestHandler(func(ctx context.Context, notification []interface{}) bool { + return true + }) + err := registry.RegisterHandler("PROTECTED_HANDLER", protectedHandler, true) + if err != nil { + t.Fatalf("Failed to register protected handler: %v", err) + } + + // Register a non-protected handler + normalHandler := newTestHandler(func(ctx context.Context, notification []interface{}) bool { + return true + }) + err = registry.RegisterHandler("NORMAL_HANDLER", normalHandler, false) + if err != nil { + t.Fatalf("Failed to register normal handler: %v", err) + } + + // Try to unregister the protected handler - should fail + err = registry.UnregisterHandler("PROTECTED_HANDLER") + if err == nil { + t.Error("Should not be able to unregister protected handler") + } + expectedError := "cannot unregister protected handler for push notification: PROTECTED_HANDLER" + if err.Error() != expectedError { + t.Errorf("Expected error '%s', got '%s'", expectedError, err.Error()) + } + + // Try to unregister the normal handler - should succeed + err = registry.UnregisterHandler("NORMAL_HANDLER") + if err != nil { + t.Errorf("Should be able to unregister normal handler: %v", err) + } + + // Verify protected handler is still registered + commands := registry.GetRegisteredPushNotificationNames() + if len(commands) != 1 || commands[0] != "PROTECTED_HANDLER" { + t.Errorf("Expected only protected handler to remain, got %v", commands) + } + + // Verify protected handler still works + ctx := context.Background() + notification := []interface{}{"PROTECTED_HANDLER", "data"} + handled := registry.HandleNotification(ctx, notification) + if !handled { + t.Error("Protected handler should still work") + } +} + func TestPushNotificationConstants(t *testing.T) { // Test that Redis Cluster push notification constants are defined correctly constants := map[string]string{ @@ -267,8 +319,8 @@ func TestPushNotificationInfo(t *testing.T) { t.Fatal("Push notification info should not be nil") } - if info.Command != "MOVING" { - t.Errorf("Expected command 'MOVING', got '%s'", info.Command) + if info.Name != "MOVING" { + t.Errorf("Expected name 'MOVING', got '%s'", info.Name) } if len(info.Args) != 2 { @@ -307,7 +359,7 @@ func TestPubSubWithGenericPushNotifications(t *testing.T) { customNotificationReceived = true t.Logf("Received custom push notification in PubSub context: %v", notification) return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -336,27 +388,6 @@ func TestPubSubWithGenericPushNotifications(t *testing.T) { } } -func TestPushNotificationMessageType(t *testing.T) { - // Test the PushNotificationMessage type - msg := &redis.PushNotificationMessage{ - Command: "CUSTOM_EVENT", - Args: []interface{}{"arg1", "arg2", 123}, - } - - if msg.Command != "CUSTOM_EVENT" { - t.Errorf("Expected command 'CUSTOM_EVENT', got '%s'", msg.Command) - } - - if len(msg.Args) != 3 { - t.Errorf("Expected 3 args, got %d", len(msg.Args)) - } - - expectedString := "push: CUSTOM_EVENT" - if msg.String() != expectedString { - t.Errorf("Expected string '%s', got '%s'", expectedString, msg.String()) - } -} - func TestPushNotificationRegistryUnregisterHandler(t *testing.T) { // Test unregistering handlers registry := redis.NewPushNotificationRegistry() @@ -368,13 +399,13 @@ func TestPushNotificationRegistryUnregisterHandler(t *testing.T) { return true }) - err := registry.RegisterHandler("TEST_CMD", handler) + err := registry.RegisterHandler("TEST_CMD", handler, false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } // Verify handler is registered - commands := registry.GetRegisteredCommands() + commands := registry.GetRegisteredPushNotificationNames() if len(commands) != 1 || commands[0] != "TEST_CMD" { t.Errorf("Expected ['TEST_CMD'], got %v", commands) } @@ -395,7 +426,7 @@ func TestPushNotificationRegistryUnregisterHandler(t *testing.T) { registry.UnregisterHandler("TEST_CMD") // Verify handler is unregistered - commands = registry.GetRegisteredCommands() + commands = registry.GetRegisteredPushNotificationNames() if len(commands) != 0 { t.Errorf("Expected no registered commands after unregister, got %v", commands) } @@ -459,24 +490,24 @@ func TestPushNotificationRegistryDuplicateHandlerError(t *testing.T) { }) // Register first handler - should succeed - err := registry.RegisterHandler("DUPLICATE_CMD", handler1) + err := registry.RegisterHandler("DUPLICATE_CMD", handler1, false) if err != nil { t.Fatalf("First handler registration should succeed: %v", err) } // Register second handler for same command - should fail - err = registry.RegisterHandler("DUPLICATE_CMD", handler2) + err = registry.RegisterHandler("DUPLICATE_CMD", handler2, false) if err == nil { t.Error("Second handler registration should fail") } - expectedError := "handler already registered for command: DUPLICATE_CMD" + expectedError := "handler already registered for push notification: DUPLICATE_CMD" if err.Error() != expectedError { t.Errorf("Expected error '%s', got '%s'", expectedError, err.Error()) } // Verify only one handler is registered - commands := registry.GetRegisteredCommands() + commands := registry.GetRegisteredPushNotificationNames() if len(commands) != 1 || commands[0] != "DUPLICATE_CMD" { t.Errorf("Expected ['DUPLICATE_CMD'], got %v", commands) } @@ -491,7 +522,7 @@ func TestPushNotificationRegistrySpecificHandlerOnly(t *testing.T) { err := registry.RegisterHandler("SPECIFIC_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool { specificCalled = true return true - })) + }), false) if err != nil { t.Fatalf("Failed to register specific handler: %v", err) } @@ -538,7 +569,7 @@ func TestPushNotificationProcessorEdgeCases(t *testing.T) { processor.RegisterHandler("TEST_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool { handlerCalled = true return true - })) + }), false) // Even with handlers registered, disabled processor shouldn't process ctx := context.Background() @@ -570,7 +601,7 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) { return true }) - err := processor.RegisterHandler("CONV_CMD", handler) + err := processor.RegisterHandler("CONV_CMD", handler, false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } @@ -580,7 +611,7 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) { err = processor.RegisterHandler("FUNC_CMD", newTestHandler(func(ctx context.Context, notification []interface{}) bool { funcHandlerCalled = true return true - })) + }), false) if err != nil { t.Fatalf("Failed to register func handler: %v", err) } @@ -628,14 +659,14 @@ func TestClientPushNotificationEdgeCases(t *testing.T) { // These should not panic even when processor is nil and should return nil error err := client.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Expected nil error when processor is nil, got: %v", err) } err = client.RegisterPushNotificationHandler("TEST_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) if err != nil { t.Errorf("Expected nil error when processor is nil, got: %v", err) } @@ -700,8 +731,8 @@ func TestPushNotificationInfoEdgeCases(t *testing.T) { t.Fatal("Info should not be nil") } - if info.Command != "COMPLEX_CMD" { - t.Errorf("Expected command 'COMPLEX_CMD', got '%s'", info.Command) + if info.Name != "COMPLEX_CMD" { + t.Errorf("Expected command 'COMPLEX_CMD', got '%s'", info.Name) } if len(info.Args) != 4 { @@ -757,7 +788,7 @@ func TestPushNotificationRegistryConcurrency(t *testing.T) { command := fmt.Sprintf("CMD_%d_%d", id, j) registry.RegisterHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) // Handle notification notification := []interface{}{command, "data"} @@ -765,7 +796,7 @@ func TestPushNotificationRegistryConcurrency(t *testing.T) { // Check registry state registry.HasHandlers() - registry.GetRegisteredCommands() + registry.GetRegisteredPushNotificationNames() } }(i) } @@ -780,7 +811,7 @@ func TestPushNotificationRegistryConcurrency(t *testing.T) { t.Error("Registry should have handlers after concurrent operations") } - commands := registry.GetRegisteredCommands() + commands := registry.GetRegisteredPushNotificationNames() if len(commands) == 0 { t.Error("Registry should have registered commands after concurrent operations") } @@ -805,7 +836,7 @@ func TestPushNotificationProcessorConcurrency(t *testing.T) { command := fmt.Sprintf("PROC_CMD_%d_%d", id, j) processor.RegisterHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) // Handle notifications notification := []interface{}{command, "data"} @@ -859,7 +890,7 @@ func TestPushNotificationClientConcurrency(t *testing.T) { command := fmt.Sprintf("CLIENT_CMD_%d_%d", id, j) client.RegisterPushNotificationHandler(command, newTestHandler(func(ctx context.Context, notification []interface{}) bool { return true - })) + }), false) // Access processor processor := client.GetPushNotificationProcessor() @@ -903,7 +934,7 @@ func TestPushNotificationConnectionHealthCheck(t *testing.T) { err := client.RegisterPushNotificationHandler("TEST_CONNCHECK", newTestHandler(func(ctx context.Context, notification []interface{}) bool { t.Logf("Received test notification: %v", notification) return true - })) + }), false) if err != nil { t.Fatalf("Failed to register handler: %v", err) } diff --git a/redis.go b/redis.go index 05f81263..462e7426 100644 --- a/redis.go +++ b/redis.go @@ -824,11 +824,12 @@ func (c *Client) initializePushProcessor() { } } -// RegisterPushNotificationHandler registers a handler for a specific push notification command. -// Returns an error if a handler is already registered for this command. -func (c *Client) RegisterPushNotificationHandler(command string, handler PushNotificationHandler) error { +// RegisterPushNotificationHandler registers a handler for a specific push notification name. +// Returns an error if a handler is already registered for this push notification name. +// If protected is true, the handler cannot be unregistered. +func (c *Client) RegisterPushNotificationHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error { if c.pushProcessor != nil { - return c.pushProcessor.RegisterHandler(command, handler) + return c.pushProcessor.RegisterHandler(pushNotificationName, handler, protected) } return nil } @@ -996,11 +997,12 @@ func (c *Conn) Process(ctx context.Context, cmd Cmder) error { return err } -// RegisterPushNotificationHandler registers a handler for a specific push notification command. -// Returns an error if a handler is already registered for this command. -func (c *Conn) RegisterPushNotificationHandler(command string, handler PushNotificationHandler) error { +// RegisterPushNotificationHandler registers a handler for a specific push notification name. +// Returns an error if a handler is already registered for this push notification name. +// If protected is true, the handler cannot be unregistered. +func (c *Conn) RegisterPushNotificationHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error { if c.pushProcessor != nil { - return c.pushProcessor.RegisterHandler(command, handler) + return c.pushProcessor.RegisterHandler(pushNotificationName, handler, protected) } return nil }