[
https://issues.apache.org/jira/browse/CONFIGURATION-712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16609247#comment-16609247
]
Rolland Hobbie commented on CONFIGURATION-712:
----------------------------------------------
I have attached a patch for the change fix.
I ended up creating a new refresh() method on FileHandlerReloadingDetector,
which initializes the last modified field. I thought it was confusing for the
factory to invoke reloadingPerformed() in order to initialize last modified. I
considered other approaches for this fix, but decided on this approach as it
was the most isolated change and the least impact.
If other *ReloadingDetector implementations will need the ability of
initializing state, maybe a new interface could be created to define refresh()?
> FileHandlerReloadingDetector Does Not Correctly Initialize File Last Modified
> -----------------------------------------------------------------------------
>
> Key: CONFIGURATION-712
> URL: https://issues.apache.org/jira/browse/CONFIGURATION-712
> Project: Commons Configuration
> Issue Type: Bug
> Components: File reloading
> Affects Versions: 2.3
> Reporter: Rolland Hobbie
> Priority: Major
> Attachments: CONFIGURATION-712-InitializeLastModified.patch,
> ReloadingFileBasedConfigurationBuilderExampleTest.java,
> ReloadingFileBasedConfigurationBuilderTest.java
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> [FileHandlerReloadingDetector|https://commons.apache.org/proper/commons-configuration/apidocs/src-html/org/apache/commons/configuration2/reloading/FileHandlerReloadingDetector.html]
> declares the following method:
>
> {code:java}
> 150 @Override
> 151 public boolean isReloadingRequired()
> 152 {
> 153 long now = System.currentTimeMillis();
> 154 if (now >= lastChecked + getRefreshDelay())
> 155 {
> 156 lastChecked = now;
> 157
> 158 long modified = getLastModificationDate();
> 159 if (modified > 0)
> 160 {
> 161 if (lastModified == 0)
> 162 {
> 163 // initialization
> 164 updateLastModified(modified);
> 165 }
> 166 else
> 167 {
> 168 if (modified != lastModified)
> 169 {
> 170 return true;
> 171 }
> 172 }
> 173 }
> 174 }
> 175
> 176 return false;
> 177 }
> {code}
>
> During initialization of FileHandlerReloadingDetector, lastModified is never
> instantiated, so the first time isReloadingRequired() is invoked lastModified
> will be 0.
>
> This results in two issues:
>
> Test #1
> * Scenario Steps
> ## Initialize ReloadingFileBasedConfigurationBuilder without invoking
> builder.getConfiguration()
> ## Wait longer than refreshDelay
> ## Invoke builder.getReloadingController().checkForReloading(null) to notify
> the FileHandlerReloadingDetector to check for reload
> ## Invoke bulider.getConfiguration()
> ## Update the properties file
> ## Wait longer than refreshDelay
> ## Invoke builder.getReloadingController().checkForReloading(null) to notify
> the FileHandlerReloadingDetector to check for reload
> ## Invoke bulider.getConfiguration()
> * Result - the Configuration does not have the updated value.
>
> Test #2
> * Scenario Steps
> ## Initialize ReloadingFileBasedConfigurationBuilder without invoking
> builder.getConfiguration()
> ## Invoke bulider.getConfiguration()
> ## Update the properties file
> ## Wait longer than refreshDelay
> ## Invoke builder.getReloadingController().checkForReloading(null) to notify
> the FileHandlerReloadingDetector to check for reload
> ## Invoke bulider.getConfiguration()
> ## Update the properties file
> ## Wait longer than refreshDelay
> ## Invoke builder.getReloadingController().checkForReloading(null) to notify
> the FileHandlerReloadingDetector to check for reload
> ## Invoke bulider.getConfiguration()
> * Result - For the first two invocations, the Configuration is not updated.
> One the third invocation of builder.getConfiguration() the property is
> updated to the new value.
>
> As potential solution, the constructor of FileHandlerReloadingDetector should
> either call isReloadingRequired() or
> updateLastModified(getLastModificationDate()) to initialize the lastModified
> instance variable to the file current lastModified value.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
