Skip to content

Commit 379fcfb

Browse files
akoako
authored
Merge pull request #456 from hjotha/submit/nested-split-parent-merge-join
fix: stop nested splits at parent merge joins
2 parents f313c9b + fcc9355 commit 379fcfb

3 files changed

Lines changed: 383 additions & 19 deletions

File tree

‎mdl/executor/cmd_microflows_show.go‎

Lines changed: 127 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -982,11 +982,13 @@ func findMergeForSplit(
982982
}
983983

984984
branchDistances := make([]map[model.ID]int, 0, len(flows))
985+
branchStarts := make([]model.ID, 0, len(flows))
985986
for _, flow := range flows {
987+
branchStarts = append(branchStarts, flow.DestinationID)
986988
branchDistances = append(branchDistances, collectReachableDistances(flow.DestinationID, flowsByOrigin))
987989
}
988990

989-
return selectNearestCommonJoin(activityMap, branchDistances)
991+
return selectNearestCommonJoin(activityMap, flowsByOrigin, branchStarts, branchDistances)
990992
}
991993

992994
// collectReachableDistances collects the shortest normal-flow distance from a
@@ -1025,6 +1027,8 @@ func collectReachableDistances(
10251027

10261028
func selectNearestCommonJoin(
10271029
activityMap map[model.ID]microflows.MicroflowObject,
1030+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1031+
branchStarts []model.ID,
10281032
branchDistances []map[model.ID]int,
10291033
) model.ID {
10301034
if len(branchDistances) < 2 {
@@ -1033,6 +1037,7 @@ func selectNearestCommonJoin(
10331037

10341038
type candidate struct {
10351039
id model.ID
1040+
reachCount int
10361041
maxDistance int
10371042
sumDistance int
10381043
}
@@ -1060,17 +1065,56 @@ func selectNearestCommonJoin(
10601065
if common {
10611066
candidates = append(candidates, candidate{
10621067
id: nodeID,
1068+
reachCount: len(branchDistances),
10631069
maxDistance: maxDistance,
10641070
sumDistance: sumDistance,
10651071
})
10661072
}
10671073
}
10681074

1075+
if len(candidates) == 0 {
1076+
byNode := map[model.ID]candidate{}
1077+
for _, distances := range branchDistances {
1078+
for nodeID, distance := range distances {
1079+
if !isSplitJoinCandidate(activityMap[nodeID]) {
1080+
continue
1081+
}
1082+
c := byNode[nodeID]
1083+
c.id = nodeID
1084+
c.reachCount++
1085+
if distance > c.maxDistance {
1086+
c.maxDistance = distance
1087+
}
1088+
c.sumDistance += distance
1089+
byNode[nodeID] = c
1090+
}
1091+
}
1092+
for _, c := range byNode {
1093+
if c.reachCount >= 2 {
1094+
candidates = append(candidates, c)
1095+
}
1096+
}
1097+
}
1098+
1099+
if len(candidates) == 0 {
1100+
return ""
1101+
}
1102+
1103+
filtered := candidates[:0]
1104+
for _, candidate := range candidates {
1105+
if splitJoinCandidateDoesNotHaveDownstreamBypass(candidate.id, activityMap, flowsByOrigin, branchStarts) {
1106+
filtered = append(filtered, candidate)
1107+
}
1108+
}
1109+
candidates = filtered
10691110
if len(candidates) == 0 {
10701111
return ""
10711112
}
10721113

10731114
sort.Slice(candidates, func(i, j int) bool {
1115+
if candidates[i].reachCount != candidates[j].reachCount {
1116+
return candidates[i].reachCount > candidates[j].reachCount
1117+
}
10741118
if candidates[i].maxDistance != candidates[j].maxDistance {
10751119
return candidates[i].maxDistance < candidates[j].maxDistance
10761120
}
@@ -1083,6 +1127,88 @@ func selectNearestCommonJoin(
10831127
return candidates[0].id
10841128
}
10851129

1130+
func splitJoinCandidateDoesNotHaveDownstreamBypass(
1131+
candidateID model.ID,
1132+
activityMap map[model.ID]microflows.MicroflowObject,
1133+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1134+
branchStarts []model.ID,
1135+
) bool {
1136+
downstream := collectReachableNonTerminalObjects(candidateID, activityMap, flowsByOrigin)
1137+
if len(downstream) == 0 {
1138+
return true
1139+
}
1140+
for _, startID := range branchStarts {
1141+
if startID == candidateID {
1142+
continue
1143+
}
1144+
if reachesAnyObjectAvoiding(startID, downstream, candidateID, activityMap, flowsByOrigin, map[model.ID]bool{}) {
1145+
return false
1146+
}
1147+
}
1148+
return true
1149+
}
1150+
1151+
func collectReachableNonTerminalObjects(
1152+
startID model.ID,
1153+
activityMap map[model.ID]microflows.MicroflowObject,
1154+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1155+
) map[model.ID]bool {
1156+
result := map[model.ID]bool{}
1157+
var walk func(model.ID)
1158+
visited := map[model.ID]bool{startID: true}
1159+
walk = func(currentID model.ID) {
1160+
if visited[currentID] {
1161+
return
1162+
}
1163+
visited[currentID] = true
1164+
if isNonTerminalMicroflowObject(activityMap[currentID]) {
1165+
result[currentID] = true
1166+
}
1167+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
1168+
walk(flow.DestinationID)
1169+
}
1170+
}
1171+
for _, flow := range findNormalFlows(flowsByOrigin[startID]) {
1172+
walk(flow.DestinationID)
1173+
}
1174+
return result
1175+
}
1176+
1177+
func reachesAnyObjectAvoiding(
1178+
currentID model.ID,
1179+
targets map[model.ID]bool,
1180+
avoidID model.ID,
1181+
activityMap map[model.ID]microflows.MicroflowObject,
1182+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1183+
visited map[model.ID]bool,
1184+
) bool {
1185+
if currentID == "" || currentID == avoidID || visited[currentID] {
1186+
return false
1187+
}
1188+
if targets[currentID] {
1189+
return true
1190+
}
1191+
if !isNonTerminalMicroflowObject(activityMap[currentID]) {
1192+
return false
1193+
}
1194+
visited[currentID] = true
1195+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
1196+
if reachesAnyObjectAvoiding(flow.DestinationID, targets, avoidID, activityMap, flowsByOrigin, visited) {
1197+
return true
1198+
}
1199+
}
1200+
return false
1201+
}
1202+
1203+
func isNonTerminalMicroflowObject(obj microflows.MicroflowObject) bool {
1204+
switch obj.(type) {
1205+
case nil, *microflows.StartEvent, *microflows.EndEvent, *microflows.ErrorEvent:
1206+
return false
1207+
default:
1208+
return true
1209+
}
1210+
}
1211+
10861212
func isSplitJoinCandidate(obj microflows.MicroflowObject) bool {
10871213
switch obj.(type) {
10881214
case nil, *microflows.StartEvent, *microflows.EndEvent:

‎mdl/executor/cmd_microflows_show_helpers.go‎

Lines changed: 68 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -668,12 +668,7 @@ func traverseFlow(
668668
emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap)
669669
emitEnumSplitStatement(ctx, currentID, mergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
670670
recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
671-
if mergeID != "" {
672-
visited[mergeID] = true
673-
for _, flow := range flowsByOrigin[mergeID] {
674-
traverseFlow(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
675-
}
676-
}
671+
continueAfterSplitJoin(ctx, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
677672
return
678673
}
679674
if stmt != "" {
@@ -831,12 +826,7 @@ func traverseFlowUntilMerge(
831826
emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap)
832827
emitEnumSplitStatement(ctx, currentID, nestedMergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
833828
recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
834-
if nestedMergeID != "" && nestedMergeID != mergeID {
835-
visited[nestedMergeID] = true
836-
for _, flow := range flowsByOrigin[nestedMergeID] {
837-
traverseFlowUntilMerge(ctx, flow.DestinationID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
838-
}
839-
}
829+
continueAfterNestedSplitJoin(ctx, nestedMergeID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
840830
return
841831
}
842832
if stmt != "" {
@@ -845,10 +835,11 @@ func traverseFlowUntilMerge(
845835
}
846836

847837
trueFlow, falseFlow := findBranchFlows(flows)
838+
nestedMergeID = resolveNestedMergeID(nestedMergeID, mergeID, trueFlow, falseFlow, flowsByOrigin)
848839

849840
// Empty-then swap: negate when true branch is empty but false branch has content.
850841
// Skip when both branches go directly to merge (both empty).
851-
if trueFlow != nil && falseFlow != nil && nestedMergeID != "" {
842+
if trueFlow != nil && falseFlow != nil && nestedMergeID != "" && nestedMergeID == mergeID {
852843
if trueFlow.DestinationID == nestedMergeID && falseFlow.DestinationID != nestedMergeID {
853844
stmt = negateIfCondition(stmt)
854845
trueFlow, falseFlow = falseFlow, trueFlow
@@ -1000,6 +991,63 @@ func continueAfterNestedSplitJoin(
1000991
traverseFlowUntilMerge(ctx, joinID, parentMergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
1001992
}
1002993

994+
func resolveNestedMergeID(
995+
nestedMergeID model.ID,
996+
parentMergeID model.ID,
997+
trueFlow *microflows.SequenceFlow,
998+
falseFlow *microflows.SequenceFlow,
999+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1000+
) model.ID {
1001+
if nestedMergeID != "" && parentMergeID != "" && nestedMergeID != parentMergeID &&
1002+
canReachNode(parentMergeID, nestedMergeID, flowsByOrigin, make(map[model.ID]bool)) {
1003+
for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} {
1004+
if flow == nil {
1005+
continue
1006+
}
1007+
if flow.DestinationID == parentMergeID ||
1008+
canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) {
1009+
return parentMergeID
1010+
}
1011+
}
1012+
}
1013+
if nestedMergeID != "" || parentMergeID == "" {
1014+
return nestedMergeID
1015+
}
1016+
for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} {
1017+
if flow == nil {
1018+
continue
1019+
}
1020+
if canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) {
1021+
return parentMergeID
1022+
}
1023+
}
1024+
return ""
1025+
}
1026+
1027+
func canReachNode(
1028+
currentID model.ID,
1029+
targetID model.ID,
1030+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1031+
visited map[model.ID]bool,
1032+
) bool {
1033+
if currentID == "" {
1034+
return false
1035+
}
1036+
if currentID == targetID {
1037+
return true
1038+
}
1039+
if visited[currentID] {
1040+
return false
1041+
}
1042+
visited[currentID] = true
1043+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
1044+
if canReachNode(flow.DestinationID, targetID, flowsByOrigin, visited) {
1045+
return true
1046+
}
1047+
}
1048+
return false
1049+
}
1050+
10031051
// traverseLoopBody traverses activities inside a loop body.
10041052
// When sourceMap is non-nil, it also records line ranges for each activity node.
10051053
func traverseLoopBody(
@@ -1421,10 +1469,13 @@ func flowLooksLikeGuardContinuation(
14211469
return false
14221470
}
14231471
// Builder-generated guard continuations sit on the split's horizontal
1424-
// centerline. This intentionally relies on mxcli's layout contract so a
1425-
// real branch that returns to a merge below the split is not collapsed into
1426-
// a guard-style continuation during describe.
1427-
return dest.GetPosition().Y == split.GetPosition().Y
1472+
// centerline and use the builder's horizontal split→tail flow. This
1473+
// intentionally relies on mxcli's layout/anchor contract so a real false
1474+
// branch whose activities happen to be aligned with the split is not
1475+
// collapsed into a guard-style continuation during describe.
1476+
return dest.GetPosition().Y == split.GetPosition().Y &&
1477+
flow.OriginConnectionIndex == AnchorRight &&
1478+
flow.DestinationConnectionIndex == AnchorLeft
14281479
}
14291480

14301481
// findErrorHandlerFlow returns the error handler flow from an activity's outgoing flows.

0 commit comments

Comments
 (0)