Thanks for checking in the unit tests.
Although, you checked in the tests running against the refactored code, without checking in the refactoring. The test cases does not compile against the code in the repository. Please commit also the following patch (which I will also try to attach):





Index: src/java/org/apache/commons/i18n/MessageManager.java =================================================================== --- src/java/org/apache/commons/i18n/MessageManager.java (revision 168095) +++ src/java/org/apache/commons/i18n/MessageManager.java (arbetskopia) @@ -20,12 +20,7 @@ package org.apache.commons.i18n;

 import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.ResourceBundle;
+import java.util.*;

/**
* The <code>MessageManager</code> provides methods for retrieving localized
@@ -46,7 +41,7 @@
* <p>
* You can call [EMAIL PROTECTED] MessageManager#getText(String,String,Object[],Locale) getText} directly,
* but if you do so, you have to ensure that the given entry key really
- * exists and to deal with the [EMAIL PROTECTED] MessageNotFound} exception that will
+ * exists and to deal with the [EMAIL PROTECTED] MessageNotFoundException} exception that will
* be thrown if you try to access a not existing entry.</p>
*
*/
@@ -61,28 +56,33 @@


public static final ResourceBundle INTERNAL_MESSAGES = ResourceBundle.getBundle("messages", Locale.getDefault());

-    private static List messageProviders = new ArrayList();
+    private static Map messageProviders = new LinkedHashMap();

