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]