gnodet commented on code in PR #24031:
URL: https://github.com/apache/camel/pull/24031#discussion_r3416184086
##########
core/camel-core-model/src/main/java/org/apache/camel/model/SendDefinition.java:
##########
@@ -42,6 +43,7 @@ public abstract class SendDefinition<Type extends
ProcessorDefinition<Type>> ext
@XmlAttribute
@Metadata(required = true)
+ @DslArg
Review Comment:
Note: this `@DslArg` (position=0 by default) is inherited by `ToDefinition`,
`ToDynamicDefinition`, `WireTapDefinition`, etc. The position interplay with
`ToDefinition.pattern` (position=1) produces a reversed argument order — see
comment on `ToDefinition.java`.
##########
core/camel-core-model/src/main/java/org/apache/camel/model/ToDefinition.java:
##########
@@ -40,6 +41,7 @@ public class ToDefinition extends
SendDefinition<ToDefinition> {
private String variableReceive;
@XmlAttribute
@Metadata(label = "advanced", javaType =
"org.apache.camel.ExchangePattern", enums = "InOnly,InOut")
+ @DslArg(position = 1, renderType = "enumString", typeName =
"ExchangePattern")
Review Comment:
🔴 **Bug: `to()` parameter ordering is reversed**
With `uri` at position 0 (inherited from `SendDefinition`) and `pattern` at
position 1, the generated code produces:
```java
.to("direct:foo", ExchangePattern.InOut)
```
But `ProcessorDefinition` only has `to(ExchangePattern pattern, String uri)`
— pattern first, uri second. There is no `to(String, ExchangePattern)`
overload, so this would fail to compile.
Fix: either swap positions (`pattern` = 0, `uri` = 1), or — since `to(uri)`
is by far the most common form and `to(pattern, uri)` is rare — consider
whether the pattern should even be a primary arg at all (it could just be a
chained attribute: `.to("uri").pattern(ExchangePattern.InOut)`).
Not caught by tests because no XML test file has `<to uri="..."
pattern="..."/>`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
