[jira] Created: (CONFIGURATION-302) FileChangedReloadingStrategy.reloadingRequired() can fail

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (CONFIGURATION-302) FileChangedReloadingStrategy.reloadingRequired() can fail

AD_LB (Jira)
FileChangedReloadingStrategy.reloadingRequired() can fail
---------------------------------------------------------

                 Key: CONFIGURATION-302
                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-302
             Project: Commons Configuration
          Issue Type: Bug
    Affects Versions: 1.5
            Reporter: Pino Navato
            Priority: Minor


If reloadingRequired() returns true and you call it again before calling reloadingPerformed(), the 2nd time it can return false (but you have not yet reloaded!) because it doesn't check the file system again until the refresh delay is expired.
Of course this is a very unusual test case (usually you reload immediately) but the behaviour of the method should be consistent in this case too: if reloadingRequired() returns true any subsequent call to this method should return true until reloadingPerformed() is called.

In my project I have fixed the method by promoting the flag called "reloading" to class scope so I that can check whether the previous call returned true or false:

protected boolean reloading = false;
public boolean reloadingRequired()
{
        if (!reloading)
        {
                long now = System.currentTimeMillis();
                if (now > lastChecked + refreshDelay)
                {
                        lastChecked = now;
                        if (hasChanged())
                        {
                                reloading = true;
                        }
                }
        }
        return reloading;
}

Of course I reset this flag in init() and reloadingPerformed().

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (CONFIGURATION-302) FileChangedReloadingStrategy.reloadingRequired() can fail

AD_LB (Jira)

    [ https://issues.apache.org/jira/browse/CONFIGURATION-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12551926 ]

Oliver Heger commented on CONFIGURATION-302:
--------------------------------------------

You are probably right. The behavior you describe seems to be more consistent. So I am +1 for applying this change.

But hey, if we now receive bug reports of this quality, this means that our code base has become pretty stable ;-)

> FileChangedReloadingStrategy.reloadingRequired() can fail
> ---------------------------------------------------------
>
>                 Key: CONFIGURATION-302
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-302
>             Project: Commons Configuration
>          Issue Type: Bug
>    Affects Versions: 1.5
>            Reporter: Pino Navato
>            Priority: Minor
>
> If reloadingRequired() returns true and you call it again before calling reloadingPerformed(), the 2nd time it can return false (but you have not yet reloaded!) because it doesn't check the file system again until the refresh delay is expired.
> Of course this is a very unusual test case (usually you reload immediately) but the behaviour of the method should be consistent in this case too: if reloadingRequired() returns true any subsequent call to this method should return true until reloadingPerformed() is called.
> In my project I have fixed the method by promoting the flag called "reloading" to class scope so I that can check whether the previous call returned true or false:
> protected boolean reloading = false;
> public boolean reloadingRequired()
> {
> if (!reloading)
> {
> long now = System.currentTimeMillis();
> if (now > lastChecked + refreshDelay)
> {
> lastChecked = now;
> if (hasChanged())
> {
> reloading = true;
> }
> }
> }
> return reloading;
> }
> Of course I reset this flag in init() and reloadingPerformed().

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (CONFIGURATION-302) FileChangedReloadingStrategy.reloadingRequired() can fail

AD_LB (Jira)
In reply to this post by AD_LB (Jira)

     [ https://issues.apache.org/jira/browse/CONFIGURATION-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated CONFIGURATION-302:
---------------------------------------

    Fix Version/s: 1.6
         Assignee: Oliver Heger

> FileChangedReloadingStrategy.reloadingRequired() can fail
> ---------------------------------------------------------
>
>                 Key: CONFIGURATION-302
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-302
>             Project: Commons Configuration
>          Issue Type: Bug
>    Affects Versions: 1.5
>            Reporter: Pino Navato
>            Assignee: Oliver Heger
>            Priority: Minor
>             Fix For: 1.6
>
>
> If reloadingRequired() returns true and you call it again before calling reloadingPerformed(), the 2nd time it can return false (but you have not yet reloaded!) because it doesn't check the file system again until the refresh delay is expired.
> Of course this is a very unusual test case (usually you reload immediately) but the behaviour of the method should be consistent in this case too: if reloadingRequired() returns true any subsequent call to this method should return true until reloadingPerformed() is called.
> In my project I have fixed the method by promoting the flag called "reloading" to class scope so I that can check whether the previous call returned true or false:
> protected boolean reloading = false;
> public boolean reloadingRequired()
> {
> if (!reloading)
> {
> long now = System.currentTimeMillis();
> if (now > lastChecked + refreshDelay)
> {
> lastChecked = now;
> if (hasChanged())
> {
> reloading = true;
> }
> }
> }
> return reloading;
> }
> Of course I reset this flag in init() and reloadingPerformed().

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (CONFIGURATION-302) FileChangedReloadingStrategy.reloadingRequired() can fail

AD_LB (Jira)
In reply to this post by AD_LB (Jira)

     [ https://issues.apache.org/jira/browse/CONFIGURATION-302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger resolved CONFIGURATION-302.
----------------------------------------

    Resolution: Fixed

A fix has been applied. Thanks.

> FileChangedReloadingStrategy.reloadingRequired() can fail
> ---------------------------------------------------------
>
>                 Key: CONFIGURATION-302
>                 URL: https://issues.apache.org/jira/browse/CONFIGURATION-302
>             Project: Commons Configuration
>          Issue Type: Bug
>    Affects Versions: 1.5
>            Reporter: Pino Navato
>            Assignee: Oliver Heger
>            Priority: Minor
>             Fix For: 1.6
>
>
> If reloadingRequired() returns true and you call it again before calling reloadingPerformed(), the 2nd time it can return false (but you have not yet reloaded!) because it doesn't check the file system again until the refresh delay is expired.
> Of course this is a very unusual test case (usually you reload immediately) but the behaviour of the method should be consistent in this case too: if reloadingRequired() returns true any subsequent call to this method should return true until reloadingPerformed() is called.
> In my project I have fixed the method by promoting the flag called "reloading" to class scope so I that can check whether the previous call returned true or false:
> protected boolean reloading = false;
> public boolean reloadingRequired()
> {
> if (!reloading)
> {
> long now = System.currentTimeMillis();
> if (now > lastChecked + refreshDelay)
> {
> lastChecked = now;
> if (hasChanged())
> {
> reloading = true;
> }
> }
> }
> return reloading;
> }
> Of course I reset this flag in init() and reloadingPerformed().

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.