gnodet commented on code in PR #22358:
URL: https://github.com/apache/camel/pull/22358#discussion_r3049972847
##########
components/camel-bean/src/main/java/org/apache/camel/component/bean/MethodInfo.java:
##########
@@ -395,16 +395,12 @@ public AccessibleObject getStaticPart() {
private void fillResult(Exchange exchange, Object result) {
LOG.trace("Setting bean invocation result : {}", result);
- // the bean component forces OUT if the MEP is OUT capable
- boolean out = exchange.hasOut() ||
ExchangeHelper.isOutCapable(exchange);
- Message old;
- if (out) {
- old = exchange.getOut();
- // propagate headers
-
exchange.getOut().getHeaders().putAll(exchange.getIn().getHeaders());
- } else {
- old = exchange.getIn();
+ // the bean component forces OUT if the MEP is OUT capable;
+ // in.copy() both creates the OUT and propagates headers in one step
+ if (ExchangeHelper.isOutCapable(exchange) &&
!ExchangeHelper.hasResponse(exchange)) {
+ ExchangeHelper.setResponse(exchange, exchange.getIn().copy());
Review Comment:
**Pattern B: `in.copy()` replaces `getOut()` + manual header propagation**
The old code did:
```java
old = exchange.getOut();
exchange.getOut().getHeaders().putAll(exchange.getIn().getHeaders());
```
`in.copy()` achieves both in one step — creates the OUT and propagates all
IN headers/body.
##########
core/camel-support/src/main/java/org/apache/camel/support/DefaultExchangeHolder.java:
##########
@@ -168,9 +169,13 @@ public static void unmarshal(Exchange exchange,
DefaultExchangeHolder payload) {
exchange.getIn().setHeaders(payload.inHeaders);
}
if (payload.outBody != null) {
- exchange.getOut().setBody(payload.outBody);
+ // reuse existing response message to preserve its type (e.g.
JmsMessage)
+ if (!ExchangeHelper.hasResponse(exchange)) {
Review Comment:
**Reuse existing response message to preserve its type**
When unmarshalling a `transferExchange` payload, the OUT message may already
be a specialized type (e.g., `JmsMessage`). The old code unconditionally
replaced it with a `DefaultMessage`, losing the type. Now we only create a new
one if none exists.
##########
core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java:
##########
@@ -1303,4 +1307,36 @@ public static <T> T getBodyAndResetStreamCache(Exchange
exchange, Class<T> type)
return exchange.getMessage().getBody(type);
}
+ /**
+ * Returns true if a response message has been explicitly set on this
exchange.
+ * <p>
+ * This is a non-deprecated equivalent of {@link Exchange#hasOut()},
provided as a utility until a proper
+ * {@code hasResponse()} method is added to the {@link Exchange} API.
+ */
+ public static boolean hasResponse(Exchange exchange) {
+ return exchange.getIn() != exchange.getMessage();
+ }
+
+ /**
Review Comment:
**Foundation: three new utility methods**
These three methods (`hasResponse`, `getResponse`, `setResponse`) centralize
all deprecated `Exchange.hasOut()`/`getOut()`/`setOut()` usage into a single
`@SuppressWarnings("deprecation")` call site.
Key semantic difference from the old API:
- `getResponse()` returns **null** when no OUT exists (unlike `getOut()`
which lazily creates an empty message)
- `hasResponse()` uses identity check (`getIn() != getMessage()`) which is
equivalent to `hasOut()`
- `setResponse()` is a direct wrapper around `setOut()`
##########
core/camel-support/src/main/java/org/apache/camel/support/processor/MarshalProcessor.java:
##########
@@ -68,8 +68,8 @@ public boolean process(Exchange exchange, AsyncCallback
callback) {
// lets setup the out message before we invoke the dataFormat
// so that it can mutate it if necessary
- Message out = exchange.getOut();
- out.copyFrom(in);
+ ExchangeHelper.setResponse(exchange, in.copy());
Review Comment:
**Pattern B: `setResponse(in.copy())` replaces `getOut().copyFrom(in)`**
Mechanically equivalent. The same pattern is used in `UnmarshalProcessor`,
`Enricher.prepareResult()`, `PollEnricher.prepareResult()`, `CamelConverter`,
and `ExchangeHelper.setInOutBodyPatternAware()`.
##########
components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrProducer.java:
##########
@@ -66,7 +68,8 @@ public void process(Exchange exchange) throws Exception {
}
}
node.addMixin("mix:referenceable");
- exchange.getOut().setBody(node.getIdentifier());
+ ExchangeHelper.setResponse(exchange, new
DefaultMessage(exchange.getContext()));
Review Comment:
**Pattern D: explicit response creation replaces `getOut().setBody()`**
The old `getOut().setBody(id)` lazily created an empty OUT and set the body.
Now we explicitly create the response. Uses `new DefaultMessage()` (not
`in.copy()`) because the response body (node UUID) is unrelated to the request
body. Same pattern in `CxfRsInvoker`.
##########
components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java:
##########
@@ -679,14 +682,19 @@ private XPathFunction createOutBodyFunction() {
return new XPathFunction() {
@SuppressWarnings("rawtypes")
public Object evaluate(List list) throws XPathFunctionException {
- if (exchange.get() != null && exchange.get().hasOut()) {
- return exchange.get().getOut().getBody();
+ if (exchange.get() != null) {
+ Message response =
ExchangeHelper.getResponse(exchange.get());
Review Comment:
**Pattern E + deprecated: `out:body()` / `out:header()` now null-safe**
The old `getOut().getBody()` would lazily create an empty OUT (side effect).
Now `getResponse()` returns null when no OUT exists, and we return null
instead. The getter/setter methods for these functions are also marked
`@Deprecated`.
##########
components/camel-bean/src/main/java/org/apache/camel/language/bean/BeanExpression.java:
##########
@@ -368,15 +369,17 @@ private static Object invokeBean(BeanHolder beanHolder,
String beanName, String
// force to use InOut to retrieve the result on the OUT message
resultExchange.setPattern(ExchangePattern.InOut);
processor.process(resultExchange);
- // the response is always stored in OUT
- result = resultExchange.hasOut() ?
resultExchange.getOut().getBody() : null;
+ // the bean component creates an OUT for non-void methods on
OUT-capable exchanges,
+ // so getResponse() returns null for void methods (no result) and
the OUT for non-void
+ Message response = ExchangeHelper.getResponse(resultExchange);
Review Comment:
**Pattern E: `getResponse()` returns null for void methods**
The bean component only creates an OUT for non-void methods on OUT-capable
exchanges. `getResponse()` returns null when no OUT exists (void method case),
unlike `getOut()` which would have lazily created an empty one. The null check
below handles this correctly.
##########
components/camel-http/src/main/java/org/apache/camel/component/http/HttpProducer.java:
##########
@@ -354,7 +355,9 @@ protected void populateResponse(
HeaderFilterStrategy strategy, int responseCode)
throws IOException, ClassNotFoundException {
- Message answer = exchange.getOut();
+ // separate response from request so response headers don't mix with
request headers
+ ExchangeHelper.setResponse(exchange, new
DefaultMessage(exchange.getContext()));
Review Comment:
**Pattern D: explicit `new DefaultMessage()` to isolate response from
request**
Uses an empty message (not `in.copy()`) so that HTTP response headers don't
mix with request headers. This preserves the old `getOut()` behavior which also
created an empty message.
##########
components/camel-netty/src/main/java/org/apache/camel/component/netty/handlers/ClientChannelHandler.java:
##########
@@ -257,10 +254,14 @@ protected Message getResponseMessage(Exchange exchange,
ChannelHandlerContext ct
// set the result on either IN or OUT on the original exchange
depending on its pattern
if (ExchangeHelper.isOutCapable(exchange)) {
- NettyPayloadHelper.setOut(exchange, body);
- return exchange.getOut();
+ // DefaultExchangeHolder unmarshals its own OUT, so only
pre-create one for normal payloads
+ if (!(body instanceof DefaultExchangeHolder)) {
Review Comment:
**Pattern D with guard: skip pre-creation for `DefaultExchangeHolder`**
When `transferExchange=true`, the body is a `DefaultExchangeHolder` whose
`unmarshal()` sets up its own OUT message. Pre-creating one here would be
overwritten. For normal payloads, we create OUT from `in.copy()` so
`setPayload()` populates the response via `getMessage()`.
##########
components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltBuilder.java:
##########
@@ -489,7 +490,11 @@ protected void configureTransformer(Transformer
transformer, Exchange exchange)
addParameters(transformer, getParameters());
transformer.setParameter("exchange", exchange);
transformer.setParameter("in", exchange.getIn());
- transformer.setParameter("out", exchange.getOut());
+ // only set "out" param if a response exists; avoids creating an empty
OUT as side effect
+ Message response = ExchangeHelper.getResponse(exchange);
Review Comment:
**Pattern E: conditional parameter avoids side-effect OUT creation**
The old code always called `exchange.getOut()` which lazily created an empty
OUT as a side effect. Now we only set the `"out"` transformer parameter if a
response actually exists.
##########
core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java:
##########
@@ -442,7 +442,11 @@ private static void copyFromOutMessage(Exchange result,
Exchange source, boolean
*/
public static Message getResultMessage(Exchange exchange) {
if (exchange.getPattern().isOutCapable()) {
- return exchange.getOut();
+ // explicitly create response if none exists (preserves old
getOut() lazy-creation semantics)
+ if (!hasResponse(exchange)) {
Review Comment:
**Explicit response creation preserves `getOut()` lazy-creation semantics**
`getResultMessage()` is the one place where we intentionally want
lazy-creation behavior (callers expect a non-null writable message). We
preserve this by explicitly creating a response if none exists.
##########
components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsProducer.java:
##########
@@ -474,10 +475,9 @@ protected Destination resolveOrCreateDestination(String
destinationName, Session
}
protected void setMessageId(Exchange exchange) {
- if (exchange.hasOut()) {
- JmsMessage out = exchange.getOut(JmsMessage.class);
+ if (ExchangeHelper.getResponse(exchange) instanceof JmsMessage out) {
Review Comment:
**Pattern E: `instanceof` replaces `hasOut()` + `getOut(Class)`**
`getResponse()` returns null when no OUT exists, and `instanceof` naturally
handles null (evaluates to false). This is more concise than the old `hasOut()`
guard + cast. Same pattern in `SjmsProducer.setMessageId()`.
--
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]
