gnodet commented on code in PR #22358:
URL: https://github.com/apache/camel/pull/22358#discussion_r3015630083


##########
components/camel-mina/src/main/java/org/apache/camel/component/mina/MinaProducer.java:
##########
@@ -184,12 +183,8 @@ protected void doProcess(Exchange exchange) throws 
Exception {
                 maybeDisconnectOnTimeout();
                 throw new ExchangeTimedOutException(exchange, timeout);
             } else {
-                // set the result on either IN or OUT on the original exchange 
depending on its pattern
-                if (ExchangeHelper.isOutCapable(exchange)) {
-                    MinaPayloadHelper.setOut(exchange, handler.getMessage());
-                } else {
-                    MinaPayloadHelper.setIn(exchange, handler.getMessage());
-                }
+                // set the result on the exchange
+                MinaPayloadHelper.setIn(exchange, handler.getMessage());

Review Comment:
   The old code conditionally called `setOut()` or `setIn()` based on 
`isOutCapable`. Since `setIn()` sets the body on the current IN message, and 
the exchange will route the result correctly regardless, we can always use 
`setIn()` here.



##########
components/camel-xmlsecurity/src/main/java/org/apache/camel/component/xmlsecurity/processor/XmlSignerProcessor.java:
##########
@@ -196,30 +196,21 @@ public XmlSignerConfiguration getConfiguration() {
     @Override
     public void process(Exchange exchange) throws Exception {
 
-        try {
-            LOG.debug("XML signature generation started using algorithm {} and 
canonicalization method {}", getConfiguration()
-                    .getSignatureAlgorithm(), 
getConfiguration().getCanonicalizationMethod().getAlgorithm());
-
-            // lets setup the out message before we invoke the signing
-            // so that it can mutate it if necessary
-            Message out = exchange.getOut();
-            out.copyFrom(exchange.getIn());
-
-            Document outputDoc = sign(out);
-
-            ByteArrayOutputStream outStream = new ByteArrayOutputStream();
-            XmlSignatureHelper.transformNonTextNodeToOutputStream(outputDoc, 
outStream, omitXmlDeclaration(out),
-                    getConfiguration().getOutputXmlEncoding());
-            byte[] data = outStream.toByteArray();
-            out.setBody(data);
-            setOutputEncodingToMessageHeader(out);
-            clearMessageHeaders(out);
-            LOG.debug("XML signature generation finished");
-        } catch (Exception e) {
-            // remove OUT message, as an exception occurred
-            exchange.setOut(null);
-            throw e;
-        }
+        LOG.debug("XML signature generation started using algorithm {} and 
canonicalization method {}", getConfiguration()
+                .getSignatureAlgorithm(), 
getConfiguration().getCanonicalizationMethod().getAlgorithm());
+
+        Message out = exchange.getMessage();

Review Comment:
   **Pattern: removed try/catch with `exchange.setOut(null)` on error**
   
   The old code created an OUT message upfront, copied IN into it, then worked 
on it. On exception, it set `exchange.setOut(null)` to clean up the 
partially-modified OUT. With `getMessage()`, we work directly on the current 
message — if an exception occurs, it propagates naturally without needing 
cleanup of a dangling OUT message. The `sign(out)` method reads from the 
message but only sets the body at the end (line 211), so an exception leaves 
the message in its original state.



##########
components/camel-xmlsecurity/src/main/java/org/apache/camel/component/xmlsecurity/processor/XmlVerifierProcessor.java:
##########
@@ -80,16 +80,9 @@ public XmlVerifierConfiguration getConfiguration() {
     public void process(Exchange exchange) throws Exception {
         InputStream stream = 
exchange.getIn().getMandatoryBody(InputStream.class);
         try {
-            // lets setup the out message before we invoke the signing
-            // so that it can mutate it if necessary
-            Message out = exchange.getOut();
-            out.copyFrom(exchange.getIn());
+            Message out = exchange.getMessage();

Review Comment:
   Same pattern as `XmlSignerProcessor`: removed `getOut()` + `copyFrom(in)` + 
try/catch with `setOut(null)`. The `verify()` method modifies the message 
in-place, so working on `getMessage()` directly is safe. The catch block that 
cleared OUT on error is no longer needed.



##########
components/camel-reactive-streams/src/main/java/org/apache/camel/component/reactive/streams/util/UnwrapStreamProcessor.java:
##########
@@ -79,7 +79,9 @@ private void addData() {
                         Exchange copy = (Exchange) body;
                         exchange.setException(copy.getException());
                         exchange.setIn(copy.getIn());
-                        if (copy.hasOut()) {
+                        @SuppressWarnings("deprecation")

Review Comment:
   **Pattern: exchange reconstruction — kept deprecated API with 
`@SuppressWarnings`**
   
   This code reconstructs a full exchange by copying IN, OUT, properties, and 
exception from a received exchange. This inherently requires the in/out 
distinction: if the source exchange has a separate OUT message, it must be 
transferred as-is to preserve the exchange state. This cannot be safely 
replaced with `getMessage()` without losing the OUT message semantics. The 
`@SuppressWarnings("deprecation")` annotation is the correct approach here.



##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/stored/SqlStoredProducer.java:
##########
@@ -80,24 +80,19 @@ public void execute(StatementWrapper ps) throws 
SQLException, DataAccessExceptio
                     
exchange.getIn().setHeader(SqlStoredConstants.SQL_STORED_UPDATE_COUNT, total);
                 } else {
                     Object result = ps.executeStatement();
-                    // preserve headers first, so we can override the 
SQL_ROW_COUNT and SQL_UPDATE_COUNT headers
-                    // if statement returns them
-                    
exchange.getOut().getHeaders().putAll(exchange.getIn().getHeaders());
 
                     if (result != null) {
-                        if (getEndpoint().isNoop()) {
-                            
exchange.getOut().setBody(exchange.getIn().getBody());
-                        } else if (getEndpoint().getOutputHeader() != null) {
-                            
exchange.getOut().setBody(exchange.getIn().getBody());
-                            
exchange.getOut().setHeader(getEndpoint().getOutputHeader(), result);
-                        } else {
-                            exchange.getOut().setBody(result);
+                        if (!getEndpoint().isNoop()) {

Review Comment:
   **Refactored logic with redundant header copy removed**
   
   The old code: (1) copied IN headers to OUT 
(`getOut().getHeaders().putAll(getIn().getHeaders())`), (2) for noop, set body 
from IN to OUT, (3) for outputHeader, set body from IN + set header, (4) 
otherwise set result as body. Since `getMessage()` returns IN (which already 
has all headers and body), for `noop=true` we now do nothing (body stays 
as-is), and for `outputHeader` we just set the header without re-setting the 
body.



##########
components/camel-xpath/src/main/java/org/apache/camel/language/xpath/MessageVariableResolver.java:
##########
@@ -85,12 +85,10 @@ public Object resolveVariable(QName name) {
                 answer = in.getBody();
             }
         } else if (uri.equals(OUT_NAMESPACE)) {
-            if (exchange.get().hasOut()) {
-                Message out = exchange.get().getOut();
-                answer = out.getHeader(localPart);
-                if (answer == null && localPart.equals("body")) {
-                    answer = out.getBody();
-                }
+            Message out = exchange.get().getMessage();

Review Comment:
   **Pattern: removed `hasOut()` guard**
   
   The old code checked `exchange.hasOut()` and only read from OUT if it 
existed. With `getMessage()`, we get the current message (OUT if it exists, IN 
otherwise) — so the guard is unnecessary. The behavior is equivalent: if OUT 
exists, we read from it; if not, we read from IN.



##########
components/camel-xpath/src/main/java/org/apache/camel/language/xpath/XPathBuilder.java:
##########
@@ -679,8 +679,8 @@ 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) {

Review Comment:
   Same `hasOut()` guard removal pattern. The old code returned `null` when no 
OUT existed; now `getMessage()` returns IN, and `getBody()` on IN is valid. The 
XPath function `$out:body` now returns the current message body rather than 
null — which is the correct modern behavior since the in/out distinction is 
deprecated.



##########
components/camel-mvel/src/main/java/org/apache/camel/language/mvel/RootObject.java:
##########
@@ -55,7 +55,7 @@ public Message getRequest() {
 
     @Deprecated
     public Message getResponse() {
-        return exchange.getOut();
+        return exchange.getMessage();

Review Comment:
   **Pattern: deprecated `getResponse()` method**
   
   The `getResponse()` method is itself `@Deprecated`. Using `getMessage()` 
instead of `getOut()` is appropriate since both the method and the underlying 
API are deprecated. Same change in `ognl/RootObject` and `spel/RootObject`.



##########
components/camel-mock/src/main/java/org/apache/camel/component/mock/MockExpressionClause.java:
##########
@@ -125,7 +125,7 @@ public T outMessage(final Function<Message, Object> 
function) {
         return delegate.expression(new ExpressionAdapter() {
             @Override
             public Object evaluate(Exchange exchange) {
-                return function.apply(exchange.getOut());
+                return function.apply(exchange.getMessage());

Review Comment:
   These methods (`outMessage()`, `outBody()`, `outBodyAs()`) use `getOut()` to 
access the response message. `getMessage()` is the correct replacement — it 
returns the response (OUT) if one exists, otherwise the request (IN). All 5 
`getOut()` calls in this file follow this same 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]

Reply via email to