diff --git a/mdl-examples/doctype-tests/20-openapi-import-examples.mdl b/mdl-examples/doctype-tests/20-openapi-import-examples.mdl index 10d3e9c8..daa532a1 100644 --- a/mdl-examples/doctype-tests/20-openapi-import-examples.mdl +++ b/mdl-examples/doctype-tests/20-openapi-import-examples.mdl @@ -17,9 +17,10 @@ describe contract operation from openapi 'https://petstore3.swagger.io/api/v3/op create module OpenApiTest; --- Minimal: spec drives everything (operations, BaseUrl, auth) +-- Minimal: spec drives operations and auth; BaseUrl set explicitly (spec uses relative /api/v3) create or modify rest client OpenApiTest.PetStoreMinimal ( - OpenAPI: 'https://petstore3.swagger.io/api/v3/openapi.json' + OpenAPI: 'https://petstore3.swagger.io/api/v3/openapi.json', + BaseUrl: 'https://petstore3.swagger.io/api/v3' ); -- With BaseUrl override: replaces servers[0].url from the spec diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 21200f44..6c646abb 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -455,8 +455,23 @@ func traverseFlow( return } - // Check if this is a merge point - if so, don't process it here (it will be handled by the split) + // Merge points are normally processed by the matching split's traversal — + // returning here avoids emitting an `end if;` twice. But a merge can also + // appear as a pure junction (multiple incoming flows converging outside + // of an IF), e.g. a manual retry loop where a "Call REST → decide → loop + // back" pattern routes the back-edge into a merge. Those merges are not + // values in splitMergeMap, and stopping there drops every activity after + // them (see issue #281). When that happens, walk through as a pass- + // through — same as traverseFlowUntilMerge already does for intermediate + // merges. if _, isMerge := obj.(*microflows.ExclusiveMerge); isMerge { + if isMergePairedWithSplit(currentID, splitMergeMap) { + return + } + visited[currentID] = true + for _, flow := range flowsByOrigin[currentID] { + traverseFlow(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) + } return } @@ -877,6 +892,24 @@ func emitLoopBody( } } +// isMergePairedWithSplit reports whether an ExclusiveMerge appears as the +// matching end-of-branch point for some ExclusiveSplit recorded in +// splitMergeMap (i.e., it closes an IF/ELSE block). Merges that aren't paired +// — e.g. a junction used as the loop-back target of a manual retry pattern — +// must be traversed as pass-through, otherwise every activity after them is +// dropped from describe output (issue #281). +// +// splitMergeMap is split-ID → merge-ID, so the merge is paired iff it appears +// as a value in the map. +func isMergePairedWithSplit(mergeID model.ID, splitMergeMap map[model.ID]model.ID) bool { + for _, v := range splitMergeMap { + if v == mergeID { + return true + } + } + return false +} + // findBranchFlows separates flows from a split into TRUE and FALSE branches based on CaseValue. // Returns (trueFlow, falseFlow). Either may be nil if the branch doesn't exist. func findBranchFlows(flows []*microflows.SequenceFlow) (trueFlow, falseFlow *microflows.SequenceFlow) { diff --git a/mdl/executor/cmd_microflows_unpaired_merge_test.go b/mdl/executor/cmd_microflows_unpaired_merge_test.go new file mode 100644 index 00000000..58d5f973 --- /dev/null +++ b/mdl/executor/cmd_microflows_unpaired_merge_test.go @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Regression tests for issue #281: DESCRIBE MICROFLOW silently truncated the +// output at an ExclusiveMerge used as a pure junction (e.g., the loop-back +// target of a manual retry-loop pattern, visible in the screenshot attached +// to the issue). The describer used to early-return on every merge, but +// merges that aren't the matching end point of an IF/ELSE block must be +// traversed as pass-through or their successors are dropped. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +func TestTraverseFlow_UnpairedMergeIsPassThrough(t *testing.T) { + // Flow graph: start → act1 → merge → act2 → end. + // The merge has no matching split (splitMergeMap is empty), so it's a + // pure junction, not the end of an IF/ELSE. + e := newTestExecutor() + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("act1"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("act1")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "before merge"}}, + }, + }, + mkID("merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("merge")}, + mkID("act2"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("act2")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "after merge"}}, + }, + }, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "act1")}, + mkID("act1"): {mkFlow("act1", "merge")}, + mkID("merge"): {mkFlow("merge", "act2")}, + mkID("act2"): {mkFlow("act2", "end")}, + } + + var lines []string + visited := make(map[model.ID]bool) + // Empty splitMergeMap — the merge isn't paired with any split. + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, visited, nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + if !strings.Contains(out, "before merge") { + t.Errorf("missing `before merge` activity (should always be emitted):\n%s", out) + } + if !strings.Contains(out, "after merge") { + t.Errorf("issue #281: `after merge` activity was dropped — describer stopped at unpaired merge:\n%s", out) + } + // The merge itself must not produce an `end if;` here — it's a + // junction, not the closing bracket of an IF/ELSE. + if strings.Contains(out, "end if;") { + t.Errorf("unpaired merge must not emit `end if;`:\n%s", out) + } +} + +func TestTraverseFlow_PairedMergeStillClosesIfElse(t *testing.T) { + // Regression guard for the partnered case: a merge that IS paired with + // a split must still be handled by the split's branch logic (not by the + // new pass-through path). Builds a standard IF/ELSE: + // + // start → split(true→thenAct, false→elseAct) → merge → end + e := newTestExecutor() + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$x > 0"}, + }, + mkID("thenAct"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("thenAct")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "yes"}}, + }, + }, + mkID("elseAct"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("elseAct")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "no"}}, + }, + }, + mkID("merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("merge")}, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "split")}, + mkID("split"): {mkBranchFlow("split", "thenAct", microflows.EnumerationCase{Value: "true"}), mkBranchFlow("split", "elseAct", microflows.EnumerationCase{Value: "false"})}, + mkID("thenAct"): {mkFlow("thenAct", "merge")}, + mkID("elseAct"): {mkFlow("elseAct", "merge")}, + mkID("merge"): {mkFlow("merge", "end")}, + } + + splitMergeMap := map[model.ID]model.ID{ + mkID("split"): mkID("merge"), + } + + var lines []string + visited := make(map[model.ID]bool) + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, splitMergeMap, visited, nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + if !strings.Contains(out, "if $x > 0 then") { + t.Errorf("missing IF header:\n%s", out) + } + if !strings.Contains(out, "else") { + t.Errorf("missing else branch:\n%s", out) + } + if !strings.Contains(out, "end if;") { + t.Errorf("missing `end if;` — paired merge must still close the IF:\n%s", out) + } + // The merge should produce exactly one `end if;`, not two. + if count := strings.Count(out, "end if;"); count != 1 { + t.Errorf("expected exactly one `end if;`, got %d:\n%s", count, out) + } +} + +func TestTraverseFlow_ManualRetryLoopPatternEmitsEverything(t *testing.T) { + // Closer reproduction of the user's microflow from issue #281. The retry + // pattern wires a back-edge from a "change retry count + delay" branch + // into a merge that sits between the setup activities and the REST call, + // so control re-enters the REST call every retry iteration. + // + // start → setup → merge → call → decide + // ↑ ↓ (retry) + // delay ← change ←──┘ + // + // Before the fix the describer emitted setup and stopped at the merge, + // dropping call, decide, change and delay — exactly what the issue + // reported. + e := newTestExecutor() + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("setup"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("setup")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "setup"}}, + }, + }, + mkID("merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("merge")}, + mkID("call"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("call")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "call-rest"}}, + }, + }, + mkID("decide"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("decide"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$retry"}, + }, + mkID("change"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("change")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "change-retry-count"}}, + }, + }, + mkID("delay"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("delay")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "delay"}}, + }, + }, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "setup")}, + mkID("setup"): {mkFlow("setup", "merge")}, + mkID("merge"): {mkFlow("merge", "call")}, + mkID("call"): {mkFlow("call", "decide")}, + mkID("decide"): {mkBranchFlow("decide", "change", microflows.EnumerationCase{Value: "true"}), mkBranchFlow("decide", "end", microflows.EnumerationCase{Value: "false"})}, + mkID("change"): {mkFlow("change", "delay")}, + mkID("delay"): {mkFlow("delay", "merge")}, + } + + var lines []string + visited := make(map[model.ID]bool) + // The `decide` split has no merge target (both branches leave the loop), + // so splitMergeMap is empty. + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, visited, nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + for _, want := range []string{"setup", "call-rest", "if $retry then", "change-retry-count", "delay"} { + if !strings.Contains(out, want) { + t.Errorf("issue #281: missing %q from describe output:\n%s", want, out) + } + } +} diff --git a/mdl/openapi/parser.go b/mdl/openapi/parser.go index 1cf3e10a..7f051c51 100644 --- a/mdl/openapi/parser.go +++ b/mdl/openapi/parser.go @@ -210,10 +210,16 @@ func ToRestClientModel(spec *Spec, serviceName string, baseUrlOverride string) ( } var warnings []string - // BaseUrl + // BaseUrl — only use absolute URLs from the spec; relative ones (e.g. "/api/v3") + // are rejected by Studio Pro CE7247 and must be overridden by the caller. baseURL := baseUrlOverride if baseURL == "" && len(spec.Servers) > 0 { - baseURL = spec.Servers[0].URL + u := spec.Servers[0].URL + if strings.HasPrefix(u, "http://") || strings.HasPrefix(u, "https://") { + baseURL = u + } else { + warnings = append(warnings, fmt.Sprintf("server URL %q is relative and cannot be used as BaseUrl; set BaseUrl explicitly in CREATE REST CLIENT", u)) + } } svc.BaseUrl = baseURL