Core Message typed property

[jmesnil]

I'm reviewing James' patch for https://jira.jboss.org/jira/browse/HORNETQ-176 and the task does not define how we want to handle property conversion.

E.g. do we return primitive types or Object subclasses? do we allow to get a String from a boolean property? What values do we return if the prop is not set? etc.

I think we should use the same semantic than the JMS Message API for conversion, null values, return types, etc. The only change to do is to throw IllegalStateException in case of conversion issues rather than a JMS specific exception. http://java.sun.com/javaee/5/docs/api/javax/jms/Message.html

Otherwise, the Core Message API would be harder to use correctly (the user will need to check for the prop before getting it, need to cast from one type to another, etc.)

Implementation-wise, this means that all the conversion code would move from HornetQMessage to MessageImpl.

wdyt?

[jamescarr]

I agree with your suggestions. :)

[timfox]

+1, just do what what JMS does

[clebert.suconic]

+1

[mural74]

Hi,

I am getting some grasp of the HornetQ codebase since I would like to collaborate with the project.

I was looking at the MessageImpl and HornetQMessage code and I have some comments regarding this issue that I would like to share.

1) Thorwing IllegalStateException in the new getters (MessageImpl) will force HornetQMessage to catch the Runtime exception because it needs to rethrow a MessageFormatException as required by the JMS API. Probably a better solution is to throw a custom exception in the getters to indicate a failure in the conversion, making possible to translate it to MessageFormatException in the proper classes.

2) Returning primitive types (from MessageImpl) as JMS does may be not the best idea. It is always possible to unbox the object (Long/Integer/etc) when it is required, eagerly unboxing the objects may lead to unnecesary reboxings (This will depend on the usage). Since eagerly unboxing the objects does not provide any benefit, it would be better to avoid possible reboxings and do the unboxing when it is required (at HornetQMessage).
In addition, PropertyValue class is maintaining the value as a primitive type but it boxes it every time it is accessed. Keeping the boxed value instead of the primitive one could be a little improvement.

[jamescarr]

I've completed this, it was pretty easy as most of the behavior was defined in the HornetQMessage, however there is some behavior I am curious if it is needed. It seems each of the typed getters include the following logic:

if (JMSXDELIVERYCOUNT.equals(name))
 {
 return message.getDeliveryCount();
 }

Is this needed in the new core MessageImpl?

Also mural74 does make some good points, however the JMS Message interface does return primitives. :S

[clebert.suconic]

I remember this being some JMS spec requirement. (I would need to double check it).

That will be needed on the JMS Layer (JMSMessage) only anyway. (No need to do it on the core message side)

You could do:

if (JMSDELIVERYCOUNT.equals(propname))
{
 return count;
}
else
{
 return coreMessage.getBlaBlaBla();
}

[timfox]

Actually, I agree with mural74 on this one:

It makes sense for the core api to return objects rather than the primitive types to avoid unnecessary boxing/unboxing.

The JMS layer can always unbox them anyway.