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-unsubscr...@tapestry.apache.org > For additional commands, e-mail: dev-h...@tapestry.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org For additional commands, e-mail: dev-h...@tapestry.apache.org
