diff --git a/module/command_handler.go b/module/command_handler.go index a9478514..37b6753d 100644 --- a/module/command_handler.go +++ b/module/command_handler.go @@ -169,7 +169,11 @@ func (h *CommandHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Type-assert to *Pipeline for concrete field access (Metadata, RoutePattern, // Execute) and execution tracker integration. All engine-registered pipelines // are *Pipeline; the interface allows custom implementations in tests/plugins. - if concretePipeline, ok := pipeline.(*Pipeline); ok { + // concretePipeline != nil: real *Pipeline. + // concretePipeline == nil && isConcrete: typed-nil – fall through to delegate/404. + // !isConcrete: different implementation – use PipelineRunner.Run() fallback. + concretePipeline, isConcrete := pipeline.(*Pipeline) + if isConcrete && concretePipeline != nil { // Inject HTTP context so delegate steps can forward directly concretePipeline.Metadata = map[string]any{ "_http_request": r, @@ -201,20 +205,26 @@ func (h *CommandHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, "failed to encode response", http.StatusInternalServerError) } return - } - // Fallback for non-*Pipeline implementations: use the PipelineRunner interface. - result, err := pipeline.Run(r.Context(), triggerData) - if err != nil { + } else if !isConcrete { + // Fallback for non-*Pipeline implementations: use the PipelineRunner interface. + result, err := pipeline.Run(r.Context(), triggerData) + if err != nil { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) + return + } + // Allow the runner to signal that it has already written the response. + if result["_response_handled"] == true { + return + } w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusInternalServerError) - _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) + if err := json.NewEncoder(w).Encode(result); err != nil { + http.Error(w, "failed to encode response", http.StatusInternalServerError) + } return } - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(result); err != nil { - http.Error(w, "failed to encode response", http.StatusInternalServerError) - } - return + // typed-nil *Pipeline: fall through to delegate/404 handling. } if h.delegateHandler != nil { diff --git a/module/command_handler_test.go b/module/command_handler_test.go index 1abee706..7819aad4 100644 --- a/module/command_handler_test.go +++ b/module/command_handler_test.go @@ -4,11 +4,26 @@ import ( "context" "encoding/json" "errors" + "log/slog" "net/http" "net/http/httptest" "testing" + + "github.com/GoCodeAlone/workflow/interfaces" ) +// mockPipelineRunner is a minimal PipelineRunner for handler tests. +type mockPipelineRunner struct { + result map[string]any + err error +} + +func (m *mockPipelineRunner) Run(_ context.Context, _ map[string]any) (map[string]any, error) { + return m.result, m.err +} +func (m *mockPipelineRunner) SetLogger(_ *slog.Logger) {} +func (m *mockPipelineRunner) SetEventRecorder(_ interfaces.EventRecorder) {} + func TestCommandHandler_Name(t *testing.T) { h := NewCommandHandler("test-commands") if h.Name() != "test-commands" { @@ -196,3 +211,87 @@ func TestCommandHandler_Handle(t *testing.T) { t.Errorf("expected 200, got %d", rr.Code) } } + +// TestCommandHandler_RoutePipeline_MockRunner verifies that a non-*Pipeline +// PipelineRunner is invoked via Run() and its result is JSON-encoded. +func TestCommandHandler_RoutePipeline_MockRunner(t *testing.T) { + h := NewCommandHandler("test") + mock := &mockPipelineRunner{result: map[string]any{"status": "processed"}} + h.routePipelines["process"] = mock + + req := httptest.NewRequest("POST", "/api/v1/engine/process", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("expected 200, got %d", rr.Code) + } + var got map[string]any + if err := json.NewDecoder(rr.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if got["status"] != "processed" { + t.Errorf("expected status=processed, got %v", got) + } +} + +// TestCommandHandler_RoutePipeline_MockRunner_ResponseHandled verifies that +// when the PipelineRunner.Run result contains _response_handled=true the +// handler does not write an additional JSON body. +func TestCommandHandler_RoutePipeline_MockRunner_ResponseHandled(t *testing.T) { + h := NewCommandHandler("test") + mock := &mockPipelineRunner{result: map[string]any{"_response_handled": true}} + h.routePipelines["process"] = mock + + req := httptest.NewRequest("POST", "/api/v1/engine/process", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Body.Len() != 0 { + t.Errorf("expected empty body when _response_handled=true, got %q", rr.Body.String()) + } +} + +// TestCommandHandler_RoutePipeline_MockRunner_Error verifies that a Run() error +// returns a 500 with the error message in the JSON body. +func TestCommandHandler_RoutePipeline_MockRunner_Error(t *testing.T) { + h := NewCommandHandler("test") + mock := &mockPipelineRunner{err: errors.New("runner failed")} + h.routePipelines["process"] = mock + + req := httptest.NewRequest("POST", "/api/v1/engine/process", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", rr.Code) + } + var got map[string]string + if err := json.NewDecoder(rr.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode error response: %v", err) + } + if got["error"] != "runner failed" { + t.Errorf("expected error=runner failed, got %v", got) + } +} + +// TestCommandHandler_RoutePipeline_TypedNil verifies that a typed-nil *Pipeline +// stored as a PipelineRunner does not panic and falls through to 404. +func TestCommandHandler_RoutePipeline_TypedNil(t *testing.T) { + h := NewCommandHandler("test") + // Store a typed-nil *Pipeline as an interfaces.PipelineRunner. + // pipeline != nil is true (interface has type info), concretePipeline == nil. + var p *Pipeline + h.routePipelines["process"] = p + + req := httptest.NewRequest("POST", "/api/v1/engine/process", nil) + rr := httptest.NewRecorder() + + // Must not panic. + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusNotFound { + t.Errorf("expected 404 for typed-nil pipeline, got %d", rr.Code) + } +} + diff --git a/module/query_handler.go b/module/query_handler.go index a98d103e..5ca04e4c 100644 --- a/module/query_handler.go +++ b/module/query_handler.go @@ -154,7 +154,11 @@ func (h *QueryHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Type-assert to *Pipeline for concrete field access (Metadata, RoutePattern, // Execute) and execution tracker integration. All engine-registered pipelines // are *Pipeline; the interface allows custom implementations in tests/plugins. - if concretePipeline, ok := pipeline.(*Pipeline); ok { + // concretePipeline != nil: real *Pipeline. + // concretePipeline == nil && isConcrete: typed-nil – fall through to delegate/404. + // !isConcrete: different implementation – use PipelineRunner.Run() fallback. + concretePipeline, isConcrete := pipeline.(*Pipeline) + if isConcrete && concretePipeline != nil { // Inject HTTP context so delegate steps can forward directly concretePipeline.Metadata = map[string]any{ "_http_request": r, @@ -188,20 +192,26 @@ func (h *QueryHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, "failed to encode response", http.StatusInternalServerError) } return - } - // Fallback for non-*Pipeline implementations: use the PipelineRunner interface. - result, err := pipeline.Run(r.Context(), triggerData) - if err != nil { + } else if !isConcrete { + // Fallback for non-*Pipeline implementations: use the PipelineRunner interface. + result, err := pipeline.Run(r.Context(), triggerData) + if err != nil { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) + return + } + // Allow the runner to signal that it has already written the response. + if result["_response_handled"] == true { + return + } w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusInternalServerError) - _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) + if err := json.NewEncoder(w).Encode(result); err != nil { + http.Error(w, "failed to encode response", http.StatusInternalServerError) + } return } - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(result); err != nil { - http.Error(w, "failed to encode response", http.StatusInternalServerError) - } - return + // typed-nil *Pipeline: fall through to delegate/404 handling. } if h.delegateHandler != nil { diff --git a/module/query_handler_test.go b/module/query_handler_test.go index ee6ed36b..a8243538 100644 --- a/module/query_handler_test.go +++ b/module/query_handler_test.go @@ -216,3 +216,87 @@ func TestLastPathSegment(t *testing.T) { } } } + +// TestQueryHandler_RoutePipeline_MockRunner verifies that a non-*Pipeline +// PipelineRunner is invoked via Run() and its result is JSON-encoded. +func TestQueryHandler_RoutePipeline_MockRunner(t *testing.T) { + h := NewQueryHandler("test") + mock := &mockPipelineRunner{result: map[string]any{"data": "value"}} + h.routePipelines["report"] = mock + + req := httptest.NewRequest("GET", "/api/v1/engine/report", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("expected 200, got %d", rr.Code) + } + var got map[string]any + if err := json.NewDecoder(rr.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if got["data"] != "value" { + t.Errorf("expected data=value, got %v", got) + } +} + +// TestQueryHandler_RoutePipeline_MockRunner_ResponseHandled verifies that +// when the PipelineRunner.Run result contains _response_handled=true the +// handler does not write an additional JSON body. +func TestQueryHandler_RoutePipeline_MockRunner_ResponseHandled(t *testing.T) { + h := NewQueryHandler("test") + mock := &mockPipelineRunner{result: map[string]any{"_response_handled": true}} + h.routePipelines["report"] = mock + + req := httptest.NewRequest("GET", "/api/v1/engine/report", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Body.Len() != 0 { + t.Errorf("expected empty body when _response_handled=true, got %q", rr.Body.String()) + } +} + +// TestQueryHandler_RoutePipeline_MockRunner_Error verifies that a Run() error +// returns a 500 with the error message in the JSON body. +func TestQueryHandler_RoutePipeline_MockRunner_Error(t *testing.T) { + h := NewQueryHandler("test") + mock := &mockPipelineRunner{err: errors.New("runner failed")} + h.routePipelines["report"] = mock + + req := httptest.NewRequest("GET", "/api/v1/engine/report", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", rr.Code) + } + var got map[string]string + if err := json.NewDecoder(rr.Body).Decode(&got); err != nil { + t.Fatalf("failed to decode error response: %v", err) + } + if got["error"] != "runner failed" { + t.Errorf("expected error=runner failed, got %v", got) + } +} + +// TestQueryHandler_RoutePipeline_TypedNil verifies that a typed-nil *Pipeline +// stored as a PipelineRunner does not panic and falls through to 404. +func TestQueryHandler_RoutePipeline_TypedNil(t *testing.T) { + h := NewQueryHandler("test") + // Store a typed-nil *Pipeline as an interfaces.PipelineRunner. + // pipeline != nil is true (interface has type info), concretePipeline == nil. + var p *Pipeline + h.routePipelines["report"] = p + + req := httptest.NewRequest("GET", "/api/v1/engine/report", nil) + rr := httptest.NewRecorder() + + // Must not panic. + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusNotFound { + t.Errorf("expected 404 for typed-nil pipeline, got %d", rr.Code) + } +} +