diff --git a/push_notification_coverage_test.go b/push_notification_coverage_test.go index 4a7bfb95..a21413bf 100644 --- a/push_notification_coverage_test.go +++ b/push_notification_coverage_test.go @@ -11,6 +11,18 @@ import ( "github.com/redis/go-redis/v9/internal/proto" ) +// Helper function to access registry for testing +func getRegistryForTestingCoverage(processor PushNotificationProcessorInterface) *PushNotificationRegistry { + switch p := processor.(type) { + case *PushNotificationProcessor: + return p.GetRegistryForTesting() + case *VoidPushNotificationProcessor: + return p.GetRegistryForTesting() + default: + return nil + } +} + // testHandler is a simple implementation of PushNotificationHandler for testing type testHandler struct { handlerFunc func(ctx context.Context, notification []interface{}) bool @@ -154,9 +166,10 @@ func TestConnPushNotificationMethods(t *testing.T) { t.Error("Conn should have push notification processor") } - // Processor should have a registry when enabled - if processor.GetRegistry() == nil { - t.Error("Conn push notification processor should have a registry when enabled") + // Test that processor can handle handlers when enabled + testHandler := processor.GetHandler("TEST") + if testHandler != nil { + t.Error("Should not have handler for TEST initially") } // Test RegisterPushNotificationHandler @@ -183,16 +196,25 @@ func TestConnPushNotificationMethods(t *testing.T) { t.Error("Should get error when registering duplicate handler") } - // Test that handlers work - registry := processor.GetRegistry() + // Test that handlers work using GetHandler ctx := context.Background() - handled := registry.HandleNotification(ctx, []interface{}{"TEST_CONN_HANDLER", "data"}) + connHandler := processor.GetHandler("TEST_CONN_HANDLER") + if connHandler == nil { + t.Error("Should have handler for TEST_CONN_HANDLER after registration") + return + } + handled := connHandler.HandlePushNotification(ctx, []interface{}{"TEST_CONN_HANDLER", "data"}) if !handled { t.Error("Handler should have been called") } - handled = registry.HandleNotification(ctx, []interface{}{"TEST_CONN_FUNC", "data"}) + funcHandler := processor.GetHandler("TEST_CONN_FUNC") + if funcHandler == nil { + t.Error("Should have handler for TEST_CONN_FUNC after registration") + return + } + handled = funcHandler.HandlePushNotification(ctx, []interface{}{"TEST_CONN_FUNC", "data"}) if !handled { t.Error("Handler func should have been called") } @@ -217,9 +239,10 @@ func TestConnWithoutPushNotifications(t *testing.T) { if processor == nil { t.Error("Conn should always have a push notification processor") } - // VoidPushNotificationProcessor should have nil registry - if processor.GetRegistry() != nil { - t.Error("VoidPushNotificationProcessor should have nil registry for RESP2") + // VoidPushNotificationProcessor should return nil for all handlers + handler := processor.GetHandler("TEST") + if handler != nil { + t.Error("VoidPushNotificationProcessor should return nil for all handlers") } // Test RegisterPushNotificationHandler returns nil (no error) @@ -297,10 +320,14 @@ func TestClonedClientPushNotifications(t *testing.T) { t.Error("Cloned client should have same push notification processor") } - // Test that handlers work on cloned client - registry := clonedProcessor.GetRegistry() + // Test that handlers work on cloned client using GetHandler ctx := context.Background() - handled := registry.HandleNotification(ctx, []interface{}{"TEST_CLONE", "data"}) + cloneHandler := clonedProcessor.GetHandler("TEST_CLONE") + if cloneHandler == nil { + t.Error("Cloned client should have TEST_CLONE handler") + return + } + handled := cloneHandler.HandlePushNotification(ctx, []interface{}{"TEST_CLONE", "data"}) if !handled { t.Error("Cloned client should handle notifications") } diff --git a/push_notifications.go b/push_notifications.go index 6777df00..6d75a5c9 100644 --- a/push_notifications.go +++ b/push_notifications.go @@ -112,7 +112,6 @@ func (r *PushNotificationRegistry) GetHandler(pushNotificationName string) PushN // PushNotificationProcessorInterface defines the interface for push notification processors. type PushNotificationProcessorInterface interface { GetHandler(pushNotificationName string) PushNotificationHandler - GetRegistry() *PushNotificationRegistry // For backward compatibility and testing ProcessPendingNotifications(ctx context.Context, rd *proto.Reader) error RegisterHandler(pushNotificationName string, handler PushNotificationHandler, protected bool) error } @@ -135,9 +134,9 @@ func (p *PushNotificationProcessor) GetHandler(pushNotificationName string) Push return p.registry.GetHandler(pushNotificationName) } -// GetRegistry returns the push notification registry for internal use. -// This method is primarily for testing and internal operations. -func (p *PushNotificationProcessor) GetRegistry() *PushNotificationRegistry { +// GetRegistryForTesting returns the push notification registry for testing. +// This method should only be used by tests. +func (p *PushNotificationProcessor) GetRegistryForTesting() *PushNotificationRegistry { return p.registry } @@ -248,8 +247,9 @@ func (v *VoidPushNotificationProcessor) GetHandler(pushNotificationName string) return nil } -// GetRegistry returns nil for void processor since it doesn't maintain handlers. -func (v *VoidPushNotificationProcessor) GetRegistry() *PushNotificationRegistry { +// GetRegistryForTesting returns nil for void processor since it doesn't maintain handlers. +// This method should only be used by tests. +func (v *VoidPushNotificationProcessor) GetRegistryForTesting() *PushNotificationRegistry { return nil } diff --git a/push_notifications_test.go b/push_notifications_test.go index 492c2734..d777eafb 100644 --- a/push_notifications_test.go +++ b/push_notifications_test.go @@ -9,6 +9,18 @@ import ( "github.com/redis/go-redis/v9/internal/pool" ) +// Helper function to access registry for testing +func getRegistryForTesting(processor redis.PushNotificationProcessorInterface) *redis.PushNotificationRegistry { + switch p := processor.(type) { + case *redis.PushNotificationProcessor: + return p.GetRegistryForTesting() + case *redis.VoidPushNotificationProcessor: + return p.GetRegistryForTesting() + default: + return nil + } +} + // testHandler is a simple implementation of PushNotificationHandler for testing type testHandler struct { handlerFunc func(ctx context.Context, notification []interface{}) bool @@ -84,8 +96,10 @@ func TestPushNotificationProcessor(t *testing.T) { // Test the push notification processor processor := redis.NewPushNotificationProcessor() - if processor.GetRegistry() == nil { - t.Error("Processor should have a registry") + // Test that we can get a handler (should be nil since none registered yet) + handler := processor.GetHandler("TEST") + if handler != nil { + t.Error("Should not have handler for TEST initially") } // Test registering handlers @@ -106,10 +120,15 @@ func TestPushNotificationProcessor(t *testing.T) { t.Fatalf("Failed to register handler: %v", err) } - // Simulate handling a notification + // Simulate handling a notification using GetHandler ctx := context.Background() notification := []interface{}{"CUSTOM_NOTIFICATION", "data"} - handled := processor.GetRegistry().HandleNotification(ctx, notification) + customHandler := processor.GetHandler("CUSTOM_NOTIFICATION") + if customHandler == nil { + t.Error("Should have handler for CUSTOM_NOTIFICATION after registration") + return + } + handled := customHandler.HandlePushNotification(ctx, notification) if !handled { t.Error("Notification should have been handled") @@ -119,9 +138,10 @@ func TestPushNotificationProcessor(t *testing.T) { t.Error("Specific handler should have been called") } - // Test that processor always has a registry (no enable/disable anymore) - if processor.GetRegistry() == nil { - t.Error("Processor should always have a registry") + // Test that processor can retrieve handlers (no enable/disable anymore) + retrievedHandler := processor.GetHandler("CUSTOM_NOTIFICATION") + if retrievedHandler == nil { + t.Error("Should be able to retrieve registered handler") } } @@ -140,7 +160,7 @@ func TestClientPushNotificationIntegration(t *testing.T) { t.Error("Push notification processor should be initialized") } - if processor.GetRegistry() == nil { + if getRegistryForTesting(processor) == nil { t.Error("Push notification processor should have a registry when enabled") } @@ -157,7 +177,7 @@ func TestClientPushNotificationIntegration(t *testing.T) { // Simulate notification handling ctx := context.Background() notification := []interface{}{"CUSTOM_EVENT", "test_data"} - handled := processor.GetRegistry().HandleNotification(ctx, notification) + handled := getRegistryForTesting(processor).HandleNotification(ctx, notification) if !handled { t.Error("Notification should have been handled") @@ -183,7 +203,7 @@ func TestClientWithoutPushNotifications(t *testing.T) { t.Error("Push notification processor should never be nil") } // VoidPushNotificationProcessor should have nil registry - if processor.GetRegistry() != nil { + if getRegistryForTesting(processor) != nil { t.Error("VoidPushNotificationProcessor should have nil registry") } @@ -211,8 +231,9 @@ func TestPushNotificationEnabledClient(t *testing.T) { t.Error("Push notification processor should be initialized when enabled") } - if processor.GetRegistry() == nil { - t.Error("Push notification processor should have a registry when enabled") + registry := getRegistryForTesting(processor) + if registry == nil { + t.Errorf("Push notification processor should have a registry when enabled. Processor type: %T", processor) } // Test registering a handler @@ -226,7 +247,6 @@ func TestPushNotificationEnabledClient(t *testing.T) { } // Test that the handler works - registry := processor.GetRegistry() ctx := context.Background() notification := []interface{}{"TEST_NOTIFICATION", "data"} handled := registry.HandleNotification(ctx, notification) @@ -375,7 +395,7 @@ func TestPubSubWithGenericPushNotifications(t *testing.T) { // Test that the processor can handle notifications notification := []interface{}{"CUSTOM_PUBSUB_EVENT", "arg1", "arg2"} - handled := processor.GetRegistry().HandleNotification(context.Background(), notification) + handled := getRegistryForTesting(processor).HandleNotification(context.Background(), notification) if !handled { t.Error("Push notification should have been handled") @@ -559,7 +579,7 @@ func TestPushNotificationProcessorEdgeCases(t *testing.T) { // Test processor with disabled state processor := redis.NewPushNotificationProcessor() - if processor.GetRegistry() == nil { + if getRegistryForTesting(processor) == nil { t.Error("Processor should have a registry") } @@ -573,7 +593,7 @@ func TestPushNotificationProcessorEdgeCases(t *testing.T) { // Even with handlers registered, disabled processor shouldn't process ctx := context.Background() notification := []interface{}{"TEST_CMD", "data"} - handled := processor.GetRegistry().HandleNotification(ctx, notification) + handled := getRegistryForTesting(processor).HandleNotification(ctx, notification) if !handled { t.Error("Registry should still handle notifications even when processor is disabled") @@ -584,7 +604,7 @@ func TestPushNotificationProcessorEdgeCases(t *testing.T) { } // Test that processor always has a registry - if processor.GetRegistry() == nil { + if getRegistryForTesting(processor) == nil { t.Error("Processor should always have a registry") } } @@ -619,7 +639,7 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) { // Test specific handler notification := []interface{}{"CONV_CMD", "data"} - handled := processor.GetRegistry().HandleNotification(ctx, notification) + handled := getRegistryForTesting(processor).HandleNotification(ctx, notification) if !handled { t.Error("Notification should be handled") @@ -635,7 +655,7 @@ func TestPushNotificationProcessorConvenienceMethods(t *testing.T) { // Test func handler notification = []interface{}{"FUNC_CMD", "data"} - handled = processor.GetRegistry().HandleNotification(ctx, notification) + handled = getRegistryForTesting(processor).HandleNotification(ctx, notification) if !handled { t.Error("Notification should be handled") @@ -676,7 +696,7 @@ func TestClientPushNotificationEdgeCases(t *testing.T) { t.Error("Processor should never be nil") } // VoidPushNotificationProcessor should have nil registry - if processor.GetRegistry() != nil { + if getRegistryForTesting(processor) != nil { t.Error("VoidPushNotificationProcessor should have nil registry when disabled") } } @@ -838,10 +858,10 @@ func TestPushNotificationProcessorConcurrency(t *testing.T) { // Handle notifications notification := []interface{}{command, "data"} - processor.GetRegistry().HandleNotification(context.Background(), notification) + getRegistryForTesting(processor).HandleNotification(context.Background(), notification) // Access processor state - processor.GetRegistry() + getRegistryForTesting(processor) } }(i) } @@ -852,7 +872,7 @@ func TestPushNotificationProcessorConcurrency(t *testing.T) { } // Verify processor is still functional - registry := processor.GetRegistry() + registry := getRegistryForTesting(processor) if registry == nil { t.Error("Processor registry should not be nil after concurrent operations") } @@ -887,7 +907,7 @@ func TestPushNotificationClientConcurrency(t *testing.T) { // Access processor processor := client.GetPushNotificationProcessor() if processor != nil { - processor.GetRegistry() + getRegistryForTesting(processor) } } }(i) @@ -921,7 +941,7 @@ func TestPushNotificationConnectionHealthCheck(t *testing.T) { if processor == nil { t.Fatal("Push notification processor should not be nil") } - if processor.GetRegistry() == nil { + if getRegistryForTesting(processor) == nil { t.Fatal("Push notification registry should not be nil when enabled") }