jbertram commented on code in PR #4833:
URL: https://github.com/apache/activemq-artemis/pull/4833#discussion_r1506660176
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java:
##########
@@ -1562,7 +1562,7 @@ public final Object getObjectProperty(String key) {
return getAMQPUserID();
case MessageUtil.CORRELATIONID_HEADER_NAME_STRING:
if (properties != null && properties.getCorrelationId() != null) {
- return
AMQPMessageIdHelper.INSTANCE.toCorrelationIdString(properties.getCorrelationId());
+ return
AMQPMessageIdHelper.INSTANCE.toCorrelationIdStringOrBytes(properties.getCorrelationId());
Review Comment:
This does change the semantics of the original `toCorrelationIdString`. Now
if the ID is a `Binary` it will return a `byte[]` instead of a `String`. If we
want to support compatibility between JMS clients as well as other protocols
which use `byte[]` correlation IDs I don't see a way around a change like this.
Currently no JMS client is compatible with another using
`setJMSCorrelationIDAsBytes`.
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
