ammachado commented on code in PR #28761:
URL: https://github.com/apache/camel/pull/28761#discussion_r4429992192
##########
core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java:
##########
@@ -1012,7 +1030,17 @@ public boolean isExternalRedelivered(Message message) {
@Override
public void copyInternalProperties(Exchange target) {
- ((AbstractExchange)
target).internalProperties.putAll(internalProperties);
+ AbstractExchange exchange = (AbstractExchange) target;
+ if (toEndpoint != null) {
Review Comment:
Added a comment in the follow-up commit. One correction to the reasoning: a
removal was not propagated before either. `EnumMap.remove` empties the slot and
`putAll` copies only occupied slots, so a key that was set and then removed
left the target alone, as it does now. The only case that changes is a source
explicitly set to `null`, which used to clear the target and now keeps it. That
is noted in the PR description, and no code in the repo sets `TO_ENDPOINT` to a
literal `null`.
_Claude Code on behalf of Adriano Machado (@ammachado)_
_This was generated by an AI agent and may contain inaccuracies. Please
verify before relying on it._
##########
core/camel-core/src/test/java/org/apache/camel/support/AbstractExchangeTest.java:
##########
@@ -77,4 +89,190 @@ void shouldPreserveDataTypeOnCopy() {
assertSame(type2, ((DataTypeAware) e2.getMessage()).getDataType());
}
+ @Test
+ void shouldNotCreateInternalPropertiesForToEndpoint() {
+ // every send sets this key, so it must not cost every exchange the
map sized for all the keys
+ DefaultExchange exchange = new DefaultExchange(context);
+ assertNull(exchange.internalProperties);
+
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ assertEquals(URI,
exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertNull(exchange.internalProperties);
+
+ AbstractExchange copy = (AbstractExchange) exchange.copy();
+ assertNull(copy.internalProperties);
+ }
+
+ @Test
+ void shouldKeepToEndpointBesideOtherInternalProperties() {
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ exchange.setProperty(ExchangePropertyKey.CHARSET_NAME, "UTF-8");
+
+ assertNotNull(exchange.internalProperties);
+ assertEquals(URI,
exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertEquals("UTF-8",
exchange.getProperty(ExchangePropertyKey.CHARSET_NAME));
+
+ assertEquals("UTF-8",
exchange.removeProperty(ExchangePropertyKey.CHARSET_NAME));
+ assertEquals(URI,
exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ }
+
+ @Test
+ void shouldReadOtherInternalPropertiesWithoutTheMap() {
+ DefaultExchange exchange = new DefaultExchange(context);
+
+ assertNull(exchange.getProperty(ExchangePropertyKey.CHARSET_NAME));
+ assertNull(exchange.getProperty(Exchange.CHARSET_NAME));
+ assertNull(exchange.removeProperty(ExchangePropertyKey.CHARSET_NAME));
+ assertTrue(exchange.getInternalProperties().isEmpty());
+ assertTrue(exchange.getAllProperties().isEmpty());
+ }
+
+ @Test
+ void shouldAccessToEndpointByKeyAndByName() {
+ // the name is the public constant that routes and components use
+ DefaultExchange exchange = new DefaultExchange(context);
+
+ exchange.setProperty(Exchange.TO_ENDPOINT, URI);
+ assertEquals(URI,
exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertEquals(URI, exchange.getProperty(Exchange.TO_ENDPOINT));
+ assertEquals(URI, exchange.getProperty(Exchange.TO_ENDPOINT,
String.class));
+
+ assertEquals(URI, exchange.removeProperty(Exchange.TO_ENDPOINT));
+ assertNull(exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ assertEquals(URI,
exchange.removeProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertNull(exchange.getProperty(Exchange.TO_ENDPOINT));
+ }
+
+ @Test
+ void shouldListToEndpointWithTheInternalProperties() {
+ // the dev consoles and tracers dump the exchange with these
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ exchange.setProperty("foo", "bar");
+
+ assertEquals(Map.of("CamelToEndpoint", URI),
exchange.getInternalProperties());
+ assertEquals(Map.of("CamelToEndpoint", URI, "foo", "bar"),
exchange.getAllProperties());
+ }
+
+ @Test
+ void shouldCopyToEndpointIndependently() {
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+
+ Exchange copy = exchange.copy();
+ assertEquals(URI, copy.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+
+ copy.setProperty(ExchangePropertyKey.TO_ENDPOINT, "mock://bar");
+ assertEquals(URI,
exchange.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ }
+
+ @Test
+ void shouldCopyInternalPropertiesIndependently() {
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.CHARSET_NAME, "UTF-8");
+
+ Exchange copy = exchange.copy();
+ assertEquals("UTF-8",
copy.getProperty(ExchangePropertyKey.CHARSET_NAME));
+
+ copy.setProperty(ExchangePropertyKey.CHARSET_NAME, "ISO-8859-1");
+ assertEquals("UTF-8",
exchange.getProperty(ExchangePropertyKey.CHARSET_NAME));
+ }
+
+ @Test
+ void shouldCreateCopyWithToEndpoint() {
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ exchange.setProperty("foo", "bar");
+
+ Exchange copy =
exchange.getExchangeExtension().createCopyWithProperties(context);
+
+ assertEquals(URI, copy.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertEquals("bar", copy.getProperty("foo"));
+ assertNull(((AbstractExchange) copy).internalProperties);
+ }
+
+ @Test
+ void shouldCreateCopyWithInternalProperties() {
+ DefaultExchange exchange = new DefaultExchange(context);
+ exchange.setProperty(ExchangePropertyKey.CHARSET_NAME, "UTF-8");
+
+ Exchange copy =
exchange.getExchangeExtension().createCopyWithProperties(context);
+
+ assertEquals("UTF-8",
copy.getProperty(ExchangePropertyKey.CHARSET_NAME));
+ }
+
+ @Test
+ void shouldCopyInternalPropertiesToTarget() {
+ DefaultExchange source = new DefaultExchange(context);
+ source.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ source.setProperty(ExchangePropertyKey.CHARSET_NAME, "UTF-8");
+
+ DefaultExchange target = new DefaultExchange(context);
+ source.getExchangeExtension().copyInternalProperties(target);
+
+ assertEquals(URI, target.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertEquals("UTF-8",
target.getProperty(ExchangePropertyKey.CHARSET_NAME));
+ // the target has its own map, so it can change without changing the
source
+ target.setProperty(ExchangePropertyKey.CHARSET_NAME, "ISO-8859-1");
+ assertEquals("UTF-8",
source.getProperty(ExchangePropertyKey.CHARSET_NAME));
+ }
+
+ @Test
+ void shouldKeepPropertiesOfTargetThatTheSourceDoesNotHave() {
+ DefaultExchange target = new DefaultExchange(context);
+ target.setProperty(ExchangePropertyKey.TO_ENDPOINT, URI);
+ target.setProperty(ExchangePropertyKey.CHARSET_NAME, "UTF-8");
+
+ new
DefaultExchange(context).getExchangeExtension().copyInternalProperties(target);
+
+ assertEquals(URI, target.getProperty(ExchangePropertyKey.TO_ENDPOINT));
+ assertEquals("UTF-8",
target.getProperty(ExchangePropertyKey.CHARSET_NAME));
Review Comment:
Added as `shouldKeepToEndpointOfTargetWhenTheSourceRemovedIt`. The target
keeps its value on this path both before and after this change, so the test
pins existing behavior rather than a new semantic. It also guards against the
`null` check being dropped, which would wrongly clear the target.
_Claude Code on behalf of Adriano Machado (@ammachado)_
_This was generated by an AI agent and may contain inaccuracies. Please
verify before relying on it._
--
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]
