You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ra...@apache.org on 2010/07/28 04:17:29 UTC
svn commit: r979933 -
/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
Author: rajith
Date: Wed Jul 28 02:17:29 2010
New Revision: 979933
URL: http://svn.apache.org/viewvc?rev=979933&view=rev
Log:
QPID-2766
Instead of doing Double.valueOf(Float.valueOf(value)) , I changed it to use Double.valueOf(value).
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java?rev=979933&r1=979932&r2=979933&view=diff
==============================================================================
--- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java (original)
+++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java Wed Jul 28 02:17:29 2010
@@ -599,7 +599,7 @@ public class AMQMessageDelegate_0_10 ext
{
try
{
- return Double.valueOf(getFloatProperty(propertyName));
+ return Double.valueOf(getStringProperty(propertyName));
}
catch(MessageFormatException e)
{
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org
Re: svn commit: r979933 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
Posted by Rajith Attapattu <ra...@gmail.com>.
On Wed, Jul 28, 2010 at 7:07 AM, Robbie Gemmell
<ro...@gmail.com> wrote:
> Whilst I agree with the reason for making it I dont think the change
> made is entirely correct as it opens us to violating the JMS spec for
> property type conversion, since it will now first convert all property
> types (except Double obviously) to a String and then on to a Double.
> Only Floats, Doubles, and Strings are allowed to be read back as a
> Double value, everything else should throw a MessageFormatException. I
> would suggest the change should be something more along the lines of:
>
> if(o instanceof Double)
> {
> return ((Double)o).doubleValue();
> }
> + else if(o instanceof String)
> + {
> + return Double.valueOf((String) o);
> + }
> else
> {
> try
> {
> return Double.valueOf(getFloatProperty(propertyName));
>
Agreed !
> In looking at this I have also come to notice that the 0-10 message
> property handling (do we really need to have seperate handler code for
> 0-8/9 and 0-10 for all of this?)
Yes when I looked at this last night I had the same question too.
I think this code could probably be moved to the abstract message delegate.
> seems to improperly convert String
> properties into short, int, and long by delegating the conversion down
> to the getByteProperty() method which then restricts it to using
> Byte.valueof(), which is rarely going to work correctly. Ill raise a
> JIRA for that.
Cool.
> Robbie
>
> On 28 July 2010 03:17, <ra...@apache.org> wrote:
>> Author: rajith
>> Date: Wed Jul 28 02:17:29 2010
>> New Revision: 979933
>>
>> URL: http://svn.apache.org/viewvc?rev=979933&view=rev
>> Log:
>> QPID-2766
>> Instead of doing Double.valueOf(Float.valueOf(value)) , I changed it to use Double.valueOf(value).
>>
>> Modified:
>> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>>
>> Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java?rev=979933&r1=979932&r2=979933&view=diff
>> ==============================================================================
>> --- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java (original)
>> +++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java Wed Jul 28 02:17:29 2010
>> @@ -599,7 +599,7 @@ public class AMQMessageDelegate_0_10 ext
>> {
>> try
>> {
>> - return Double.valueOf(getFloatProperty(propertyName));
>> + return Double.valueOf(getStringProperty(propertyName));
>> }
>> catch(MessageFormatException e)
>> {
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
--
Regards,
Rajith Attapattu
Red Hat
http://rajith.2rlabs.com/
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r979933 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
Posted by Robbie Gemmell <ro...@gmail.com>.
Whilst I agree with the reason for making it I dont think the change
made is entirely correct as it opens us to violating the JMS spec for
property type conversion, since it will now first convert all property
types (except Double obviously) to a String and then on to a Double.
Only Floats, Doubles, and Strings are allowed to be read back as a
Double value, everything else should throw a MessageFormatException. I
would suggest the change should be something more along the lines of:
if(o instanceof Double)
{
return ((Double)o).doubleValue();
}
+ else if(o instanceof String)
+ {
+ return Double.valueOf((String) o);
+ }
else
{
try
{
return Double.valueOf(getFloatProperty(propertyName));
In looking at this I have also come to notice that the 0-10 message
property handling (do we really need to have seperate handler code for
0-8/9 and 0-10 for all of this?) seems to improperly convert String
properties into short, int, and long by delegating the conversion down
to the getByteProperty() method which then restricts it to using
Byte.valueof(), which is rarely going to work correctly. Ill raise a
JIRA for that.
Robbie
On 28 July 2010 03:17, <ra...@apache.org> wrote:
> Author: rajith
> Date: Wed Jul 28 02:17:29 2010
> New Revision: 979933
>
> URL: http://svn.apache.org/viewvc?rev=979933&view=rev
> Log:
> QPID-2766
> Instead of doing Double.valueOf(Float.valueOf(value)) , I changed it to use Double.valueOf(value).
>
> Modified:
> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>
> Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java?rev=979933&r1=979932&r2=979933&view=diff
> ==============================================================================
> --- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java (original)
> +++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java Wed Jul 28 02:17:29 2010
> @@ -599,7 +599,7 @@ public class AMQMessageDelegate_0_10 ext
> {
> try
> {
> - return Double.valueOf(getFloatProperty(propertyName));
> + return Double.valueOf(getStringProperty(propertyName));
> }
> catch(MessageFormatException e)
> {
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org