[
https://issues.apache.org/jira/browse/CONFIGURATION-712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16597622#comment-16597622
]
Rolland Hobbie commented on CONFIGURATION-712:
----------------------------------------------
I saw the comment in FileHandlerReloadingDetector, however the
ReloadingFileBasedConfigurationBuilder initializes this class using
DefaultReloadingDetectorFactory so wouldn't the responsibility be for the
factory to fully initialize FileHandlerReloadingDetector?
I also saw other places in the documentation state, "If the underlying file has
not been changed, this check has no effect. However, if the file has been
changed by an external source, an updated last-modified date is detected, and
the reloading detector signals a need for a reload"
([https://commons.apache.org/proper/commons-configuration/userguide/howto_reloading.html)],
with no mention of requiring further initialization of the ReloadingDetector.
Given the example in the documentation, both test scenarios fail.
* Test #1 behaves the same as my example - incorrectly returns the initial
configuration value ("1" not "2")
* Test #2 behaves the same as my example - incorrectly returns the initial
configuration value for both the first and second invocations, and returns the
updated value for the third invocation ("3" and the change of "2" is never seen)
I am unable to take my implementation to production because these tests
scenarios fail. Given Test #1, if I change the trigger period to be 24 hours,
and update the properties after 24 hours, the application would not see the
updated changes. And, given Test #2 the changes would only be seen if the
properties were updated a second time, and then only after another cycle of the
triggering period (in the case of 24 hours only after 48 hours), but the first
change would never be seen by the application.
> 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: 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)
