portals-jetspeed-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santiago Gala <sg...@hisitech.com>
Subject Re: [patch] JetspeedLocalizationService
Date Thu, 06 Mar 2003 21:30:21 GMT
Jon Evans wrote:
> Hi David,
> 
> David Sean Taylor wrote:
> 
>> The patch isn't necessary I just committed a patch from
>> Missimilliano Dessi. It works fine as is. Although I thought we had
>> country flag icons with the original portlet.
> 
> 
> I have just got round to using the latest CVS.  I like the fact that
> the changeLanguage portlet is now a .vm, it will make it a lot easier
> to change the look and feel for my site.
> 
> I'm still concerned though with
> org.apache.jetspeed.services.customlocalization.JetspeedLocalizationService. 
> 
> 
> It still doesn't seem to work if you have more than one bundle defined
> in locale.default.bundles.  It only seems to look at the first bundle.
> 
> If you look at the class, it extends TurbineLocalizationService.
> JetspeedLocalizationService implements an initBundleNames() method
> which populates an instance variable String[] bundleNames.  But all of
> the getString(...) methods call super.getString(...), where
> TurbineLocalizationService will use its own instance variables and will
> not find the strings.
> 
> 
> Let me put it another way.  Each class defines an identical set of 
> instance variables, like this:
> 
> JetspeedLocalizationService     TurbineLocalizationService
> =============================== ===============================
> private Hashtable bundles;      private Hashtable bundles;
> private String bundleNames[];   private String bundleNames[];
> private Locale defaultLocale;   private Locale defaultLocale;
> private String defaultLanguage; private String defaultLanguage;
> private String defaultCountry;  private String defaultCountry;
> 
> After JetspeedLocalizationService.init() is called they look like this:
> 
> JetspeedLocalizationService TurbineLocalizationService
> =========================== ==========================
> bundles          null       bundles         set
> bundleNames[]    set        bundleNames[]   null
> defaultLocale    null       defaultLocale   set
> defaultLanguage  null       defaultLanguage set
> defaultCountry   null       defaultCountry  set
> 
> when getString(String bundleName, Locale locale, String key) is called
> in JetspeedLocalizationService, it calls super.getString(...).  This
> calls getBundle(String, Locale), which is implemented in
> JetspeedLocalizationService as a call to super.getBundle(String,
> Locale), so we are back to the code in TurbineLocalizationService.
> 
> getBundle(String, Locale) in TurbineLocalizationService returns the
> default bundle to the getString(...) method.
> 
> Back in getString(String bundleName, Locale locale, String key), we
> look up the requested key in the bundle, using getStringOrNull(), which 
> returns null because the string I'm looking for isn't in the first 
> bundle, it's in the second.
> 
> The next test is intended to determine whether we need to check the
> other bundles for the key.  The test is:
> 
>         // Look for text in list of default bundles.
>         if (value == null && bundleNames.length > 0)
> 
> At this point we throw a NullPointerException because bundleNames is
> null (it was never set in TurbineLocalizationService, only in
> JetspeedLocalizationService).
> 
> The whole of the JetspeedLocalizationService class can be reduced to a 
> single function, getLocale(RunData runData).
> 
> The attached patch does this, and seems to work.  Yes, I compiled from 
> clean this time. :-)
> 
> Jon
> 
> 


It looks right, and removes a lot of code (I have no time to test it), 
but...

In the process of evolution that is coming towards us (I mean JSR 
results, etc.) it is quite possible that changes are needed, looking 
from the original proposal and industry trends. So, I would prefer a 
tested patch which would just stopped inheriting from the turbine one 
but is still functionally complete. I mean inheriting from 
TurbineBaseService, which limits dependency to initialization code.

I say so because Turbine is moving towards less coupled classes. 
Currently services are being decoupled and ported to fulcrum, for use in 
turbine 2.2+ and 3. So I think keeping the whole implementation here 
until we ensure that the needed changes are in Turbine does not look bad.

The way I propose will make easier in the future:
* backport our needed functionality to the service already present in 
Fulcrum
* just change the interfaces it implements and have it running in 
Fulcrum, through the magics of Avalon.

Regards,
      Santiago


> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: src/java/org/apache/jetspeed/services/customlocalization/JetspeedLocalizationService.java
> ===================================================================
> RCS file: /home/cvspublic/jakarta-jetspeed/src/java/org/apache/jetspeed/services/customlocalization/JetspeedLocalizationService.java,v
> retrieving revision 1.4
> diff -u -r1.4 JetspeedLocalizationService.java
> --- src/java/org/apache/jetspeed/services/customlocalization/JetspeedLocalizationService.java
26 Feb 2003 17:46:34 -0000	1.4
> +++ src/java/org/apache/jetspeed/services/customlocalization/JetspeedLocalizationService.java
4 Mar 2003 12:49:37 -0000
> @@ -62,14 +62,10 @@
>   */
>  
>  // Java imports
> -import java.util.Hashtable;
>  import java.util.Locale;
> -import java.util.ResourceBundle;
>  
>  // Turbine imports
>  import org.apache.turbine.services.localization.TurbineLocalizationService;
> -import org.apache.turbine.services.InitializationException;
> -import org.apache.turbine.services.resources.TurbineResources;
>  import org.apache.turbine.util.RunData;
>  
>  // Jetspeed imports
> @@ -78,81 +74,6 @@
>  
>  public class JetspeedLocalizationService extends TurbineLocalizationService implements
CustomLocalizationService
>  {
> -    public JetspeedLocalizationService()
> -    {
> -        super();
> -    }
> -
> -    public void init() throws InitializationException
> -    {
> -        super.init();
> -    }
> -
> -    protected void initBundleNames(String ignored[])
> -    {
> -        bundleNames = TurbineResources.getStringArray("locale.default.bundles");
> -        String name = TurbineResources.getString("locale.default.bundle");
> -        if (name != null && name.length() > 0)
> -        {
> -            if (bundleNames == null || bundleNames.length <= 0)
> -            {
> -              bundleNames = (new String[] {name});
> -            }
> -            else
> -            {
> -              String array[] = new String[bundleNames.length + 1];
> -              array[0] = name;
> -              System.arraycopy(bundleNames, 0, array, 1, bundleNames.length);
> -              bundleNames = array;
> -            }
> -        }
> -        if (bundleNames == null)
> -        {
> -            bundleNames = new String[0];
> -        }
> -    }
> -
> -    public String getDefaultBundleName()
> -    {
> -        return bundleNames.length > 0 ? bundleNames[0] : "";
> -    }
> -
> -    public ResourceBundle getBundle()
> -    {
> -        return super.getBundle();
> -    }
> -
> -    public ResourceBundle getBundle(String bundleName)
> -    {
> -        return super.getBundle(bundleName);
> -    }
> -
> -    public ResourceBundle getBundle(String bundleName, String languageHeader)
> -    {
> -      return super.getBundle(bundleName, languageHeader);
> -    }
> -
> -    public ResourceBundle getBundle(RunData data)
> -    {
> -      return super.getBundle(data);
> -    }
> -
> -    public ResourceBundle getBundle(String bundleName, RunData data)
> -    {
> -      return super.getBundle(bundleName, data);
> -    }
> -
> -    public ResourceBundle getBundle(String bundleName, Locale locale)
> -    {
> -     return super.getBundle(bundleName, locale);
> -    }
> -
> -    public void setBundle(String defaultBundle)
> -    {
> -      super.setBundle(defaultBundle);
> -    }
> -
> -
>      public final Locale getLocale(RunData data)
>      {
>          JetspeedUser user = (JetspeedUser) data.getUser();
> @@ -183,21 +104,4 @@
>              }
>          }
>      }
> -
> -    public Locale getLocale(String header)
> -    {
> -        return super.getLocale(header);
> -    }
> -
> -    public String getString(String bundleName, Locale locale, String key)
> -    {
> -        return super.getString(bundleName, locale, key);
> -    }
> -
> -    private Hashtable bundles;
> -    private String bundleNames[];
> -    private Locale defaultLocale;
> -    private String defaultLanguage;
> -    private String defaultCountry;
> -
> -}
> +}
> \ No newline at end of file
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jetspeed-dev-help@jakarta.apache.org


Mime
View raw message