Issue 511 - Fix review comments

pkg/plugin/implementation/registry/cmd/plugin.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strconv"
 	"time"
 
@@ -14,39 +15,61 @@ import (
 // registryProvider implements the RegistryLookupProvider interface for the registry plugin.
 type registryProvider struct{}
 
+// newRegistryFunc is a function type that creates a new Registry instance.
+var newRegistryFunc = registry.New
+
+// parseConfig parses the configuration map and returns a registry.Config with optional parameters.
+func (r registryProvider) parseConfig(config map[string]string) (*registry.Config, error) {
+	registryConfig := &registry.Config{
+		URL: config["url"],
+	}
+
+	// Parse retry_max
+	if retryMaxStr, exists := config["retry_max"]; exists && retryMaxStr != "" {
+		retryMax, err := strconv.Atoi(retryMaxStr)
+		if err != nil {
+			return nil, fmt.Errorf("invalid retry_max value '%s': %w", retryMaxStr, err)
+		}
+		registryConfig.RetryMax = retryMax
+	}
+
+	// Parse retry_wait_min
+	if retryWaitMinStr, exists := config["retry_wait_min"]; exists && retryWaitMinStr != "" {
+		retryWaitMin, err := time.ParseDuration(retryWaitMinStr)
+		if err != nil {
+			return nil, fmt.Errorf("invalid retry_wait_min value '%s': %w", retryWaitMinStr, err)
+		}
+		registryConfig.RetryWaitMin = retryWaitMin
+	}
+
+	// Parse retry_wait_max
+	if retryWaitMaxStr, exists := config["retry_wait_max"]; exists && retryWaitMaxStr != "" {
+		retryWaitMax, err := time.ParseDuration(retryWaitMaxStr)
+		if err != nil {
+			return nil, fmt.Errorf("invalid retry_wait_max value '%s': %w", retryWaitMaxStr, err)
+		}
+		registryConfig.RetryWaitMax = retryWaitMax
+	}
+
+	return registryConfig, nil
+}
+
 // New creates a new registry plugin instance.
 func (r registryProvider) New(ctx context.Context, config map[string]string) (definition.RegistryLookup, func() error, error) {
 	if ctx == nil {
 		return nil, nil, errors.New("context cannot be nil")
 	}
 
-	// Parse configuration from map
-	registryConfig := &registry.Config{
-		URL: config["url"],
-	}
-
-	// Parse optional retry settings
-	if retryMaxStr, exists := config["retry_max"]; exists && retryMaxStr != "" {
-		if retryMax, err := strconv.Atoi(retryMaxStr); err == nil {
-			registryConfig.RetryMax = retryMax
-		}
-	}
-
-	if retryWaitMinStr, exists := config["retry_wait_min"]; exists && retryWaitMinStr != "" {
-		if retryWaitMin, err := time.ParseDuration(retryWaitMinStr); err == nil {
-			registryConfig.RetryWaitMin = retryWaitMin
-		}
-	}
-
-	if retryWaitMaxStr, exists := config["retry_wait_max"]; exists && retryWaitMaxStr != "" {
-		if retryWaitMax, err := time.ParseDuration(retryWaitMaxStr); err == nil {
-			registryConfig.RetryWaitMax = retryWaitMax
-		}
+	// Parse configuration from map using the dedicated method
+	registryConfig, err := r.parseConfig(config)
+	if err != nil {
+		log.Errorf(ctx, err, "Failed to parse registry configuration")
+		return nil, nil, fmt.Errorf("failed to parse registry configuration: %w", err)
 	}
 
 	log.Debugf(ctx, "Registry config mapped: %+v", registryConfig)
 
-	registryClient, closer, err := registry.New(ctx, registryConfig)
+	registryClient, closer, err := newRegistryFunc(ctx, registryConfig)
 	if err != nil {
 		log.Errorf(ctx, err, "Failed to create registry instance")
 		return nil, nil, err

pkg/plugin/implementation/registry/cmd/plugin_test.go

@@ -145,7 +145,6 @@ func TestRegistryProvider_IntegrationTest(t *testing.T) {
 	require.NotNil(t, closer)
 	defer closer()
 
-	// Test Subscribe
 	subscription := &model.Subscription{
 		Subscriber: model.Subscriber{
 			SubscriberID: "test-subscriber",
@@ -161,9 +160,6 @@ func TestRegistryProvider_IntegrationTest(t *testing.T) {
 		Status:           "SUBSCRIBED",
 	}
 
-	err = registry.Subscribe(ctx, subscription)
-	require.NoError(t, err)
-
 	// Test Lookup
 	results, err := registry.Lookup(ctx, subscription)
 	require.NoError(t, err)
@@ -249,18 +245,6 @@ func TestRegistryProvider_ConfigurationParsing(t *testing.T) {
 			require.NotNil(t, closer)
 			defer closer()
 
-			// The registry should work regardless of invalid config values
-			subscription := &model.Subscription{
-				KeyID:            "test-key",
-				SigningPublicKey: "test-signing-key",
-				EncrPublicKey:    "test-encryption-key",
-				ValidFrom:        time.Now(),
-				ValidUntil:       time.Now().Add(24 * time.Hour),
-				Status:           "SUBSCRIBED",
-			}
-
-			err = registry.Subscribe(ctx, subscription)
-			assert.NoError(t, err)
 		})
 	}
 }