Skip to content

Commit 8ab166f

Browse files
reid-spencerclaude
reid-spencer
and
claude
committed
Fix MessageFlowPass producing empty results for adaptor models
Create AdaptorBridge edges from adaptor declarations regardless of handler content. Fix direction bug (was always inbound). Make messageType optional. Add warnings on resolution failures. Add domain/context scoping helpers to MessageFlowOutput. Add 6 tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 77e56d8 commit 8ab166f

4 files changed

Lines changed: 266 additions & 18 deletions

File tree

‎passes/jvm-native/src/test/scala/com/ossuminc/riddl/passes/analysis/MessageFlowPassTest.scala‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
/*
2+
* Copyright 2019-2026 Ossum Inc.
3+
*
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package com.ossuminc.riddl.passes.analysis
8+
9+
import com.ossuminc.riddl.language.AST.*
10+
import com.ossuminc.riddl.language.Messages
11+
import com.ossuminc.riddl.language.parsing.RiddlParserInput
12+
import com.ossuminc.riddl.passes.{Pass, PassInput, PassesResult}
13+
import com.ossuminc.riddl.passes.validate.AbstractValidatingTest
14+
import com.ossuminc.riddl.utils.{pc, ec}
15+
16+
import org.scalatest.*
17+
18+
class MessageFlowPassTest extends AbstractValidatingTest {
19+
20+
private def runMessageFlowPass(
21+
input: String,
22+
origin: String = "test"
23+
)(
24+
check: (MessageFlowOutput, Messages.Messages) => Assertion
25+
): Assertion =
26+
val rpi = RiddlParserInput(input, origin)
27+
parseValidateAndThen(rpi, shouldFailOnErrors = false) {
28+
(pr: PassesResult, root: Root, _, msgs: Messages.Messages) =>
29+
val passInput = PassInput(root)
30+
val outputs = pr.outputs
31+
val pass = MessageFlowPass(passInput, outputs)
32+
val mfo = Pass.runPass[MessageFlowOutput](passInput, outputs, pass)
33+
check(mfo, msgs)
34+
}
35+
end runMessageFlowPass
36+
37+
"MessageFlowPass" should {
38+
"detect adaptor bridge edge from declaration with ??? body" in {
39+
(td: TestData) =>
40+
runMessageFlowPass(
41+
"""domain D is {
42+
| context Source is {
43+
| adaptor ToTarget to context D.Target is { ??? }
44+
| }
45+
| context Target is { ??? }
46+
|}
47+
|""".stripMargin
48+
) { (mfo, _) =>
49+
mfo.edges must not be empty
50+
val bridges = mfo.edges.filter(
51+
_.mechanism == FlowMechanism.AdaptorBridge
52+
)
53+
bridges must not be empty
54+
// Outbound adaptor: producer=Source, consumer=Target
55+
bridges.head.producer.id.value mustBe "Source"
56+
bridges.head.consumer.id.value mustBe "Target"
57+
bridges.head.messageType mustBe None
58+
}
59+
}
60+
61+
"detect adaptor bridge edge with prompt-only handler" in {
62+
(td: TestData) =>
63+
runMessageFlowPass(
64+
"""domain D is {
65+
| context Source is {
66+
| adaptor ToTarget to context D.Target is {
67+
| handler Routing is {
68+
| on command D.Target.DoSomething is {
69+
| prompt "convert and forward"
70+
| }
71+
| }
72+
| }
73+
| type DoSomething is command { ??? }
74+
| }
75+
| context Target is {
76+
| type DoSomething is command { ??? }
77+
| }
78+
|}
79+
|""".stripMargin
80+
) { (mfo, _) =>
81+
val bridges = mfo.edges.filter(
82+
_.mechanism == FlowMechanism.AdaptorBridge
83+
)
84+
// At minimum, the declaration-level edge exists
85+
bridges must not be empty
86+
bridges.exists(_.messageType.isEmpty) mustBe true
87+
}
88+
}
89+
90+
"set correct direction for inbound adaptor" in {
91+
(td: TestData) =>
92+
runMessageFlowPass(
93+
"""domain D is {
94+
| context Receiver is {
95+
| adaptor FromSender from context D.Sender is { ??? }
96+
| }
97+
| context Sender is { ??? }
98+
|}
99+
|""".stripMargin
100+
) { (mfo, _) =>
101+
val bridges = mfo.edges.filter(
102+
_.mechanism == FlowMechanism.AdaptorBridge
103+
)
104+
bridges must not be empty
105+
// Inbound adaptor: producer=Sender(referent), consumer=Receiver(source)
106+
bridges.head.producer.id.value mustBe "Sender"
107+
bridges.head.consumer.id.value mustBe "Receiver"
108+
}
109+
}
110+
111+
"set correct direction for outbound adaptor" in {
112+
(td: TestData) =>
113+
runMessageFlowPass(
114+
"""domain D is {
115+
| context Source is {
116+
| adaptor ToTarget to context D.Target is { ??? }
117+
| }
118+
| context Target is { ??? }
119+
|}
120+
|""".stripMargin
121+
) { (mfo, _) =>
122+
val bridges = mfo.edges.filter(
123+
_.mechanism == FlowMechanism.AdaptorBridge
124+
)
125+
bridges must not be empty
126+
// Outbound adaptor: producer=Source, consumer=Target(referent)
127+
bridges.head.producer.id.value mustBe "Source"
128+
bridges.head.consumer.id.value mustBe "Target"
129+
}
130+
}
131+
132+
"detect tell statement edges" in { (td: TestData) =>
133+
runMessageFlowPass(
134+
"""domain D is {
135+
| context C is {
136+
| type Cmd is command { ??? }
137+
| type Evt is event { ??? }
138+
| entity Sender is {
139+
| handler H is {
140+
| on command D.C.Cmd is {
141+
| tell event D.C.Evt to entity D.C.Receiver
142+
| }
143+
| }
144+
| }
145+
| entity Receiver is {
146+
| handler H is {
147+
| on event D.C.Evt is { ??? }
148+
| }
149+
| }
150+
| }
151+
|}
152+
|""".stripMargin
153+
) { (mfo, _) =>
154+
val tells = mfo.edges.filter(
155+
_.mechanism == FlowMechanism.Tell
156+
)
157+
tells must not be empty
158+
tells.head.producer.id.value mustBe "Sender"
159+
tells.head.consumer.id.value mustBe "Receiver"
160+
tells.head.messageType mustBe defined
161+
tells.head.messageType.get.id.value mustBe "Evt"
162+
}
163+
}
164+
165+
"emit warnings on failed resolution" in { (td: TestData) =>
166+
runMessageFlowPass(
167+
"""domain D is {
168+
| context C is {
169+
| entity E is {
170+
| handler H is {
171+
| on command D.C.NonExistent is {
172+
| tell event D.C.AlsoMissing to entity D.C.Ghost
173+
| }
174+
| }
175+
| }
176+
| }
177+
|}
178+
|""".stripMargin
179+
) { (mfo, _) =>
180+
val warnings = mfo.messages.filter(_.kind.isWarning)
181+
warnings must not be empty
182+
}
183+
}
184+
}
185+
}

‎passes/shared/src/main/scala/com/ossuminc/riddl/passes/analysis/MessageFlowPass.scala‎

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ enum FlowMechanism:
3737
case class MessageFlowEdge(
3838
producer: Definition,
3939
consumer: Definition,
40-
messageType: Type,
40+
messageType: Option[Type],
4141
mechanism: FlowMechanism
4242
)
4343

@@ -65,7 +65,23 @@ case class MessageFlowOutput(
6565
producerIndex: Map[Definition, Seq[MessageFlowEdge]] = Map.empty,
6666
consumerIndex: Map[Definition, Seq[MessageFlowEdge]] = Map.empty,
6767
messageIndex: Map[Type, Seq[MessageFlowEdge]] = Map.empty
68-
) extends PassOutput
68+
) extends PassOutput:
69+
70+
/** Edges where producer or consumer is within the given domain */
71+
def edgesForDomain(domain: Domain, symbols: SymbolsOutput): Seq[MessageFlowEdge] =
72+
edges.filter: edge =>
73+
isWithin(edge.producer, domain, symbols) ||
74+
isWithin(edge.consumer, domain, symbols)
75+
76+
/** Edges where producer or consumer is within the given context */
77+
def edgesForContext(context: Context, symbols: SymbolsOutput): Seq[MessageFlowEdge] =
78+
edges.filter: edge =>
79+
isWithin(edge.producer, context, symbols) ||
80+
isWithin(edge.consumer, context, symbols)
81+
82+
private def isWithin(defn: Definition, ancestor: Definition, symbols: SymbolsOutput): Boolean =
83+
(defn == ancestor) || symbols.parentsOf(defn).contains(ancestor)
84+
end MessageFlowOutput
6985