- static {
- // Add default message providers
- messageProviders.add(new XMLMessageProvider());
- messageProviders.add(new ResourceBundleMessageProvider());
- }
-
/**
* Add a custom <code>[EMAIL PROTECTED] MessageProvider}</code> to the
* <code>MessageManager</code>. It will be incorporated in later calls of
* the [EMAIL PROTECTED] MessageManager#getText(String,String,Object[],Locale) getText}
* or [EMAIL PROTECTED] #getEntries(String,Locale) getEntries}methods.
- *
+ *
+ * @param providerId Id of the provider used for uninstallation and
+ * qualified naming.
* @param messageProvider
* The <code>MessageProvider</code> to be added.
*/
- public static void addMessageProvider(MessageProvider messageProvider) {
- messageProviders.add(messageProvider);
+ public static void addMessageProvider(String providerId, MessageProvider messageProvider) {
+ messageProviders.put(providerId, messageProvider);
}


/**
+ * Remove custom <code>[EMAIL PROTECTED] MessageProvider}</code> from the
+ * <code>MessageManager</code>. Used for tearing down unit tests.
+ *
+ * @param providerId The ID of the provider to remove.
+ */
+ static void removeMessageProvider(String providerId) {
+ messageProviders.remove(providerId);
+ }
+ /**
* Iterates over all registered message providers in order to find the given
* entry in the requested message bundle.
*
@@ -104,8 +104,10 @@
*/
public static String getText(String id, String entry, Object[] arguments,
Locale locale) throws MessageNotFoundException {
+ if(messageProviders.isEmpty())
+ throw new MessageNotFoundException("No MessageProvider registered");
MessageNotFoundException exception = null;
- for (Iterator i = messageProviders.iterator(); i.hasNext();) {
+ for (Iterator i = messageProviders.values().iterator(); i.hasNext();) {
try {
String text = ((MessageProvider) i.next()).getText(id, entry,
locale);
@@ -140,10 +142,9 @@
public static String getText(String id, String entry, Object[] arguments,
Locale locale, String defaultText) {
try {
- String text = getText(id, entry, arguments, locale);
- return MessageFormat.format(text, arguments);
+ return getText(id, entry, arguments, locale);
} catch (MessageNotFoundException e) {
- return defaultText;
+ return MessageFormat.format(defaultText, arguments);
}
}


@@ -155,12 +156,12 @@
*/
public static Map getEntries(String id, Locale locale)
throws MessageNotFoundException {
+ if(messageProviders.isEmpty())
+ throw new MessageNotFoundException("No MessageProvider registered");
MessageNotFoundException exception = null;
- for (Iterator i = messageProviders.iterator(); i.hasNext();) {
+ for (Iterator i = messageProviders.values().iterator(); i.hasNext();) {
try {
- Map entries = ((MessageProvider) i.next()).getEntries(id,
- locale);
- return entries;
+ return ((MessageProvider) i.next()).getEntries(id, locale);
} catch (MessageNotFoundException e) {
exception = e;
}
Index: src/java/org/apache/commons/i18n/XMLMessageProvider.java
===================================================================
--- src/java/org/apache/commons/i18n/XMLMessageProvider.java (revision 168095)
+++ src/java/org/apache/commons/i18n/XMLMessageProvider.java (arbetskopia)
@@ -21,9 +21,7 @@


import java.io.InputStream;
import java.text.MessageFormat;
-import java.util.Collection;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.Locale;
import java.util.Map;
import java.util.logging.Level;
@@ -41,13 +39,32 @@
*
*/
public class XMLMessageProvider implements MessageProvider {
- private static Logger logger = Logger.getLogger(XMLMessageProvider.class.getName());
+ private static final Logger logger = Logger.getLogger(XMLMessageProvider.class.getName());


     private static SAXParserFactory factory = SAXParserFactory.newInstance();

- private static Map installedMessages = new HashMap();
- private static Map messages = new HashMap();
-
+ private final String id;
+
+ private Map messages = new HashMap();
+
+ public XMLMessageProvider(String id, InputStream inputStream) {
+ this.id = id;
+ try {
+ Map applicationMessages = new HashMap();
+ SAXParser parser = factory.newSAXParser();
+ ConfigurationHandler handler = new ConfigurationHandler();
+ parser.parse(new InputSource(inputStream), handler);
+ Map parsedMessages = handler.getMessages();
+ applicationMessages.putAll(parsedMessages);
+ messages.putAll(applicationMessages);
+ } catch (Exception exception) {
+ logger.log(Level.SEVERE,
+ MessageFormat.format(
+ MessageManager.INTERNAL_MESSAGES.getString(MessageManager.MESSAGE_PARSING_ERROR),
+ new String[] { id }), exception);
+ }
+ }
+
/* (non-Javadoc)
* @see org.apache.commons.i18n.MessageProvider#getText(java.lang.String, java.lang.String, java.util.Locale)
*/
@@ -72,33 +89,14 @@
* @param inputStream providing the messages in the required XML format
*/
public static void install(String id, InputStream inputStream) {
- try {
- Map applicationMessages = new HashMap();
- SAXParser parser = factory.newSAXParser();
- ConfigurationHandler handler = new ConfigurationHandler();
- parser.parse(new InputSource(inputStream), handler);
- Map parsedMessages = handler.getMessages();
- applicationMessages.putAll(parsedMessages);
- messages.putAll(applicationMessages);
- installedMessages.put(id, applicationMessages.keySet());
- } catch (Exception exception) {
- logger.log(Level.SEVERE,
- MessageFormat.format(
- MessageManager.INTERNAL_MESSAGES.getString(MessageManager.MESSAGE_PARSING_ERROR),
- new String[] { id }), exception);
- }
+ MessageManager.addMessageProvider(id, new XMLMessageProvider(id, inputStream));
}


     /**
      * @param id unique identifier for the messages to uninstall
      */
     public static void uninstall(String id) {
-        Collection messageKeys = (Collection)installedMessages.get(id);
-        for ( Iterator i = messageKeys.iterator(); i.hasNext(); ) {
-            String messageKey = (String)i.next();
-            messages.remove(messageKey);
-        }
-        installedMessages.remove(id);
+        MessageManager.removeMessageProvider(id);
     }

     /**
@@ -110,7 +108,7 @@
         install(id, inputStream);
     }

-    private static Message findMessage(String id, Locale locale) {
+    private Message findMessage(String id, Locale locale) {
         Message message = lookupMessage(id, locale);
         if (message == null) {
             message = lookupMessage(id, Locale.getDefault());
@@ -122,13 +120,8 @@
         return message;
     }

- private static Message lookupMessage(String id, Locale locale) {
- StringBuffer keyBuffer = new StringBuffer(64);
- keyBuffer.append(id);
- if (locale.getLanguage() != null) keyBuffer.append("_" + locale.getLanguage());
- if (locale.getCountry() != null) keyBuffer.append("_" + locale.getCountry());
- if (locale.getVariant() != null) keyBuffer.append("_" + locale.getVariant());
- String key = keyBuffer.toString();
+ private Message lookupMessage(String id, Locale locale) {
+ String key = id + '_' + locale.toString();
if (messages.containsKey(key)) return (Message)messages.get(key);
while (key.lastIndexOf('_') > 0) {
key = key.substring(0, key.lastIndexOf('_'));
@@ -137,7 +130,7 @@
return null;
}


-    static class ConfigurationHandler extends DefaultHandler {
+    class ConfigurationHandler extends DefaultHandler {
         private String id, key;
         private Message message;
         private StringBuffer cData;
@@ -178,7 +171,8 @@
     }

     static class Message {
-        private String id, language, country, variant;
+        private final String id;
+        private String language, country, variant;
         private Map entries = new HashMap();

         public Message(String id) {
@@ -210,12 +204,9 @@
         }

public String getKey() {
- StringBuffer key = new StringBuffer(64);
- key.append(id);
- if (language != null) key.append("_" + language);
- if (country != null) key.append("_" + country);
- if (variant != null) key.append("_" + variant);
- return key.toString();
+ return id + '_' + new Locale((language != null) ? language : "",
+ (country != null) ? country : "",
+ (variant != null) ? variant : "").toString();
}
}
}
\ No newline at end of file
Index: src/java/org/apache/commons/i18n/LocalizedBundle.java
===================================================================
--- src/java/org/apache/commons/i18n/LocalizedBundle.java (revision 168095)
+++ src/java/org/apache/commons/i18n/LocalizedBundle.java (arbetskopia)
@@ -37,11 +37,10 @@
* the key of the desired message entry.</p>
* This class should not be used directly in order to retrieve entries of a message bundle. It is recommended
* to subclass the <code>LocalizedBundle</code> class in order to define a specific localized bundle.
- * @see TextBundle, MessageBundle, ErrorBundle
+ * @see org.apache.commons.i18n.bundles.TextBundle, MessageBundle, ErrorBundle
*/
public class LocalizedBundle implements Serializable {
public final static String ID = "id";
- public final static String ARGUMENTS = "arguments";


protected String id;
protected Object[] arguments;
@@ -97,7 +96,7 @@
* @param defaultText the text to be returned if no entry was found for the given key
* @return returns the text of the desired message entry for the given locale
*/
- public String getEntry(String key, String defaultText, Locale locale) {
+ public String getEntry(String key, Locale locale, String defaultText) {
return MessageManager.getText(id, key, arguments, locale, defaultText);
}
}
\ No newline at end of file
Index: src/java/org/apache/commons/i18n/ResourceBundleMessageProvider.java
===================================================================
--- src/java/org/apache/commons/i18n/ResourceBundleMessageProvider.java (revision 168095)
+++ src/java/org/apache/commons/i18n/ResourceBundleMessageProvider.java (arbetskopia)
@@ -20,11 +20,8 @@
package org.apache.commons.i18n;


import java.text.MessageFormat;
-import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.MissingResourceException;
@@ -40,35 +37,33 @@
public class ResourceBundleMessageProvider implements MessageProvider {
private static Logger logger = Logger.getLogger(ResourceBundleMessageProvider.class.getName());


-    private static List installedResourceBundles = new ArrayList();
+    private final String baseName;

+ public ResourceBundleMessageProvider(String baseName) {
+ this.baseName = baseName;
+ }
+
/* (non-Javadoc)
* @see org.apache.commons.i18n.MessageProvider#getText(java.lang.String, java.lang.String, java.util.Locale)
*/
public String getText(String id, String entry, Locale locale) throws MessageNotFoundException {
- for ( Iterator i = installedResourceBundles.iterator(); i.hasNext(); ) {
- String baseName = (String)i.next();
- try {
- ResourceBundle resourceBundle = ResourceBundle.getBundle(baseName, locale);
- try {
- return resourceBundle.getString(id+"."+entry);
- } catch ( ClassCastException e ) {
- // ignore all entries that are not of type String
- } catch ( MissingResourceException e ) {
- // skip resource bundle if it is not containing the desired entry
- }
- } catch ( MissingResourceException e ) {
- logger.log(
- Level.WARNING,
- MessageFormat.format(
- MessageManager.INTERNAL_MESSAGES.getString(MessageManager.RESOURCE_BUNDLE_NOT_FOUND),
- new String[] { baseName }));
- i.remove();
- }
+ // TODO: Revise try/catch
+ try {
+ ResourceBundle resourceBundle = ResourceBundle.getBundle(baseName, locale);
+ return resourceBundle.getString(id+"."+entry);
+ } catch ( ClassCastException e ) {
+ // ignore all entries that are not of type String
}
+ catch ( MissingResourceException e ) {
+ logger.log(
+ Level.WARNING,
+ MessageFormat.format(
+ MessageManager.INTERNAL_MESSAGES.getString(MessageManager.RESOURCE_BUNDLE_NOT_FOUND),
+ new String[] { baseName }));
+ }
throw new MessageNotFoundException(MessageFormat.format(
- MessageManager.INTERNAL_MESSAGES.getString(MessageManager.NO_MESSAGE_ENTRIES_FOUND),
- new String[] { id }));
+ MessageManager.INTERNAL_MESSAGES.getString(MessageManager.NO_MESSAGE_ENTRIES_FOUND),
+ new String[] { id }));
}


/* (non-Javadoc)
@@ -77,27 +72,25 @@
public Map getEntries(String id, Locale locale) {
String messageIdentifier = id+".";
Map entries = null;
- for ( Iterator i = installedResourceBundles.iterator(); i.hasNext(); ) {
- String baseName = (String)i.next();
- try {
- ResourceBundle resourceBundle = ResourceBundle.getBundle(baseName, locale);
- Enumeration keys = resourceBundle.getKeys();
- while ( keys.hasMoreElements() ) {
- String key = (String)keys.nextElement();
- if ( key.startsWith(messageIdentifier) ) {
- if ( entries == null ) {
- entries = new HashMap();
- }
- entries.put(key.substring(messageIdentifier.length()), resourceBundle.getString(key));
+ try {
+ ResourceBundle resourceBundle = ResourceBundle.getBundle(baseName, locale);
+ Enumeration keys = resourceBundle.getKeys();
+ while ( keys.hasMoreElements() ) {
+ String key = (String)keys.nextElement();
+ if ( key.startsWith(messageIdentifier) ) {
+ if ( entries == null ) {
+ entries = new HashMap();
}
+ entries.put(key.substring(messageIdentifier.length()), resourceBundle.getString(key));
}
- } catch ( MissingResourceException e ) {
- logger.log(
- Level.WARNING,
- MessageFormat.format(
- MessageManager.INTERNAL_MESSAGES.getString(MessageManager.RESOURCE_BUNDLE_NOT_FOUND),
- new String[] { baseName }));
}
+ } catch ( MissingResourceException e ) {
+ logger.log(
+ Level.WARNING,
+ MessageFormat.format(
+ MessageManager.INTERNAL_MESSAGES.getString(MessageManager.RESOURCE_BUNDLE_NOT_FOUND),
+ new String[] { baseName }));
+ // TODO: Consider uninstalling
}
if ( entries == null ) {
throw new MessageNotFoundException(MessageFormat.format(
@@ -117,15 +110,15 @@
*
*/
public static void install(String baseName) {
- if ( !installedResourceBundles.contains(baseName) )
- installedResourceBundles.add(baseName);
+ MessageManager.addMessageProvider(baseName,
+ new ResourceBundleMessageProvider(baseName));
}


/**
* @param baseName unique identifier for the resource bundle to uninstall
*/
public static void uninstall(String baseName) {
- installedResourceBundles.remove(baseName);
+ MessageManager.removeMessageProvider(baseName); // TODO: Consider checkning type
}


     /**
Index: src/java/org/apache/commons/i18n/bundles/ErrorBundle.java
===================================================================
--- src/java/org/apache/commons/i18n/bundles/ErrorBundle.java   (revision 
168095)
+++ src/java/org/apache/commons/i18n/bundles/ErrorBundle.java   (arbetskopia)
@@ -64,11 +64,11 @@

/**
* @param locale The locale that is used to find the appropriate localized text
- * @param defaultText The default text will be returned, if no entry with key <code>summary</code> could be found in the message bundle identified by the given message identifier
+ * @param defaultSummary The default text will be returned, if no entry with key <code>summary</code> could be found in the message bundle identified by the given message identifier
* @return returns the localized message entry with the key <code>summary</code>
*/
public String getSummary(Locale locale, String defaultSummary) {
- return getEntry(SUMMARY, defaultSummary, locale);
+ return getEntry(SUMMARY, locale, defaultSummary);
}



@@ -83,10 +83,10 @@

/**
* @param locale The locale that is used to find the appropriate localized text
- * @param defaultText The default text will be returned, if no entry with key <code>details</code> could be found in the message bundle identified by the given message identifier
+ * @param defaultDetails The default text will be returned, if no entry with key <code>details</code> could be found in the message bundle identified by the given message identifier
* @return returns the localized message entry with the key <code>details</code>
*/
public String getDetails(Locale locale, String defaultDetails) {
- return getEntry(DETAILS, defaultDetails, locale);
+ return getEntry(DETAILS, locale, defaultDetails);
}
}
\ No newline at end of file
Index: src/java/org/apache/commons/i18n/bundles/TextBundle.java
===================================================================
--- src/java/org/apache/commons/i18n/bundles/TextBundle.java (revision 168095)
+++ src/java/org/apache/commons/i18n/bundles/TextBundle.java (arbetskopia)
@@ -67,6 +67,6 @@
* @return returns the localized message entry with the key <code>text</code>
*/
public String getText(Locale locale, String defaultText) {
- return getEntry(TEXT, defaultText, locale);
+ return getEntry(TEXT, locale, defaultText);
}
}
\ No newline at end of file
Index: src/java/org/apache/commons/i18n/bundles/MessageBundle.java
===================================================================
--- src/java/org/apache/commons/i18n/bundles/MessageBundle.java (revision 168095)
+++ src/java/org/apache/commons/i18n/bundles/MessageBundle.java (arbetskopia)
@@ -62,10 +62,10 @@


/**
* @param locale The locale that is used to find the appropriate localized text
- * @param defaultText The default text will be returned, if no entry with key <code>title</code> could be found in the message bundle identified by the given message identifier
+ * @param defaultTitle The default text will be returned, if no entry with key <code>title</code> could be found in the message bundle identified by the given message identifier
* @return returns the localized message entry with the key <code>title</code>
*/
- public String getTitle(Locale locale, String defaultSummary) {
- return getEntry(TITLE, defaultSummary, locale);
+ public String getTitle(Locale locale, String defaultTitle) {
+ return getEntry(TITLE, locale, defaultTitle);
}
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to