Skip to content

Commit e9dfbc3

Browse files
authored
Merge branch 'main' into pr4-nanoflows-all
2 parents f914d3d + 1613f08 commit e9dfbc3

12 files changed

Lines changed: 928 additions & 13 deletions
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
-- ============================================================================
2+
-- Bug #350: Manual `while true` retry-loop roundtrip
3+
-- ============================================================================
4+
--
5+
-- Symptom (before fix):
6+
-- Studio Pro projects with manual retry loops authored as
7+
-- while true
8+
-- begin
9+
-- <work>
10+
-- if <done> then return ...; end if;
11+
-- continue;
12+
-- end while;
13+
-- could be described correctly, but `mxcli exec` rebuilt the same MDL
14+
-- into either a `LoopedActivity` (collection-style loop) or a flow
15+
-- with a synthetic fallthrough `EndEvent`. The resulting MPR diverged
16+
-- from the original — a back-edge merge expected by Studio Pro was
17+
-- missing — and `mx check` could fail or the model's runtime semantics
18+
-- could change silently.
19+
--
20+
-- Root cause:
21+
-- The builder accepted `while`, `break`, `continue` syntax but never
22+
-- materialized BreakEvent/ContinueEvent objects. `continue` in a
23+
-- manual `while true` body was treated as plain fallthrough.
24+
--
25+
-- After fix:
26+
-- - `BreakEvent` and `ContinueEvent` objects are produced for break/continue.
27+
-- - Both are treated as terminal branch statements for control-flow analysis.
28+
-- - Manual `while true` bodies that need a graph-level back-edge are
29+
-- built as an `ExclusiveMerge` loop header.
30+
-- - `continue` inside that manual loop routes back to the merge instead
31+
-- of producing a standalone `ContinueEvent`.
32+
--
33+
-- Usage:
34+
-- mxcli exec mdl-examples/bug-tests/350-manual-while-true-loop-roundtrip.mdl -p app.mpr
35+
-- mxcli -p app.mpr -c "describe microflow BugTest350.MF_ManualRetryLoop"
36+
-- `mx check` against the resulting MPR must report 0 errors.
37+
-- ============================================================================
38+
39+
create module BugTest350;
40+
41+
create entity BugTest350.Item (
42+
IsDone : boolean
43+
);
44+
/
45+
46+
-- Manual while-true retry loop. The graph rebuild must use an
47+
-- ExclusiveMerge back-edge — not a LoopedActivity and not a fallthrough
48+
-- EndEvent — and the `continue` must route to that merge.
49+
create microflow BugTest350.MF_ManualRetryLoop (
50+
$Item: BugTest350.Item
51+
)
52+
returns boolean as $Done
53+
begin
54+
while true
55+
begin
56+
if $Item/IsDone then
57+
return true;
58+
end if;
59+
continue;
60+
end while;
61+
end;
62+
/

