Skip to content

Commit 0c5da0e

Browse files
committed
refactor: simplify push notification interface
- 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
1 parent 3733bad commit 0c5da0e

File tree

4 files changed

+89
-142
lines changed

4 files changed

+89
-142
lines changed

push_notification_coverage_test.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,20 @@ import (
1111
"github.com/redis/go-redis/v9/internal/proto"
1212
)
1313

14+
// testHandler is a simple implementation of PushNotificationHandler for testing
15+
type testHandler struct {
16+
handlerFunc func(ctx context.Context, notification []interface{}) bool
17+
}
18+
19+
func (h *testHandler) HandlePushNotification(ctx context.Context, notification []interface{}) bool {
20+
return h.handlerFunc(ctx, notification)
21+
}
22+
23+
// newTestHandler creates a test handler from a function
24+
func newTestHandler(f func(ctx context.Context, notification []interface{}) bool) *testHandler {
25+
return &testHandler{handlerFunc: f}
26+
}
27+
1428
// TestConnectionPoolPushNotificationIntegration tests the connection pool's
1529
// integration with push notifications for 100% coverage.
1630
func TestConnectionPoolPushNotificationIntegration(t *testing.T) {
@@ -102,9 +116,9 @@ func TestConnectionHealthCheckWithPushNotifications(t *testing.T) {
102116
defer client.Close()
103117

104118
// Register a handler to ensure processor is active
105-
err := client.RegisterPushNotificationHandlerFunc("TEST_HEALTH", func(ctx context.Context, notification []interface{}) bool {
119+
err := client.RegisterPushNotificationHandler("TEST_HEALTH", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
106120
return true
107-
})
121+
}))
108122
if err != nil {
109123
t.Fatalf("Failed to register handler: %v", err)
110124
}
@@ -147,7 +161,7 @@ func TestConnPushNotificationMethods(t *testing.T) {
147161
}
148162

149163
// Test RegisterPushNotificationHandler
150-
handler := PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
164+
handler := newTestHandler(func(ctx context.Context, notification []interface{}) bool {
151165
return true
152166
})
153167

@@ -156,10 +170,10 @@ func TestConnPushNotificationMethods(t *testing.T) {
156170
t.Errorf("Failed to register handler on Conn: %v", err)
157171
}
158172

159-
// Test RegisterPushNotificationHandlerFunc
160-
err = conn.RegisterPushNotificationHandlerFunc("TEST_CONN_FUNC", func(ctx context.Context, notification []interface{}) bool {
173+
// Test RegisterPushNotificationHandler with function wrapper
174+
err = conn.RegisterPushNotificationHandler("TEST_CONN_FUNC", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
161175
return true
162-
})
176+
}))
163177
if err != nil {
164178
t.Errorf("Failed to register handler func on Conn: %v", err)
165179
}
@@ -206,17 +220,17 @@ func TestConnWithoutPushNotifications(t *testing.T) {
206220
}
207221

208222
// Test RegisterPushNotificationHandler returns nil (no error)
209-
err := conn.RegisterPushNotificationHandler("TEST", PushNotificationHandlerFunc(func(ctx context.Context, notification []interface{}) bool {
223+
err := conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
210224
return true
211225
}))
212226
if err != nil {
213227
t.Errorf("Should return nil error when no processor: %v", err)
214228
}
215229

216-
// Test RegisterPushNotificationHandlerFunc returns nil (no error)
217-
err = conn.RegisterPushNotificationHandlerFunc("TEST", func(ctx context.Context, notification []interface{}) bool {
230+
// Test RegisterPushNotificationHandler returns nil (no error)
231+
err = conn.RegisterPushNotificationHandler("TEST", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
218232
return true
219-
})
233+
}))
220234
if err != nil {
221235
t.Errorf("Should return nil error when no processor: %v", err)
222236
}
@@ -263,9 +277,9 @@ func TestClonedClientPushNotifications(t *testing.T) {
263277
}
264278

