This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch att in repository https://gitbox.apache.org/repos/asf/camel.git
commit a35c008ccffaf75928fa8dab96fd51ee9a2148ad Author: Claus Ibsen <[email protected]> AuthorDate: Mon Feb 24 09:29:55 2025 +0100 CAMEL-21755: Adjust attachments API on Message to avoid issue during routing --- .../org/apache/camel/attachment/AttachmentMap.java | 31 ------- .../camel/attachment/DefaultAttachmentMessage.java | 102 +++++++-------------- .../java/org/apache/camel/ExchangeExtension.java | 5 - .../src/main/java/org/apache/camel/Message.java | 9 ++ .../org/apache/camel/support/AbstractExchange.java | 5 +- .../org/apache/camel/support/DefaultMessage.java | 19 ++++ .../org/apache/camel/support/ExchangeHelper.java | 16 +--- .../camel/support/ExtendedExchangeExtension.java | 5 - .../org/apache/camel/support/MessageSupport.java | 22 ++++- .../apache/camel/support/MessageHelperTest.java | 23 +++-- 10 files changed, 99 insertions(+), 138 deletions(-) diff --git a/components/camel-attachments/src/main/java/org/apache/camel/attachment/AttachmentMap.java b/components/camel-attachments/src/main/java/org/apache/camel/attachment/AttachmentMap.java deleted file mode 100644 index bf4eb24c2cc..00000000000 --- a/components/camel-attachments/src/main/java/org/apache/camel/attachment/AttachmentMap.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.camel.attachment; - -import java.util.LinkedHashMap; - -import org.apache.camel.SafeCopyProperty; - -public final class AttachmentMap extends LinkedHashMap<String, Attachment> implements SafeCopyProperty { - - @Override - public SafeCopyProperty safeCopy() { - AttachmentMap clone = new AttachmentMap(); - clone.putAll(this); - return clone; - } -} diff --git a/components/camel-attachments/src/main/java/org/apache/camel/attachment/DefaultAttachmentMessage.java b/components/camel-attachments/src/main/java/org/apache/camel/attachment/DefaultAttachmentMessage.java index 10e53c88b78..49fdb908c73 100644 --- a/components/camel-attachments/src/main/java/org/apache/camel/attachment/DefaultAttachmentMessage.java +++ b/components/camel-attachments/src/main/java/org/apache/camel/attachment/DefaultAttachmentMessage.java @@ -30,18 +30,10 @@ import org.apache.camel.trait.message.MessageTrait; public final class DefaultAttachmentMessage implements AttachmentMessage { - /* - * Attachments are stores as a property on the {@link Exchange} which ensures they are propagated - * during routing and we dont have to pollute the generic {@link Message} with attachment APIs - */ - private static final String ATTACHMENT_OBJECTS = "CamelAttachmentObjects"; - private final Message delegate; - private final Exchange exchange; public DefaultAttachmentMessage(Message delegate) { this.delegate = delegate; - this.exchange = delegate.getExchange(); } @Override @@ -49,6 +41,11 @@ public final class DefaultAttachmentMessage implements AttachmentMessage { return delegate; } + @Override + public Message newInstance() { + return new DefaultAttachmentMessage(delegate); + } + @Override public void reset() { delegate.reset(); @@ -176,12 +173,12 @@ public final class DefaultAttachmentMessage implements AttachmentMessage { @Override public Message copy() { - return delegate.copy(); + return new DefaultAttachmentMessage(delegate.copy()); } @Override public void copyFrom(Message message) { - delegate.copyFrom(message); + } @Override @@ -190,106 +187,77 @@ public final class DefaultAttachmentMessage implements AttachmentMessage { } @Override - @SuppressWarnings("unchecked") public DataHandler getAttachment(String id) { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map != null) { - Attachment att = map.get(id); - if (att != null) { - return att.getDataHandler(); - } + Attachment att = (Attachment) getAttachmentsMap().get(id); + if (att != null) { + return att.getDataHandler(); } return null; } @Override - @SuppressWarnings("unchecked") public Attachment getAttachmentObject(String id) { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map != null) { - return map.get(id); - } - return null; + return (Attachment) getAttachmentsMap().get(id); } @Override - @SuppressWarnings("unchecked") public Set<String> getAttachmentNames() { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map != null) { - return map.keySet(); - } - return null; + return getAttachmentsMap().keySet(); } @Override - @SuppressWarnings("unchecked") public void removeAttachment(String id) { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map != null) { - map.remove(id); - } + getAttachmentsMap().remove(id); } @Override - @SuppressWarnings("unchecked") public void addAttachment(String id, DataHandler content) { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map == null) { - map = new AttachmentMap(); - exchange.getExchangeExtension().setSafeCopyProperty(ATTACHMENT_OBJECTS, map); - } - map.put(id, new DefaultAttachment(content)); + getAttachmentsMap().put(id, new DefaultAttachment(content)); } @Override - @SuppressWarnings("unchecked") public void addAttachmentObject(String id, Attachment content) { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - if (map == null) { - map = new AttachmentMap(); - exchange.getExchangeExtension().setSafeCopyProperty(ATTACHMENT_OBJECTS, map); - } - map.put(id, content); + getAttachmentsMap().put(id, content); } @Override - @SuppressWarnings("unchecked") public Map<String, DataHandler> getAttachments() { - Map<String, Attachment> map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, Map.class); - if (map != null) { - Map<String, DataHandler> answer = new LinkedHashMap<>(); - map.forEach((id, att) -> answer.put(id, att.getDataHandler())); - return answer; - } - return null; + Map<String, DataHandler> answer = new LinkedHashMap<>(); + getAttachmentsMap().forEach((id, att) -> { + Attachment a = (Attachment) att; + answer.put(id, a.getDataHandler()); + }); + return answer; } @Override - @SuppressWarnings("unchecked") public Map<String, Attachment> getAttachmentObjects() { - return exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, Map.class); + Map<String, Attachment> answer = new LinkedHashMap<>(); + getAttachmentsMap().forEach((id, att) -> { + Attachment a = (Attachment) att; + answer.put(id, a); + }); + return answer; } @Override public void setAttachments(Map<String, DataHandler> attachments) { - AttachmentMap map = new AttachmentMap(); - attachments.forEach((id, dh) -> map.put(id, new DefaultAttachment(dh))); - exchange.getExchangeExtension().setSafeCopyProperty(ATTACHMENT_OBJECTS, map); + attachments.forEach((id, dh) -> getAttachmentsMap().put(id, new DefaultAttachment(dh))); } @Override public void setAttachmentObjects(Map<String, Attachment> attachments) { - AttachmentMap map = new AttachmentMap(); - map.putAll(attachments); - exchange.getExchangeExtension().setSafeCopyProperty(ATTACHMENT_OBJECTS, map); + getAttachmentsMap().putAll(attachments); } @Override - @SuppressWarnings("unchecked") public boolean hasAttachments() { - AttachmentMap map = exchange.getExchangeExtension().getSafeCopyProperty(ATTACHMENT_OBJECTS, AttachmentMap.class); - return map != null && !map.isEmpty(); + return delegate.hasAttachments(); + } + + @Override + public Map<String, Object> getAttachmentsMap() { + return delegate.getAttachmentsMap(); } @Override diff --git a/core/camel-api/src/main/java/org/apache/camel/ExchangeExtension.java b/core/camel-api/src/main/java/org/apache/camel/ExchangeExtension.java index 9d13bcd16b1..ded6208da5b 100644 --- a/core/camel-api/src/main/java/org/apache/camel/ExchangeExtension.java +++ b/core/camel-api/src/main/java/org/apache/camel/ExchangeExtension.java @@ -38,11 +38,6 @@ public interface ExchangeExtension { */ <T> T getInOrNull(Class<T> type); - /** - * Prepares the exchange by setting IN as OUT - */ - void prepareInToOut(); - /** * Sets the endpoint which originated this message exchange. This method should typically only be called by * {@link Endpoint} implementations diff --git a/core/camel-api/src/main/java/org/apache/camel/Message.java b/core/camel-api/src/main/java/org/apache/camel/Message.java index a056aeb2dcb..1211354c25b 100644 --- a/core/camel-api/src/main/java/org/apache/camel/Message.java +++ b/core/camel-api/src/main/java/org/apache/camel/Message.java @@ -33,6 +33,11 @@ import org.apache.camel.trait.message.MessageTrait; */ public interface Message { + /** + * Returns a new instance of this type. + */ + Message newInstance(); + /** * Clears the message from user data, so the message can be reused. * <p/> @@ -215,6 +220,10 @@ public interface Message { */ boolean hasHeaders(); + boolean hasAttachments(); + + Map<String, Object> getAttachmentsMap(); + /** * Returns the body of the message as a POJO * <p/> diff --git a/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java b/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java index 98d1cabd2cb..f158bd54f2f 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java @@ -515,10 +515,7 @@ abstract class AbstractExchange implements Exchange { } private Message newOutMessage() { - if (in instanceof MessageSupport messageSupport) { - return messageSupport.newInstance(); - } - return new DefaultMessage(getContext()); + return in.newInstance(); } @SuppressWarnings("deprecated") diff --git a/core/camel-support/src/main/java/org/apache/camel/support/DefaultMessage.java b/core/camel-support/src/main/java/org/apache/camel/support/DefaultMessage.java index fdec471fc0f..7af576a13c4 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/DefaultMessage.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/DefaultMessage.java @@ -17,6 +17,7 @@ package org.apache.camel.support; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.function.Supplier; @@ -36,6 +37,7 @@ import org.apache.camel.spi.HeadersMapFactory; */ public class DefaultMessage extends MessageSupport { private Map<String, Object> headers; + private Map<String, Object> attachments; public DefaultMessage(Exchange exchange) { setExchange(exchange); @@ -55,6 +57,9 @@ public class DefaultMessage extends MessageSupport { if (headers != null) { headers.clear(); } + if (attachments != null) { + attachments.clear(); + } } @Override @@ -292,6 +297,20 @@ public class DefaultMessage extends MessageSupport { return !headers.isEmpty(); } + @Override + public boolean hasAttachments() { + return attachments != null && !attachments.isEmpty(); + } + + @Override + public Map<String, Object> getAttachmentsMap() { + if (attachments == null) { + // force creating attachments + attachments = new LinkedHashMap<>(); + } + return attachments; + } + @Override public DefaultMessage newInstance() { return new DefaultMessage(camelContext); diff --git a/core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java b/core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java index 054b2d904b5..1d09ab4315c 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/ExchangeHelper.java @@ -398,13 +398,8 @@ public final class ExchangeHelper { // we just need to ensure MEP is as expected (eg copy result to OUT if out capable) // and the result is not failed if (result.getPattern().isOutCapable() && !result.hasOut() && !result.isFailed()) { - // prepare IN as OUT as we expect a OUT response - if (result == source) { - // optimized - result.getExchangeExtension().prepareInToOut(); - } else { - result.getOut().copyFrom(source.getIn()); - } + // copy IN to OUT as we expect a OUT response + result.getOut().copyFrom(source.getIn()); } } @@ -415,12 +410,7 @@ public final class ExchangeHelper { // so lets assume the last IN is the OUT if (!preserverPattern && result.getPattern().isOutCapable()) { // only set OUT if its OUT capable - if (result == source) { - // optimized - result.getExchangeExtension().prepareInToOut(); - } else { - result.getOut().copyFrom(source.getIn()); - } + result.getOut().copyFrom(source.getIn()); } else { // if not replace IN instead to keep the MEP result.getIn().copyFrom(source.getIn()); diff --git a/core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java b/core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java index 8b383af7163..bb41cac492f 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java @@ -271,11 +271,6 @@ public class ExtendedExchangeExtension implements ExchangeExtension { return this.exchange.getInOrNull(type); } - @Override - public void prepareInToOut() { - this.exchange.out = this.exchange.in; - } - @Override public AsyncCallback getDefaultConsumerCallback() { return this.defaultConsumerCallback; diff --git a/core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java b/core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java index f1813d0b32b..baf81854880 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java @@ -226,12 +226,29 @@ public abstract class MessageSupport implements Message, CamelContextAware, Data getHeaders().putAll(that.getHeaders()); } } + + // the attachments may be the same instance if the end user has made some mistake + // and set the OUT message with the same attachment instance of the IN message etc + if (!sameAttachments(that)) { + if (hasAttachments()) { + // okay its safe to clear the attachments + getAttachmentsMap().clear(); + } + if (that.hasAttachments()) { + getAttachmentsMap().putAll(that.getAttachmentsMap()); + } + } + } private boolean sameHeaders(Message that) { return hasHeaders() && that.hasHeaders() && getHeaders() == that.getHeaders(); } + private boolean sameAttachments(Message that) { + return hasAttachments() && that.hasAttachments() && getAttachmentsMap() == that.getAttachmentsMap(); + } + @Override public Exchange getExchange() { return exchange; @@ -252,11 +269,6 @@ public abstract class MessageSupport implements Message, CamelContextAware, Data this.typeConverter = camelContext.getTypeConverter(); } - /** - * Returns a new instance - */ - public abstract Message newInstance(); - /** * A factory method to allow a provider to lazily create the message body for inbound messages from other sources * diff --git a/core/camel-support/src/test/java/org/apache/camel/support/MessageHelperTest.java b/core/camel-support/src/test/java/org/apache/camel/support/MessageHelperTest.java index 6ba63e877b9..32f251c710a 100644 --- a/core/camel-support/src/test/java/org/apache/camel/support/MessageHelperTest.java +++ b/core/camel-support/src/test/java/org/apache/camel/support/MessageHelperTest.java @@ -92,8 +92,12 @@ class MessageHelperTest { } @Override - public void reset() { + public Message newInstance() { + return null; + } + @Override + public void reset() { } @Override @@ -108,7 +112,6 @@ class MessageHelperTest { @Override public void setMessageId(String messageId) { - } @Override @@ -153,7 +156,6 @@ class MessageHelperTest { @Override public void setHeader(String name, Object value) { - } @Override @@ -178,7 +180,6 @@ class MessageHelperTest { @Override public void setHeaders(Map<String, Object> headers) { - } @Override @@ -186,6 +187,16 @@ class MessageHelperTest { return false; } + @Override + public boolean hasAttachments() { + return false; + } + + @Override + public Map<String, Object> getAttachmentsMap() { + return null; + } + @Override public Object getBody() { return body; @@ -213,7 +224,6 @@ class MessageHelperTest { @Override public <T> void setBody(Object body, Class<T> type) { - } @Override @@ -223,12 +233,10 @@ class MessageHelperTest { @Override public void copyFrom(Message message) { - } @Override public void copyFromWithNewBody(Message message, Object newBody) { - } @Override @@ -243,7 +251,6 @@ class MessageHelperTest { @Override public void setPayloadForTrait(MessageTrait trait, Object object) { - } } }
