tabish121 commented on code in PR #4833:
URL: https://github.com/apache/activemq-artemis/pull/4833#discussion_r1505003120
##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireMessageConverter.java:
##########
@@ -590,9 +593,14 @@ private static ActiveMQMessage
toAMQMessage(MessageReference reference,
}
amqMsg.setCommandId(commandId);
- final SimpleString corrId = getObjectProperty(coreMessage,
SimpleString.class, OpenWireConstants.JMS_CORRELATION_ID_PROPERTY);
- if (corrId != null) {
- amqMsg.setCorrelationId(corrId.toString());
+ final Object correlationID = coreMessage.getCorrelationID();
+ if (correlationID != null) {
+ if (correlationID instanceof String || correlationID instanceof
SimpleString) {
Review Comment:
Can the core message carry something else besides SimpleString, String or
byte[] and if so should it be ignored as it is now? If not should the if be
inverted to simply check for byte[] and just toString anything else?
##########
artemis-protocols/artemis-amqp-protocol/src/test/java/org/apache/activemq/artemis/protocol/amqp/converter/message/AMQPMessageIdHelperTest.java:
##########
@@ -458,22 +459,21 @@ public void testToCorrelationIdStringWithUnsignedLong() {
}
/**
- * Test that {@link AMQPMessageIdHelper#toCorrelationIdString(Object)}
- * returns a string indicating an AMQP encoded binary when given a Binary
- * object.
+ * Test that {@link
AMQPMessageIdHelper#toCorrelationIdStringOrBytes(Object)}
+ * returns a byte[] when given a Binary object.
*/
@Test
- public void testToCorrelationIdStringWithBinary() {
+ public void testToCorrelationIdByteArrayWithBinary() {
byte[] bytes = new byte[] {(byte) 0x00, (byte) 0xAB, (byte) 0x09, (byte)
0xFF};
Binary binary = new Binary(bytes);
- String expected = AMQPMessageIdHelper.JMS_ID_PREFIX +
AMQPMessageIdHelper.AMQP_BINARY_PREFIX + "00AB09FF";
-
- doToCorrelationIDTestImpl(binary, expected);
+ Object idString = messageIdHelper.toCorrelationIdStringOrBytes(binary);
Review Comment:
If the result isn't expected to be a string then naming it 'idString' seems
misleading to the reader
--
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