mdl/executor/cmd_microflows_builder.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,13 @@ type flowBuilder struct {
3737
// continues) so the continuing branch's @anchor survives to the actual
3838
// splitID→nextActivity flow — which is emitted one iteration later by the
3939
// outer loop, not by addIfStatement.
40-
nextFlowAnchor *ast.FlowAnchors
41-
backend backend.FullBackend // For looking up page/microflow references
42-
hierarchy *ContainerHierarchy // For resolving container IDs to module names
43-
pendingAnnotations *ast.ActivityAnnotations // Pending annotations to attach to next activity
44-
restServices []*model.ConsumedRestService // Cached REST services for parameter classification
40+
nextFlowAnchor *ast.FlowAnchors
41+
backend backend.FullBackend // For looking up page/microflow references
42+
hierarchy *ContainerHierarchy // For resolving container IDs to module names
43+
pendingAnnotations *ast.ActivityAnnotations // Pending annotations to attach to next activity
44+
restServices []*model.ConsumedRestService // Cached REST services for parameter classification
45+
listInputVariables map[string]bool // Variables later consumed by list-only actions
46+
objectInputVariables map[string]bool // Variables later consumed through object/attribute access
4547
// previousStmtAnchor holds the Anchor annotation of the statement that
4648
// just emitted an activity, so the next flow's OriginConnectionIndex can
4749
// be overridden by the user. Cleared after each flow is created.
@@ -51,6 +53,8 @@ type flowBuilder struct {
5153
microflowsCacheLoaded bool
5254
nanoflowsCache []*microflows.Nanoflow
5355
nanoflowsCacheLoaded bool
56+
previousStmtAnchor *ast.FlowAnchors
57+
manualLoopBackTarget model.ID
5458
}
5559

5660
// addError records a validation error during flow building.

mdl/executor/cmd_microflows_builder_actions.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,19 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID {
304304
startVarType = fb.varTypes[s.StartVariable]
305305
}
306306

307-
if assocInfo != nil && assocInfo.Type == domainmodel.AssociationTypeReference &&
307+
outputUsedAsList := fb.listInputVariables != nil && fb.listInputVariables[s.Variable]
308+
outputUsedAsObject := fb.objectInputVariables != nil && fb.objectInputVariables[s.Variable]
309+
// Owner-both Reference associations need later usage context: the same
310+
// compact retrieve can be consumed as either a list or a single object.
311+
expandReverseReference := assocInfo != nil &&
312+
assocInfo.Type == domainmodel.AssociationTypeReference &&
308313
assocInfo.Owner != "" &&
309314
assocInfo.parentPersistable &&
310-
assocInfo.childEntityQN != "" && startVarType == assocInfo.childEntityQN {
315+
assocInfo.childEntityQN != "" &&
316+
startVarType == assocInfo.childEntityQN &&
317+
(assocInfo.Owner != domainmodel.AssociationOwnerBoth || outputUsedAsList && !outputUsedAsObject)
318+
319+
if expandReverseReference {
311320
// Reverse traversal on Reference: child → parent (one-to-many)
312321
// Use DatabaseRetrieveSource with XPath to get a list of parent entities
313322
dbSource := &microflows.DatabaseRetrieveSource{
@@ -335,7 +344,7 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID {
335344
if startVarType == assocInfo.childEntityQN {
336345
otherEntity = assocInfo.parentEntityQN
337346
}
338-
if startVarType == assocInfo.childEntityQN {
347+
if startVarType == assocInfo.childEntityQN && !outputUsedAsObject {
339348
fb.varTypes[s.Variable] = "List of " + otherEntity
340349
} else {
341350
fb.varTypes[s.Variable] = otherEntity

mdl/executor/cmd_microflows_builder_control.go

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,9 +436,150 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
436436
return loop.ID
437437
}
438438

439+
func isManualWhileTrueCandidate(s *ast.WhileStmt) bool {
440+
if s == nil || containsBreakForCurrentLoop(s.Body) || (!containsContinueStmt(s.Body) && !containsTerminalStmt(s.Body)) {
441+
return false
442+
}
443+
lit, ok := s.Condition.(*ast.LiteralExpr)
444+
if !ok || lit.Kind != ast.LiteralBoolean {
445+
return false
446+
}
447+
value, ok := lit.Value.(bool)
448+
return ok && value
449+
}
450+
451+
func containsBreakForCurrentLoop(stmts []ast.MicroflowStatement) bool {
452+
for _, stmt := range stmts {
453+
switch s := stmt.(type) {
454+
case *ast.BreakStmt:
455+
return true
456+
case *ast.IfStmt:
457+
if containsBreakForCurrentLoop(s.ThenBody) || containsBreakForCurrentLoop(s.ElseBody) {
458+
return true
459+
}
460+
case *ast.LoopStmt, *ast.WhileStmt:
461+
// A break inside a nested loop exits that nested loop, not this
462+
// manual while-true back-edge.
463+
continue
464+
}
465+
}
466+
return false
467+
}
468+
469+
func containsContinueStmt(stmts []ast.MicroflowStatement) bool {
470+
for _, stmt := range stmts {
471+
switch s := stmt.(type) {
472+
case *ast.ContinueStmt:
473+
return true
474+
case *ast.IfStmt:
475+
if containsContinueStmt(s.ThenBody) || containsContinueStmt(s.ElseBody) {
476+
return true
477+
}
478+
case *ast.LoopStmt:
479+
if containsContinueStmt(s.Body) {
480+
return true
481+
}
482+
case *ast.WhileStmt:
483+
if containsContinueStmt(s.Body) {
484+
return true
485+
}
486+
}
487+
}
488+
return false
489+
}
490+
491+
func (fb *flowBuilder) addManualWhileTrueStatement(s *ast.WhileStmt) model.ID {
492+
mergeX := fb.posX
493+
mergeY := fb.posY
494+
if s.Annotations != nil && s.Annotations.Position != nil {
495+
mergeX = s.Annotations.Position.X
496+
mergeY = s.Annotations.Position.Y
497+
}
498+
499+
merge := &microflows.ExclusiveMerge{
500+
BaseMicroflowObject: microflows.BaseMicroflowObject{
501+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
502+
Position: model.Point{X: mergeX, Y: mergeY},
503+
Size: model.Size{Width: MergeSize, Height: MergeSize},
504+
},
505+
}
506+
fb.objects = append(fb.objects, merge)
507+
if fb.pendingAnnotations != nil {
508+
fb.applyAnnotations(merge.ID, fb.pendingAnnotations)
509+
fb.pendingAnnotations = nil
510+
}
511+
512+
savedBackTarget := fb.manualLoopBackTarget
513+
fb.manualLoopBackTarget = merge.ID
514+
defer func() { fb.manualLoopBackTarget = savedBackTarget }()
515+
516+
fb.posX = mergeX + MergeSize + HorizontalSpacing/2
517+
fb.posY = mergeY
518+
519+
lastBodyID := merge.ID
520+
var pendingBodyCase string
521+
var pendingBodyAnchor *ast.FlowAnchors
522+
var prevBodyAnchor *ast.FlowAnchors
523+
for _, stmt := range s.Body {
524+
thisAnchor := stmtOwnAnchor(stmt)
525+
actID := fb.addStatement(stmt)
526+
if actID == "" {
527+
continue
528+
}
529+
if fb.pendingAnnotations != nil {
530+
fb.applyAnnotations(actID, fb.pendingAnnotations)
531+
fb.pendingAnnotations = nil
532+
}
533+
if lastBodyID != "" && lastBodyID != actID {
534+
var flow *microflows.SequenceFlow
535+
if pendingBodyCase != "" {
536+
flow = newHorizontalFlowWithCase(lastBodyID, actID, pendingBodyCase)
537+
if pendingBodyAnchor != nil {
538+
prevBodyAnchor = pendingBodyAnchor
539+
}
540+
pendingBodyCase = ""
541+
pendingBodyAnchor = nil
542+
} else {
543+
flow = newHorizontalFlow(lastBodyID, actID)
544+
}
545+
applyUserAnchors(flow, prevBodyAnchor, thisAnchor)
546+
fb.flows = append(fb.flows, flow)
547+
}
548+
prevBodyAnchor = thisAnchor
549+
if fb.nextConnectionPoint != "" {
550+
lastBodyID = fb.nextConnectionPoint
551+
fb.nextConnectionPoint = ""
552+
pendingBodyCase = fb.nextFlowCase
553+
fb.nextFlowCase = ""
554+
pendingBodyAnchor = fb.nextFlowAnchor
555+
fb.nextFlowAnchor = nil
556+
} else {
557+
lastBodyID = actID
558+
}
559+
}
560+
561+
if lastBodyID != "" && lastBodyID != merge.ID && !lastStmtIsReturn(s.Body) {
562+
var flow *microflows.SequenceFlow
563+
if pendingBodyCase != "" {
564+
flow = newHorizontalFlowWithCase(lastBodyID, merge.ID, pendingBodyCase)
565+
} else {
566+
flow = newHorizontalFlow(lastBodyID, merge.ID)
567+
}
568+
applyUserAnchors(flow, pendingBodyAnchor, pendingBodyAnchor)
569+
fb.flows = append(fb.flows, flow)
570+
}
571+
fb.endsWithReturn = true
572+
573+
return merge.ID
574+
}
575+
439576
// addWhileStatement creates a WHILE loop using LoopedActivity with WhileLoopCondition.
440577
// Layout matches addLoopStatement but without iterator icon space.
441578
func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
579+
if isManualWhileTrueCandidate(s) {
580+
return fb.addManualWhileTrueStatement(s)
581+
}
582+
442583
// Snapshot & clear this WHILE's own annotations so the body's recursive
443584
// addStatement calls can't consume them (see addLoopStatement).
444585
savedWhileAnnotations := fb.pendingAnnotations
@@ -523,3 +664,33 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
523664

524665
return loop.ID
525666
}
667+
668+
func (fb *flowBuilder) addBreakEvent() model.ID {
669+
event := &microflows.BreakEvent{
670+
BaseMicroflowObject: microflows.BaseMicroflowObject{
671+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
672+
Position: model.Point{X: fb.posX, Y: fb.posY},
673+
Size: model.Size{Width: EventSize, Height: EventSize},
674+
},
675+
}
676+
fb.objects = append(fb.objects, event)
677+
fb.posX += fb.spacing
678+
return event.ID
679+
}
680+
681+
func (fb *flowBuilder) addContinueEvent() model.ID {
682+
if fb.manualLoopBackTarget != "" {
683+
return fb.manualLoopBackTarget
684+
}
685+
686+
event := &microflows.ContinueEvent{
687+
BaseMicroflowObject: microflows.BaseMicroflowObject{
688+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
689+
Position: model.Point{X: fb.posX, Y: fb.posY},
690+
Size: model.Size{Width: EventSize, Height: EventSize},
691+
},
692+
}
693+
fb.objects = append(fb.objects, event)
694+
fb.posX += fb.spacing
695+
return event.ID
696+
}

mdl/executor/cmd_microflows_builder_flows.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,13 @@ func pendingFlowAnchors(previousAnchor, pendingAnchor, stmtAnchor *ast.FlowAncho
206206
}
207207

208208
// lastStmtIsReturn reports whether execution of a body is guaranteed to terminate
209-
// (via RETURN or RAISE ERROR) on every path — i.e. control can never fall off the
210-
// end of the body into the parent flow.
209+
// (via RETURN, RAISE ERROR, BREAK, or CONTINUE) on every path — i.e. control can
210+
// never fall off the end of the body into the parent flow.
211211
//
212-
// Terminal statements: ReturnStmt, RaiseErrorStmt. An IfStmt is terminal iff it
213-
// has an ELSE and both branches are terminal (recursively). A LoopStmt is never
214-
// terminal — BREAK can exit the loop even if the body returns.
212+
// Terminal statements: ReturnStmt, RaiseErrorStmt, BreakStmt, ContinueStmt. An
213+
// IfStmt is terminal iff it has an ELSE and both branches are terminal
214+
// (recursively). A LoopStmt is never terminal — BREAK can exit the loop even if
215+
// the body returns.
215216
//
216217
// Naming kept for history; the predicate is really "last stmt is a guaranteed
217218
// terminator". Missing this case causes the outer IF to emit a dangling
@@ -230,12 +231,40 @@ func isTerminalStmt(stmt ast.MicroflowStatement) bool {
230231
return true
231232
case *ast.RaiseErrorStmt:
232233
return true
234+
case *ast.BreakStmt:
235+
return true
236+
case *ast.ContinueStmt:
237+
return true
233238
case *ast.IfStmt:
234239
if len(s.ElseBody) == 0 {
235240
return false
236241
}
237242
return lastStmtIsReturn(s.ThenBody) && lastStmtIsReturn(s.ElseBody)
243+
case *ast.WhileStmt:
244+
return isManualWhileTrueCandidate(s)
238245
default:
239246
return false
240247
}
241248
}
249+
250+
func containsTerminalStmt(stmts []ast.MicroflowStatement) bool {
251+
for _, stmt := range stmts {
252+
switch s := stmt.(type) {
253+
case *ast.ReturnStmt, *ast.RaiseErrorStmt:
254+
return true
255+
case *ast.IfStmt:
256+
if containsTerminalStmt(s.ThenBody) || containsTerminalStmt(s.ElseBody) {
257+
return true
258+
}
259+
case *ast.LoopStmt:
260+
if containsTerminalStmt(s.Body) {
261+
return true
262+
}
263+
case *ast.WhileStmt:
264+
if containsTerminalStmt(s.Body) {
265+
return true
266+
}
267+
}
268+
}
269+
return false
270+
}

0 commit comments

Comments
 (0)