Merge pull request #378 from hjotha/submit/builder-rest-httpresponse-result-handling

fix: emit ResultHandlingHttpResponse for REST 'returns response'

Author: ako

mdl-examples/bug-tests/375-rest-httpresponse-result-type.mdl

@@ -0,0 +1,69 @@
+-- ============================================================================
+-- Bug #375: REST HttpResponse result handling serialized as string
+-- ============================================================================
+--
+-- Symptom (before fix):
+--   When a Studio Pro REST call action bound the complete HTTP response
+--   to an output variable, the parser/AST captured this as
+--   `ResultHandlingHttpResponse` and the action-level
+--   `ResultHandlingType` was `HttpResponse`. The writer, however, did
+--   NOT have an explicit branch for `ResultHandlingHttpResponse` inside
+--   `serializeRestResultHandling`, so it fell through to the string
+--   default. The serialized result variable was emitted as
+--   `DataTypes$StringType`, not the expected
+--   `DataTypes$ObjectType` bound to `System.HttpResponse`.
+--   Studio Pro then surfaced a type mismatch on the bound output
+--   variable, and dependent activities that reached into the response
+--   (`$Response/Content`, `$Response/StatusCode`) failed validation
+--   with CE0117.
+--
+-- After fix (writer only):
+--   `serializeRestResultHandling` now has an explicit
+--   `ResultHandlingHttpResponse` branch that emits a `DataTypes$ObjectType`
+--   variable type bound to `System.HttpResponse`.
+--
+-- Scope note — known related gap:
+--   The MDL builder for `rest call ... returns response` still creates
+--   a `ResultHandlingString` (not `ResultHandlingHttpResponse`). See
+--   `mdl/executor/cmd_microflows_builder_calls.go` — the
+--   `case ast.RestResultResponse` arm carries the comment
+--   "we would need a different result type, but using String for now".
+--   That means newly authored MDL with `returns response` does NOT yet
+--   trigger the new writer branch. PR #376 fixes the symptom for
+--   existing Studio Pro models that already contain
+--   `ResultHandlingHttpResponse` in their BSON; the BUILDER side is
+--   tracked separately and verified by the SDK-level Go test
+--   `TestSerializeRestResultHandlingHttpResponseUsesObjectType`, which
+--   constructs the AST directly.
+--
+-- Usage (positive control):
+--   mxcli exec mdl-examples/bug-tests/375-rest-httpresponse-result-type.mdl -p app.mpr
+--   `mx check` against the resulting MPR must report 0 errors. The
+--   negative case (a Studio Pro REST call with HttpResponse binding)
+--   needs the existing BSON shape and is covered by the Go test above.
+-- ============================================================================
+
+create module BugTest375;
+
+-- Inline REST call returning a string. Once the builder learns to emit
+-- ResultHandlingHttpResponse for `returns response`, this script can be
+-- updated to use `returns response` and exercise the writer branch end
+-- to end.
+create microflow BugTest375.MF_RestCall ()
+returns boolean as $Ok
+begin
+  declare $Ok boolean = false;
+
+  $Body = rest call get 'https://httpbin.org/get'
+    header Accept = 'application/json'
+    timeout 15
+    returns string
+    on error continue;
+
+  if $Body != '' then
+    set $Ok = true;
+  end if;
+
+  return $Ok;
+end;
+/

mdl-examples/bug-tests/377-rest-returns-response-builder.mdl

