Skip to content

Commit 2243188

Browse files
committed
feat: add protected handler support and rename command to pushNotificationName
- Add protected flag to RegisterHandler methods across all types - Protected handlers cannot be unregistered, UnregisterHandler returns error - Rename 'command' parameter to 'pushNotificationName' for clarity - Update PushNotificationInfo.Command field to Name field - Add comprehensive test for protected handler functionality - Update all existing tests to use new protected parameter (false by default) - Improve error messages to use 'push notification' terminology Benefits: - Critical handlers can be protected from accidental unregistration - Clearer naming reflects that these are notification names, not commands - Better error handling with informative error messages - Backward compatible (existing handlers work with protected=false)
1 parent 550762f commit 2243188

File tree

4 files changed

+154
-109
lines changed

4 files changed

+154
-109
lines changed

push_notification_coverage_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func TestConnectionHealthCheckWithPushNotifications(t *testing.T) {
118118
// Register a handler to ensure processor is active
119119
err := client.RegisterPushNotificationHandler("TEST_HEALTH", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
120120
return true
121-
}))
121+
}), false)
122122
if err != nil {
123123
t.Fatalf("Failed to register handler: %v", err)
124124
}
@@ -165,21 +165,21 @@ func TestConnPushNotificationMethods(t *testing.T) {
165165
return true
166166
})
167167

168-
err := conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler)
168+
err := conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler, false)
169169
if err != nil {
170170
t.Errorf("Failed to register handler on Conn: %v", err)
171171
}
172172

173173
// Test RegisterPushNotificationHandler with function wrapper
174174
err = conn.RegisterPushNotificationHandler("TEST_CONN_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
175175
return true
176-
}))
176+
}), false)
177177
if err != nil {
178178
t.Errorf("Failed to register handler func on Conn: %v", err)
179179
}
180180

181181
// Test duplicate handler error
182-
err = conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler)
182+
err = conn.RegisterPushNotificationHandler("TEST_CONN_HANDLER", handler, false)
183183
if err == nil {
184184
t.Error("Should get error when registering duplicate handler")
185185
}
@@ -222,15 +222,15 @@ func TestConnWithoutPushNotifications(t *testing.T) {
222222
// Test RegisterPushNotificationHandler returns nil (no error)
223223
err := conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
224224
return true
225-
}))
225+
}), false)
226226
if err != nil {
227227
t.Errorf("Should return nil error when no processor: %v", err)
228228
}
229229

