Skip to content

Commit bcbe34e

Browse files
hjothahjothamendix
authored andcommitted
fix: harden microflow describe roundtrip
MDL text produced by `describe microflow` was failing to re-parse on several shapes: multiline captions/messages lost their newlines, tabs were silently dropped when unquoting strings, and loop positions anchored with `@position` drifted by half a loop width per roundtrip. Centralise string escaping in an mdlQuote helper that doubles single quotes and encodes `\n`, `\r`, `\t`, and `\\`. Switch every call site (`cmd_microflows_show.go`, `cmd_microflows_show_helpers.go`, `cmd_microflows_format_action.go`, `cmd_microflows_helpers.go`, `cmd_pages_describe_output.go`) to use it. Mirror the encoding in the visitor by rewriting `unquoteString` to walk the input and decode each escape explicitly instead of doing naive sequential `ReplaceAll` calls, which mangled `\\n` and similar sequences. Loops now compute `loopLeftX`/`loopCenterX` up front and use the annotated `@position` when set, then advance `fb.posX` past the actual left edge of the loop instead of the stale pre-annotation position. `addWhileStatement` gets the same treatment. Regression tests cover: - `emitObjectAnnotations` emitting escaped `@caption` / `@annotation` - `formatAction` for `ShowMessage` and `LogMessage` with multiline text - `unquoteString` decoding `\n`, `\t`, `\\`, and doubled quotes - `addLoopStatement` / `addWhileStatement` honouring `@position` and leaving `fb.posX` past the loop's right edge
1 parent 5720eb0 commit bcbe34e

11 files changed

Lines changed: 227 additions & 38 deletions

mdl/executor/bugfix_regression_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,81 @@ func TestFormatActivity_WhileLoop(t *testing.T) {
6565
}
6666
}
6767

68+
func TestAddLoopStatement_PreservesAnnotatedPosition(t *testing.T) {
69+
fb := &flowBuilder{
70+
posX: 350,
71+
posY: 200,
72+
spacing: HorizontalSpacing,
73+
varTypes: map[string]string{"Items": "List of Test.Item"},
74+
declaredVars: map[string]string{},
75+
measurer: &layoutMeasurer{varTypes: map[string]string{"Items": "List of Test.Item"}},
76+
}
77+
78+
stmt := &ast.LoopStmt{
79+
LoopVariable: "Item",
80+
ListVariable: "Items",
81+
Annotations: &ast.ActivityAnnotations{
82+
Position: &ast.Position{X: 350, Y: 200},
83+
},
84+
}
85+
86+
id := fb.addLoopStatement(stmt)
87+
if id == "" {
88+
t.Fatal("expected loop activity ID")
89+
}
90+
91+
loop, ok := fb.objects[len(fb.objects)-1].(*microflows.LoopedActivity)
92+
if !ok {
93+
t.Fatalf("expected LoopedActivity, got %T", fb.objects[len(fb.objects)-1])
94+
}
95+
if loop.Position.X != 350 || loop.Position.Y != 200 {
96+
t.Fatalf("got loop position (%d, %d), want (350, 200)", loop.Position.X, loop.Position.Y)
97+
}
98+
wantNextX := 350 + loop.Size.Width/2 + HorizontalSpacing
99+
if fb.posX != wantNextX {
100+
t.Fatalf("got next posX %d, want %d", fb.posX, wantNextX)
101+
}
102+
}
103+
104+
func TestAddWhileStatement_PreservesAnnotatedPosition(t *testing.T) {
105+
fb := &flowBuilder{
106+
posX: 420,
107+
posY: 180,
108+
spacing: HorizontalSpacing,
109+
varTypes: map[string]string{},
110+
declaredVars: map[string]string{},
111+
measurer: &layoutMeasurer{varTypes: map[string]string{}},
112+
}
113+
114+
stmt := &ast.WhileStmt{
115+
Condition: &ast.BinaryExpr{
116+
Left: &ast.VariableExpr{Name: "Count"},
117+
Operator: "<",
118+
Right: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: int64(10)},
119+
},
120+
Annotations: &ast.ActivityAnnotations{
121+
Position: &ast.Position{X: 420, Y: 180},
122+
},
123+
}
124+
125+
id := fb.addWhileStatement(stmt)
126+
if id == "" {
127+
t.Fatal("expected while activity ID")
128+
}
129+
130+
loop, ok := fb.objects[len(fb.objects)-1].(*microflows.LoopedActivity)
131+
if !ok {
132+
t.Fatalf("expected LoopedActivity, got %T", fb.objects[len(fb.objects)-1])
133+
}
134+
if loop.Position.X != 420 || loop.Position.Y != 180 {
135+
t.Fatalf("got while position (%d, %d), want (420, 180)", loop.Position.X, loop.Position.Y)
136+
}
137+
wantNextX := 420 + loop.Size.Width/2 + HorizontalSpacing
138+
if fb.posX != wantNextX {
139+
t.Fatalf("got next posX %d, want %d", fb.posX, wantNextX)
140+
}
141+
}
142+
68143
// =============================================================================
69144
// Issue #19: Long type must not be downgraded to Integer
70145
// =============================================================================

