You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by ta...@apache.org on 2011/12/04 19:07:53 UTC

svn commit: r1210164 - in /tapestry/tapestry5/branches/5.3/tapestry-core/src: main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Author: tawus
Date: Sun Dec  4 18:07:53 2011
New Revision: 1210164

URL: http://svn.apache.org/viewvc?rev=1210164&view=rev
Log:
TAP5-1763: NPE is caused when you create a Translator with null message key. Fixed by using the 'translator-name'-message in case the message key is null

Modified:
    tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
    tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Modified: tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java?rev=1210164&r1=1210163&r2=1210164&view=diff
==============================================================================
--- tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java (original)
+++ tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java Sun Dec  4 18:07:53 2011
@@ -125,7 +125,14 @@ public class FieldTranslatorSourceImpl i
             return overrideMessages.getFormatter(overrideKey);
 
         // Otherwise, use the built-in validation message appropriate to this validator.
+        String messageKey = translator.getMessageKey();
 
-        return globalMessages.getFormatter(translator.getMessageKey());
+        // If no key has been specified, use translator name to create a key
+        if(messageKey == null)
+        {
+            messageKey = translatorName + "-message";
+        }
+
+        return globalMessages.getFormatter(messageKey);
     }
 }

Modified: tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java?rev=1210164&r1=1210163&r2=1210164&view=diff
==============================================================================
--- tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java (original)
+++ tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java Sun Dec  4 18:07:53 2011
@@ -113,6 +113,50 @@ public class FieldTranslatorSourceImplTe
     }
 
     @Test
