Merge branch 'beckn-onix-v1.0-develop' of https://github.com/beckn/beckn-onix into fix/publisher

pkg/plugin/implementation/reqpreprocessor/cmd/plugin.go

@@ -3,7 +3,6 @@ package main
 import (
 	"context"
 	"net/http"
-	"strings"
 
 	"github.com/beckn/beckn-onix/pkg/plugin/implementation/reqpreprocessor"
 )
@@ -12,9 +11,6 @@ type provider struct{}
 
 func (p provider) New(ctx context.Context, c map[string]string) (func(http.Handler) http.Handler, error) {
 	config := &reqpreprocessor.Config{}
-	if contextKeysStr, ok := c["contextKeys"]; ok {
-		config.ContextKeys = strings.Split(contextKeysStr, ",")
-	}
 	if role, ok := c["role"]; ok {
 		config.Role = role
 	}

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

@@ -32,9 +32,9 @@ func TestProviderNew(t *testing.T) {
 			},
 		},
 		{
-			name: "With Check Keys",
+			name: "Success with BPP role",
 			config: map[string]string{
-				"contextKeys": "message_id,transaction_id",
+				"role": "bpp",
 			},
 			expectedError:  false,
 			expectedStatus: http.StatusOK,
@@ -42,6 +42,29 @@ func TestProviderNew(t *testing.T) {
 				// Add headers matching the check keys.
 				req.Header.Set("context", "test-context")
 				req.Header.Set("transaction_id", "test-transaction")
+				req.Header.Set("bpp_id", "bpp-456")
+			},
+		},
+		{
+			name:   "Missing role configuration",
+			config: map[string]string{
+				// No role specified
+			},
+			expectedError: true,
+			prepareRequest: func(req *http.Request) {
+				req.Header.Set("context", "test-context")
+				req.Header.Set("transaction_id", "test-transaction")
+			},
+		},
+		{
+			name: "Invalid role configuration",
+			config: map[string]string{
+				"role": "invalid-role",
+			},
+			expectedError: true,
+			prepareRequest: func(req *http.Request) {
+				req.Header.Set("context", "test-context")
+				req.Header.Set("transaction_id", "test-transaction")
 			},
 		},
 	}

pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go

@@ -10,103 +10,70 @@ import (
 	"net/http"
 
 	"github.com/beckn/beckn-onix/pkg/log"
-	"github.com/google/uuid"
 )
 
-// Config holds the configuration settings for the application.
 type Config struct {
-	ContextKeys []string // ContextKeys is a list of context keys used for request processing.
-	Role        string   // Role specifies the role of the entity (e.g., subscriber, gateway).
+	Role string
 }
 
-type becknRequest struct {
-	Context map[string]any `json:"context"`
-}
+type keyType string
 
-const contextKey = "context"
-const subscriberIDKey = "subscriber_id"
+const (
+	contextKey      keyType = "context"
+	subscriberIDKey keyType = "subscriber_id"
+)
 
-// NewPreProcessor creates a middleware that processes incoming HTTP requests by extracting
-// and modifying the request context based on the provided configuration.
 func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) {
 	if err := validateConfig(cfg); err != nil {
 		return nil, err
 	}
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			body, _ := io.ReadAll(r.Body)
-			var req becknRequest
+			body, err := io.ReadAll(r.Body)
+			if err != nil {
+				http.Error(w, "Failed to read request body", http.StatusBadRequest)
+				return
+			}
+			var req map[string]interface{}
 			ctx := r.Context()
 			if err := json.Unmarshal(body, &req); err != nil {
 				http.Error(w, "Failed to decode request body", http.StatusBadRequest)
 				return
 			}
-			if req.Context == nil {
-				http.Error(w, fmt.Sprintf("%s field not found.", contextKey), http.StatusBadRequest)
+
+			// Extract context from request
+			reqContext, ok := req["context"].(map[string]interface{})
+			if !ok {
+				http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKey), http.StatusBadRequest)
 				return
 			}
 			var subID any
 			switch cfg.Role {
 			case "bap":
-				subID = req.Context["bap_id"]
+				subID = reqContext["bap_id"]
 			case "bpp":
-				subID = req.Context["bpp_id"]
+				subID = reqContext["bpp_id"]
 			}
 			if subID != nil {
 				log.Debugf(ctx, "adding subscriberId to request:%s, %v", subscriberIDKey, subID)
-				// TODO: Add a ContextKey type in model and use it here instead of raw context key.
 				ctx = context.WithValue(ctx, subscriberIDKey, subID)
 			}