230230
// Test RegisterPushNotificationHandler returns nil (no error)
231231
err = conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
232232
return true
233-
}))
233+
}), false)
234234
if err != nil {
235235
t.Errorf("Should return nil error when no processor: %v", err)
236236
}
@@ -279,7 +279,7 @@ func TestClonedClientPushNotifications(t *testing.T) {
279279
// Register handler on original
280280
err := client.RegisterPushNotificationHandler("TEST_CLONE", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
281281
return true
282-
}))
282+
}), false)
283283
if err != nil {
284284
t.Fatalf("Failed to register handler: %v", err)
285285
}
@@ -305,7 +305,7 @@ func TestClonedClientPushNotifications(t *testing.T) {
305305
// Test registering new handler on cloned client
306306
err = clonedClient.RegisterPushNotificationHandler("TEST_CLONE_NEW", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
307307
return true
308-
}))
308+
}), false)
309309
if err != nil {
310310
t.Errorf("Failed to register handler on cloned client: %v", err)
311311
}
@@ -350,22 +350,22 @@ func TestPushNotificationInfoStructure(t *testing.T) {
350350
t.Run(tc.name, func(t *testing.T) {
351351
info := ParsePushNotificationInfo(tc.notification)
352352

353-
if info.Command != tc.expectedCmd {
354-
t.Errorf("Expected command %s, got %s", tc.expectedCmd, info.Command)
353+
if info.Name != tc.expectedCmd {
354+
t.Errorf("Expected name %s, got %s", tc.expectedCmd, info.Name)
355355
}
356356

357357
if len(info.Args) != tc.expectedArgs {
358358
t.Errorf("Expected %d args, got %d", tc.expectedArgs, len(info.Args))
359359
}
360360

361-
// Verify no unused fields exist by checking the struct only has Command and Args
361+
// Verify no unused fields exist by checking the struct only has Name and Args
362362
// This is a compile-time check - if unused fields were added back, this would fail
363363
_ = struct {
364-
Command string
365-
Args []interface{}
364+
Name string
365+
Args []interface{}
366366
}{
367-
Command: info.Command,
368-
Args: info.Args,
367+
Name: info.Name,
368+
Args: info.Args,
369369
}
370370
})
371371
}

push_notifications.go

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,36 +18,47 @@ type PushNotificationHandler interface {
1818

1919
// PushNotificationRegistry manages handlers for different types of push notifications.
2020
type PushNotificationRegistry struct {
21-
mu sync.RWMutex
22-
handlers map[string]PushNotificationHandler // command -> single handler
21+
mu sync.RWMutex
22+
handlers map[string]PushNotificationHandler // pushNotificationName -> single handler
23+
protected map[string]bool // pushNotificationName -> protected flag
2324
}
2425

2526
// NewPushNotificationRegistry creates a new push notification registry.
2627
func NewPushNotificationRegistry() *PushNotificationRegistry {
2728
return &PushNotificationRegistry{
28-
handlers: make(map[string]PushNotificationHandler),
29+
handlers: make(map[string]PushNotificationHandler),
30+
protected: make(map[string]bool),
2931
}
3032
}
3133

32-
// RegisterHandler registers a handler for a specific push notification command.
33-
// Returns an error if a handler is already registered for this command.
34-
func (r *PushNotificationRegistry) RegisterHandler(command string, handler PushNotificationHandler) error {
34+
// RegisterHandler registers a handler for a specific push notification name.
35+
// Returns an error if a handler is already registered for this push notification name.
36+
// If protected is true, the handler cannot be unregistered.
37+
func (r *PushNotificationRegistry) RegisterHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error {
3538
r.mu.Lock()
3639
defer r.mu.Unlock()
3740

38-
if _, exists := r.handlers[command]; exists {
39-
return fmt.Errorf("handler already registered for command: %s", command)
41+
if _, exists := r.handlers[pushNotificationName]; exists {
42+
return fmt.Errorf("handler already registered for push notification: %s", pushNotificationName)
4043
}
41-
r.handlers[command] = handler
44+
r.handlers[pushNotificationName] = handler
45+
r.protected[pushNotificationName] = protected
4246
return nil
4347
}
4448

45-
// UnregisterHandler removes the handler for a specific push notification command.
46-
func (r *PushNotificationRegistry) UnregisterHandler(command string) {
49+
// UnregisterHandler removes the handler for a specific push notification name.
50+
// Returns an error if the handler is protected.
51+
func (r *PushNotificationRegistry) UnregisterHandler(pushNotificationName string) error {
4752
r.mu.Lock()
4853
defer r.mu.Unlock()
4954

50-
delete(r.handlers, command)
55+
if r.protected[pushNotificationName] {
56+
return fmt.Errorf("cannot unregister protected handler for push notification: %s", pushNotificationName)
57+
}
58+
59+
delete(r.handlers, pushNotificationName)
60+
delete(r.protected, pushNotificationName)
61+
return nil
5162
}
5263

5364
// HandleNotification processes a push notification by calling the registered handler.
@@ -56,8 +67,8 @@ func (r *PushNotificationRegistry) HandleNotification(ctx context.Context, notif
5667
return false
5768
}
5869

59-
// Extract command from notification
60-
command, ok := notification[0].(string)
70+
// Extract push notification name from notification
71+
pushNotificationName, ok := notification[0].(string)
6172
if !ok {
6273
return false
6374
}
@@ -66,23 +77,23 @@ func (r *PushNotificationRegistry) HandleNotification(ctx context.Context, notif
6677
defer r.mu.RUnlock()
6778

6879
// Call specific handler
69-
if handler, exists := r.handlers[command]; exists {
80+
if handler, exists := r.handlers[pushNotificationName]; exists {
7081
return handler.HandlePushNotification(ctx, notification)
7182
}
7283

7384
return false
7485
}
7586

76-
// GetRegisteredCommands returns a list of commands that have registered handlers.
77-
func (r *PushNotificationRegistry) GetRegisteredCommands() []string {
87+
// GetRegisteredPushNotificationNames returns a list of push notification names that have registered handlers.
88+
func (r *PushNotificationRegistry) GetRegisteredPushNotificationNames() []string {
7889
r.mu.RLock()
7990
defer r.mu.RUnlock()
8091

81-
commands := make([]string, 0, len(r.handlers))
82-
for command := range r.handlers {
83-
commands = append(commands, command)
92+
names := make([]string, 0, len(r.handlers))
93+
for name := range r.handlers {
94+
names = append(names, name)
8495
}
85-
return commands
96+
return names
8697
}
8798

8899
// HasHandlers returns true if there are any handlers registered.
@@ -176,13 +187,14 @@ func (p *PushNotificationProcessor) ProcessPendingNotifications(ctx context.Cont
176187
return nil
177188
}
178189

179-
// RegisterHandler is a convenience method to register a handler for a specific command.
180-
// Returns an error if a handler is already registered for this command.
181-
func (p *PushNotificationProcessor) RegisterHandler(command string, handler PushNotificationHandler) error {
182-
return p.registry.RegisterHandler(command, handler)
190+
// RegisterHandler is a convenience method to register a handler for a specific push notification name.
191+
// Returns an error if a handler is already registered for this push notification name.
192+
// If protected is true, the handler cannot be unregistered.
193+
func (p *PushNotificationProcessor) RegisterHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error {
194+
return p.registry.RegisterHandler(pushNotificationName, handler, protected)
183195
}
184196

185-
// Redis Cluster push notification commands
197+
// Redis Cluster push notification names
186198
const (
187199
PushNotificationMoving = "MOVING"
188200
PushNotificationMigrating = "MIGRATING"
@@ -193,8 +205,8 @@ const (
193205

194206
// PushNotificationInfo contains metadata about a push notification.
195207
type PushNotificationInfo struct {
196-
Command string
197-
Args []interface{}
208+
Name string
209+
Args []interface{}
198210
}
199211

200212
// ParsePushNotificationInfo extracts information from a push notification.
@@ -203,14 +215,14 @@ func ParsePushNotificationInfo(notification []interface{}) *PushNotificationInfo
203215
return nil
204216
}
205217

206-
command, ok := notification[0].(string)
218+
name, ok := notification[0].(string)
207219
if !ok {
208220
return nil
209221
}
210222

211223
return &PushNotificationInfo{
212-
Command: command,
213-
Args: notification[1:],
224+
Name: name,
225+
Args: notification[1:],
214226
}
215227
}
216228

@@ -219,5 +231,5 @@ func (info *PushNotificationInfo) String() string {
219231
if info == nil {
220232
return "<nil>"
221233
}
222-
return info.Command
234+
return info.Name
223235
}

0 commit comments

Comments
 (0)