+    public void create_default_translator_with_name_and_null_key()
+    {
+        Field field = mockField();
+        Messages messages = mockMessages();
+        Locale locale = Locale.ENGLISH;
+        Class propertyType = Map.class;
+        TranslatorSource ts = mockTranslatorSource();
+        FormSupport fs = mockFormSupport();
+        Translator translator = mockTranslator("maptrans", Map.class);
+        Messages globalMessages = mockMessages();
+        MessageFormatter formatter = mockMessageFormatter();
+        MarkupWriter writer = mockMarkupWriter();
+        String label = "Field Label";
+        String message = "Woops, did it again.";
+        AnnotationProvider ap = mockAnnotationProvider(null);
+
+        train_findByType(ts, propertyType, translator);
+
+        train_getFormValidationId(fs, "myform");
+
+        train_contains(messages, "myform-myfield-maptrans-message", false);
+        train_contains(messages, "myfield-maptrans-message", false);
+        train_getMessageKey(translator, null);
+
+        train_getMessageFormatter(globalMessages, "maptrans-message", formatter);
+        train_getLabel(field, label);
+        train_format(formatter, message, label);
+
+        translator.render(field, message, writer, fs);
+
+        replay();
+
+        FieldTranslatorSource source = new FieldTranslatorSourceImpl(ts, globalMessages, fs);
+
+        FieldTranslator ft = source.createDefaultTranslator(field, "myfield", messages, locale, propertyType, ap);
+
+        assertEquals(ft.getType(), Map.class);
+
+        ft.render(writer);
+
+        verify();
+    }
+
+    @Test
     public void create_default_translator_with_name()
     {
         Field field = mockField();



Re: svn commit: r1210164 - in /tapestry/tapestry5/branches/5.3/tapestry-core/src: main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Posted by Howard Lewis Ship <hl...@gmail.com>.
Warnings are useless, at least when logged to the console.  I'm
increasingly of the opinion that warnings should be logged to
AlertManager instead to force action from the developer.

Also, any plans to port this forward to 5.4 (trunk)?

On Mon, Dec 5, 2011 at 1:47 AM, Christian Riedel
<cr...@googlemail.com> wrote:
> Getting some kind of warning would be nice, though. Maybe you could at least add a log.warn statement to inform the developer?
>
>
> Am 05.12.2011 um 02:34 schrieb Taha Hafeez Siddiqi:
>
>> Hi Igor,
>>
>> I also thought of that but then I settled for this solution because 1) it doesn't break 5.2 code 2) we are already using translator-name in the overriding messages 3) throwing an exception because of a missing message-key doesn't seem right.
>>
>> regards
>> Taha
>>
>> On Dec 5, 2011, at 3:43 AM, Igor Drobiazko wrote:
>>
>>> Wouldn't it be better to validate each translator in TranslatorSourceImpl
>>> and throw an IllegalRgumentException with a detailed message if a
>>> translator doesn't provide a message key?  This way the user would
>>> immediately at startup know that he needs to provide a key. A default key
>>> without any value doesn't make any sense to me.
>>>
>>> On Sun, Dec 4, 2011 at 7:07 PM, <ta...@apache.org> wrote:
>>>
>>>> Author: tawus
>>>> Date: Sun Dec  4 18:07:53 2011
>>>> New Revision: 1210164
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1210164&view=rev
>>>> Log:
>>>> TAP5-1763: NPE is caused when you create a Translator with null message
>>>> key. Fixed by using the 'translator-name'-message in case the message key
>>>> is null
>>>>
>>>> Modified:
>>>>
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>>>
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>>>
>>>> Modified:
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>>> (original)
>>>> +++
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>>> Sun Dec  4 18:07:53 2011
>>>> @@ -125,7 +125,14 @@ public class FieldTranslatorSourceImpl i
>>>>           return overrideMessages.getFormatter(overrideKey);
>>>>
>>>>       // Otherwise, use the built-in validation message appropriate to
>>>> this validator.
>>>> +        String messageKey = translator.getMessageKey();
>>>>
>>>> -        return globalMessages.getFormatter(translator.getMessageKey());
>>>> +        // If no key has been specified, use translator name to create a
>>>> key
>>>> +        if(messageKey == null)
>>>> +        {
>>>> +            messageKey = translatorName + "-message";
>>>> +        }
>>>> +
>>>> +        return globalMessages.getFormatter(messageKey);
>>>>   }
>>>> }
>>>>
>>>> Modified:
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>>> (original)
>>>> +++
>>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>>> Sun Dec  4 18:07:53 2011
>>>> @@ -113,6 +113,50 @@ public class FieldTranslatorSourceImplTe
>>>>   }
>>>>
>>>>   @Test
>>>> +    public void create_default_translator_with_name_and_null_key()
>>>> +    {
>>>> +        Field field = mockField();
>>>> +        Messages messages = mockMessages();
>>>> +        Locale locale = Locale.ENGLISH;
>>>> +        Class propertyType = Map.class;
>>>> +        TranslatorSource ts = mockTranslatorSource();
>>>> +        FormSupport fs = mockFormSupport();
>>>> +        Translator translator = mockTranslator("maptrans", Map.class);
>>>> +        Messages globalMessages = mockMessages();
>>>> +        MessageFormatter formatter = mockMessageFormatter();
>>>> +        MarkupWriter writer = mockMarkupWriter();
>>>> +        String label = "Field Label";
>>>> +        String message = "Woops, did it again.";
>>>> +        AnnotationProvider ap = mockAnnotationProvider(null);
>>>> +
>>>> +        train_findByType(ts, propertyType, translator);
>>>> +
>>>> +        train_getFormValidationId(fs, "myform");
>>>> +
>>>> +        train_contains(messages, "myform-myfield-maptrans-message",
>>>> false);
>>>> +        train_contains(messages, "myfield-maptrans-message", false);
>>>> +        train_getMessageKey(translator, null);
>>>> +
>>>> +        train_getMessageFormatter(globalMessages, "maptrans-message",
>>>> formatter);
>>>> +        train_getLabel(field, label);
>>>> +        train_format(formatter, message, label);
>>>> +
>>>> +        translator.render(field, message, writer, fs);
>>>> +
>>>> +        replay();
>>>> +
>>>> +        FieldTranslatorSource source = new FieldTranslatorSourceImpl(ts,
>>>> globalMessages, fs);
>>>> +
>>>> +        FieldTranslator ft = source.createDefaultTranslator(field,
>>>> "myfield", messages, locale, propertyType, ap);
>>>> +
>>>> +        assertEquals(ft.getType(), Map.class);
>>>> +
>>>> +        ft.render(writer);
>>>> +
>>>> +        verify();
>>>> +    }
>>>> +
>>>> +    @Test
>>>>   public void create_default_translator_with_name()
>>>>   {
>>>>       Field field = mockField();
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>>
>>> Igor Drobiazko
>>> http://tapestry5.de
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
>> For additional commands, e-mail: dev-help@tapestry.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>



-- 
Howard M. Lewis Ship

Creator of Apache Tapestry

The source for Tapestry training, mentoring and support. Contact me to
learn how I can get you up and productive in Tapestry fast!

(971) 678-5210
http://howardlewisship.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r1210164 - in /tapestry/tapestry5/branches/5.3/tapestry-core/src: main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Posted by Christian Riedel <cr...@googlemail.com>.
Getting some kind of warning would be nice, though. Maybe you could at least add a log.warn statement to inform the developer?


Am 05.12.2011 um 02:34 schrieb Taha Hafeez Siddiqi:

> Hi Igor,
> 
> I also thought of that but then I settled for this solution because 1) it doesn't break 5.2 code 2) we are already using translator-name in the overriding messages 3) throwing an exception because of a missing message-key doesn't seem right.
> 
> regards
> Taha
> 
> On Dec 5, 2011, at 3:43 AM, Igor Drobiazko wrote:
> 
>> Wouldn't it be better to validate each translator in TranslatorSourceImpl
>> and throw an IllegalRgumentException with a detailed message if a
>> translator doesn't provide a message key?  This way the user would
>> immediately at startup know that he needs to provide a key. A default key
>> without any value doesn't make any sense to me.
>> 
>> On Sun, Dec 4, 2011 at 7:07 PM, <ta...@apache.org> wrote:
>> 
>>> Author: tawus
>>> Date: Sun Dec  4 18:07:53 2011
>>> New Revision: 1210164
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1210164&view=rev
>>> Log:
>>> TAP5-1763: NPE is caused when you create a Translator with null message
>>> key. Fixed by using the 'translator-name'-message in case the message key
>>> is null
>>> 
>>> Modified:
>>> 
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>> 
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>> 
>>> Modified:
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>> URL:
>>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>>> 
>>> ==============================================================================
>>> ---
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>> (original)
>>> +++
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>>> Sun Dec  4 18:07:53 2011
>>> @@ -125,7 +125,14 @@ public class FieldTranslatorSourceImpl i
>>>           return overrideMessages.getFormatter(overrideKey);
>>> 
>>>       // Otherwise, use the built-in validation message appropriate to
>>> this validator.
>>> +        String messageKey = translator.getMessageKey();
>>> 
>>> -        return globalMessages.getFormatter(translator.getMessageKey());
>>> +        // If no key has been specified, use translator name to create a
>>> key
>>> +        if(messageKey == null)
>>> +        {
>>> +            messageKey = translatorName + "-message";
>>> +        }
>>> +
>>> +        return globalMessages.getFormatter(messageKey);
>>>   }
>>> }
>>> 
>>> Modified:
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>> URL:
>>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>>> 
>>> ==============================================================================
>>> ---
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>> (original)
>>> +++
>>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>>> Sun Dec  4 18:07:53 2011
>>> @@ -113,6 +113,50 @@ public class FieldTranslatorSourceImplTe
>>>   }
>>> 
>>>   @Test
>>> +    public void create_default_translator_with_name_and_null_key()
>>> +    {
>>> +        Field field = mockField();
>>> +        Messages messages = mockMessages();
>>> +        Locale locale = Locale.ENGLISH;
>>> +        Class propertyType = Map.class;
>>> +        TranslatorSource ts = mockTranslatorSource();
>>> +        FormSupport fs = mockFormSupport();
>>> +        Translator translator = mockTranslator("maptrans", Map.class);
>>> +        Messages globalMessages = mockMessages();
>>> +        MessageFormatter formatter = mockMessageFormatter();
>>> +        MarkupWriter writer = mockMarkupWriter();
>>> +        String label = "Field Label";
>>> +        String message = "Woops, did it again.";
>>> +        AnnotationProvider ap = mockAnnotationProvider(null);
>>> +
>>> +        train_findByType(ts, propertyType, translator);
>>> +
>>> +        train_getFormValidationId(fs, "myform");
>>> +
>>> +        train_contains(messages, "myform-myfield-maptrans-message",
>>> false);
>>> +        train_contains(messages, "myfield-maptrans-message", false);
>>> +        train_getMessageKey(translator, null);
>>> +
>>> +        train_getMessageFormatter(globalMessages, "maptrans-message",
>>> formatter);
>>> +        train_getLabel(field, label);
>>> +        train_format(formatter, message, label);
>>> +
>>> +        translator.render(field, message, writer, fs);
>>> +
>>> +        replay();
>>> +
>>> +        FieldTranslatorSource source = new FieldTranslatorSourceImpl(ts,
>>> globalMessages, fs);
>>> +
>>> +        FieldTranslator ft = source.createDefaultTranslator(field,
>>> "myfield", messages, locale, propertyType, ap);
>>> +
>>> +        assertEquals(ft.getType(), Map.class);
>>> +
>>> +        ft.render(writer);
>>> +
>>> +        verify();
>>> +    }
>>> +
>>> +    @Test
>>>   public void create_default_translator_with_name()
>>>   {
>>>       Field field = mockField();
>>> 
>>> 
>>> 
>> 
>> 
>> -- 
>> Best regards,
>> 
>> Igor Drobiazko
>> http://tapestry5.de
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r1210164 - in /tapestry/tapestry5/branches/5.3/tapestry-core/src: main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Posted by Taha Hafeez Siddiqi <ta...@gmail.com>.
Hi Igor,

I also thought of that but then I settled for this solution because 1) it doesn't break 5.2 code 2) we are already using translator-name in the overriding messages 3) throwing an exception because of a missing message-key doesn't seem right.

