qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Kennedy <andrewinternatio...@gmail.com>
Subject Re: svn commit: r979316 - /qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java
Date Sun, 01 Aug 2010 14:48:35 GMT
Agreed,

In fact, I've changed all of the places that check for this situation  
to use the same message. The only problem is, I had to find somewhere  
to put the exception message being used, since it needed to be  
available for two different exceptions, one of which is a JMS  
provided class. In the end, I chose a public static final field in  
the AMQPInvalidClassException class, since it is accessible by all  
the code paths. However I view this as a stop-gap solution, since if  
we ever want to embark on any i18n of messages this isn't the best  
option. Should there be some kind of error message resource file,  
perhaps as part of the common module?

I used a similar mechanism for error messages in the access control  
plugin with printf style placeholders, but since one of the things  
that is being worked on currently is broker and client exceptions, we  
should put some more thought into the message text handling. As  
mentioned, the obvious ways are final Strings in a single interface  
or class, or resources in property files - with either printf '%s'  
style placeholders for String.format(...) or '{0}' MessageFormat  
style placeholders. Since resource files are more suitable for i18n,  
I'd prefer this way, although it involves a bit more work, but the  
choice of printf versus MessageFormat is just a matter of preference.

Any ideas appreciated...

Andrew.
-- 
-- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
-- http://grkvlt.blogspot.com/ ? edinburgh : +44 7941 197 134 ;

On 1 Aug 2010, at 00:25, Martin Ritchie wrote:

> On 26 July 2010 15:39,  <grkvlt@apache.org> wrote:
>> Author: grkvlt
>> Date: Mon Jul 26 14:39:33 2010
>> New Revision: 979316
>>
>> URL: http://svn.apache.org/viewvc?rev=979316&view=rev
>> Log:
>> QPID-2744: Make JMSPropertiesTest deal with both types of error  
>> messages
>>
>> Modified:
>>    qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ 
>> test/unit/message/JMSPropertiesTest.java
>>
>> Modified: qpid/trunk/qpid/java/systests/src/main/java/org/apache/ 
>> qpid/test/unit/message/JMSPropertiesTest.java
>> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/systests/ 
>> src/main/java/org/apache/qpid/test/unit/message/ 
>> JMSPropertiesTest.java?rev=979316&r1=979315&r2=979316&view=diff
>> ===================================================================== 
>> =========
>> --- qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ 
>> test/unit/message/JMSPropertiesTest.java (original)
>> +++ qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ 
>> test/unit/message/JMSPropertiesTest.java Mon Jul 26 14:39:33 2010
>> @@ -100,7 +100,8 @@ public class JMSPropertiesTest extends Q
>>         catch (MessageFormatException mfe)
>>         {
>>             // Check the error message
>> -            assertTrue("Incorrect error message: " +  
>> mfe.getMessage(), mfe.getMessage().contains("Object is null"));
>> +            assertEquals("Incorrect error message",
>> +                    isBroker010() ? "Object is null" : "Only  
>> Primitives objects allowed Object is:null", mfe.getMessage());
>>         }
>>
>>         // send it
>
> As the 0-10 code base was recently modified to throw "Object is null"
> I would suggest we update it to throw the same exception text as the
> 0-8/9 code path.. this will simplify testing and simplify the user
> experience.
>
> Martin

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Mime
View raw message