-			for _, key := range cfg.ContextKeys {
-				value := uuid.NewString()
-				updatedValue := update(req.Context, key, value)
-				ctx = context.WithValue(ctx, key, updatedValue)
-			}
-			reqData := map[string]any{"context": req.Context}
-			updatedBody, _ := json.Marshal(reqData)
-			r.Body = io.NopCloser(bytes.NewBuffer(updatedBody))
-			r.ContentLength = int64(len(updatedBody))
+
+			r.Body = io.NopCloser(bytes.NewBuffer(body))
+			r.ContentLength = int64(len(body))
 			r = r.WithContext(ctx)
 			next.ServeHTTP(w, r)
 		})
 	}, nil
 }
 
-func update(wrapper map[string]any, key string, value any) any {
-	field, exists := wrapper[key]
-	if !exists || isEmpty(field) {
-		wrapper[key] = value
-		return value
-	}
-
-	return field
-}
-func isEmpty(v any) bool {
-	switch v := v.(type) {
-	case string:
-		return v == ""
-	case nil:
-		return true
-	default:
-		return false
-	}
-}
-
 func validateConfig(cfg *Config) error {
 	if cfg == nil {
 		return errors.New("config cannot be nil")
 	}
 
-	// Check if ContextKeys is empty.
-	if len(cfg.ContextKeys) == 0 {
-		return errors.New("ContextKeys cannot be empty")
-	}
-
-	// Validate that ContextKeys does not contain empty strings.
-	for _, key := range cfg.ContextKeys {
-		if key == "" {
-			return errors.New("ContextKeys cannot contain empty strings")
-		}
+	if cfg.Role != "bap" && cfg.Role != "bpp" {
+		return errors.New("role must be either 'bap' or 'bpp'")
 	}
 	return nil
 }

pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go

@@ -5,48 +5,49 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
-func TestNewUUIDSetterSuccessCases(t *testing.T) {
+// ToDo Separate Middleware creation and execution.
+func TestNewPreProcessorSuccessCases(t *testing.T) {
 	tests := []struct {
 		name        string
 		config      *Config
 		requestBody map[string]any
-		expectedKeys []string
-		role         string
+		expectedID  string
 	}{
 		{
-			name: "Valid keys, update missing keys with bap role",
+			name: "BAP role with valid context",
 			config: &Config{
-				ContextKeys: []string{"transaction_id", "message_id"},
 				Role: "bap",
 			},
-			requestBody: map[string]any{
-				"context": map[string]any{
-					"transaction_id": "",
-					"message_id":     nil,
+			requestBody: map[string]interface{}{
+				"context": map[string]interface{}{
 					"bap_id":     "bap-123",
+					"message_id": "msg-123",
+				},
+				"message": map[string]interface{}{
+					"key": "value",
 				},
 			},
-			expectedKeys: []string{"transaction_id", "message_id", "bap_id"},
-			role:         "bap",
+			expectedID: "bap-123",
 		},
 		{
-			name: "Valid keys, do not update existing keys with bpp role",
+			name: "BPP role with valid context",
 			config: &Config{
-				ContextKeys: []string{"transaction_id", "message_id"},
 				Role: "bpp",
 			},
-			requestBody: map[string]any{
-				"context": map[string]any{
-					"transaction_id": "existing-transaction",
-					"message_id":     "existing-message",
+			requestBody: map[string]interface{}{
+				"context": map[string]interface{}{
 					"bpp_id":     "bpp-456",
+					"message_id": "msg-456",
+				},
+				"message": map[string]interface{}{
+					"key": "value",
 				},
 			},
-			expectedKeys: []string{"transaction_id", "message_id", "bpp_id"},
-			role:         "bpp",
+			expectedID: "bpp-456",
 		},
 	}
 
@@ -54,29 +55,40 @@ func TestNewUUIDSetterSuccessCases(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			middleware, err := NewPreProcessor(tt.config)
 			if err != nil {
-				t.Fatalf("Unexpected error while creating middleware: %v", err)
+				t.Fatalf("NewPreProcessor() error = %v", err)
 			}
 
-			bodyBytes, _ := json.Marshal(tt.requestBody)
+			bodyBytes, err := json.Marshal(tt.requestBody)
+			if err != nil {
+				t.Fatalf("Failed to marshal request body: %v", err)
+			}
 			req := httptest.NewRequest(http.MethodPost, "/test", bytes.NewReader(bodyBytes))
 			req.Header.Set("Content-Type", "application/json")
 
 			rec := httptest.NewRecorder()
 
+			var gotSubID interface{}
+
 			dummyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				ctx := r.Context()
+				gotSubID = ctx.Value(subscriberIDKey)
 				w.WriteHeader(http.StatusOK)
 
-				subID, ok := ctx.Value(subscriberIDKey).(string)
-				if !ok {
-					http.Error(w, "Subscriber ID not found", http.StatusInternalServerError)
+				// Verify subscriber ID
+				subID := ctx.Value(subscriberIDKey)
+				if subID == nil {
+					t.Errorf("Expected subscriber ID but got none %s", ctx)
 					return
 				}
 
-				response := map[string]any{"subscriber_id": subID}
-				if err := json.NewEncoder(w).Encode(response); err != nil {
-					http.Error(w, "Internal Server Error", http.StatusInternalServerError)
-					return
+				// Verify the correct ID was set based on role
+				expectedKey := "bap_id"
+				if tt.config.Role == "bpp" {
+					expectedKey = "bpp_id"
+				}
+				expectedID := tt.requestBody["context"].(map[string]interface{})[expectedKey]
+				if subID != expectedID {
+					t.Errorf("Expected subscriber ID %v, got %v", expectedID, subID)
 				}
 			})
 
@@ -87,71 +99,113 @@ func TestNewUUIDSetterSuccessCases(t *testing.T) {
 				return
 			}
 
-			var responseBody map[string]any
-			if err := json.Unmarshal(rec.Body.Bytes(), &responseBody); err != nil {
-				t.Fatal("Failed to unmarshal response body:", err)
-			}
-
-			expectedSubIDKey := "bap_id"
-			if tt.role == "bpp" {
-				expectedSubIDKey = "bpp_id"
-			}
-
-			subID, ok := responseBody["subscriber_id"].(string)
-			if !ok {
-				t.Error("subscriber_id not found in response")
+			// Verify subscriber ID
+			if gotSubID == nil {
+				t.Error("Expected subscriber_id to be set in context but got nil")
 				return
 			}
 
-			expectedSubID := tt.requestBody["context"].(map[string]any)[expectedSubIDKey]
-			if subID != expectedSubID {
-				t.Errorf("Expected subscriber_id %v, but got %v", expectedSubID, subID)
+			subID, ok := gotSubID.(string)
+			if !ok {
+				t.Errorf("Expected subscriber_id to be string, got %T", gotSubID)
+				return
+			}
+
+			if subID != tt.expectedID {
+				t.Errorf("Expected subscriber_id %q, got %q", tt.expectedID, subID)
 			}
 		})
 	}
 }
 
-func TestNewUUIDSetterErrorCases(t *testing.T) {
+func TestNewPreProcessorErrorCases(t *testing.T) {
 	tests := []struct {
 		name         string
 		config       *Config
-		requestBody  map[string]any
+		requestBody  interface{}
 		expectedCode int
+		expectErr    bool
+		errMsg       string
 	}{
 		{
-			name: "Missing context key",
+			name: "Missing context",
 			config: &Config{
-				ContextKeys: []string{"transaction_id"},
+				Role: "bap",
 			},
 			requestBody: map[string]any{
 				"otherKey": "value",
 			},
 			expectedCode: http.StatusBadRequest,
+			expectErr:    false,
+			errMsg:       "context field not found or invalid",
 		},
 		{
 			name: "Invalid context type",
 			config: &Config{
-				ContextKeys: []string{"transaction_id"},
+				Role: "bap",
 			},
 			requestBody: map[string]any{
 				"context": "not-a-map",
 			},
 			expectedCode: http.StatusBadRequest,
+			expectErr:    false,
+			errMsg:       "context field not found or invalid",
 		},
 		{
 			name:         "Nil config",
 			config:       nil,
 			requestBody:  map[string]any{},
 			expectedCode: http.StatusInternalServerError,
+			expectErr:    true,
+			errMsg:       "config cannot be nil",
+		},
+		{
+			name: "Invalid role",
+			config: &Config{
+				Role: "invalid-role",
+			},
+			requestBody: map[string]interface{}{
+				"context": map[string]interface{}{
+					"bap_id": "bap-123",
+				},
+			},
+			expectedCode: http.StatusInternalServerError,
+			expectErr:    true,
+			errMsg:       "role must be either 'bap' or 'bpp'",
+		},
+		{
+			name: "Missing subscriber ID",
+			config: &Config{
+				Role: "bap",
+			},
+			requestBody: map[string]interface{}{
+				"context": map[string]interface{}{
+					"message_id": "msg-123",
+				},
+			},
+			expectedCode: http.StatusOK,
+			expectErr:    false,
+		},
+		{
+			name: "Invalid JSON body",
+			config: &Config{
+				Role: "bap",
+			},
+			requestBody:  "{invalid-json}",
+			expectedCode: http.StatusBadRequest,
+			expectErr:    false,
+			errMsg:       "failed to decode request body",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			middleware, err := NewPreProcessor(tt.config)
-			if tt.config == nil {
+			if tt.expectErr {
 				if err == nil {
-					t.Error("Expected an error for nil config, but got none")
+					t.Errorf("Expected an error for NewPreProcessor(%s), but got none", tt.config)
+				} else if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) {
+					t.Errorf("Expected error to contain %q, got %v", tt.errMsg, err)
 				}
 				return
 			}