7086
@JSExportTopLevel("MessageFlowPass$")
7187
object MessageFlowPass extends PassInfo[PassOptions] {
@@ -143,11 +159,23 @@ case class MessageFlowPass(
143159
MessageFlowEdge(
144160
producer = processor,
145161
consumer = target,
146-
messageType = msgType,
162+
messageType = Some(msgType),
147163
mechanism = FlowMechanism.Tell
148164
)
149165
)
150-
case _ => ()
166+
case _ =>
167+
if maybeTarget.isEmpty then
168+
messages.addWarning(
169+
tell.loc,
170+
s"MessageFlowPass: could not resolve tell target" +
171+
s" '${tell.processorRef.format}' in ${processor.identify}"
172+
)
173+
if maybeType.isEmpty then
174+
messages.addWarning(
175+
tell.loc,
176+
s"MessageFlowPass: could not resolve message type" +
177+
s" '${tell.msg.format}' in ${processor.identify}"
178+
)
151179
}
152180

153181
sends.foreach { send =>
@@ -164,26 +192,54 @@ case class MessageFlowPass(
164192
MessageFlowEdge(
165193
producer = processor,
166194
consumer = target,
167-
messageType = msgType,
195+
messageType = Some(msgType),
168196
mechanism = FlowMechanism.Send
169197
)
170198
)
171199
}
172-
case _ => ()
200+
case _ =>
201+
if maybePortlet.isEmpty then
202+
messages.addWarning(
203+
send.loc,
204+
s"MessageFlowPass: could not resolve send portlet" +
205+
s" '${send.portlet.format}' in ${processor.identify}"
206+
)
207+
if maybeType.isEmpty then
208+
messages.addWarning(
209+
send.loc,
210+
s"MessageFlowPass: could not resolve message type" +
211+
s" '${send.msg.format}' in ${processor.identify}"
212+
)
173213
}
174214
}
175215

