JavaMoney / jsr354-ri

JSR 354 - Moneta: Reference Implementation
Other
343 stars 101 forks source link

Fallback file is ignored on exchange rate loading failure #177

Closed marikt closed 6 years ago

marikt commented 6 years ago

Hi,

I use
groupId: org.javamoney artifactId: moneta version: 1.1

I have created new exchange rate provider (CNB = czech national bank). Its endpoint is pretty unstable so I have to take advantage of having the fallback file with the historic exchange rates. Here is my javamoney.properties file:

{1}load.CNBCurrentRateProvider.type=SCHEDULED {1}load.CNBCurrentRateProvider.at=16:00 {1}load.CNBCurrentRateProvider.resource=/javamoney/CNBCurrentRateProviderFallback.dat {1}load.CNBCurrentRateProvider.urls=https://www.cnb.cz/en/financial_markets/foreign_exchange_market/exchange_rate_fixing/daily.txt {1}load.CNBCurrentRateProvider.startRemote=true

Problem is that when the loading from URL failed, the fallback CNBCurrentRateProviderFallback.dat file is not used at all. I found that in the LoadableResource.shouldReadDataFromCallBack() there is !loadRemote() which has the wrong implementation as it should return true, on success, but it returns false on success.

keilw commented 6 years ago

Is there a small code snippet or JUnit test you could provide to replicate this?

marikt commented 6 years ago

Well I don't have JUnit, but if I take a look at the code of the class LoadableResource:

   /**
     * @return true, on success.
     */
    public boolean loadRemote() {
           ....
           return !load(itemToLoad, false);
          ....
    } 

and if you look on the impl of the load(itemToLoad, false); it returns TRUE on success so the ! should be deleted from loadRemote() method otherwise the javadoc is wrong

marikt commented 6 years ago

You can replicate it by setting the unreachable url in javamoney.property:

{1}load.ECBCurrentRateProvider.urls=https://www.blablabla.bla {1}load.ECBCurrentRateProvider.resource=/javamoney/ECBCurrentRateProviderFallback.dat

it will:

  1. try to read exchange rate form https://www.blablabla.bla which will fail
  2. so it should try to read data from ECBCurrentRateProviderCallback.dat but it will not try because there is wrongly implemented check:

LoadableResource.shouldReadDataFromCallBack()

In the log, there will be the message that reading from the https://www.blablabla.bla & reading from the callback failed.

heli80 commented 6 years ago

Hi! This bug is already fixed on the master branch: https://github.com/JavaMoney/jsr354-ri/pull/157

I hope moneta version 1.2 is released soon... Br, Heli

marikt commented 6 years ago

Great, thanks!

marikt commented 6 years ago

Is there an estimated day of release 1.2?

keilw commented 6 years ago

@marikt I'm afraid not. @atsticks planned to approach the current Spec Lead company Credit Suisse. Without their collaboration and support either finding a new Spec Lead or a transfer (similar to e.g. MVC) there will be no release or progress.

atsticks commented 6 years ago

Will be released with 1.2