Merge pull request #265 from hjotha/submit/microflow-split-subtype-preservation

fix: preserve rule-based decision subtype across microflow roundtrips

Author: ako

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)
 }

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)

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
+}

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()

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 &microflows.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 := &microflows.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, &microflows.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
+}

mdl/executor/cmd_microflows_builder_control.go

@@ -43,19 +43,20 @@ 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 := &microflows.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 := &microflows.ExclusiveSplit{
 		BaseMicroflowObject: microflows.BaseMicroflowObject{
 			BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
 			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,
 	}

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")
+}

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).