[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1684
e.g.
```
public class OpenwireMessage extends RefCountMessage {
org.apache.activemq.command.Message message;
public OpenwireMessage(org.apache.activemq.command.Message message){
this.message = message;
}
@Override
public SimpleString getReplyTo() {
return
SimpleString.toSimpleString(message.getReplyTo().getPhysicalName());
}
@Override
public Message setReplyTo(SimpleString address) {
message.setReplyTo(ActiveMQDestination.createDestination(address.toString(),
ActiveMQDestination.QUEUE_TYPE));
return this;
}
@Override
public Object getUserID() {
return message.getUserID();
}
@Override
public Message setUserID(Object userID) {
message.setUserID(userID.toString());
return this;
}
@Override
public boolean isDurable() {
return message.isPersistent();
}
@Override
public Message setDurable(boolean durable) {
message.setPersistent(durable);
return this;
}
.
}
```
---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 @RaiSaurabh this isn't what was quite meant, the idea of implementing Message is to avoid conversion, so it only needs to convert if cross protocol on consume. You have only extended and kept the conversion to CoreMessage, not entirely the intent expected. E.g. the idea is that you have similar to what occurs with AMQPMessage, that internally it is kept unconverted in AMQP form, so that if AMQP producer and AMQP consumer, it doesn't convert to CoreMessage. ---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user RaiSaurabh commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 @michaelandrepearce I have taken your suggestion and implemented it. Currently, I am testing it will push it in a day or two. I am ensuring that all the headers are retained on conversion of protocols. Hope it is alright for you. ---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 @RaiSaurabh seems you've reverted this, are you working on an alternative solution? Is it worth closing this PR till ready? ---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1684
On that note, going through the OpenWireConverter that currently converts
OpenWire to Core on produce, there is many places where its not setting the
correct matching properties (or methods) on core because it has them hardcoded
as string, instead of using the constants from Message (in core).
Example
```
String groupId = messageSend.getGroupID();
if (groupId != null) {
coreMessage.putStringProperty(AMQ_MSG_GROUP_ID, groupId);
}
```
Here the property being set is "__HDR_GROUP_ID" where as in core Message if
using the constants from there, then actually what should be set is
```
String groupId = messageSend.getGroupID();
if (groupId != null) {
coreMessage.putStringProperty(org.apache.activemq.artemis.api.core.Message.HDR_GROUP_ID,
SimpleString.toSimpleString(groupId));
}
```
so that it actually sets the correct property than then is handled by other
converters properly. (its seems this is quite numerous in the converter, just
briefly going through it. )
(please be aware this are of code I'm less familiar in artemis with but a
brief look through, i see such oddities)
---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 So here I would either suggest that these are not copied to core message when converting to core, but if need because of consuming with openwire consumer then probably better but a bit more work is to make OpenWire Message so it doesnât convert on produce only on consume eg some work is done to make it implement some internal interfaces as like what was for for amqp, this would save conversion to core also if producer and consumer are openwire making it more performant. ---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user RaiSaurabh commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 @michaelandrepearce @clebertsuconic Please correct me if my understanding is wrong. I checked the code of OpenwireMesageConverter when we send a message using client if comes to (https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireMessageConverter.java#L122) where we set all the internal headers with __HDR as message property and return the CoreMessage. If we try to receive the message from the Openwire client we use these message properties to set them as ActiveMQMessage object properties like ID etc. Now with the case, if I use the AMQP/ Core receiver the message that is fetched is in form of ICoreMessage in CoreAmqpConverter class (https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/CoreAmqpConverter.java#L104) and all the internal headers of Openwire are present as message properties. Hence I decided to stip it off from the CoreAmqpConverter class. I was not able to find any better place. If you could point me that would be great. ---
[GitHub] activemq-artemis issue #1684: ARTEMIS-1498: Openwire internal headers should...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1684 If this is specific to OpenWire headers, should this not be stripped during converting open wire messages within the open wire protocol support module, it shouldn't be leaking into other protocols. ---