regards
Taha
 
On Dec 5, 2011, at 3:43 AM, Igor Drobiazko wrote:

> Wouldn't it be better to validate each translator in TranslatorSourceImpl
> and throw an IllegalRgumentException with a detailed message if a
> translator doesn't provide a message key?  This way the user would
> immediately at startup know that he needs to provide a key. A default key
> without any value doesn't make any sense to me.
> 
> On Sun, Dec 4, 2011 at 7:07 PM, <ta...@apache.org> wrote:
> 
>> Author: tawus
>> Date: Sun Dec  4 18:07:53 2011
>> New Revision: 1210164
>> 
>> URL: http://svn.apache.org/viewvc?rev=1210164&view=rev
>> Log:
>> TAP5-1763: NPE is caused when you create a Translator with null message
>> key. Fixed by using the 'translator-name'-message in case the message key
>> is null
>> 
>> Modified:
>> 
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>> 
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>> 
>> Modified:
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>> URL:
>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>> 
>> ==============================================================================
>> ---
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>> (original)
>> +++
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>> Sun Dec  4 18:07:53 2011
>> @@ -125,7 +125,14 @@ public class FieldTranslatorSourceImpl i
>>            return overrideMessages.getFormatter(overrideKey);
>> 
>>        // Otherwise, use the built-in validation message appropriate to
>> this validator.
>> +        String messageKey = translator.getMessageKey();
>> 
>> -        return globalMessages.getFormatter(translator.getMessageKey());
>> +        // If no key has been specified, use translator name to create a
>> key
>> +        if(messageKey == null)
>> +        {
>> +            messageKey = translatorName + "-message";
>> +        }
>> +
>> +        return globalMessages.getFormatter(messageKey);
>>    }
>> }
>> 
>> Modified:
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>> URL:
>> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>> 
>> ==============================================================================
>> ---
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>> (original)
>> +++
>> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>> Sun Dec  4 18:07:53 2011
>> @@ -113,6 +113,50 @@ public class FieldTranslatorSourceImplTe
>>    }
>> 
>>    @Test
>> +    public void create_default_translator_with_name_and_null_key()
>> +    {
>> +        Field field = mockField();
>> +        Messages messages = mockMessages();
>> +        Locale locale = Locale.ENGLISH;
>> +        Class propertyType = Map.class;
>> +        TranslatorSource ts = mockTranslatorSource();
>> +        FormSupport fs = mockFormSupport();
>> +        Translator translator = mockTranslator("maptrans", Map.class);
>> +        Messages globalMessages = mockMessages();
>> +        MessageFormatter formatter = mockMessageFormatter();
>> +        MarkupWriter writer = mockMarkupWriter();
>> +        String label = "Field Label";
>> +        String message = "Woops, did it again.";
>> +        AnnotationProvider ap = mockAnnotationProvider(null);
>> +
>> +        train_findByType(ts, propertyType, translator);
>> +
>> +        train_getFormValidationId(fs, "myform");
>> +
>> +        train_contains(messages, "myform-myfield-maptrans-message",
>> false);
>> +        train_contains(messages, "myfield-maptrans-message", false);
>> +        train_getMessageKey(translator, null);
>> +
>> +        train_getMessageFormatter(globalMessages, "maptrans-message",
>> formatter);
>> +        train_getLabel(field, label);
>> +        train_format(formatter, message, label);
>> +
>> +        translator.render(field, message, writer, fs);
>> +
>> +        replay();
>> +
>> +        FieldTranslatorSource source = new FieldTranslatorSourceImpl(ts,
>> globalMessages, fs);
>> +
>> +        FieldTranslator ft = source.createDefaultTranslator(field,
>> "myfield", messages, locale, propertyType, ap);
>> +
>> +        assertEquals(ft.getType(), Map.class);
>> +
>> +        ft.render(writer);
>> +
>> +        verify();
>> +    }
>> +
>> +    @Test
>>    public void create_default_translator_with_name()
>>    {
>>        Field field = mockField();
>> 
>> 
>> 
> 
> 
> -- 
> Best regards,
> 
> Igor Drobiazko
> http://tapestry5.de


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: svn commit: r1210164 - in /tapestry/tapestry5/branches/5.3/tapestry-core/src: main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java

Posted by Igor Drobiazko <ig...@gmail.com>.
Wouldn't it be better to validate each translator in TranslatorSourceImpl
and throw an IllegalRgumentException with a detailed message if a
translator doesn't provide a message key?  This way the user would
immediately at startup know that he needs to provide a key. A default key
without any value doesn't make any sense to me.

On Sun, Dec 4, 2011 at 7:07 PM, <ta...@apache.org> wrote:

> Author: tawus
> Date: Sun Dec  4 18:07:53 2011
> New Revision: 1210164
>
> URL: http://svn.apache.org/viewvc?rev=1210164&view=rev
> Log:
> TAP5-1763: NPE is caused when you create a Translator with null message
> key. Fixed by using the 'translator-name'-message in case the message key
> is null
>
> Modified:
>
>  tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
>
>  tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
>
> Modified:
> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
> URL:
> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>
> ==============================================================================
> ---
> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
> (original)
> +++
> tapestry/tapestry5/branches/5.3/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImpl.java
> Sun Dec  4 18:07:53 2011
> @@ -125,7 +125,14 @@ public class FieldTranslatorSourceImpl i
>             return overrideMessages.getFormatter(overrideKey);
>
>         // Otherwise, use the built-in validation message appropriate to
> this validator.
> +        String messageKey = translator.getMessageKey();
>
> -        return globalMessages.getFormatter(translator.getMessageKey());
> +        // If no key has been specified, use translator name to create a
> key
> +        if(messageKey == null)
> +        {
> +            messageKey = translatorName + "-message";
> +        }
> +
> +        return globalMessages.getFormatter(messageKey);
>     }
>  }
>
> Modified:
> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
> URL:
> http://svn.apache.org/viewvc/tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java?rev=1210164&r1=1210163&r2=1210164&view=diff
>
> ==============================================================================
> ---
> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
> (original)
> +++
> tapestry/tapestry5/branches/5.3/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/FieldTranslatorSourceImplTest.java
> Sun Dec  4 18:07:53 2011
> @@ -113,6 +113,50 @@ public class FieldTranslatorSourceImplTe
>     }
>
>     @Test
> +    public void create_default_translator_with_name_and_null_key()
> +    {
> +        Field field = mockField();
> +        Messages messages = mockMessages();
> +        Locale locale = Locale.ENGLISH;
> +        Class propertyType = Map.class;
> +        TranslatorSource ts = mockTranslatorSource();
> +        FormSupport fs = mockFormSupport();
> +        Translator translator = mockTranslator("maptrans", Map.class);
> +        Messages globalMessages = mockMessages();
> +        MessageFormatter formatter = mockMessageFormatter();
> +        MarkupWriter writer = mockMarkupWriter();
> +        String label = "Field Label";
> +        String message = "Woops, did it again.";
> +        AnnotationProvider ap = mockAnnotationProvider(null);
> +
> +        train_findByType(ts, propertyType, translator);
> +
> +        train_getFormValidationId(fs, "myform");
> +
> +        train_contains(messages, "myform-myfield-maptrans-message",
> false);
> +        train_contains(messages, "myfield-maptrans-message", false);
> +        train_getMessageKey(translator, null);
> +
> +        train_getMessageFormatter(globalMessages, "maptrans-message",
> formatter);
> +        train_getLabel(field, label);
> +        train_format(formatter, message, label);
> +
> +        translator.render(field, message, writer, fs);
> +
> +        replay();
> +
> +        FieldTranslatorSource source = new FieldTranslatorSourceImpl(ts,
> globalMessages, fs);
> +
> +        FieldTranslator ft = source.createDefaultTranslator(field,
> "myfield", messages, locale, propertyType, ap);
> +
> +        assertEquals(ft.getType(), Map.class);
> +
> +        ft.render(writer);
> +
> +        verify();
> +    }
> +
> +    @Test
>     public void create_default_translator_with_name()
>     {
>         Field field = mockField();
>
>
>


-- 
Best regards,

Igor Drobiazko
http://tapestry5.de