Yamini,
Looks good. Some minor things:
1) Generic types make casts unecessary and make for cleaner code:
for( final ResourceBundled resourceBundle : resourceBundles ) { ... }
Then you won't need a cast, or Iterator either.
2) A comment is worthwhile, indicating that 'Vector' is used for its
thread-safe properties.
3) While iterating/looping, addition of a resource bundle might do
what? (cause an exception, work fine?). A comment is in order:
"If {_at_link #addResourceBundle} is called while getting a
description, ...".
You could call toArray() I suppose, that would preclude an issue (I
think--check what Vector javadoc says).
Thanks,
Lloyd
On Nov 14, 2007, at 9:02 PM, Yamini K B wrote:
> Hi Lloyd,
>
> Implemented your suggestion as follows, please review the same:
>
> 1. StatsDescription will load a default resource bundle.
> 2. Exposed a method to add resource bundles externally.
> 3. Exposed getInstance() to allow for only one instance of this class.
>
> Thanks,
> -Yamini
>
> public class StatsDescriptionHelper {
> private final Vector<ResourceBundle> resourceBundles = new
> Vector<ResourceBundle>();
> public static final String NO_DESCRIPTION_AVAILABLE = "No
> Description was available";
> private static final String DEFAULT_PACKAGE =
> StatsDescriptionHelper.class.getPackage().getName();
> private static final String DEFAULT_FILE = "LocalStrings";
> private static final StatsDescriptionHelper sdh = new
> StatsDescriptionHelper();
>
> public StatsDescriptionHelper() {
> ResourceBundle resourceBundle =
> ResourceBundle.getBundle(DEFAULT_PACKAGE + "." + DEFAULT_FILE);
> resourceBundles.add(resourceBundle);
> }
>
> public static StatsDescriptionHelper getInstance() {
> return sdh;
> }
>
> /*
> * Add a resource bundle
> */
> public void addResourceBundle(ResourceBundle rb)
> {
> resourceBundles.add(rb);
> }
> /*
> * Returns the localized description
> */
> public String getDescription(String key)
> {
> Iterator it = resourceBundles.iterator();
> String value = NO_DESCRIPTION_AVAILABLE;
> while (it.hasNext()) {
> ResourceBundle resourceBundle = (ResourceBundle)it.next();
> try {
> value = resourceBundle.getString(key);
> break;
> }
> catch (MissingResourceException mre) {
> // continue search in next resource bundle
> }
> }
> return value;
> }
> }
>
>
>
>
> Lloyd L Chambers wrote On 11/09/07 22:47,:
>
>> Yamini,
>>
>> This is the unmodified class I see:
>>
>> class StatsDescriptionHelper {
>> private LocalStringManager mgr;
>> static final String NO_DESCRIPTION_AVAILABLE="No Description
>> was available";
>> StatsDescriptionHelper(){
>> mgr = new LocalStringManagerImpl(this.getClass());
>> }
>> String getDescription(java.lang.String name) {
>> return mgr.getLocalString(this.getClass(), name,
>> NO_DESCRIPTION_AVAILABLE);
>> }
>> }
>>
>> Comments:
>> - 'mgr' should be 'final'.
>> - StatsDescriptionHelper is instantiated multiple times, but
>> actual use appears to be as a singleton; all clients apparently
>> should be accessing exactly the same strings. So make it a
>> singleton with a private constructor, a 'private static final'
>> instance, and a public getInstance() method.
>> - It's unclear (in your modified code) whether setDescriptionClass
>> () will be called before StatsDescriptionHelper is instantiated.
>> I see no way of guaranteeing that, so that's a bug.
>>
>> The apparent intent is to set the 'descriptionClass' static
>> variable externally and therefore cause the create() method to
>> use some subclass of StatsDescriptionHelper, obtained by
>> Class.forName().
>>
>> Thinking about this problem in general, such a "helper" class
>> might in general need to find strings from more than one place eg
>> search locations A, B, C, etc in order until a String is found.
>> That would be a general solution, and a dynamic one too, better
>> for a dynamic environment in which modules might load and unload.
>>
>> My suggestion is this: create a general purpose base class (of
>> which StatsDescriptionHelper is a subclass) that allows the
>> addition of more than one resource bundle (removal is probably
>> unnecessary). Any subclass can insert its default resource
>> bundle via super(), and external code can insert (in a thread-
>> safe way) additional resource bundle(s) dynamically. With this
>> capability, the SIP code can do:
>>
>> StatsDescriptionHelper.getInstance().addResourceBundle( myBundle1 );
>> StatsDescriptionHelper.getInstance().addResourceBundle( myBundle2 );
>> etc
>>
>> Of course, this has global and dynamic effect for all clients of
>> StatsDescriptionHelper, but this is apparently the desired semantic.
>>
>> The internal list of resource bundles will need to be dealt with
>> in a thread-safe manner of course.
>>
>> Lloyd
>>
>> On Nov 6, 2007, at 9:30 AM, Yamini K B wrote:
>>
>>>
>>> Description: GlassFish monitoring module stores the description
>>> texts for all the statistics in LocalStrings.properties packaged
>>> under com.sun.enterprise.admin.monitor.registry.spi. There is a
>>> StatsDescriptionHelper class within the same package that looks
>>> up for the description text during the registration of the
>>> monitoring objects. Recently, we extended GlassFish monitoring
>>> to load external monitoring modules (to be able to support
>>> SailFin monitoring) The problem is that description texts fail
>>> to show up for the SIP monitoring statistics, the reason being
>>> that StatsDescriptionHelper tries to load the resource bundle
>>> from the same package as itself. So the requirement is that
>>> StatsDescriptionHelper should be able to pick up description
>>> texts for modules loaded externally.
>>>
>>> Solution: Provided a hook in StatsDescriptionHelper to load the
>>> resource bundle from the caller class. Attached diffs contain
>>> the following changes:
>>>
>>> 1. Exposed a method setDescriptionClass() that will be called
>>> from some external module.
>>> 2. Create the StatsDescriptionHelper based on the the
>>> description class.
>>>
>>> To make this work end-to-end the following changes was done on
>>> SailFin side:
>>>
>>> 1. Created an empty class SipStatsDescriptionHelper which
>>> extends StatsDescriptionHelper.
>>> 2. Modified SipMonitoringManagerImpl (which is the
>>> initialization class for SIP monitoring) to set the description
>>> class just before registering the SIP stats. The stats
>>> description class needs to be unset as soon as the job is done
>>> since we wouldn't want this to be used when some GlassFish
>>> monitoring registration/unregistration happens.
>>> 3. Created LocalStrings.properties within the same package as
>>> SipStatsDescriptionHelper.
>>>
>>> Would appreciate your comments on the suggested changes.
>>>
>>> Thanks,
>>> -Yamini
>>>
>>>
>>>
>>> Index: StatisticWorkaround.java
>>> ===================================================================
>>> RCS file: /cvs/glassfish/admin/monitor/src/java/com/sun/
>>> enterprise/ admin/monitor/registry/spi/StatisticWorkaround.java,v
>>> retrieving revision 1.5
>>> diff -u -r1.5 StatisticWorkaround.java
>>> --- StatisticWorkaround.java 5 May 2007 05:24:19 -0000 1.5
>>> +++ StatisticWorkaround.java 5 Nov 2007 15:40:20 -0000
>>> @@ -64,7 +64,7 @@
>>> */
>>>
>>> final class StatisticWorkaround {
>>> - private static final StatsDescriptionHelper helper = new
>>> StatsDescriptionHelper();
>>> + private static final StatsDescriptionHelper helper =
>>> StatsDescriptionHelper.create();
>>> private static Logger logger = Logger.getLogger
>>> (AdminConstants.kLoggerName);
>>> static final Statistic[] populateDescriptions(final Statistic
>>> [] from) {
>>> if (from == null) {
>>> Index: StatsMediatorImpl.java
>>> ===================================================================
>>> RCS file: /cvs/glassfish/admin/monitor/src/java/com/sun/
>>> enterprise/ admin/monitor/registry/spi/StatsMediatorImpl.java,v
>>> retrieving revision 1.7
>>> diff -u -r1.7 StatsMediatorImpl.java
>>> --- StatsMediatorImpl.java 5 May 2007 05:24:19 -0000 1.7
>>> +++ StatsMediatorImpl.java 5 Nov 2007 15:40:21 -0000
>>> @@ -88,7 +88,7 @@
>>> private final String OLD_DELIMITER = "_";
>>> // some JTA specific constants
>>> private Map opsMap;
>>> - private final StatsDescriptionHelper helper = new
>>> StatsDescriptionHelper();
>>> + private final StatsDescriptionHelper helper =
>>> StatsDescriptionHelper.create();
>>> private final String DESCRIPTION_GETTER = "getDescription";
>>> private static final String DOTTED_NAME = "dotted-name";
>>>
>>> Index: StatsDescriptionHelper.java
>>> ===================================================================
>>> RCS file: /cvs/glassfish/admin/monitor/src/java/com/sun/
>>> enterprise/ admin/monitor/registry/spi/StatsDescriptionHelper.java,v
>>> retrieving revision 1.4
>>> diff -u -r1.4 StatsDescriptionHelper.java
>>> --- StatsDescriptionHelper.java 5 May 2007 05:24:19 -0000 1.4
>>> +++ StatsDescriptionHelper.java 6 Nov 2007 16:56:59 -0000
>>> @@ -41,6 +41,7 @@
>>> */
>>>
>>> package com.sun.enterprise.admin.monitor.registry.spi;
>>> +
>>> import com.sun.enterprise.util.LocalStringManager;
>>> import com.sun.enterprise.util.LocalStringManagerImpl;
>>> /**
>>> @@ -48,14 +49,37 @@
>>> * given statistic.
>>> * @author Shreedhar Ganapathy<mailto:shreedhar.ganapathy_at_sun.com>
>>> */
>>> -class StatsDescriptionHelper {
>>> - private LocalStringManager mgr;
>>> +public class StatsDescriptionHelper {
>>> + protected LocalStringManager mgr;
>>> static final String NO_DESCRIPTION_AVAILABLE="No Description
>>> was available";
>>> + static String descriptionClass = null;
>>> - StatsDescriptionHelper(){
>>> + public StatsDescriptionHelper(){
>>> mgr = new LocalStringManagerImpl(this.getClass());
>>> }
>>> + public static StatsDescriptionHelper create() {
>>> + if (descriptionClass == null) {
>>> + return new StatsDescriptionHelper();
>>> + } else {
>>> + StatsDescriptionHelper sdh = null;
>>> + try {
>>> + sdh = (StatsDescriptionHelper)
>>> Class.forName (descriptionClass).newInstance();
>>> + } catch (Exception e) {
>>> + // Fall back on default in case of error
>>> + sdh = new StatsDescriptionHelper();
>>> + }
>>> + return sdh;
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * Set the description class
>>> + */
>>> + public static void setDescriptionClass(String c) {
>>> + descriptionClass = c;
>>> + }
>>> +
>>> String getDescription(java.lang.String name) {
>>> return mgr.getLocalString(this.getClass(), name,
>>> NO_DESCRIPTION_AVAILABLE);
>>> }
>>>
>>>
>>> --------------------------------------------------------------------
>>> -
>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>>
>> ---
>> Lloyd L Chambers
>> lloyd.chambers_at_sun.com
>> Sun Microsystems, Inc
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>
---
Lloyd L Chambers
lloyd.chambers_at_sun.com
Sun Microsystems, Inc