265279
// Register handler on original
266-
err := client.RegisterPushNotificationHandlerFunc("TEST_CLONE", func(ctx context.Context, notification []interface{}) bool {
280+
err := client.RegisterPushNotificationHandler("TEST_CLONE", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
267281
return true
268-
})
282+
}))
269283
if err != nil {
270284
t.Fatalf("Failed to register handler: %v", err)
271285
}
@@ -289,9 +303,9 @@ func TestClonedClientPushNotifications(t *testing.T) {
289303
}
290304

291305
// Test registering new handler on cloned client
292-
err = clonedClient.RegisterPushNotificationHandlerFunc("TEST_CLONE_NEW", func(ctx context.Context, notification []interface{}) bool {
306+
err = clonedClient.RegisterPushNotificationHandler("TEST_CLONE_NEW", newTestHandler(func(ctx context.Context, notification []interface{}) bool {
293307
return true
294-
})
308+
}))
295309
if err != nil {
296310
t.Errorf("Failed to register handler on cloned client: %v", err)
297311
}

push_notifications.go

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ type PushNotificationHandler interface {
1616
HandlePushNotification(ctx context.Context, notification []interface{}) bool
1717
}
1818

19-
// PushNotificationHandlerFunc is a function adapter for PushNotificationHandler.
20-
type PushNotificationHandlerFunc func(ctx context.Context, notification []interface{}) bool
21-
22-
// HandlePushNotification implements PushNotificationHandler.
23-
func (f PushNotificationHandlerFunc) HandlePushNotification(ctx context.Context, notification []interface{}) bool {
24-
return f(ctx, notification)
25-
}
26-
2719
// PushNotificationRegistry manages handlers for different types of push notifications.
2820
type PushNotificationRegistry struct {
2921
mu sync.RWMutex
@@ -185,42 +177,13 @@ func (p *PushNotificationProcessor) RegisterHandler(command string, handler Push
185177
return p.registry.RegisterHandler(command, handler)
186178
}
187179

188-
// RegisterHandlerFunc is a convenience method to register a function as a handler.
189-
// Returns an error if a handler is already registered for this command.
190-
func (p *PushNotificationProcessor) RegisterHandlerFunc(command string, handlerFunc func(ctx context.Context, notification []interface{}) bool) error {
191-
return p.registry.RegisterHandler(command, PushNotificationHandlerFunc(handlerFunc))
192-
}
193-
194-
// Common push notification commands
180+
// Redis Cluster push notification commands
195181
const (
196-
// Redis Cluster notifications
197182
PushNotificationMoving = "MOVING"
198183
PushNotificationMigrating = "MIGRATING"
199184
PushNotificationMigrated = "MIGRATED"
200185
PushNotificationFailingOver = "FAILING_OVER"
201186
PushNotificationFailedOver = "FAILED_OVER"
202-
203-
// Redis Pub/Sub notifications
204-
PushNotificationPubSubMessage = "message"
205-
PushNotificationPMessage = "pmessage"
206-
PushNotificationSubscribe = "subscribe"
207-
PushNotificationUnsubscribe = "unsubscribe"
208-
PushNotificationPSubscribe = "psubscribe"
209-
PushNotificationPUnsubscribe = "punsubscribe"
210-
211-
// Redis Stream notifications
212-
PushNotificationXRead = "xread"
213-
PushNotificationXReadGroup = "xreadgroup"
214-
215-
// Redis Keyspace notifications
216-
PushNotificationKeyspace = "keyspace"
217-
PushNotificationKeyevent = "keyevent"
218-
219-
// Redis Module notifications
220-
PushNotificationModule = "module"
221-
222-
// Custom application notifications
223-
PushNotificationCustom = "custom"
224187
)
225188

226189
// PushNotificationInfo contains metadata about a push notification.

0 commit comments

Comments
 (0)