mdl/executor/cmd_microflows_builder_control.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,13 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
288288
innerStartX := LoopPadding + iteratorSpace // Extra offset for iterator icon and label
289289
innerStartY := LoopPadding + ActivityHeight/2 // Center activities vertically with some padding
290290

291+
loopLeftX := fb.posX
292+
loopCenterX := loopLeftX + loopWidth/2
293+
if s.Annotations != nil && s.Annotations.Position != nil {
294+
loopCenterX = s.Annotations.Position.X
295+
loopLeftX = loopCenterX - loopWidth/2
296+
}
297+
291298
// Add loop variable to varTypes with element type derived from list type
292299
// If $ProductList is "List of MfTest.Product", then $Product is "MfTest.Product"
293300
if fb.varTypes != nil {
@@ -335,7 +342,7 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
335342
loop := &microflows.LoopedActivity{
336343
BaseMicroflowObject: microflows.BaseMicroflowObject{
337344
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
338-
Position: model.Point{X: fb.posX + loopWidth/2, Y: fb.posY},
345+
Position: model.Point{X: loopCenterX, Y: fb.posY},
339346
Size: model.Size{Width: loopWidth, Height: loopHeight},
340347
},
341348
LoopSource: &microflows.IterableList{
@@ -362,7 +369,7 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
362369
fb.applyAnnotations(loop.ID, savedLoopAnnotations)
363370
}
364371

365-
fb.posX += loopWidth + HorizontalSpacing
372+
fb.posX = loopLeftX + loopWidth + HorizontalSpacing
366373

367374
return loop.ID
368375
}
@@ -383,6 +390,13 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
383390
innerStartX := LoopPadding
384391
innerStartY := LoopPadding + ActivityHeight/2
385392

393+
loopLeftX := fb.posX
394+
loopCenterX := loopLeftX + loopWidth/2
395+
if s.Annotations != nil && s.Annotations.Position != nil {
396+
loopCenterX = s.Annotations.Position.X
397+
loopLeftX = loopCenterX - loopWidth/2
398+
}
399+
386400
loopBuilder := &flowBuilder{
387401
posX: innerStartX,
388402
posY: innerStartY,
@@ -417,7 +431,7 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
417431
loop := &microflows.LoopedActivity{
418432
BaseMicroflowObject: microflows.BaseMicroflowObject{
419433
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
420-
Position: model.Point{X: fb.posX + loopWidth/2, Y: fb.posY},
434+
Position: model.Point{X: loopCenterX, Y: fb.posY},
421435
Size: model.Size{Width: loopWidth, Height: loopHeight},
422436
},
423437
LoopSource: &microflows.WhileLoopCondition{
@@ -439,7 +453,7 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
439453
fb.applyAnnotations(loop.ID, savedWhileAnnotations)
440454
}
441455

442-
fb.posX += loopWidth + HorizontalSpacing
456+
fb.posX = loopLeftX + loopWidth + HorizontalSpacing
443457

444458
return loop.ID
445459
}

mdl/executor/cmd_microflows_format_action.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,7 @@ func formatAction(
372372
if text, ok := a.MessageTemplate.Translations["en_US"]; ok {
373373
message = text
374374
}
375-
// Wrap message in quotes for MDL syntax (escape any existing single quotes)
376-
message = "'" + strings.ReplaceAll(message, "'", "''") + "'"
375+
message = mdlQuote(message)
377376
}
378377

379378
// Build WITH clause if there are template parameters
@@ -448,7 +447,7 @@ func formatAction(
448447
}
449448
case *microflows.EntityTypeCodeActionParameterValue:
450449
if v.Entity != "" {
451-
valueStr = "'" + v.Entity + "'"
450+
valueStr = mdlQuote(v.Entity)
452451
}
453452
}
454453
params = append(params, fmt.Sprintf("%s = %s", paramName, valueStr))
@@ -549,8 +548,7 @@ func formatAction(
549548
if text, ok := a.Template.Translations["en_US"]; ok {
550549
message = text
551550
}
552-
// Wrap message in quotes for MDL syntax (escape any existing single quotes)
553-
message = "'" + strings.ReplaceAll(message, "'", "''") + "'"
551+
message = mdlQuote(message)
554552
}
555553
result := fmt.Sprintf("show message %s type %s", message, msgType)
556554
if len(a.TemplateParameters) > 0 {
@@ -569,8 +567,7 @@ func formatAction(
569567
if text, ok := a.Template.Translations["en_US"]; ok {
570568
msgText = text
571569
}
572-
// Wrap message in quotes for MDL syntax (escape any existing single quotes)
573-
msgText = "'" + strings.ReplaceAll(msgText, "'", "''") + "'"
570+
msgText = mdlQuote(msgText)
574571
}
575572
// Build attribute path from variable and attribute name
576573
// AttributeName format: Module.Entity.Attribute
@@ -636,7 +633,7 @@ func formatAction(
636633
return formatWorkflowOperationAction(ctx, a)
637634

638635
case *microflows.SetTaskOutcomeAction:
639-
return fmt.Sprintf("set task outcome $%s '%s';", a.WorkflowTaskVariable, a.OutcomeValue)
636+
return fmt.Sprintf("set task outcome $%s %s;", a.WorkflowTaskVariable, mdlQuote(a.OutcomeValue))
640637

641638
case *microflows.OpenUserTaskAction:
642639
return fmt.Sprintf("open user task $%s;", a.UserTaskVariable)
@@ -684,7 +681,7 @@ func formatWorkflowOperationAction(ctx *ExecContext, a *microflows.WorkflowOpera
684681
switch op := a.Operation.(type) {
685682
case *microflows.AbortOperation:
686683
if op.Reason != "" {
687-
return fmt.Sprintf("workflow operation abort $%s reason '%s';", op.WorkflowVariable, strings.ReplaceAll(op.Reason, "'", "''"))
684+
return fmt.Sprintf("workflow operation abort $%s reason %s;", op.WorkflowVariable, mdlQuote(op.Reason))
688685
}
689686
return fmt.Sprintf("workflow operation abort $%s;", op.WorkflowVariable)
690687
case *microflows.ContinueOperation:
@@ -843,7 +840,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string
843840
// URL
844841
url := "''"
845842
if a.HttpConfiguration != nil && a.HttpConfiguration.LocationTemplate != "" {
846-
url = "'" + strings.ReplaceAll(a.HttpConfiguration.LocationTemplate, "'", "''") + "'"
843+
url = mdlQuote(a.HttpConfiguration.LocationTemplate)
847844
}
848845
sb.WriteString(url)
849846

@@ -862,9 +859,9 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string
862859
// Headers
863860
if a.HttpConfiguration != nil && len(a.HttpConfiguration.CustomHeaders) > 0 {
864861
for _, h := range a.HttpConfiguration.CustomHeaders {
865-
sb.WriteString("\n header '")
866-
sb.WriteString(strings.ReplaceAll(h.Name, "'", "''"))
867-
sb.WriteString("' = ")
862+
sb.WriteString("\n header ")
863+
sb.WriteString(mdlQuote(h.Name))
864+
sb.WriteString(" = ")
868865
sb.WriteString(h.Value)
869866
}
870867
}
@@ -882,9 +879,8 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string
882879
switch rh := a.RequestHandling.(type) {
883880
case *microflows.CustomRequestHandling:
884881
if rh.Template != "" {
885-
sb.WriteString("\n body '")
886-
sb.WriteString(strings.ReplaceAll(rh.Template, "'", "''"))
887-
sb.WriteString("'")
882+
sb.WriteString("\n body ")
883+
sb.WriteString(mdlQuote(rh.Template))
888884
// Add template parameters if present
889885
if len(rh.TemplateParams) > 0 {
890886
sb.WriteString(" with (")

mdl/executor/cmd_microflows_format_action_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,20 @@ func TestFormatAction_ShowMessage_EscapesQuotes(t *testing.T) {
462462
}
463463
}
464464

465+
func TestFormatAction_ShowMessage_EscapesMultiline(t *testing.T) {
466+
e := newTestExecutor()
467+
action := &microflows.ShowMessageAction{
468+
Type: microflows.MessageTypeInformation,
469+
Template: &model.Text{
470+
Translations: map[string]string{"en_US": "Line 1\nLine 2\tTabbed"},
471+
},
472+
}
473+
got := e.formatAction(action, nil, nil)
474+
if got != "show message 'Line 1\\nLine 2\\tTabbed' type Information;" {
475+
t.Errorf("got %q", got)
476+
}
477+
}
478+
465479
func TestFormatAction_ValidationFeedback(t *testing.T) {
466480
e := newTestExecutor()
467481
action := &microflows.ValidationFeedbackAction{
@@ -511,6 +525,22 @@ func TestFormatAction_LogMessage_WithTemplateParams(t *testing.T) {
511525
}
512526
}
513527

528+
func TestFormatAction_LogMessage_EscapesMultiline(t *testing.T) {
529+
e := newTestExecutor()
530+
action := &microflows.LogMessageAction{
531+
LogLevel: microflows.LogLevelInfo,
532+
LogNodeName: "'App'",
533+
MessageTemplate: &model.Text{
534+
Translations: map[string]string{"en_US": "Line 1\nLine 2"},
535+
},
536+
}
537+
got := e.formatAction(action, nil, nil)
538+
want := "LOG INFO NODE 'App' 'Line 1\\nLine 2';"
539+
if got != want {
540+
t.Errorf("got %q, want %q", got, want)
541+
}
542+
}
543+
514544
func TestFormatAction_UnknownAction(t *testing.T) {
515545
e := newTestExecutor()
516546
action := &microflows.UnknownAction{TypeName: "SomeNewAction"}

mdl/executor/cmd_microflows_helpers.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,7 @@ func expressionToString(expr ast.Expression) string {
182182
case *ast.LiteralExpr:
183183
switch e.Kind {
184184
case ast.LiteralString:
185-
// Escape single quotes for Mendix expression syntax (use '' inside strings)
186-
strVal := fmt.Sprintf("%v", e.Value)
187-
strVal = strings.ReplaceAll(strVal, `'`, `''`)
188-
return "'" + strVal + "'"
185+
return mdlQuote(fmt.Sprintf("%v", e.Value))
189186
case ast.LiteralBoolean:
190187
if e.Value.(bool) {
191188
return "true"

mdl/executor/cmd_microflows_show.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ func describeMicroflow(ctx *ExecContext, name ast.QualifiedName) error {
267267

268268
// Folder
269269
if folderPath := h.BuildFolderPath(targetMf.ContainerID); folderPath != "" {
270-
lines = append(lines, fmt.Sprintf("folder '%s'", folderPath))
270+
lines = append(lines, fmt.Sprintf("folder %s", mdlQuote(folderPath)))
271271
}
272272

273273
// BEGIN block
@@ -402,7 +402,7 @@ func describeNanoflow(ctx *ExecContext, name ast.QualifiedName) error {
402402

403403
// Folder
404404
if folderPath := h.BuildFolderPath(targetNf.ContainerID); folderPath != "" {
405-
lines = append(lines, fmt.Sprintf("folder '%s'", folderPath))
405+
lines = append(lines, fmt.Sprintf("folder %s", mdlQuote(folderPath)))
406406
}
407407

408408
// BEGIN block with activities

mdl/executor/cmd_microflows_show_helpers.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ func emitObjectAnnotations(obj microflows.MicroflowObject, lines *[]string, inde
7676
*lines = append(*lines, indentStr+"@excluded")
7777
}
7878
if !activity.AutoGenerateCaption && activity.Caption != "" {
79-
escapedCaption := strings.ReplaceAll(activity.Caption, "'", "''")
80-
*lines = append(*lines, indentStr+fmt.Sprintf("@caption '%s'", escapedCaption))
79+
*lines = append(*lines, indentStr+fmt.Sprintf("@caption %s", mdlQuote(activity.Caption)))
8180
}
8281
if activity.BackgroundColor != "" && activity.BackgroundColor != "Default" {
8382
*lines = append(*lines, indentStr+fmt.Sprintf("@color %s", activity.BackgroundColor))
@@ -96,8 +95,7 @@ func emitObjectAnnotations(obj microflows.MicroflowObject, lines *[]string, inde
9695
// @annotation (attached Annotation objects)
9796
if annotationsByTarget != nil {
9897
for _, caption := range annotationsByTarget[currentID] {
99-
escapedCaption := strings.ReplaceAll(caption, "'", "''")
100-
*lines = append(*lines, indentStr+fmt.Sprintf("@annotation '%s'", escapedCaption))
98+
*lines = append(*lines, indentStr+fmt.Sprintf("@annotation %s", mdlQuote(caption)))
10199
}
102100
}
103101
}

mdl/executor/cmd_microflows_show_helpers_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
package executor
44

55
import (
6+
"strings"
67
"testing"
78

9+
"github.com/mendixlabs/mxcli/model"
810
"github.com/mendixlabs/mxcli/sdk/microflows"
911
)
1012

@@ -129,6 +131,34 @@ func TestFindNormalFlows_AllErrors(t *testing.T) {
129131
}
130132
}
131133

134+
func TestEmitObjectAnnotations_EscapesMultilineText(t *testing.T) {
135+
obj := &microflows.ActionActivity{
136+
BaseActivity: microflows.BaseActivity{
137+
BaseMicroflowObject: microflows.BaseMicroflowObject{
138+
BaseElement: model.BaseElement{ID: mkID("act")},
139+
Position: model.Point{X: 100, Y: 200},
140+
},
141+
Caption: "Caption\nLine",
142+
AutoGenerateCaption: false,
143+
},
144+
}
145+
146+
annotationsByTarget := map[model.ID][]string{
147+
mkID("act"): {"Note\nLine\tTabbed"},
148+
}
149+
150+
var lines []string
151+
emitObjectAnnotations(obj, &lines, "", annotationsByTarget)
152+
153+
got := strings.Join(lines, "\n")
154+
if !strings.Contains(got, "@caption 'Caption\\nLine'") {
155+
t.Fatalf("expected escaped caption, got:\n%s", got)
156+
}
157+
if !strings.Contains(got, "@annotation 'Note\\nLine\\tTabbed'") {
158+
t.Fatalf("expected escaped annotation, got:\n%s", got)
159+
}
160+
}
161+
132162
// =============================================================================
133163
// formatErrorHandlingSuffix
134164
// =============================================================================

mdl/executor/cmd_pages_describe_output.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,16 @@ import (
1313
"go.mongodb.org/mongo-driver/bson/primitive"
1414
)
1515

16-
// mdlQuote wraps a string in single quotes, escaping any embedded single quotes
17-
// by doubling them (MDL convention: 'it”s here').
16+
// mdlQuote wraps a string in single quotes and escapes MDL-sensitive characters.
1817
func mdlQuote(s string) string {
19-
return "'" + strings.ReplaceAll(s, "'", "''") + "'"
18+
escaped := strings.NewReplacer(
19+
"\\", "\\\\",
20+
"\n", "\\n",
21+
"\r", "\\r",
22+
"\t", "\\t",
23+
"'", "''",
24+
).Replace(s)
25+
return "'" + escaped + "'"
2026
}
2127

2228
// appendDataGridPagingProps appends non-default paging properties for DataGrid2.

0 commit comments

Comments
 (0)