176216
private def processAdaptor(
177217
adaptor: Adaptor,
178218
parents: Parents
179219
): Unit = {
180-
val maybeTargetContext =
220+
val maybeReferentContext =
181221
refMap.definitionOf[Context](adaptor.referent, adaptor)
182-
val sourceContext = parents.headOption.collect {
222+
val maybeSourceContext = parents.headOption.collect {
183223
case c: Context => c
184224
}
185-
(sourceContext, maybeTargetContext) match
186-
case (Some(source), Some(target)) =>
225+
(maybeSourceContext, maybeReferentContext) match
226+
case (Some(source), Some(referent)) =>
227+
// Determine producer/consumer based on adaptor direction
228+
val (producer, consumer) = adaptor.direction match
229+
case _: InboundAdaptor => (referent, source)
230+
case _: OutboundAdaptor => (source, referent)
231+
232+
// Always create a declaration-level edge (no message type)
233+
collectedEdges.addOne(
234+
MessageFlowEdge(
235+
producer = producer,
236+
consumer = consumer,
237+
messageType = None,
238+
mechanism = FlowMechanism.AdaptorBridge
239+
)
240+
)
241+
242+
// Create additional typed edges from handler clauses
187243
adaptor.handlers.foreach { handler =>
188244
handler.clauses.foreach {
189245
case omc: OnMessageClause =>
@@ -192,17 +248,22 @@ case class MessageFlowPass(
192248
maybeType.foreach { msgType =>
193249
collectedEdges.addOne(
194250
MessageFlowEdge(
195-
producer = target,
196-
consumer = source,
197-
messageType = msgType,
251+
producer = producer,
252+
consumer = consumer,
253+
messageType = Some(msgType),
198254
mechanism = FlowMechanism.AdaptorBridge
199255
)
200256
)
201257
}
202258
case _ => ()
203259
}
204260
}
205-
case _ => ()
261+
case _ =>
262+
messages.addWarning(
263+
adaptor.loc,
264+
s"MessageFlowPass: could not resolve adaptor context references" +
265+
s" for ${adaptor.identify}"
266+
)
206267
}
207268

208269
private def processConnector(
@@ -257,7 +318,7 @@ case class MessageFlowPass(
257318
MessageFlowEdge(
258319
producer = from,
259320
consumer = to,
260-
messageType = msgType,
321+
messageType = Some(msgType),
261322
mechanism = FlowMechanism.ConnectorPipe
262323
)
263324
)
@@ -273,7 +334,9 @@ case class MessageFlowPass(
273334
edges = edges,
274335
producerIndex = edges.groupBy(_.producer),
275336
consumerIndex = edges.groupBy(_.consumer),
276-
messageIndex = edges.groupBy(_.messageType)
337+
messageIndex = edges.collect {
338+
case e if e.messageType.isDefined => e
339+
}.groupBy(_.messageType.get)
277340
)
278341
}
279342
}

‎riddlLib/js/src/main/scala/com/ossuminc/riddl/RiddlAPI.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ object RiddlAPI {
531531
producerKind = edge.producer.kind,
532532
consumerId = edge.consumer.id.value,
533533
consumerKind = edge.consumer.kind,
534-
messageTypeId = edge.messageType.id.value,
534+
messageTypeId = edge.messageType.map(_.id.value).getOrElse(""),
535535
mechanism = edge.mechanism.toString
536536
)
537537
}.toJSArray,

‎riddlLib/js/types/index.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1039,7 +1039,7 @@ export interface MessageFlowEdge {
10391039
consumerId: string;
10401040
/** Consumer definition kind (e.g., "Entity") */
10411041
consumerKind: string;
1042-
/** Message type identifier */
1042+
/** Message type identifier (empty string if not available) */
10431043
messageTypeId: string;
10441044
/** Flow mechanism */
10451045
mechanism: 'Tell' | 'Send' | 'AdaptorBridge' | 'ConnectorPipe';

0 commit comments

Comments
 (0)