@@ -0,0 +1,67 @@
+-- ============================================================================
+-- Bug #377: Builder emitted ResultHandlingString for `returns response`
+-- ============================================================================
+--
+-- Symptom (before fix):
+--   When MDL authored a REST call with `returns response` (intent: bind
+--   the full HttpResponse object to the output variable), the microflow
+--   builder constructed a `ResultHandlingString` instead of a
+--   `ResultHandlingHttpResponse`. The action-level `ResultHandlingType`
+--   then derived as `String`, and the writer serialized the result
+--   variable as `DataTypes$StringType`. Studio Pro treated the bound
+--   variable as a string, so dependent activities reaching into the
+--   response (`$Response/Content`, `$Response/StatusCode`) failed
+--   validation with CE0117.
+--
+--   The builder file `cmd_microflows_builder_calls.go` carried an
+--   explicit TODO comment for this case:
+--     "we would need a different result type, but using String for now".
+--
+-- After fix:
+--   The `ast.RestResultResponse` arm now constructs a
+--   `ResultHandlingHttpResponse{VariableName: s.OutputVariable}`. The
+--   writer's existing branch (added in PR #376) then emits a bound
+--   `Microflows$ResultHandling` whose `VariableType` is
+--   `DataTypes$ObjectType` with entity `System.HttpResponse`, and the
+--   action-level `ResultHandlingType` is correctly derived as
+--   `HttpResponse`.
+--
+-- Dependency:
+--   This bug-test relies on the writer fix from PR #376 being present
+--   in the same branch — without it, the action-level type is still
+--   `HttpResponse` but the nested ResultHandling falls through to the
+--   string default. This branch is stacked on PR #376 specifically so
+--   the round-trip can be verified end to end.
+--
+-- Usage:
+--   mxcli exec mdl-examples/bug-tests/377-rest-returns-response-builder.mdl -p app.mpr
+--   mxcli -p app.mpr -c "describe microflow BugTest377.MF_GetResponse"
+--   `mx check` against the resulting MPR must report 0 errors and the
+--   `$Response/Content` / `$Response/StatusCode` references must
+--   resolve in Studio Pro.
+-- ============================================================================
+
+create module BugTest377;
+
+-- Inline REST call binding the full HttpResponse to the output variable.
+-- The downstream `if` and `log` reach into the response, so the bound
+-- variable's type must resolve to `System.HttpResponse` for `mx check`
+-- to pass.
+create microflow BugTest377.MF_GetResponse ()
+returns boolean as $Ok
+begin
+  declare $Ok boolean = false;
+
+  $Response = rest call get 'https://httpbin.org/get'
+    header Accept = 'application/json'
+    timeout 15
+    returns response
+    on error continue;
+
+  if $Response != empty and $Response/StatusCode = 200 then
+    set $Ok = true;
+  end if;
+
+  return $Ok;
+end;
+/

mdl/executor/cmd_microflows_builder_calls.go

@@ -729,10 +729,14 @@ func (fb *flowBuilder) addRestCallAction(s *ast.RestCallStmt) model.ID {
 			BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
 		}
 	case ast.RestResultResponse:
-		resultHandling = &microflows.ResultHandlingString{
-			BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
+		// Bind the full HTTP response object to the output variable. The writer
+		// emits the matching `DataTypes$ObjectType` bound to System.HttpResponse;
+		// the action-level `ResultHandlingType` is derived as "HttpResponse" from
+		// this concrete type.
+		resultHandling = &microflows.ResultHandlingHttpResponse{
+			BaseElement:  model.BaseElement{ID: model.ID(types.GenerateID())},
+			VariableName: s.OutputVariable,
 		}
-		// Note: For HttpResponse, we would need a different result type, but using String for now
 	case ast.RestResultMapping:
 		mappingQN := s.Result.MappingName.Module + "." + s.Result.MappingName.Name
 		entityQN := s.Result.ResultEntity.Module + "." + s.Result.ResultEntity.Name

mdl/executor/cmd_microflows_builder_rest_response_test.go

@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// TestAddRestCallAction_ReturnsResponseUsesHttpResponseHandling pins the
+// builder behavior for issue #377: when MDL authors `rest call ... returns
+// response`, the builder must construct a ResultHandlingHttpResponse so the
+// writer's matching branch (PR #376) emits a DataTypes$ObjectType bound to
+// System.HttpResponse, instead of falling back to a string variable.
+func TestAddRestCallAction_ReturnsResponseUsesHttpResponseHandling(t *testing.T) {
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		varTypes:     map[string]string{},
+		declaredVars: map[string]string{},
+		measurer:     &layoutMeasurer{},
+	}
+
+	stmt := &ast.RestCallStmt{
+		OutputVariable: "Response",
+		Method:         ast.HttpMethodGet,
+		URL:            &ast.LiteralExpr{Kind: ast.LiteralString, Value: "https://example.com"},
+		Result:         ast.RestResult{Type: ast.RestResultResponse},
+	}
+	fb.addRestCallAction(stmt)
+
+	if len(fb.objects) == 0 {
+		t.Fatalf("expected one activity, got %d", len(fb.objects))
+	}
+
+	activity, ok := fb.objects[0].(*microflows.ActionActivity)
+	if !ok {
+		t.Fatalf("first object is %T, want *microflows.ActionActivity", fb.objects[0])
+	}
+	action, ok := activity.Action.(*microflows.RestCallAction)
+	if !ok {
+		t.Fatalf("activity.Action is %T, want *microflows.RestCallAction", activity.Action)
+	}
+
+	httpResponse, ok := action.ResultHandling.(*microflows.ResultHandlingHttpResponse)
+	if !ok {
+		t.Fatalf("ResultHandling is %T, want *microflows.ResultHandlingHttpResponse", action.ResultHandling)
+	}
+	if httpResponse.VariableName != "Response" {
+		t.Errorf("VariableName = %q, want %q", httpResponse.VariableName, "Response")
+	}
+}

