diff --git a/mdl-examples/bug-tests/263-nested-caption-preservation.mdl b/mdl-examples/bug-tests/263-nested-caption-preservation.mdl new file mode 100644 index 00000000..115ddad5 --- /dev/null +++ b/mdl-examples/bug-tests/263-nested-caption-preservation.mdl @@ -0,0 +1,45 @@ +-- ============================================================================ +-- Bug #263: Preserve decision/loop captions across nested control flow +-- ============================================================================ +-- +-- Symptom (before fix): +-- `@caption` on an outer IF/LOOP/WHILE was being overwritten by the inner +-- IF/LOOP's caption because `pendingAnnotations` was shared mutable state +-- across recursive addStatement calls. Annotations attached to the outer +-- split ended up bound to the inner split, and the outer split inherited +-- whatever caption the inner statement carried. +-- +-- After fix: +-- addIfStatement / addLoopStatement / addWhileStatement snapshot + clear +-- `pendingAnnotations` before recursing, then re-apply to their own activity +-- after it's created. The WHILE case also gained explicit handling in +-- mergeStatementAnnotations (previously fell through to `default: nil`). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/263-nested-caption-preservation.mdl -p app.mpr +-- Open in Studio Pro — each split/loop displays its own caption, not +-- inherited from a nested statement. +-- ============================================================================ + +create module BugTest263; + +create microflow BugTest263.MF_NestedCaptions ( + $S: string +) +returns boolean as $ok +begin + declare $ok boolean = false; + + @caption 'String not empty?' + if $S != empty then + @caption 'Right format?' + if isMatch($S, 'x') then + return true; + else + return false; + end if; + else + return false; + end if; +end; +/ diff --git a/mdl/backend/microflow.go b/mdl/backend/microflow.go index 1b788720..c839eb02 100644 --- a/mdl/backend/microflow.go +++ b/mdl/backend/microflow.go @@ -26,4 +26,10 @@ type MicroflowBackend interface { UpdateNanoflow(nf *microflows.Nanoflow) error DeleteNanoflow(id model.ID) error MoveNanoflow(nf *microflows.Nanoflow) error + + // IsRule reports whether the given qualified name refers to a rule + // (Microflows$Rule) rather than a microflow. The flow builder uses this + // to decide whether an IF condition that looks like a function call + // (Module.Name(...)) should be serialized as a RuleSplitCondition. + IsRule(qualifiedName string) (bool, error) } diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index 0a30eed9..2e07a1e1 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -88,6 +88,7 @@ type MockBackend struct { UpdateNanoflowFunc func(nf *microflows.Nanoflow) error DeleteNanoflowFunc func(id model.ID) error MoveNanoflowFunc func(nf *microflows.Nanoflow) error + IsRuleFunc func(qualifiedName string) (bool, error) // PageBackend ListPagesFunc func() ([]*pages.Page, error) diff --git a/mdl/backend/mock/mock_microflow.go b/mdl/backend/mock/mock_microflow.go index 3e9d5784..f6b77fe7 100644 --- a/mdl/backend/mock/mock_microflow.go +++ b/mdl/backend/mock/mock_microflow.go @@ -97,3 +97,10 @@ func (m *MockBackend) MoveNanoflow(nf *microflows.Nanoflow) error { } return nil } + +func (m *MockBackend) IsRule(qualifiedName string) (bool, error) { + if m.IsRuleFunc != nil { + return m.IsRuleFunc(qualifiedName) + } + return false, nil +} diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index 48e5426b..43a7bd10 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -221,6 +221,9 @@ func (b *MprBackend) DeleteMicroflow(id model.ID) error { return b.writer.Delete func (b *MprBackend) MoveMicroflow(mf *microflows.Microflow) error { return b.writer.MoveMicroflow(mf) } +func (b *MprBackend) IsRule(qualifiedName string) (bool, error) { + return b.reader.IsRule(qualifiedName) +} func (b *MprBackend) ListNanoflows() ([]*microflows.Nanoflow, error) { return b.reader.ListNanoflows() diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 238e4c12..acfb1436 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -9,6 +9,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" ) @@ -170,3 +171,103 @@ func (fb *flowBuilder) resolvePathSegments(path []string) []string { } return resolved } + +// buildSplitCondition constructs the right SplitCondition variant for an IF +// statement. When the condition is a qualified call into a rule, it emits a +// RuleSplitCondition (nested RuleCall with ParameterMappings). Everything else +// falls back to ExpressionSplitCondition. +// +// Studio Pro enforces this distinction: a rule reference stored as an +// expression fails validation with CE0117, which is the regression this +// helper prevents on describe → exec roundtrips. +func (fb *flowBuilder) buildSplitCondition(expr ast.Expression, fallbackExpression string) microflows.SplitCondition { + if ruleCond := fb.tryBuildRuleSplitCondition(expr); ruleCond != nil { + return ruleCond + } + return µflows.ExpressionSplitCondition{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + Expression: fallbackExpression, + } +} + +// tryBuildRuleSplitCondition returns a RuleSplitCondition when the expression +// is a qualified function call that resolves to a rule via the backend. +// Returns nil if the expression isn't a qualified call, if the backend is +// unavailable, or if the name doesn't resolve to a rule. +func (fb *flowBuilder) tryBuildRuleSplitCondition(expr ast.Expression) *microflows.RuleSplitCondition { + if fb.backend == nil { + return nil + } + call := unwrapParenCall(expr) + if call == nil { + return nil + } + // Only qualified names (Module.Name) can refer to rules; bare identifiers + // are built-ins (length, contains, etc.). + if !strings.Contains(call.Name, ".") { + return nil + } + isRule, err := fb.backend.IsRule(call.Name) + if err != nil || !isRule { + return nil + } + + cond := µflows.RuleSplitCondition{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + RuleQualifiedName: call.Name, + } + for _, arg := range call.Arguments { + name, value := extractNamedArg(arg) + if name == "" { + // Positional arguments aren't representable in RuleCall — skip + // rather than fabricate a parameter mapping that Studio Pro + // would reject. + continue + } + cond.ParameterMappings = append(cond.ParameterMappings, µflows.RuleCallParameterMapping{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + ParameterName: call.Name + "." + name, + Argument: fb.exprToString(value), + }) + } + return cond +} + +// unwrapParenCall peels outer ParenExprs and returns the inner FunctionCallExpr +// if present. Describer output wraps rule calls in parens when they sit inside +// boolean expressions, so we must see through them. +func unwrapParenCall(expr ast.Expression) *ast.FunctionCallExpr { + for { + switch e := expr.(type) { + case *ast.FunctionCallExpr: + return e + case *ast.ParenExpr: + expr = e.Inner + default: + return nil + } + } +} + +// extractNamedArg recognises `Name = value` BinaryExprs and returns the +// parameter name + value. Anything else returns "", nil. +// +// The left side of a named-arg expression can surface as either an +// IdentifierExpr (bare parameter name) or an AttributePathExpr with an empty +// Variable — both forms come out of the visitor depending on surrounding +// context, so handle them both. +func extractNamedArg(expr ast.Expression) (string, ast.Expression) { + bin, ok := expr.(*ast.BinaryExpr) + if !ok || bin.Operator != "=" { + return "", nil + } + switch left := bin.Left.(type) { + case *ast.IdentifierExpr: + return left.Name, bin.Right + case *ast.AttributePathExpr: + if left.Variable == "" && len(left.Path) == 1 { + return left.Path[0], bin.Right + } + } + return "", nil +} diff --git a/mdl/executor/cmd_microflows_builder_annotations.go b/mdl/executor/cmd_microflows_builder_annotations.go index ed9b0e32..f652374f 100644 --- a/mdl/executor/cmd_microflows_builder_annotations.go +++ b/mdl/executor/cmd_microflows_builder_annotations.go @@ -37,6 +37,8 @@ func getStatementAnnotations(stmt ast.MicroflowStatement) *ast.ActivityAnnotatio return s.Annotations case *ast.LoopStmt: return s.Annotations + case *ast.WhileStmt: + return s.Annotations case *ast.LogStmt: return s.Annotations case *ast.CallMicroflowStmt: @@ -116,8 +118,8 @@ func (fb *flowBuilder) applyAnnotations(activityID model.ID, ann *ast.ActivityAn continue } - // @caption, @color, and @excluded — only applicable to ActionActivity - if activity, ok := obj.(*microflows.ActionActivity); ok { + switch activity := obj.(type) { + case *microflows.ActionActivity: if ann.Caption != "" { activity.Caption = ann.Caption activity.AutoGenerateCaption = false @@ -128,6 +130,22 @@ func (fb *flowBuilder) applyAnnotations(activityID model.ID, ann *ast.ActivityAn if ann.Excluded { activity.Disabled = true } + case *microflows.ExclusiveSplit: + // Splits carry a human-readable Caption (e.g. "Right format?") + // independent of the expression/rule being evaluated. + if ann.Caption != "" { + activity.Caption = ann.Caption + } + case *microflows.InheritanceSplit: + if ann.Caption != "" { + activity.Caption = ann.Caption + } + case *microflows.LoopedActivity: + // LOOP / WHILE activities can carry a caption just like + // splits and action activities. + if ann.Caption != "" { + activity.Caption = ann.Caption + } } break diff --git a/mdl/executor/cmd_microflows_builder_annotations_test.go b/mdl/executor/cmd_microflows_builder_annotations_test.go new file mode 100644 index 00000000..bd471d8d --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_annotations_test.go @@ -0,0 +1,312 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// TestNestedIfPreservesCaptions is a regression test for the bug where +// the outer IF's @caption would be overwritten by the inner IF's @caption +// because pendingAnnotations is shared mutable state across recursive +// addStatement calls. +// +// Before the fix: +// - outer ExclusiveSplit received caption "Right format?" (from inner IF) +// - inner ExclusiveSplit kept its condition expression as caption +// - inner IF's @annotation got attached to the outer split +// +// After the fix: +// - addIfStatement consumes its own pendingAnnotations right after +// creating its split, so outer and inner captions stay bound to the +// correct splits. +func TestNestedIfPreservesCaptions(t *testing.T) { + // Build an AST equivalent to: + // if $S != empty @caption 'String not empty?' + // if isMatch($S, 'x') @caption 'Right format?' + // return true + // else + // return false + // else + // return false + innerIf := &ast.IfStmt{ + Condition: &ast.FunctionCallExpr{ + Name: "isMatch", + Arguments: []ast.Expression{&ast.VariableExpr{Name: "S"}, &ast.LiteralExpr{Value: "x", Kind: ast.LiteralString}}, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{Caption: "Right format?"}, + } + outerIf := &ast.IfStmt{ + Condition: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "S"}, + Operator: "!=", + Right: &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull}, + }, + ThenBody: []ast.MicroflowStatement{innerIf}, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{Caption: "String not empty?"}, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"S": "String"}, + declaredVars: map[string]string{"S": "String"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{outerIf}, nil) + + // Collect ExclusiveSplits with their captions. The outer split is created + // first, so objects[1] is the outer split (objects[0] is the StartEvent). + var splits []*microflows.ExclusiveSplit + for _, obj := range fb.objects { + if sp, ok := obj.(*microflows.ExclusiveSplit); ok { + splits = append(splits, sp) + } + } + + if len(splits) != 2 { + t.Fatalf("expected 2 ExclusiveSplits, got %d", len(splits)) + } + + // Splits are appended in creation order: outer first (from outerIf), + // then inner (when recursion into ThenBody creates the nested IF's split). + outerSplit, innerSplit := splits[0], splits[1] + + if outerSplit.Caption != "String not empty?" { + t.Errorf("outer split caption: got %q, want %q", outerSplit.Caption, "String not empty?") + } + if innerSplit.Caption != "Right format?" { + t.Errorf("inner split caption: got %q, want %q", innerSplit.Caption, "Right format?") + } +} + +// TestIfCaptionWithoutNesting confirms a single IF with @caption still gets +// the right caption after the fix (baseline sanity check). +func TestIfCaptionWithoutNesting(t *testing.T) { + ifStmt := &ast.IfStmt{ + Condition: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "S"}, + Operator: "!=", + Right: &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull}, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{Caption: "String not empty?"}, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"S": "String"}, + declaredVars: map[string]string{"S": "String"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ifStmt}, nil) + + for _, obj := range fb.objects { + if sp, ok := obj.(*microflows.ExclusiveSplit); ok { + if sp.Caption != "String not empty?" { + t.Errorf("split caption: got %q, want %q", sp.Caption, "String not empty?") + } + return + } + } + t.Fatal("no ExclusiveSplit found") +} + +// TestIfAnnotationStaysWithCorrectSplit confirms @annotation on a nested IF +// attaches to that IF's split, not to the outer one. +func TestIfAnnotationStaysWithCorrectSplit(t *testing.T) { + innerIf := &ast.IfStmt{ + Condition: &ast.FunctionCallExpr{ + Name: "isMatch", + Arguments: []ast.Expression{&ast.VariableExpr{Name: "S"}, &ast.LiteralExpr{Value: "x", Kind: ast.LiteralString}}, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{ + Caption: "Right format?", + AnnotationText: "Inner IF note", + }, + } + outerIf := &ast.IfStmt{ + Condition: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "S"}, + Operator: "!=", + Right: &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull}, + }, + ThenBody: []ast.MicroflowStatement{innerIf}, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{ + Caption: "String not empty?", + AnnotationText: "Outer IF note", + }, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"S": "String"}, + declaredVars: map[string]string{"S": "String"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{outerIf}, nil) + + var splits []*microflows.ExclusiveSplit + var annotations []*microflows.Annotation + for _, obj := range fb.objects { + switch o := obj.(type) { + case *microflows.ExclusiveSplit: + splits = append(splits, o) + case *microflows.Annotation: + annotations = append(annotations, o) + } + } + + if len(splits) != 2 { + t.Fatalf("expected 2 splits, got %d", len(splits)) + } + if len(annotations) != 2 { + t.Fatalf("expected 2 annotations, got %d", len(annotations)) + } + + outerSplit, innerSplit := splits[0], splits[1] + + // AnnotationFlow links Annotation -> activity. Verify each flow points + // from the annotation with the expected text to the expected split. + var outerNoteDestID, innerNoteDestID string + for _, af := range fb.annotationFlows { + // Find the Annotation referenced by OriginID + for _, ann := range annotations { + if ann.ID != af.OriginID { + continue + } + switch ann.Caption { + case "Outer IF note": + outerNoteDestID = string(af.DestinationID) + case "Inner IF note": + innerNoteDestID = string(af.DestinationID) + } + } + } + + if outerNoteDestID != string(outerSplit.ID) { + t.Errorf("outer note destination: got %q, want %q (outer split)", outerNoteDestID, outerSplit.ID) + } + if innerNoteDestID != string(innerSplit.ID) { + t.Errorf("inner note destination: got %q, want %q (inner split)", innerNoteDestID, innerSplit.ID) + } +} + +// TestLoopCaptionPreserved covers the loop caption case — previously untested +// per PR review. The fix for the outer-IF caption contamination bug also applied +// the same snapshot/restore pattern to addLoopStatement and addWhileStatement. +func TestLoopCaptionPreserved(t *testing.T) { + innerReturn := &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}} + loop := &ast.LoopStmt{ + LoopVariable: "item", + ListVariable: "items", + Body: []ast.MicroflowStatement{innerReturn}, + Annotations: &ast.ActivityAnnotations{Caption: "Process each item"}, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"items": "List of MyMod.Item"}, + declaredVars: map[string]string{"items": "List of MyMod.Item"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{loop}, nil) + + var loops []*microflows.LoopedActivity + for _, obj := range fb.objects { + if l, ok := obj.(*microflows.LoopedActivity); ok { + loops = append(loops, l) + } + } + + if len(loops) != 1 { + t.Fatalf("expected 1 LoopedActivity, got %d", len(loops)) + } + if loops[0].Caption != "Process each item" { + t.Errorf("loop caption: got %q, want %q", loops[0].Caption, "Process each item") + } +} + +// TestWhileLoopCaptionPreserved — same coverage for the WHILE shape. +func TestWhileLoopCaptionPreserved(t *testing.T) { + whileStmt := &ast.WhileStmt{ + Condition: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "n"}, + Operator: "<", + Right: &ast.LiteralExpr{Value: int64(10), Kind: ast.LiteralInteger}, + }, + Body: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + Annotations: &ast.ActivityAnnotations{Caption: "Until n >= 10"}, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"n": "Integer"}, + declaredVars: map[string]string{"n": "Integer"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{whileStmt}, nil) + + var loops []*microflows.LoopedActivity + for _, obj := range fb.objects { + if l, ok := obj.(*microflows.LoopedActivity); ok { + loops = append(loops, l) + } + } + + if len(loops) != 1 { + t.Fatalf("expected 1 LoopedActivity (WHILE), got %d", len(loops)) + } + if loops[0].Caption != "Until n >= 10" { + t.Errorf("while caption: got %q, want %q", loops[0].Caption, "Until n >= 10") + } +} + +// TestInheritanceSplitCaptionApplied — InheritanceSplit is not produced by the +// executor builder (only parsed from BSON for roundtrip), but applyAnnotations +// gained an InheritanceSplit case in the fix. Test the applicator directly. +func TestInheritanceSplitCaptionApplied(t *testing.T) { + split := µflows.InheritanceSplit{} + split.ID = "inh-split-1" + + fb := &flowBuilder{} + fb.objects = append(fb.objects, split) + + fb.applyAnnotations(split.ID, &ast.ActivityAnnotations{Caption: "Customer type?"}) + + if split.Caption != "Customer type?" { + t.Errorf("inheritance split caption: got %q, want %q", split.Caption, "Customer type?") + } +} diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go index 8f4d20cd..ff9c16da 100644 --- a/mdl/executor/cmd_microflows_builder_control.go +++ b/mdl/executor/cmd_microflows_builder_control.go @@ -43,11 +43,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { splitX := fb.posX centerY := fb.posY // This is the center line for the happy path - // Create ExclusiveSplit with expression condition - splitCondition := µflows.ExpressionSplitCondition{ - BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - Expression: fb.exprToString(s.Condition), - } + // Decide whether the IF condition is a rule call or a plain expression. + // A rule-based split must be serialized as Microflows$RuleSplitCondition; + // emitting ExpressionSplitCondition for a rule call causes Studio Pro to + // raise CE0117 "Error(s) in expression". + caption := fb.exprToString(s.Condition) + splitCondition := fb.buildSplitCondition(s.Condition, caption) split := µflows.ExclusiveSplit{ BaseMicroflowObject: microflows.BaseMicroflowObject{ @@ -55,13 +56,22 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { Position: model.Point{X: splitX, Y: centerY}, Size: model.Size{Width: SplitWidth, Height: SplitHeight}, }, - Caption: fb.exprToString(s.Condition), + Caption: caption, SplitCondition: splitCondition, ErrorHandlingType: microflows.ErrorHandlingTypeRollback, } fb.objects = append(fb.objects, split) splitID := split.ID + // Apply this IF's own annotations (e.g. @caption, @annotation) to the split + // BEFORE recursing into branch bodies. Otherwise a nested statement's annotations + // would overwrite fb.pendingAnnotations (shared state) and the outer loop in + // buildFlowGraph would then attach the wrong caption/annotation to this split. + if fb.pendingAnnotations != nil { + fb.applyAnnotations(splitID, fb.pendingAnnotations) + fb.pendingAnnotations = nil + } + // Calculate merge position (after the longest branch) mergeX := splitX + SplitWidth + HorizontalSpacing/2 + branchWidth + HorizontalSpacing/2 @@ -251,6 +261,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { // addLoopStatement creates a LOOP statement using LoopedActivity. // Layout: Auto-sizes the loop box to fit content with padding func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { + // Snapshot & clear this loop's own annotations so the body's recursive + // addStatement calls can't consume them. We re-apply to the loop activity + // after it's created below. + savedLoopAnnotations := fb.pendingAnnotations + fb.pendingAnnotations = nil + // First, measure the loop body to determine size bodyBounds := fb.measurer.measureStatements(s.Body) @@ -335,6 +351,11 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { // This is how Mendix stores them - all flows at the microflow level fb.flows = append(fb.flows, loopBuilder.flows...) + // Re-apply this loop's own annotations now that its activity exists. + if savedLoopAnnotations != nil { + fb.applyAnnotations(loop.ID, savedLoopAnnotations) + } + fb.posX += loopWidth + HorizontalSpacing return loop.ID @@ -343,6 +364,11 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { // addWhileStatement creates a WHILE loop using LoopedActivity with WhileLoopCondition. // Layout matches addLoopStatement but without iterator icon space. func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID { + // Snapshot & clear this WHILE's own annotations so the body's recursive + // addStatement calls can't consume them (see addLoopStatement). + savedWhileAnnotations := fb.pendingAnnotations + fb.pendingAnnotations = nil + bodyBounds := fb.measurer.measureStatements(s.Body) loopWidth := max(bodyBounds.Width+2*LoopPadding, MinLoopWidth) @@ -402,6 +428,11 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID { fb.objects = append(fb.objects, loop) fb.flows = append(fb.flows, loopBuilder.flows...) + + if savedWhileAnnotations != nil { + fb.applyAnnotations(loop.ID, savedWhileAnnotations) + } + fb.posX += loopWidth + HorizontalSpacing return loop.ID diff --git a/mdl/executor/cmd_microflows_builder_rule_split_test.go b/mdl/executor/cmd_microflows_builder_rule_split_test.go new file mode 100644 index 00000000..c4730511 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_rule_split_test.go @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// TestIfWithRuleCall_EmitsRuleSplitCondition is the regression test for the +// Rule vs Expression subtype preservation bug. Prior to the fix, an IF whose +// condition was a call into a rule (e.g. ControlCenterCommons.IsNotEmptyString) +// was serialized as ExpressionSplitCondition, causing Mendix Studio Pro to +// raise CE0117 "Error(s) in expression" and demoting the decision's subtype +// from Rule to Expression on every describe → exec roundtrip. +func TestIfWithRuleCall_EmitsRuleSplitCondition(t *testing.T) { + mb := &mock.MockBackend{ + IsRuleFunc: func(qualifiedName string) (bool, error) { + return qualifiedName == "Module.IsEligible", nil + }, + } + + // if Module.IsEligible(Customer = $Customer) then return true else return false + ifStmt := &ast.IfStmt{ + Condition: &ast.FunctionCallExpr{ + Name: "Module.IsEligible", + Arguments: []ast.Expression{ + &ast.BinaryExpr{ + Left: &ast.IdentifierExpr{Name: "Customer"}, + Operator: "=", + Right: &ast.VariableExpr{Name: "Customer"}, + }, + }, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}}, + }, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + backend: mb, + varTypes: map[string]string{"Customer": "Module.Customer"}, + declaredVars: map[string]string{"Customer": "Module.Customer"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ifStmt}, nil) + + var split *microflows.ExclusiveSplit + for _, obj := range fb.objects { + if sp, ok := obj.(*microflows.ExclusiveSplit); ok { + split = sp + break + } + } + if split == nil { + t.Fatal("expected an ExclusiveSplit, found none") + } + + rule, ok := split.SplitCondition.(*microflows.RuleSplitCondition) + if !ok { + t.Fatalf("split condition: got %T, want *microflows.RuleSplitCondition", split.SplitCondition) + } + if rule.RuleQualifiedName != "Module.IsEligible" { + t.Errorf("rule name: got %q, want %q", rule.RuleQualifiedName, "Module.IsEligible") + } + if len(rule.ParameterMappings) != 1 { + t.Fatalf("parameter mappings: got %d, want 1", len(rule.ParameterMappings)) + } + pm := rule.ParameterMappings[0] + if pm.ParameterName != "Module.IsEligible.Customer" { + t.Errorf("parameter name: got %q, want %q", pm.ParameterName, "Module.IsEligible.Customer") + } + if pm.Argument != "$Customer" { + t.Errorf("argument: got %q, want %q", pm.Argument, "$Customer") + } +} + +// TestIfWithNonRuleCall_EmitsExpressionSplitCondition confirms that a plain +// expression-level function call (built-in or sub-microflow, not a rule) still +// produces an ExpressionSplitCondition — the fix must not over-trigger. +func TestIfWithNonRuleCall_EmitsExpressionSplitCondition(t *testing.T) { + mb := &mock.MockBackend{ + IsRuleFunc: func(qualifiedName string) (bool, error) { + return false, nil + }, + } + + ifStmt := &ast.IfStmt{ + Condition: &ast.FunctionCallExpr{ + Name: "empty", + Arguments: []ast.Expression{&ast.VariableExpr{Name: "S"}}, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + backend: mb, + varTypes: map[string]string{"S": "String"}, + declaredVars: map[string]string{"S": "String"}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ifStmt}, nil) + + var split *microflows.ExclusiveSplit + for _, obj := range fb.objects { + if sp, ok := obj.(*microflows.ExclusiveSplit); ok { + split = sp + break + } + } + if split == nil { + t.Fatal("expected an ExclusiveSplit, found none") + } + if _, ok := split.SplitCondition.(*microflows.ExpressionSplitCondition); !ok { + t.Fatalf("split condition: got %T, want *microflows.ExpressionSplitCondition", split.SplitCondition) + } +} + +// TestIfWithoutBackend_FallsBackToExpression confirms that when the flow +// builder has no backend (e.g. disconnected check mode), it can't tell whether +// a qualified call is a rule — it must default to ExpressionSplitCondition so +// that syntax-only checks don't crash. +func TestIfWithoutBackend_FallsBackToExpression(t *testing.T) { + ifStmt := &ast.IfStmt{ + Condition: &ast.FunctionCallExpr{ + Name: "Module.IsEligible", + Arguments: []ast.Expression{}, + }, + ThenBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}, + }, + } + + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{}, + declaredVars: map[string]string{}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ifStmt}, nil) + + for _, obj := range fb.objects { + if sp, ok := obj.(*microflows.ExclusiveSplit); ok { + if _, ok := sp.SplitCondition.(*microflows.ExpressionSplitCondition); !ok { + t.Fatalf("without backend, split condition: got %T, want *microflows.ExpressionSplitCondition", sp.SplitCondition) + } + return + } + } + t.Fatal("expected an ExclusiveSplit, found none") +} diff --git a/sdk/microflows/microflows.go b/sdk/microflows/microflows.go index 8bec5638..91ed69a3 100644 --- a/sdk/microflows/microflows.go +++ b/sdk/microflows/microflows.go @@ -290,19 +290,24 @@ type ExpressionSplitCondition struct { func (ExpressionSplitCondition) isSplitCondition() {} // RuleSplitCondition represents a rule-based split condition. +// In Mendix BSON the rule is referenced by qualified name under the +// "RuleCall.Microflow" field (rules and microflows share a namespace). type RuleSplitCondition struct { model.BaseElement - RuleID model.ID `json:"ruleId"` + RuleQualifiedName string `json:"ruleQualifiedName,omitempty"` ParameterMappings []*RuleCallParameterMapping `json:"parameterMappings,omitempty"` } func (RuleSplitCondition) isSplitCondition() {} // RuleCallParameterMapping maps a parameter to a value. +// ParameterName is the fully qualified parameter name (e.g. "Module.Rule.ParamName") +// as stored in BSON; used when the describer renders the rule call. type RuleCallParameterMapping struct { model.BaseElement - ParameterID model.ID `json:"parameterId"` - Argument string `json:"argument"` + ParameterID model.ID `json:"parameterId"` + ParameterName string `json:"parameterName,omitempty"` + Argument string `json:"argument"` } // LoopSource is the source for a LoopedActivity. Either IterableList (FOR EACH) or WhileLoopCondition (WHILE). diff --git a/sdk/mpr/parser_microflow.go b/sdk/mpr/parser_microflow.go index d4f58c98..f93aa01b 100644 --- a/sdk/mpr/parser_microflow.go +++ b/sdk/mpr/parser_microflow.go @@ -456,20 +456,26 @@ func parseSplitCondition(raw map[string]any) microflows.SplitCondition { Expression: extractString(raw["Expression"]), } case "Microflows$RuleSplitCondition": - cond := µflows.RuleSplitCondition{ - RuleID: model.ID(extractBsonID(raw["Rule"])), + cond := µflows.RuleSplitCondition{} + // Mendix nests the rule reference under a RuleCall sub-document whose + // "Microflow" field holds the rule's qualified name (rules share the + // microflow namespace). Parameter mappings are scoped inside RuleCall too. + rcSource := raw + if rc := extractBsonMap(raw["RuleCall"]); rc != nil { + cond.RuleQualifiedName = extractString(rc["Microflow"]) + rcSource = rc } - // Parse parameter mappings if present - if mappings, ok := raw["ParameterMappings"].([]any); ok { - for _, m := range mappings { - if mMap, ok := m.(map[string]any); ok { - mapping := µflows.RuleCallParameterMapping{ - ParameterID: model.ID(extractBsonID(mMap["Parameter"])), - Argument: extractString(mMap["Argument"]), - } - cond.ParameterMappings = append(cond.ParameterMappings, mapping) - } + for _, m := range extractBsonArray(rcSource["ParameterMappings"]) { + mMap := extractBsonMap(m) + if mMap == nil { + continue + } + mapping := µflows.RuleCallParameterMapping{ + ParameterID: model.ID(extractBsonID(mMap["Parameter"])), + ParameterName: extractString(mMap["Parameter"]), + Argument: extractString(mMap["Argument"]), } + cond.ParameterMappings = append(cond.ParameterMappings, mapping) } return cond default: diff --git a/sdk/mpr/reader_documents.go b/sdk/mpr/reader_documents.go index ef96d741..0e24117f 100644 --- a/sdk/mpr/reader_documents.go +++ b/sdk/mpr/reader_documents.go @@ -216,6 +216,55 @@ func (r *Reader) GetMicroflow(id model.ID) (*microflows.Microflow, error) { return nil, fmt.Errorf("microflow not found: %s", id) } +// IsRule reports whether the given qualified name refers to a rule +// (Microflows$Rule). Rules share the microflow namespace but are stored +// under a distinct BSON type — the flow-builder needs this distinction so +// it can emit RuleSplitCondition instead of ExpressionSplitCondition for +// rule-based IF statements. +func (r *Reader) IsRule(qualifiedName string) (bool, error) { + if qualifiedName == "" { + return false, nil + } + units, err := r.listUnitsByType("Microflows$Rule") + if err != nil { + return false, err + } + if len(units) == 0 { + return false, nil + } + modules, err := r.ListModules() + if err != nil { + return false, err + } + moduleMap := make(map[string]string, len(modules)) + for _, m := range modules { + moduleMap[string(m.ID)] = m.Name + } + containerParent, err := r.buildContainerParent() + if err != nil { + return false, err + } + for _, u := range units { + var raw map[string]any + if err := bson.Unmarshal(u.Contents, &raw); err != nil { + continue + } + name, _ := raw["Name"].(string) + if name == "" { + continue + } + moduleName := resolveModuleName(u.ContainerID, moduleMap, containerParent) + fullName := name + if moduleName != "" { + fullName = moduleName + "." + name + } + if fullName == qualifiedName { + return true, nil + } + } + return false, nil +} + // ListNanoflows returns all nanoflows in the project. func (r *Reader) ListNanoflows() ([]*microflows.Nanoflow, error) { units, err := r.listUnitsByType("Microflows$Nanoflow") diff --git a/sdk/mpr/writer_microflow.go b/sdk/mpr/writer_microflow.go index 957a1515..07f54fec 100644 --- a/sdk/mpr/writer_microflow.go +++ b/sdk/mpr/writer_microflow.go @@ -463,6 +463,37 @@ func serializeMicroflowObject(obj microflows.MicroflowObject) bson.D { {Key: "$Type", Value: "Microflows$ExpressionSplitCondition"}, {Key: "Expression", Value: sc.Expression}, }}) + case *microflows.RuleSplitCondition: + // Mendix nests the rule reference under a RuleCall sub-document + // whose Microflow field holds the rule's qualified name + // (rules share the microflow namespace). ParameterMappings are + // scoped inside RuleCall too — see parser_microflow.go. + ruleCall := bson.D{ + {Key: "$ID", Value: idToBsonBinary(generateUUID())}, + {Key: "$Type", Value: "Microflows$RuleCall"}, + {Key: "Microflow", Value: sc.RuleQualifiedName}, + } + if len(sc.ParameterMappings) > 0 { + var mappings bson.A + mappings = append(mappings, int32(2)) // Array marker + for _, pm := range sc.ParameterMappings { + mapping := bson.D{ + {Key: "$ID", Value: idToBsonBinary(string(pm.ID))}, + {Key: "$Type", Value: "Microflows$RuleCallParameterMapping"}, + {Key: "Parameter", Value: pm.ParameterName}, + {Key: "Argument", Value: pm.Argument}, + } + mappings = append(mappings, mapping) + } + ruleCall = append(ruleCall, bson.E{Key: "ParameterMappings", Value: mappings}) + } else { + ruleCall = append(ruleCall, bson.E{Key: "ParameterMappings", Value: bson.A{int32(2)}}) + } + doc = append(doc, bson.E{Key: "SplitCondition", Value: bson.D{ + {Key: "$ID", Value: idToBsonBinary(string(sc.ID))}, + {Key: "$Type", Value: "Microflows$RuleSplitCondition"}, + {Key: "RuleCall", Value: ruleCall}, + }}) } } return doc diff --git a/sdk/mpr/writer_rule_split_test.go b/sdk/mpr/writer_rule_split_test.go new file mode 100644 index 00000000..fab267c0 --- /dev/null +++ b/sdk/mpr/writer_rule_split_test.go @@ -0,0 +1,119 @@ +// 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" +) + +// TestSerializeExclusiveSplit_RuleSplitCondition_Roundtrip verifies that an +// ExclusiveSplit whose SplitCondition is a RuleSplitCondition survives +// serialize → BSON → parse without losing the rule reference or its parameter +// mappings. This is the BSON-level regression guard for the CE0117 Studio Pro +// error that appears when a rule-based decision is stored as an expression. +func TestSerializeExclusiveSplit_RuleSplitCondition_Roundtrip(t *testing.T) { + split := µflows.ExclusiveSplit{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "11111111-1111-1111-1111-111111111111"}, + Position: model.Point{X: 100, Y: 200}, + Size: model.Size{Width: 50, Height: 50}, + }, + Caption: "Module.IsEligible($Customer)", + ErrorHandlingType: microflows.ErrorHandlingTypeRollback, + SplitCondition: µflows.RuleSplitCondition{ + BaseElement: model.BaseElement{ID: "22222222-2222-2222-2222-222222222222"}, + RuleQualifiedName: "Module.IsEligible", + ParameterMappings: []*microflows.RuleCallParameterMapping{ + { + BaseElement: model.BaseElement{ID: "33333333-3333-3333-3333-333333333333"}, + ParameterName: "Module.IsEligible.Customer", + Argument: "$Customer", + }, + }, + }, + } + + doc := serializeMicroflowObject(split) + if doc == nil { + t.Fatal("serializeMicroflowObject returned nil") + } + + data, err := bson.Marshal(doc) + if err != nil { + t.Fatalf("bson.Marshal failed: %v", err) + } + + var raw map[string]any + if err := bson.Unmarshal(data, &raw); err != nil { + t.Fatalf("bson.Unmarshal failed: %v", err) + } + + parsed := parseMicroflowObject(raw) + roundtripSplit, ok := parsed.(*microflows.ExclusiveSplit) + if !ok { + t.Fatalf("parsed object: got %T, want *microflows.ExclusiveSplit", parsed) + } + + ruleCond, ok := roundtripSplit.SplitCondition.(*microflows.RuleSplitCondition) + if !ok { + t.Fatalf("split condition after roundtrip: got %T, want *microflows.RuleSplitCondition", roundtripSplit.SplitCondition) + } + if ruleCond.RuleQualifiedName != "Module.IsEligible" { + t.Errorf("rule qualified name: got %q, want %q", ruleCond.RuleQualifiedName, "Module.IsEligible") + } + if len(ruleCond.ParameterMappings) != 1 { + t.Fatalf("parameter mappings: got %d, want 1", len(ruleCond.ParameterMappings)) + } + pm := ruleCond.ParameterMappings[0] + if pm.ParameterName != "Module.IsEligible.Customer" { + t.Errorf("parameter name: got %q, want %q", pm.ParameterName, "Module.IsEligible.Customer") + } + if pm.Argument != "$Customer" { + t.Errorf("argument: got %q, want %q", pm.Argument, "$Customer") + } +} + +// TestSerializeExclusiveSplit_ExpressionSplitCondition_Roundtrip is the +// complementary baseline that ensures the existing expression path still +// roundtrips correctly after the Rule branch was added to the writer switch. +func TestSerializeExclusiveSplit_ExpressionSplitCondition_Roundtrip(t *testing.T) { + split := µflows.ExclusiveSplit{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "44444444-4444-4444-4444-444444444444"}, + Position: model.Point{X: 100, Y: 200}, + Size: model.Size{Width: 50, Height: 50}, + }, + Caption: "$Var = 'x'", + ErrorHandlingType: microflows.ErrorHandlingTypeRollback, + SplitCondition: µflows.ExpressionSplitCondition{ + BaseElement: model.BaseElement{ID: "55555555-5555-5555-5555-555555555555"}, + Expression: "$Var = 'x'", + }, + } + + doc := serializeMicroflowObject(split) + data, err := bson.Marshal(doc) + if err != nil { + t.Fatalf("bson.Marshal failed: %v", err) + } + + var raw map[string]any + if err := bson.Unmarshal(data, &raw); err != nil { + t.Fatalf("bson.Unmarshal failed: %v", err) + } + + parsed := parseMicroflowObject(raw) + roundtripSplit := parsed.(*microflows.ExclusiveSplit) + + exprCond, ok := roundtripSplit.SplitCondition.(*microflows.ExpressionSplitCondition) + if !ok { + t.Fatalf("split condition after roundtrip: got %T, want *microflows.ExpressionSplitCondition", roundtripSplit.SplitCondition) + } + if exprCond.Expression != "$Var = 'x'" { + t.Errorf("expression: got %q, want %q", exprCond.Expression, "$Var = 'x'") + } +}