From 0ee2010338d7c4505928612151b5320052ba9821 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Thu, 3 Apr 2025 12:35:13 +0530 Subject: [PATCH 01/15] change for the context keys --- pkg/model/model.go | 7 +++++-- .../implementation/reqpreprocessor/reqpreprocessor.go | 8 ++++---- .../reqpreprocessor/reqpreprocessor_test.go | 4 +++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/model/model.go b/pkg/model/model.go index 621bdeb..53ff27e 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -38,10 +38,13 @@ const ( UnaAuthorizedHeaderGateway string = "Proxy-Authenticate" ) -type contextKey string +type ContextKey string // MsgIDKey is the context key used to store and retrieve the message ID in a request context. -const MsgIDKey = contextKey("message_id") +const MsgIDKey = ContextKey("message_id") + +// SubscriberIDKey is the context key used to store and retrieve the subscriber ID in a request context. +const SubscriberIDKey = ContextKey("subscriber_id") // Role defines the type of participant in the network. type Role string diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 24ffa67..ca9f2eb 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -10,6 +10,7 @@ import ( "net/http" "github.com/beckn/beckn-onix/pkg/log" + "github.com/beckn/beckn-onix/pkg/model" "github.com/google/uuid" ) @@ -24,7 +25,6 @@ type becknRequest struct { } const contextKey = "context" -const subscriberIDKey = "subscriber_id" // NewPreProcessor creates a middleware that processes incoming HTTP requests by extracting // and modifying the request context based on the provided configuration. @@ -53,14 +53,14 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { subID = req.Context["bpp_id"] } if subID != nil { - log.Debugf(ctx, "adding subscriberId to request:%s, %v", subscriberIDKey, subID) + log.Debugf(ctx, "adding subscriberId to request:%s, %v", model.SubscriberIDKey, subID) // TODO: Add a ContextKey type in model and use it here instead of raw context key. - ctx = context.WithValue(ctx, subscriberIDKey, subID) + ctx = context.WithValue(ctx, model.SubscriberIDKey, subID) } for _, key := range cfg.ContextKeys { value := uuid.NewString() updatedValue := update(req.Context, key, value) - ctx = context.WithValue(ctx, key, updatedValue) + ctx = context.WithValue(ctx, model.ContextKey(key), updatedValue) } reqData := map[string]any{"context": req.Context} updatedBody, _ := json.Marshal(reqData) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go index d70af8e..f35ee68 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go @@ -6,6 +6,8 @@ import ( "net/http" "net/http/httptest" "testing" + + "github.com/beckn/beckn-onix/pkg/model" ) func TestNewUUIDSetterSuccessCases(t *testing.T) { @@ -67,7 +69,7 @@ func TestNewUUIDSetterSuccessCases(t *testing.T) { ctx := r.Context() w.WriteHeader(http.StatusOK) - subID, ok := ctx.Value(subscriberIDKey).(string) + subID, ok := ctx.Value(model.SubscriberIDKey).(string) if !ok { http.Error(w, "Subscriber ID not found", http.StatusInternalServerError) return From 01588fc866686b9c252a377583b59b8931328571 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Thu, 3 Apr 2025 15:17:02 +0530 Subject: [PATCH 02/15] Added test case for the contextKeys --- pkg/model/model.go | 10 ++-- .../reqpreprocessor/reqpreprocessor_test.go | 53 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/pkg/model/model.go b/pkg/model/model.go index 53ff27e..8555046 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -40,11 +40,13 @@ const ( type ContextKey string -// MsgIDKey is the context key used to store and retrieve the message ID in a request context. -const MsgIDKey = ContextKey("message_id") +const ( + // MsgIDKey is the context key used to store and retrieve the message ID in a request context. + MsgIDKey = ContextKey("message_id") -// SubscriberIDKey is the context key used to store and retrieve the subscriber ID in a request context. -const SubscriberIDKey = ContextKey("subscriber_id") + // SubscriberIDKey is the context key used to store and retrieve the subscriber ID in a request context. + SubscriberIDKey = ContextKey("subscriber_id") +) // Role defines the type of participant in the network. type Role string diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go index f35ee68..87d464f 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/beckn/beckn-onix/pkg/model" + "github.com/stretchr/testify/require" ) func TestNewUUIDSetterSuccessCases(t *testing.T) { @@ -178,3 +179,55 @@ func TestNewUUIDSetterErrorCases(t *testing.T) { }) } } + +// Mock configuration +var testConfig = &Config{ + Role: "bap", + ContextKeys: []string{"message_id"}, +} + +// Mock request payload +var testPayload = `{ + "context": { + "message_id": "test-msg-id", + "bap_id": "test-bap-id" + } +}` + +// Mock handler to capture processed request context +func captureContextHandler(t *testing.T, expectedMsgID string, expectedSubID string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Retrieve values from context + msgID, ok := r.Context().Value(model.MsgIDKey).(string) + require.True(t, ok, "message_id should be set") + require.Equal(t, expectedMsgID, msgID, "message_id should match") + + subID, ok := r.Context().Value(model.SubscriberIDKey).(string) + require.True(t, ok, "subscriber_id should be set") + require.Equal(t, expectedSubID, subID, "subscriber_id should match") + + w.WriteHeader(http.StatusOK) + }) +} + +// Test NewPreProcessor middleware +func TestNewPreProcessor(t *testing.T) { + preProcessor, err := NewPreProcessor(testConfig) + require.NoError(t, err) + + // Create test request + req := httptest.NewRequest(http.MethodPost, "/test", bytes.NewBufferString(testPayload)) + req.Header.Set("Content-Type", "application/json") + + // Create response recorder + recorder := httptest.NewRecorder() + + // Wrap handler with middleware + handler := preProcessor(captureContextHandler(t, "test-msg-id", "test-bap-id")) + + // Serve request + handler.ServeHTTP(recorder, req) + + // Check response status + require.Equal(t, http.StatusOK, recorder.Code, "Middleware should process correctly") +} From 251457480e4beac061eb506a062b6ea4b40bb435 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 12:04:45 +0530 Subject: [PATCH 03/15] Resolved router test cases --- pkg/plugin/implementation/router/router.go | 22 +++++++++++++++++++ .../implementation/router/router_test.go | 6 ++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/plugin/implementation/router/router.go b/pkg/plugin/implementation/router/router.go index 52e628b..f9316b0 100644 --- a/pkg/plugin/implementation/router/router.go +++ b/pkg/plugin/implementation/router/router.go @@ -153,6 +153,10 @@ func validateRules(rules []routingRule) error { if rule.Target.URL == "" { return fmt.Errorf("invalid rule: url is required for targetType 'url'") } + // Validate HTTPS scheme + if err := isValidHTTPSURL(rule.Target.URL); err != nil { + return fmt.Errorf("invalid URI %s in request body for url: %v", rule.Target.URL, err) + } if _, err := url.Parse(rule.Target.URL); err != nil { return fmt.Errorf("invalid URL - %s: %w", rule.Target.URL, err) } @@ -162,6 +166,9 @@ func validateRules(rules []routingRule) error { } case targetTypeBPP, targetTypeBAP: if rule.Target.URL != "" { + if err := isValidHTTPSURL(rule.Target.URL); err != nil { + return fmt.Errorf("invalid URI %s in request body for %s: %v", rule.Target.URL, rule.TargetType, err) + } if _, err := url.Parse(rule.Target.URL); err != nil { return fmt.Errorf("invalid URL - %s defined in routing config for target type %s: %w", rule.Target.URL, rule.TargetType, err) } @@ -234,6 +241,9 @@ func handleProtocolMapping(route *model.Route, npURI, endpoint string) (*model.R }, }, nil } + if err := isValidHTTPSURL(target); err != nil { + return nil, fmt.Errorf("invalid %s URI - %s in request body for %s: %v", strings.ToUpper(route.TargetType), target, endpoint, err) + } targetURL, err := url.Parse(target) if err != nil { return nil, fmt.Errorf("invalid %s URI - %s in request body for %s: %w", strings.ToUpper(route.TargetType), target, endpoint, err) @@ -248,3 +258,15 @@ func handleProtocolMapping(route *model.Route, npURI, endpoint string) (*model.R }, }, nil } + +// isValidHTTPSURL ensures the provided URL is a valid HTTPS URL. +func isValidHTTPSURL(rawURL string) error { + parsedURL, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid URL - %s: %v", rawURL, err) + } + if parsedURL.Scheme != "https" { + return fmt.Errorf("URL '%s' must use https scheme", rawURL) + } + return nil +} diff --git a/pkg/plugin/implementation/router/router_test.go b/pkg/plugin/implementation/router/router_test.go index 7937ca5..7688066 100644 --- a/pkg/plugin/implementation/router/router_test.go +++ b/pkg/plugin/implementation/router/router_test.go @@ -296,7 +296,7 @@ func TestValidateRulesFailure(t *testing.T) { Endpoints: []string{"search"}, }, }, - wantErr: "invalid URL - htp://invalid-url.com: URL 'htp://invalid-url.com' must use https scheme", + wantErr: "invalid URI htp://invalid-url.com in request body for url: URL 'htp://invalid-url.com' must use https scheme", }, { name: "Missing topic_id for targetType: publisher", @@ -326,7 +326,7 @@ func TestValidateRulesFailure(t *testing.T) { Endpoints: []string{"search"}, }, }, - wantErr: "invalid URL - htp://invalid-url.com defined in routing config for target type bpp", + wantErr: "invalid URI htp://invalid-url.com in request body for bpp: URL 'htp://invalid-url.com' must use https scheme", }, { name: "Invalid URL for BAP targetType", @@ -341,7 +341,7 @@ func TestValidateRulesFailure(t *testing.T) { Endpoints: []string{"search"}, }, }, - wantErr: "invalid URL - http://[invalid].com defined in routing config for target type bap", + wantErr: "invalid URI http://[invalid].com in request body for bap", }, } From 593b76427c3e3725e5770baa5f3990036678eb2e Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 12:48:36 +0530 Subject: [PATCH 04/15] resolved merge conflict --- .../reqpreprocessor/reqpreprocessor_test.go | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go index fe0c6fa..8f34759 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/beckn/beckn-onix/pkg/model" - "github.com/stretchr/testify/require" ) // ToDo Separate Middleware creation and execution. @@ -74,11 +73,11 @@ func TestNewPreProcessorSuccessCases(t *testing.T) { dummyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - gotSubID = ctx.Value(subscriberIDKey) + gotSubID = ctx.Value(model.SubscriberIDKey) w.WriteHeader(http.StatusOK) // Verify subscriber ID - subID := ctx.Value(subscriberIDKey) + subID := ctx.Value(model.SubscriberIDKey) if subID == nil { t.Errorf("Expected subscriber ID but got none %s", ctx) return @@ -236,8 +235,7 @@ func TestNewPreProcessorErrorCases(t *testing.T) { // Mock configuration var testConfig = &Config{ - Role: "bap", - ContextKeys: []string{"message_id"}, + Role: "bap", } // Mock request payload @@ -249,16 +247,15 @@ var testPayload = `{ }` // Mock handler to capture processed request context -func captureContextHandler(t *testing.T, expectedMsgID string, expectedSubID string) http.Handler { +func captureContextHandler(t *testing.T, expectedSubID string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Retrieve values from context - msgID, ok := r.Context().Value(model.MsgIDKey).(string) - require.True(t, ok, "message_id should be set") - require.Equal(t, expectedMsgID, msgID, "message_id should match") - + // Retrieve subscriber_id from context subID, ok := r.Context().Value(model.SubscriberIDKey).(string) - require.True(t, ok, "subscriber_id should be set") - require.Equal(t, expectedSubID, subID, "subscriber_id should match") + if !ok { + t.Error("subscriber_id should be set in context") + } else if subID != expectedSubID { + t.Errorf("expected subscriber_id %s, got %s", expectedSubID, subID) + } w.WriteHeader(http.StatusOK) }) @@ -266,8 +263,20 @@ func captureContextHandler(t *testing.T, expectedMsgID string, expectedSubID str // Test NewPreProcessor middleware func TestNewPreProcessor(t *testing.T) { + testConfig := &Config{ + Role: "bap", + } + + testPayload := `{ + "context": { + "bap_id": "test-bap-id" + } + }` + preProcessor, err := NewPreProcessor(testConfig) - require.NoError(t, err) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } // Create test request req := httptest.NewRequest(http.MethodPost, "/test", bytes.NewBufferString(testPayload)) @@ -277,11 +286,13 @@ func TestNewPreProcessor(t *testing.T) { recorder := httptest.NewRecorder() // Wrap handler with middleware - handler := preProcessor(captureContextHandler(t, "test-msg-id", "test-bap-id")) + handler := preProcessor(captureContextHandler(t, "test-bap-id")) // Serve request handler.ServeHTTP(recorder, req) // Check response status - require.Equal(t, http.StatusOK, recorder.Code, "Middleware should process correctly") + if recorder.Code != http.StatusOK { + t.Errorf("expected status %d, got %d", http.StatusOK, recorder.Code) + } } From 49e460f61dcc11ad2567f5efa18ed9037cdae08c Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 14:59:40 +0530 Subject: [PATCH 05/15] fixed linting iessues --- .../reqpreprocessor/reqpreprocessor.go | 4 +--- .../reqpreprocessor/reqpreprocessor_test.go | 13 ------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 554a9b5..441a3a7 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -17,8 +17,6 @@ type Config struct { Role string } -type keyType string - const contextKey = "context" func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { @@ -39,7 +37,7 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { return } - // Extract context from request + // 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) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go index 8f34759..20beb18 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go @@ -233,19 +233,6 @@ func TestNewPreProcessorErrorCases(t *testing.T) { } } -// Mock configuration -var testConfig = &Config{ - Role: "bap", -} - -// Mock request payload -var testPayload = `{ - "context": { - "message_id": "test-msg-id", - "bap_id": "test-bap-id" - } -}` - // Mock handler to capture processed request context func captureContextHandler(t *testing.T, expectedSubID string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From fc296b8ef36f0420045e830e34f599987a0add7a Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 15:45:15 +0530 Subject: [PATCH 06/15] code fix as per the review comments --- pkg/model/model.go | 12 ++-- .../reqpreprocessor/reqpreprocessor.go | 12 ++-- .../reqpreprocessor/reqpreprocessor_test.go | 70 +++++++------------ pkg/response/response.go | 6 +- pkg/response/response_test.go | 4 +- 5 files changed, 48 insertions(+), 56 deletions(-) diff --git a/pkg/model/model.go b/pkg/model/model.go index 8555046..13802f4 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -38,14 +38,18 @@ const ( UnaAuthorizedHeaderGateway string = "Proxy-Authenticate" ) +// ContextKey is a custom type used as a key for storing and retrieving values in a context. type ContextKey string const ( - // MsgIDKey is the context key used to store and retrieve the message ID in a request context. - MsgIDKey = ContextKey("message_id") + // ContextKeyMsgID is the context key used to store and retrieve the message ID in a request context. + ContextKeyMsgID ContextKey = "message_id" - // SubscriberIDKey is the context key used to store and retrieve the subscriber ID in a request context. - SubscriberIDKey = ContextKey("subscriber_id") + // ContextKeySubscriberID is the context key used to store and retrieve the subscriber ID in a request context. + ContextKeySubscriberID ContextKey = "subscriber_id" + + // ContextKeyModelID is the context key for storing and retrieving the model ID from a request context. + ContextKeyModelID ContextKey = "model_id" ) // Role defines the type of participant in the network. diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 441a3a7..6653051 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -13,12 +13,16 @@ import ( "github.com/beckn/beckn-onix/pkg/model" ) +// Config represents the configuration for the request preprocessor middleware. type Config struct { Role string } -const contextKey = "context" +// contextKeyContext is the typed context key for request context. +var contextKeyContext = model.ContextKey("context") +// NewPreProcessor returns a middleware that processes the incoming request, +// extracts the context field from the body, and adds relevant values (like subscriber ID). func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { if err := validateConfig(cfg); err != nil { return nil, err @@ -40,7 +44,7 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { // 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) + http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKeyContext), http.StatusBadRequest) return } var subID any @@ -51,8 +55,8 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { subID = reqContext["bpp_id"] } if subID != nil { - log.Debugf(ctx, "adding subscriberId to request:%s, %v", model.SubscriberIDKey, subID) - ctx = context.WithValue(ctx, model.SubscriberIDKey, subID) + log.Debugf(ctx, "adding subscriberId to request:%s, %v", model.ContextKeySubscriberID, subID) + ctx = context.WithValue(ctx, model.ContextKeySubscriberID, subID) } r.Body = io.NopCloser(bytes.NewBuffer(body)) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go index 20beb18..ed78d2d 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor_test.go @@ -73,11 +73,11 @@ func TestNewPreProcessorSuccessCases(t *testing.T) { dummyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - gotSubID = ctx.Value(model.SubscriberIDKey) + gotSubID = ctx.Value(model.ContextKeySubscriberID) w.WriteHeader(http.StatusOK) // Verify subscriber ID - subID := ctx.Value(model.SubscriberIDKey) + subID := ctx.Value(model.ContextKeySubscriberID) if subID == nil { t.Errorf("Expected subscriber ID but got none %s", ctx) return @@ -233,53 +233,37 @@ func TestNewPreProcessorErrorCases(t *testing.T) { } } -// Mock handler to capture processed request context -func captureContextHandler(t *testing.T, expectedSubID string) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Retrieve subscriber_id from context - subID, ok := r.Context().Value(model.SubscriberIDKey).(string) - if !ok { - t.Error("subscriber_id should be set in context") - } else if subID != expectedSubID { - t.Errorf("expected subscriber_id %s, got %s", expectedSubID, subID) - } +func TestNewPreProcessorAddsSubscriberIDToContext(t *testing.T) { + cfg := &Config{Role: "bap"} + middleware, err := NewPreProcessor(cfg) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + samplePayload := map[string]interface{}{ + "context": map[string]interface{}{ + "bap_id": "bap.example.com", + }, + } + bodyBytes, _ := json.Marshal(samplePayload) + + var receivedSubscriberID interface{} + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedSubscriberID = r.Context().Value(model.ContextKeySubscriberID) w.WriteHeader(http.StatusOK) }) -} -// Test NewPreProcessor middleware -func TestNewPreProcessor(t *testing.T) { - testConfig := &Config{ - Role: "bap", - } - - testPayload := `{ - "context": { - "bap_id": "test-bap-id" - } - }` - - preProcessor, err := NewPreProcessor(testConfig) - if err != nil { - t.Fatalf("expected no error, got: %v", err) - } - - // Create test request - req := httptest.NewRequest(http.MethodPost, "/test", bytes.NewBufferString(testPayload)) + req := httptest.NewRequest("POST", "/", strings.NewReader(string(bodyBytes))) req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() - // Create response recorder - recorder := httptest.NewRecorder() + middleware(handler).ServeHTTP(rr, req) - // Wrap handler with middleware - handler := preProcessor(captureContextHandler(t, "test-bap-id")) - - // Serve request - handler.ServeHTTP(recorder, req) - - // Check response status - if recorder.Code != http.StatusOK { - t.Errorf("expected status %d, got %d", http.StatusOK, recorder.Code) + if rr.Code != http.StatusOK { + t.Fatalf("Expected status 200 OK, got %d", rr.Code) + } + if receivedSubscriberID != "bap.example.com" { + t.Errorf("Expected subscriber ID 'bap.example.com', got %v", receivedSubscriberID) } } diff --git a/pkg/response/response.go b/pkg/response/response.go index a5ab0c4..f9ac9d5 100644 --- a/pkg/response/response.go +++ b/pkg/response/response.go @@ -47,8 +47,8 @@ func nack(ctx context.Context, w http.ResponseWriter, err *model.Error, status i w.WriteHeader(status) _, er := w.Write(data) if er != nil { - fmt.Printf("Error writing response: %v, MessageID: %s", er, ctx.Value(model.MsgIDKey)) - http.Error(w, fmt.Sprintf("Internal server error, MessageID: %s", ctx.Value(model.MsgIDKey)), http.StatusInternalServerError) + fmt.Printf("Error writing response: %v, MessageID: %s", er, ctx.Value(model.ContextKeyMsgID)) + http.Error(w, fmt.Sprintf("Internal server error, MessageID: %s", ctx.Value(model.ContextKeyMsgID)), http.StatusInternalServerError) return } } @@ -57,7 +57,7 @@ func nack(ctx context.Context, w http.ResponseWriter, err *model.Error, status i func internalServerError(ctx context.Context) *model.Error { return &model.Error{ Code: http.StatusText(http.StatusInternalServerError), - Message: fmt.Sprintf("Internal server error, MessageID: %s", ctx.Value(model.MsgIDKey)), + Message: fmt.Sprintf("Internal server error, MessageID: %s", ctx.Value(model.ContextKeyMsgID)), } } diff --git a/pkg/response/response_test.go b/pkg/response/response_test.go index 96f1caa..f0c1c6d 100644 --- a/pkg/response/response_test.go +++ b/pkg/response/response_test.go @@ -46,7 +46,7 @@ func TestSendAck(t *testing.T) { } func TestSendNack(t *testing.T) { - ctx := context.WithValue(context.Background(), model.MsgIDKey, "123456") + ctx := context.WithValue(context.Background(), model.ContextKeyMsgID, "123456") tests := []struct { name string @@ -197,7 +197,7 @@ func TestNack_1(t *testing.T) { if err != nil { t.Fatal(err) } - ctx := context.WithValue(req.Context(), model.MsgIDKey, "12345") + ctx := context.WithValue(req.Context(), model.ContextKeyMsgID, "12345") var w http.ResponseWriter if tt.useBadWrite { From f3a3b5230f187f26a7df4863c91749d8ff0794e9 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 16:12:37 +0530 Subject: [PATCH 07/15] fixes as per the review comments --- pkg/plugin/implementation/router/router.go | 22 ------------------- .../implementation/router/router_test.go | 4 ++-- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/pkg/plugin/implementation/router/router.go b/pkg/plugin/implementation/router/router.go index f9316b0..52e628b 100644 --- a/pkg/plugin/implementation/router/router.go +++ b/pkg/plugin/implementation/router/router.go @@ -153,10 +153,6 @@ func validateRules(rules []routingRule) error { if rule.Target.URL == "" { return fmt.Errorf("invalid rule: url is required for targetType 'url'") } - // Validate HTTPS scheme - if err := isValidHTTPSURL(rule.Target.URL); err != nil { - return fmt.Errorf("invalid URI %s in request body for url: %v", rule.Target.URL, err) - } if _, err := url.Parse(rule.Target.URL); err != nil { return fmt.Errorf("invalid URL - %s: %w", rule.Target.URL, err) } @@ -166,9 +162,6 @@ func validateRules(rules []routingRule) error { } case targetTypeBPP, targetTypeBAP: if rule.Target.URL != "" { - if err := isValidHTTPSURL(rule.Target.URL); err != nil { - return fmt.Errorf("invalid URI %s in request body for %s: %v", rule.Target.URL, rule.TargetType, err) - } if _, err := url.Parse(rule.Target.URL); err != nil { return fmt.Errorf("invalid URL - %s defined in routing config for target type %s: %w", rule.Target.URL, rule.TargetType, err) } @@ -241,9 +234,6 @@ func handleProtocolMapping(route *model.Route, npURI, endpoint string) (*model.R }, }, nil } - if err := isValidHTTPSURL(target); err != nil { - return nil, fmt.Errorf("invalid %s URI - %s in request body for %s: %v", strings.ToUpper(route.TargetType), target, endpoint, err) - } targetURL, err := url.Parse(target) if err != nil { return nil, fmt.Errorf("invalid %s URI - %s in request body for %s: %w", strings.ToUpper(route.TargetType), target, endpoint, err) @@ -258,15 +248,3 @@ func handleProtocolMapping(route *model.Route, npURI, endpoint string) (*model.R }, }, nil } - -// isValidHTTPSURL ensures the provided URL is a valid HTTPS URL. -func isValidHTTPSURL(rawURL string) error { - parsedURL, err := url.Parse(rawURL) - if err != nil { - return fmt.Errorf("invalid URL - %s: %v", rawURL, err) - } - if parsedURL.Scheme != "https" { - return fmt.Errorf("URL '%s' must use https scheme", rawURL) - } - return nil -} diff --git a/pkg/plugin/implementation/router/router_test.go b/pkg/plugin/implementation/router/router_test.go index 7688066..8ee777e 100644 --- a/pkg/plugin/implementation/router/router_test.go +++ b/pkg/plugin/implementation/router/router_test.go @@ -291,12 +291,12 @@ func TestValidateRulesFailure(t *testing.T) { Version: "1.0.0", TargetType: "url", Target: target{ - URL: "htp://invalid-url.com", // Invalid scheme + URL: "htp:// invalid-url.com", // Invalid scheme }, Endpoints: []string{"search"}, }, }, - wantErr: "invalid URI htp://invalid-url.com in request body for url: URL 'htp://invalid-url.com' must use https scheme", + wantErr: `invalid URI htp:// invalid-url.com in request body for url: invalid URL - htp:// invalid-url.com: parse "htp:// invalid-url.com": invalid character " " in host name`, }, { name: "Missing topic_id for targetType: publisher", From 278e217c64e57e1501d2d0181d708d1c9ad59bec Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Fri, 4 Apr 2025 16:15:16 +0530 Subject: [PATCH 08/15] added const for contextKey --- pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 6653051..a7f4842 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -18,8 +18,7 @@ type Config struct { Role string } -// contextKeyContext is the typed context key for request context. -var contextKeyContext = model.ContextKey("context") +const contextKey = "context" // NewPreProcessor returns a middleware that processes the incoming request, // extracts the context field from the body, and adds relevant values (like subscriber ID). @@ -44,7 +43,7 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { // Extract context from request. reqContext, ok := req["context"].(map[string]interface{}) if !ok { - http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKeyContext), http.StatusBadRequest) + http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKey), http.StatusBadRequest) return } var subID any From ec62a3242baa5266615f96c08c294e6f237de350 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Sun, 6 Apr 2025 17:09:59 +0530 Subject: [PATCH 09/15] resolved reqpreprocessing review comments 1. added logs for contextKey parsing and unmarshaling yml file. 2. resolved router test case issue. 2. resolved logger test case issues. --- core/module/handler/stdHandler.go | 2 +- pkg/log/log.go | 9 +- pkg/log/log_test.go | 117 ++++++++++-------- pkg/model/error_test.go | 53 +++++++- pkg/model/model.go | 35 ++++++ .../reqpreprocessor/reqpreprocessor.go | 16 ++- .../implementation/router/router_test.go | 14 +-- 7 files changed, 176 insertions(+), 70 deletions(-) diff --git a/core/module/handler/stdHandler.go b/core/module/handler/stdHandler.go index 251102e..3a3caaa 100644 --- a/core/module/handler/stdHandler.go +++ b/core/module/handler/stdHandler.go @@ -101,7 +101,7 @@ func (h *stdHandler) stepCtx(r *http.Request, rh http.Header) (*model.StepContex // subID retrieves the subscriber ID from the request context. func (h *stdHandler) subID(ctx context.Context) string { - rSubID, ok := ctx.Value("subscriber_id").(string) + rSubID, ok := ctx.Value(model.ContextKeySubscriberID).(string) if ok { return rSubID } diff --git a/pkg/log/log.go b/pkg/log/log.go index eabd9f0..1f5f59f 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/beckn/beckn-onix/pkg/model" "github.com/rs/zerolog" "gopkg.in/natefinch/lumberjack.v2" ) @@ -52,9 +53,9 @@ var logLevels = map[level]zerolog.Level{ // Config represents the configuration for logging. type Config struct { - Level level `yaml:"level"` - Destinations []destination `yaml:"destinations"` - ContextKeys []string `yaml:"contextKeys"` + Level level `yaml:"level"` + Destinations []destination `yaml:"destinations"` + ContextKeys []model.ContextKey `yaml:"contextKeys"` } var ( @@ -277,7 +278,7 @@ func addCtx(ctx context.Context, event *zerolog.Event) { if !ok { continue } - keyStr := key + keyStr := string(key) event.Any(keyStr, val) } } diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 2e874ae..8fa4eac 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -13,6 +13,8 @@ import ( "strings" "testing" "time" + + "github.com/beckn/beckn-onix/pkg/model" ) type ctxKey any @@ -63,7 +65,12 @@ func setupLogger(t *testing.T, l level) string { }, }, }, - ContextKeys: []string{"userID", "requestID"}, + ContextKeys: []model.ContextKey{ + model.ContextKeyTxnID, + model.ContextKeyMsgID, + model.ContextKeySubscriberID, + model.ContextKeyModelID, + }, } // Initialize logger with the given config @@ -97,16 +104,16 @@ func parseLogLine(t *testing.T, line string) map[string]interface{} { func TestDebug(t *testing.T) { t.Helper() logPath := setupLogger(t, DebugLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Debug(ctx, "Debug message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "debug", - "userID": "12345", - "message": "Debug message", + "level": "debug", + "subscriber_id": "12345", + "message": "Debug message", } var found bool @@ -129,16 +136,16 @@ func TestDebug(t *testing.T) { func TestInfo(t *testing.T) { logPath := setupLogger(t, InfoLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Info(ctx, "Info message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "info", - "userID": "12345", - "message": "Info message", + "level": "info", + "subscriber_id": "12345", + "message": "Info message", } var found bool @@ -161,16 +168,16 @@ func TestInfo(t *testing.T) { func TestWarn(t *testing.T) { logPath := setupLogger(t, WarnLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Warn(ctx, "Warning message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "warn", - "userID": "12345", - "message": "Warning message", + "level": "warn", + "subscriber_id": "12345", + "message": "Warning message", } var found bool @@ -189,17 +196,17 @@ func TestWarn(t *testing.T) { func TestError(t *testing.T) { logPath := setupLogger(t, ErrorLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Error(ctx, fmt.Errorf("test error"), "Error message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "error", - "userID": "12345", - "message": "Error message", - "error": "test error", + "level": "error", + "subscriber_id": "12345", + "message": "Error message", + "error": "test error", } var found bool @@ -277,17 +284,17 @@ func TestResponse(t *testing.T) { func TestFatal(t *testing.T) { logPath := setupLogger(t, FatalLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Fatal(ctx, fmt.Errorf("fatal error"), "Fatal message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "fatal", - "userID": "12345", - "message": "Fatal message", - "error": "fatal error", + "level": "fatal", + "subscriber_id": "12345", + "message": "Fatal message", + "error": "fatal error", } var found bool @@ -308,17 +315,17 @@ func TestFatal(t *testing.T) { func TestPanic(t *testing.T) { logPath := setupLogger(t, PanicLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Panic(ctx, fmt.Errorf("panic error"), "Panic message") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "panic", - "userID": "12345", - "message": "Panic message", - "error": "panic error", + "level": "panic", + "subscriber_id": "12345", + "message": "Panic message", + "error": "panic error", } var found bool @@ -339,16 +346,16 @@ func TestPanic(t *testing.T) { func TestDebugf(t *testing.T) { logPath := setupLogger(t, DebugLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Debugf(ctx, "Debugf message: %s", "test") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "debug", - "userID": "12345", - "message": "Debugf message: test", + "level": "debug", + "subscriber_id": "12345", + "message": "Debugf message: test", } var found bool @@ -370,16 +377,16 @@ func TestDebugf(t *testing.T) { func TestInfof(t *testing.T) { logPath := setupLogger(t, InfoLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Infof(ctx, "Infof message: %s", "test") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "info", - "userID": "12345", - "message": "Infof message: test", + "level": "info", + "subscriber_id": "12345", + "message": "Infof message: test", } var found bool @@ -400,16 +407,16 @@ func TestInfof(t *testing.T) { func TestWarnf(t *testing.T) { logPath := setupLogger(t, WarnLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") Warnf(ctx, "Warnf message: %s", "test") lines := readLogFile(t, logPath) if len(lines) == 0 { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "warn", - "userID": "12345", - "message": "Warnf message: test", + "level": "warn", + "subscriber_id": "12345", + "message": "Warnf message: test", } var found bool @@ -430,7 +437,7 @@ func TestWarnf(t *testing.T) { func TestErrorf(t *testing.T) { logPath := setupLogger(t, ErrorLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") err := fmt.Errorf("error message") Errorf(ctx, err, "Errorf message: %s", "test") lines := readLogFile(t, logPath) @@ -438,10 +445,10 @@ func TestErrorf(t *testing.T) { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "error", - "userID": "12345", - "message": "Errorf message: test", - "error": "error message", + "level": "error", + "subscriber_id": "12345", + "message": "Errorf message: test", + "error": "error message", } var found bool @@ -462,7 +469,7 @@ func TestErrorf(t *testing.T) { func TestFatalf(t *testing.T) { logPath := setupLogger(t, FatalLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") err := fmt.Errorf("fatal error") Fatalf(ctx, err, "Fatalf message: %s", "test") lines := readLogFile(t, logPath) @@ -470,10 +477,10 @@ func TestFatalf(t *testing.T) { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "fatal", - "userID": "12345", - "message": "Fatalf message: test", - "error": "fatal error", + "level": "fatal", + "subscriber_id": "12345", + "message": "Fatalf message: test", + "error": "fatal error", } var found bool @@ -494,7 +501,7 @@ func TestFatalf(t *testing.T) { func TestPanicf(t *testing.T) { logPath := setupLogger(t, PanicLevel) - ctx := context.WithValue(context.Background(), userID, "12345") + ctx := context.WithValue(context.Background(), model.ContextKeySubscriberID, "12345") err := fmt.Errorf("panic error") Panicf(ctx, err, "Panicf message: %s", "test") lines := readLogFile(t, logPath) @@ -503,10 +510,10 @@ func TestPanicf(t *testing.T) { t.Fatal("No logs were written.") } expected := map[string]interface{}{ - "level": "panic", - "userID": "12345", - "message": "Panicf message: test", - "error": "panic error", + "level": "panic", + "subscriber_id": "12345", + "message": "Panicf message: test", + "error": "panic error", } var found bool diff --git a/pkg/model/error_test.go b/pkg/model/error_test.go index ee295e6..519b54a 100644 --- a/pkg/model/error_test.go +++ b/pkg/model/error_test.go @@ -3,8 +3,8 @@ package model import ( "errors" "fmt" - "testing" "net/http" + "testing" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" @@ -198,3 +198,54 @@ func TestSchemaValidationErr_BecknError_NoErrors(t *testing.T) { t.Errorf("beErr.Code = %s, want %s", beErr.Code, expectedCode) } } + +func TestParseContextKey_ValidKeys(t *testing.T) { + tests := []struct { + input string + expected ContextKey + }{ + {"message_id", ContextKeyMsgID}, + {"subscriber_id", ContextKeySubscriberID}, + {"model_id", ContextKeyModelID}, + } + + for _, tt := range tests { + key, err := ParseContextKey(tt.input) + if err != nil { + t.Errorf("unexpected error for input %s: %v", tt.input, err) + } + if key != tt.expected { + t.Errorf("expected %s, got %s", tt.expected, key) + } + } +} + +func TestParseContextKey_InvalidKey(t *testing.T) { + _, err := ParseContextKey("invalid_key") + if err == nil { + t.Error("expected error for invalid context key, got nil") + } +} + +func TestContextKey_UnmarshalYAML_Valid(t *testing.T) { + yamlData := []byte("message_id") + var key ContextKey + + err := yaml.Unmarshal(yamlData, &key) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if key != ContextKeyMsgID { + t.Errorf("expected %s, got %s", ContextKeyMsgID, key) + } +} + +func TestContextKey_UnmarshalYAML_Invalid(t *testing.T) { + yamlData := []byte("invalid_key") + var key ContextKey + + err := yaml.Unmarshal(yamlData, &key) + if err == nil { + t.Error("expected error for invalid context key, got nil") + } +} diff --git a/pkg/model/model.go b/pkg/model/model.go index 13802f4..7d39eb1 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -42,6 +42,9 @@ const ( type ContextKey string const ( + // ContextKeyTxnID is the context key used to store and retrieve the transaction ID in a request context. + ContextKeyTxnID ContextKey = "transaction_id" + // ContextKeyMsgID is the context key used to store and retrieve the message ID in a request context. ContextKeyMsgID ContextKey = "message_id" @@ -52,6 +55,38 @@ const ( ContextKeyModelID ContextKey = "model_id" ) +var contextKeys = map[string]ContextKey{ + "message_id": ContextKeyMsgID, + "subscriber_id": ContextKeySubscriberID, + "model_id": ContextKeyModelID, +} + +// ParseContextKey converts a string into a valid ContextKey. +func ParseContextKey(v string) (ContextKey, error) { + key, ok := contextKeys[v] + if !ok { + return "", fmt.Errorf("invalid context key: %s", key) + } + return key, nil +} + +// UnmarshalYAML ensures that only known context keys are accepted during YAML unmarshalling. +func (k *ContextKey) UnmarshalYAML(unmarshal func(interface{}) error) error { + var keyStr string + if err := unmarshal(&keyStr); err != nil { + return err + } + + parsedKey, err := ParseContextKey(keyStr) + if err != nil { + return err + } + + *k = parsedKey + return nil + +} + // Role defines the type of participant in the network. type Role string diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index a7f4842..7ca2901 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -15,7 +15,8 @@ import ( // Config represents the configuration for the request preprocessor middleware. type Config struct { - Role string + Role string + ContextKeys []string } const contextKey = "context" @@ -57,7 +58,12 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { log.Debugf(ctx, "adding subscriberId to request:%s, %v", model.ContextKeySubscriberID, subID) ctx = context.WithValue(ctx, model.ContextKeySubscriberID, subID) } - + for _, key := range cfg.ContextKeys { + ctxKey, _ := model.ParseContextKey(key) + if v, ok := reqContext[key]; ok { + ctx = context.WithValue(ctx, ctxKey, v) + } + } r.Body = io.NopCloser(bytes.NewBuffer(body)) r.ContentLength = int64(len(body)) r = r.WithContext(ctx) @@ -74,5 +80,11 @@ func validateConfig(cfg *Config) error { if cfg.Role != "bap" && cfg.Role != "bpp" { return errors.New("role must be either 'bap' or 'bpp'") } + + for _, key := range cfg.ContextKeys { + if _, err := model.ParseContextKey(key); err != nil { + return err + } + } return nil } diff --git a/pkg/plugin/implementation/router/router_test.go b/pkg/plugin/implementation/router/router_test.go index 8ee777e..d0bb271 100644 --- a/pkg/plugin/implementation/router/router_test.go +++ b/pkg/plugin/implementation/router/router_test.go @@ -296,7 +296,7 @@ func TestValidateRulesFailure(t *testing.T) { Endpoints: []string{"search"}, }, }, - wantErr: `invalid URI htp:// invalid-url.com in request body for url: invalid URL - htp:// invalid-url.com: parse "htp:// invalid-url.com": invalid character " " in host name`, + wantErr: `invalid URL - htp:// invalid-url.com: parse "htp:// invalid-url.com": invalid character " " in host name`, }, { name: "Missing topic_id for targetType: publisher", @@ -321,12 +321,12 @@ func TestValidateRulesFailure(t *testing.T) { Version: "1.0.0", TargetType: "bpp", Target: target{ - URL: "htp://invalid-url.com", // Invalid URL + URL: "htp:// invalid-url.com", // Invalid URL }, Endpoints: []string{"search"}, }, }, - wantErr: "invalid URI htp://invalid-url.com in request body for bpp: URL 'htp://invalid-url.com' must use https scheme", + wantErr: `invalid URL - htp:// invalid-url.com defined in routing config for target type bpp: parse "htp:// invalid-url.com": invalid character " " in host name`, }, { name: "Invalid URL for BAP targetType", @@ -336,12 +336,12 @@ func TestValidateRulesFailure(t *testing.T) { Version: "1.0.0", TargetType: "bap", Target: target{ - URL: "http://[invalid].com", // Invalid host + URL: "http:// [invalid].com", // Invalid host }, Endpoints: []string{"search"}, }, }, - wantErr: "invalid URI http://[invalid].com in request body for bap", + wantErr: `invalid URL - http:// [invalid].com defined in routing config for target type bap: parse "http:// [invalid].com": invalid character " " in host name`, }, } @@ -464,8 +464,8 @@ func TestRouteFailure(t *testing.T) { name: "Invalid bpp_uri format in request", configFile: "bap_caller.yaml", url: "https://example.com/v1/ondc/select", - body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0", "bpp_uri": "htp://invalid-url"}}`, // Invalid scheme (htp instead of http) - wantErr: "invalid BPP URI - htp://invalid-url in request body for select: URL 'htp://invalid-url' must use https scheme", + body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0", "bpp_uri": "htp:// invalid-url"}}`, // Invalid scheme (htp instead of http) + wantErr: `invalid BPP URI - htp:// invalid-url in request body for select: parse "htp:// invalid-url": invalid character " " in host name`, }, } From c70ff1084374f847751a5721826d183555c0a2ee Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Tue, 8 Apr 2025 15:05:07 +0530 Subject: [PATCH 10/15] update on the review comments 1. change ContextKeyModelId to ContextKeyModuleId 2. added ContextKeyTxnId to the constants. --- pkg/log/log_test.go | 2 +- pkg/model/error_test.go | 3 ++- pkg/model/model.go | 13 +++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 8fa4eac..7646cb7 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -69,7 +69,7 @@ func setupLogger(t *testing.T, l level) string { model.ContextKeyTxnID, model.ContextKeyMsgID, model.ContextKeySubscriberID, - model.ContextKeyModelID, + model.ContextKeyModuleID, }, } diff --git a/pkg/model/error_test.go b/pkg/model/error_test.go index 519b54a..1ac952e 100644 --- a/pkg/model/error_test.go +++ b/pkg/model/error_test.go @@ -204,9 +204,10 @@ func TestParseContextKey_ValidKeys(t *testing.T) { input string expected ContextKey }{ + {"transaction_id", ContextKeyTxnID}, {"message_id", ContextKeyMsgID}, {"subscriber_id", ContextKeySubscriberID}, - {"model_id", ContextKeyModelID}, + {"module_id", ContextKeyModuleID}, } for _, tt := range tests { diff --git a/pkg/model/model.go b/pkg/model/model.go index 7d39eb1..a91e2a3 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -51,21 +51,22 @@ const ( // ContextKeySubscriberID is the context key used to store and retrieve the subscriber ID in a request context. ContextKeySubscriberID ContextKey = "subscriber_id" - // ContextKeyModelID is the context key for storing and retrieving the model ID from a request context. - ContextKeyModelID ContextKey = "model_id" + // ContextKeyModuleID is the context key for storing and retrieving the model ID from a request context. + ContextKeyModuleID ContextKey = "module_id" ) var contextKeys = map[string]ContextKey{ - "message_id": ContextKeyMsgID, - "subscriber_id": ContextKeySubscriberID, - "model_id": ContextKeyModelID, + "transaction_id": ContextKeyTxnID, + "message_id": ContextKeyMsgID, + "subscriber_id": ContextKeySubscriberID, + "module_id": ContextKeyModuleID, } // ParseContextKey converts a string into a valid ContextKey. func ParseContextKey(v string) (ContextKey, error) { key, ok := contextKeys[v] if !ok { - return "", fmt.Errorf("invalid context key: %s", key) + return "", fmt.Errorf("invalid context key: %s", v) } return key, nil } From 0f9ca0cd62413c2b23c57401a44f3d95ecc47c21 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Tue, 8 Apr 2025 16:35:50 +0530 Subject: [PATCH 11/15] resolved linting issue --- pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 7ca2901..72f1a28 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -19,7 +19,7 @@ type Config struct { ContextKeys []string } -const contextKey = "context" +const contextKeyField = "context" // NewPreProcessor returns a middleware that processes the incoming request, // extracts the context field from the body, and adds relevant values (like subscriber ID). @@ -44,7 +44,7 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { // 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) + http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKeyField), http.StatusBadRequest) return } var subID any From e50dd96a3420aa0d558e2004aecf8e822767e83a Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Wed, 9 Apr 2025 08:21:32 +0530 Subject: [PATCH 12/15] Update reqpreprocessor.go --- pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go index 72f1a28..7ca2901 100644 --- a/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go +++ b/pkg/plugin/implementation/reqpreprocessor/reqpreprocessor.go @@ -19,7 +19,7 @@ type Config struct { ContextKeys []string } -const contextKeyField = "context" +const contextKey = "context" // NewPreProcessor returns a middleware that processes the incoming request, // extracts the context field from the body, and adds relevant values (like subscriber ID). @@ -44,7 +44,7 @@ func NewPreProcessor(cfg *Config) (func(http.Handler) http.Handler, error) { // Extract context from request. reqContext, ok := req["context"].(map[string]interface{}) if !ok { - http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKeyField), http.StatusBadRequest) + http.Error(w, fmt.Sprintf("%s field not found or invalid.", contextKey), http.StatusBadRequest) return } var subID any From cfdf589f43c9d7cb59359ce7f14ebf84e04fd6e5 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Wed, 9 Apr 2025 08:25:05 +0530 Subject: [PATCH 13/15] Resolved linitng issue --- core/module/module.go | 2 +- core/module/module_test.go | 2 +- pkg/model/model.go | 4 ---- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/core/module/module.go b/core/module/module.go index 4641fcb..2a28e62 100644 --- a/core/module/module.go +++ b/core/module/module.go @@ -76,7 +76,7 @@ func addMiddleware(ctx context.Context, mgr handler.PluginManager, handler http. func moduleCtxMiddleware(moduleName string, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := context.WithValue(r.Context(), model.ContextKeyModuleId, moduleName) + ctx := context.WithValue(r.Context(), model.ContextKeyModuleID, moduleName) next.ServeHTTP(w, r.WithContext(ctx)) }) } diff --git a/core/module/module_test.go b/core/module/module_test.go index 6091fc1..ffeaafe 100644 --- a/core/module/module_test.go +++ b/core/module/module_test.go @@ -107,7 +107,7 @@ func TestRegisterSuccess(t *testing.T) { // Create a handler that extracts context var capturedModuleName any testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedModuleName = r.Context().Value(model.ContextKeyModuleId) + capturedModuleName = r.Context().Value(model.ContextKeyModuleID) w.WriteHeader(http.StatusOK) }) diff --git a/pkg/model/model.go b/pkg/model/model.go index 1193dee..a91e2a3 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -16,10 +16,6 @@ type Subscriber struct { Domain string `json:"domain"` } -type ContextKey string - -const ContextKeyModuleId ContextKey = "module_id" - // Subscription represents subscription details of a network participant. type Subscription struct { Subscriber `json:",inline"` From eeaa857963049e4053cd50a96a1238b9fa8686e8 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Wed, 9 Apr 2025 08:26:45 +0530 Subject: [PATCH 14/15] resolved linting issue --- pkg/log/log_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 7646cb7..2b93d73 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -20,7 +20,6 @@ import ( type ctxKey any var requestID ctxKey = "requestID" -var userID ctxKey = "userID" const testLogFilePath = "./test_logs/test.log" From 5432e94d6c2334d99ef972f124ea03da41fd3f01 Mon Sep 17 00:00:00 2001 From: MohitKatare-protean Date: Wed, 9 Apr 2025 19:27:03 +0530 Subject: [PATCH 15/15] response.go change for log debug --- pkg/response/response.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/response/response.go b/pkg/response/response.go index f9ac9d5..0ced3de 100644 --- a/pkg/response/response.go +++ b/pkg/response/response.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" + "github.com/beckn/beckn-onix/pkg/log" "github.com/beckn/beckn-onix/pkg/model" ) @@ -47,7 +48,7 @@ func nack(ctx context.Context, w http.ResponseWriter, err *model.Error, status i w.WriteHeader(status) _, er := w.Write(data) if er != nil { - fmt.Printf("Error writing response: %v, MessageID: %s", er, ctx.Value(model.ContextKeyMsgID)) + log.Debugf(ctx, "Error writing response: %v, MessageID: %s", er, ctx.Value(model.ContextKeyMsgID)) http.Error(w, fmt.Sprintf("Internal server error, MessageID: %s", ctx.Value(model.ContextKeyMsgID)), http.StatusInternalServerError) return }