mdl/executor/cmd_microflows_format_action.go

@@ -937,6 +937,8 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string
 		case *microflows.ResultHandlingString:
 			sb.WriteString("String")
 			_ = rh // used for type assertion only
+		case *microflows.ResultHandlingHttpResponse:
+			sb.WriteString("response")
 		case *microflows.ResultHandlingMapping:
 			sb.WriteString("mapping ")
 			sb.WriteString(string(rh.MappingID))

mdl/executor/cmd_microflows_format_restcall_test.go

@@ -31,6 +31,20 @@ func TestFormatRestCallAction_GET(t *testing.T) {
 	assertContains(t, got, "returns String")
 }
 
+func TestFormatRestCallAction_HttpResponse(t *testing.T) {
+	e := newTestExecutor()
+	action := &microflows.RestCallAction{
+		HttpConfiguration: &microflows.HttpConfiguration{
+			HttpMethod:       microflows.HttpMethodGet,
+			LocationTemplate: "https://api.example.com/orders",
+		},
+		ResultHandling: &microflows.ResultHandlingHttpResponse{VariableName: "Response"},
+	}
+	got := e.formatRestCallAction(action)
+	assertContains(t, got, "$Response = ")
+	assertContains(t, got, "returns response")
+}
+
 func TestFormatRestCallAction_POST_CustomBody(t *testing.T) {
 	e := newTestExecutor()
 	action := &microflows.RestCallAction{

sdk/mpr/writer_microflow_actions.go

@@ -813,6 +813,20 @@ func serializeRestResultHandling(rh microflows.ResultHandling, outputVar string)
 		)
 		return doc
 
+	case *microflows.ResultHandlingHttpResponse:
+		return bson.D{
+			{Key: "$ID", Value: idToBsonBinary(string(h.ID))},
+			{Key: "$Type", Value: "Microflows$ResultHandling"},
+			{Key: "Bind", Value: outputVar != ""},
+			{Key: "ImportMappingCall", Value: nil},
+			{Key: "ResultVariableName", Value: outputVar},
+			{Key: "VariableType", Value: bson.D{
+				{Key: "$ID", Value: idToBsonBinary(GenerateID())},
+				{Key: "$Type", Value: "DataTypes$ObjectType"},
+				{Key: "Entity", Value: "System.HttpResponse"},
+			}},
+		}
+
 	case *microflows.ResultHandlingNone:
 		return bson.D{
 			{Key: "$ID", Value: idToBsonBinary(string(h.ID))},

sdk/mpr/writer_rest_httpresponse_test.go

@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package mpr
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+	"go.mongodb.org/mongo-driver/bson"
+)
+
+func TestSerializeRestResultHandlingHttpResponseUsesObjectType(t *testing.T) {
+	handling := &microflows.ResultHandlingHttpResponse{
+		BaseElement:  model.BaseElement{ID: "result-1"},
+		VariableName: "HttpResponse",
+	}
+
+	doc := serializeRestResultHandling(handling, "HttpResponse")
+
+	if got := getBSONField(doc, "ResultVariableName"); got != "HttpResponse" {
+		t.Fatalf("ResultVariableName = %#v, want HttpResponse", got)
+	}
+	variableType, ok := getBSONField(doc, "VariableType").(bson.D)
+	if !ok {
+		t.Fatalf("VariableType is %T, want bson.D", getBSONField(doc, "VariableType"))
+	}
+	if got := getBSONField(variableType, "$Type"); got != "DataTypes$ObjectType" {
+		t.Fatalf("VariableType.$Type = %#v, want DataTypes$ObjectType", got)
+	}
+	if got := getBSONField(variableType, "Entity"); got != "System.HttpResponse" {
+		t.Fatalf("VariableType.Entity = %#v, want System.HttpResponse", got)
+	}
+}