From 9b01e60c89e30395042a0b8f2f3e8c9f52ec64ff Mon Sep 17 00:00:00 2001 From: sernamar Date: Sun, 1 Sep 2024 10:40:38 +0200 Subject: [PATCH 1/2] Add retry mechanism to `CompoundRateProvider.getExchangeRate` on failure Modified the getExchangeRate method in the CompoundRateProvider class to introduce a failFast parameter. Previously, the method would immediately throw an exception on the first provider failure. This change allows for a more flexible approach by adding an option to continue attempting to get an exchange rate from subsequent providers if failFast is set to false. - Added a new overloaded getExchangeRate method with a failFast boolean parameter. - If failFast is true, the method retains the original behavior of failing immediately on the first exception. - If failFast is false, the method logs a warning and continues to the next provider in case of an exception. This update addresses some of the limitations described in issue #385. --- moneta-core/pom.xml | 7 ++ .../moneta/spi/CompoundRateProvider.java | 22 +++++-- .../moneta/spi/CompoundRateProviderTest.java | 66 +++++++++++++++++++ 3 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java diff --git a/moneta-core/pom.xml b/moneta-core/pom.xml index c48ebffeb..95e3067be 100644 --- a/moneta-core/pom.xml +++ b/moneta-core/pom.xml @@ -216,6 +216,13 @@ + + org.javamoney + moneta + 1.4.4 + pom + test + javax.money money-api diff --git a/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java b/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java index 79eb35874..16502f9ad 100644 --- a/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java +++ b/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java @@ -28,6 +28,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.logging.Logger; /** * This class implements a {@link ExchangeRateProvider} that delegates calls to @@ -48,6 +49,8 @@ public class CompoundRateProvider extends AbstractRateProvider { */ private final List providers = new ArrayList<>(); + private static final Logger logger = Logger.getLogger(CompoundRateProvider.class.getName()); + /** * Constructor. * @@ -102,6 +105,10 @@ private void addProvider(ExchangeRateProvider prov) { */ @Override public ExchangeRate getExchangeRate(ConversionQuery conversionQuery) { + return getExchangeRate(conversionQuery, false); + } + + public ExchangeRate getExchangeRate(ConversionQuery conversionQuery, boolean failFast) { for (ExchangeRateProvider prov : this.providers) { try { if (prov.isAvailable(conversionQuery)) { @@ -111,15 +118,18 @@ public ExchangeRate getExchangeRate(ConversionQuery conversionQuery) { } } } catch (Exception e) { - throw new CurrencyConversionException(conversionQuery.getBaseCurrency(), conversionQuery.getCurrency(), null, - "Rate Provider did not return data though at check before data was flagged as available," + - " provider=" + prov.getContext().getProviderName() + ", query=" + conversionQuery, e); + if (failFast) { + throw new CurrencyConversionException(conversionQuery.getBaseCurrency(), conversionQuery.getCurrency(), null, + "Rate Provider did not return data though at check before data was flagged as available," + + " provider=" + prov.getContext().getProviderName() + ", query=" + conversionQuery, e); + } else { + logger.warning("Rate Provider did not return data though at check before data was flagged as available," + + " provider=" + prov.getContext().getProviderName() + ", query=" + conversionQuery + ", error=" + e.getMessage()); + } } } throw new CurrencyConversionException(conversionQuery.getBaseCurrency(), conversionQuery.getCurrency(), null, "All delegate providers failed to deliver rate, providers=" + this.providers + ", query=" + conversionQuery); } - - -} \ No newline at end of file +} diff --git a/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java b/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java new file mode 100644 index 000000000..fbaa74880 --- /dev/null +++ b/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java @@ -0,0 +1,66 @@ +package org.javamoney.moneta.spi; + +import org.testng.annotations.Test; + +import javax.money.convert.*; + +import java.time.DayOfWeek; +import java.time.LocalDate; +import java.time.temporal.TemporalAdjusters; + +import static org.testng.Assert.*; + +public class CompoundRateProviderTest { + + @Test + public void testGetExchangeRate() { + String baseCurrency = "EUR"; + String termCurrency = "GBP"; + LocalDate date = lastWeekTuesday(); + ConversionQuery conversionQuery = ConversionQueryBuilder.of() + .setBaseCurrency(baseCurrency) + .setTermCurrency(termCurrency) + .set(date) + .build(); + + ExchangeRateProvider compoundRateProvider = MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); + assertNotNull(compoundRateProvider.getExchangeRate(conversionQuery)); + } + + @Test + public void testGetExchangeRateAllProvidersFail() { + String baseCurrency = "EUR"; + String termCurrency = "GBP"; + LocalDate date = lastWeekTuesday(); + ConversionQuery conversionQuery = ConversionQueryBuilder.of() + .setBaseCurrency(baseCurrency) + .setTermCurrency(termCurrency) + .set(date) + .build(); + + ExchangeRateProvider compoundRateProvider = MonetaryConversions.getExchangeRateProvider("ECB", "IMF"); + assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery)); + } + + @Test + public void testGetExchangeRateFailingFast() { + String baseCurrency = "EUR"; + String termCurrency = "GBP"; + LocalDate date = lastWeekTuesday(); + ConversionQuery conversionQuery = ConversionQueryBuilder.of() + .setBaseCurrency(baseCurrency) + .setTermCurrency(termCurrency) + .set(date) + .build(); + + // Need to cast to CompoundRateProvider to access the getExchangeRate method that takes a boolean parameter + CompoundRateProvider compoundRateProvider = (CompoundRateProvider) MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); + assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery, true)); + } + + private LocalDate lastWeekTuesday() { + LocalDate today = LocalDate.now(); + LocalDate lastTuesday = today.with(TemporalAdjusters.previousOrSame(DayOfWeek.TUESDAY)); + return lastTuesday.minusWeeks(1); + } +} From d3fbfae1a5ecceab0a2c996706efd52774614645 Mon Sep 17 00:00:00 2001 From: sernamar Date: Sun, 1 Sep 2024 19:50:09 +0200 Subject: [PATCH 2/2] Use `true` as the default value for the `failFast` parameter --- .../moneta/spi/CompoundRateProvider.java | 2 +- .../moneta/spi/CompoundRateProviderTest.java | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java b/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java index 16502f9ad..2dbc29cef 100644 --- a/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java +++ b/moneta-core/src/main/java/org/javamoney/moneta/spi/CompoundRateProvider.java @@ -105,7 +105,7 @@ private void addProvider(ExchangeRateProvider prov) { */ @Override public ExchangeRate getExchangeRate(ConversionQuery conversionQuery) { - return getExchangeRate(conversionQuery, false); + return getExchangeRate(conversionQuery, true); } public ExchangeRate getExchangeRate(ConversionQuery conversionQuery, boolean failFast) { diff --git a/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java b/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java index fbaa74880..fc4a8f425 100644 --- a/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java +++ b/moneta-core/src/test/java/org/javamoney/moneta/spi/CompoundRateProviderTest.java @@ -23,8 +23,9 @@ public void testGetExchangeRate() { .set(date) .build(); - ExchangeRateProvider compoundRateProvider = MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); - assertNotNull(compoundRateProvider.getExchangeRate(conversionQuery)); + // Need to cast to CompoundRateProvider to access the getExchangeRate method that takes a boolean parameter + CompoundRateProvider compoundRateProvider = (CompoundRateProvider) MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); + assertNotNull(compoundRateProvider.getExchangeRate(conversionQuery, false)); } @Test @@ -38,8 +39,9 @@ public void testGetExchangeRateAllProvidersFail() { .set(date) .build(); - ExchangeRateProvider compoundRateProvider = MonetaryConversions.getExchangeRateProvider("ECB", "IMF"); - assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery)); + // Need to cast to CompoundRateProvider to access the getExchangeRate method that takes a boolean parameter + CompoundRateProvider compoundRateProvider = (CompoundRateProvider) MonetaryConversions.getExchangeRateProvider("ECB", "IMF"); + assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery, false)); } @Test @@ -53,9 +55,8 @@ public void testGetExchangeRateFailingFast() { .set(date) .build(); - // Need to cast to CompoundRateProvider to access the getExchangeRate method that takes a boolean parameter - CompoundRateProvider compoundRateProvider = (CompoundRateProvider) MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); - assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery, true)); + ExchangeRateProvider compoundRateProvider = MonetaryConversions.getExchangeRateProvider("ECB", "ECB-HIST90"); + assertThrows(CurrencyConversionException.class, () -> compoundRateProvider.getExchangeRate(conversionQuery)); } private LocalDate lastWeekTuesday() {