ScheduledDataLoaderService disregards the time zone

[2kewl4u]

The org.javamoney.moneta.spi.loader.ScheduledDataLoaderService can be configured by entries in in the 'javamoney.properties' files. Some of these properties are:
'period' and 'delay' used for timer.scheduleAtFixedRate(createTimerTask(load), delayMS, periodMS);
'at' used for timer.schedule(createTimerTask(load), date.getTime(), 3_600_000 * 24 /* daily */)

The format of the 'at' property is limited to what ScheduledDataLoaderService.parseDates(..) expects, thus hours, minutes, seconds and milliseconds. The timer however will be triggered with a Date, thus a specific timestamp, which is not possible to specify with only these parameters.

The worst part is that a new GregorianCalendar() is used, which

Constructs a default GregorianCalendar using the current timein the default time zone with the default FORMAT locale.

The entry in the configuration file will work differently depending on the environment. Furthermore, the period of 24hours only applies to timezones without time shift.

As an example, take the ECBCurrentRateProvider that should load the latest exchange rates. According to the website of the ECB
https://www.ecb.europa.eu/stats/policy_and_exchange_rates/euro_reference_exchange_rates/html/index.en.html

Euro foreign exchange reference rates
The reference rates are usually updated around 16:00 CET on every working day, except on TARGET closing days.

The ECB uses the CET timezone, thus even if the property 'at' would be configured to '16:00' (+x as a tolerance if you want) and you can configure the environment to use CET as a default timezone, which might effect other parts of the application as well, then a the hard coded period of 3_600_000 * 24 /* daily */ will then break whenever a time shift occurs, since not every day is 24 hours.

Further considerations:
The ScheduledDataLoaderService uses the old java.util.Timer class, which is fine for some tasks, but it uses only a single Thread and tasks that should already have been scheduled are delayed as long as other tasks are running. Especially with a fixed-delay execution, where

each execution is scheduled relative tothe actual execution time of the previous execution. If an executionis delayed for any reason (such as garbage collection or otherbackground activity), subsequent executions will be delayed as well.
Also note that the Timer is shared accross multiple providers.

[keilw]

Thanks for documenting that in such detail.
As Java 8 wasn't even out till 2014 and the Backport always had to run exactly the same on at least Java 7 this was likely done in parallel for both. Now the BP is pretty much frozen and the repo may be archived soon with no further development, so we could use java.time here if it works better to do the scheduling.

Regarding the java.util.Timer it is not deprecated or anything in Java 14 and I don't see that to be removed anytime soon (similar for GregorianCalender btw, not sure, if we are still alive when that goes away?;-) but it mentions ScheduledThreadPoolExecutor in the JavaDoc, so any suggestion how to use that here instead?
I would say that is maybe for JavaMoney 2 while the scheduling improvement might fit into an update.
I mark it as ".next" for now, but please if there is a good PR or something happy to apply that in the next 1.4.x release and split the issue into more than one.

[keilw]

The internal implementation class was renamed to URLConnectionScheduler and replacec by OkHttpScheduler.
The biggest priority of 1.4.3 was to actually fix the connection problems to both ECB (in that case more encoding issues) and IMF (with inevitable timeouts), not really the scheduler.
Replacing Timer with ScheduledThreadPoolExecutor is the most likely improvement. Anything else like introducing Quartz I think would be an overkill.