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
