[jira] [Comment Edited] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones

["Christian P. MOMON (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14525802#comment-14525802 ] 

Christian P. MOMON edited comment on LANG-1127 at 5/3/15 12:08 PM:
-------------------------------------------------------------------

Hello, I support the goal of this ticket. I want to draw your attention to a technical element that seems important to take into account. (!)

Many methods from time package are using the FormatCache class. Example:

{noformat}
class FastDateFormat {
[...]
public static FastDateFormat getInstance(final String pattern, final TimeZone timeZone) {
        return cache.getInstance(pattern, timeZone, null);
    }
[...]
{noformat}

FormatCache.java :
{noformat}
    public F getInstance(final String pattern, TimeZone timeZone, Locale locale) {
        if (pattern == null) {
            throw new NullPointerException("pattern must not be null");
        }
        if (timeZone == null) {
            timeZone = TimeZone.getDefault();
        }
        if (locale == null) {
            locale = Locale.getDefault();
        }
        final MultipartKey key = new MultipartKey(pattern, timeZone, locale);
        F format = cInstanceCache.get(key);
        if (format == null) {           
            format = createInstance(pattern, timeZone, locale);
            final F previousValue= cInstanceCache.putIfAbsent(key, format);
            if (previousValue != null) {
                // another thread snuck in and did the same work
                // we should return the instance that is in ConcurrentMap
                format= previousValue;              
            }
        }
        return format;
    }
{noformat}

So, unit tests will fill the FormatCache object with some default system values. If you change the default system value then the old cache values are in it again. (!)

Change the default system timezone requires to reset the FormatCache objet. (+)

I suggest to add a reset method in FormatCache class. (on)
I suggest to add cache reset on default system timezone value. (on)
Another way is to make FormatCache detect any timezone system change and then reset himself the cache. But what about performance?

Hope this will help you. :)





was (Author: cpm):
Hello, I support the goal of this ticket. I want to draw your attention to a technical element that seems important to take into account. (!)

Many methods from time package are using the FormatCache class. Example:

{noformat}
class FastDateFormat {
[...]
public static FastDateFormat getInstance(final String pattern, final TimeZone timeZone) {
        return cache.getInstance(pattern, timeZone, null);
    }
[...]
{noformat}

FormatCache.java :
{noformat}
    public F getInstance(final String pattern, TimeZone timeZone, Locale locale) {
        if (pattern == null) {
            throw new NullPointerException("pattern must not be null");
        }
        if (timeZone == null) {
            timeZone = TimeZone.getDefault();
        }
        if (locale == null) {
            locale = Locale.getDefault();
        }
        final MultipartKey key = new MultipartKey(pattern, timeZone, locale);
        F format = cInstanceCache.get(key);
        if (format == null) {           
            format = createInstance(pattern, timeZone, locale);
            final F previousValue= cInstanceCache.putIfAbsent(key, format);
            if (previousValue != null) {
                // another thread snuck in and did the same work
                // we should return the instance that is in ConcurrentMap
                format= previousValue;              
            }
        }
        return format;
    }
{noformat}

So, unit tests will fill the FormatCache object with some default system values. If you change the default system value then the old cache values are in it again. (!)

Change the default system timezone requires to reset the FormatCache objet. (+)

I suggest to add a reset method in FormatCache class. (on)

Hope this will help you. :)




> Create a base test for the time package, which sets and resets default Locales and TimeZones
> --------------------------------------------------------------------------------------------
>
>                 Key: LANG-1127
>                 URL: https://issues.apache.org/jira/browse/LANG-1127
>             Project: Commons Lang
>          Issue Type: Test
>          Components: lang.time.*
>            Reporter: Benedikt Ritter
>            Assignee: Charles Honton
>             Fix For: Patch Needed
>
>
> We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)