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, <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
