Skip to content

Commit

Permalink
Wrap comments at 90 characters
Browse files Browse the repository at this point in the history 
Update comments to reflect bisq-network/style#5 guideline
  • Loading branch information
@cd2357
cd2357 committed Jul 11, 2020
1 parent fcf713d commit 7976798
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 40 deletions.
30 changes: 20 additions & 10 deletions pricenode/src/main/java/bisq/price/spot/ExchangeRateProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@ protected void onRefresh() {
}

/**
* @param exchangeClass Class of the {@link Exchange} for which the rates should be polled
* @return Exchange rates for Bisq-supported fiat currencies and altcoins in the specified {@link Exchange}
* @param exchangeClass Class of the {@link Exchange} for which the rates should be
* polled
* @return Exchange rates for Bisq-supported fiat currencies and altcoins in the
* specified {@link Exchange}
*
* @see CurrencyUtil#getAllSortedFiatCurrencies()
* @see CurrencyUtil#getAllSortedCryptoCurrencies()
Expand Down Expand Up @@ -117,12 +119,16 @@ protected Set<ExchangeRate> doGet(Class<? extends Exchange> exchangeClass) {
this.getName()
));
} catch (CurrencyPairNotValidException cpnve) {
// Some exchanges support certain currency pairs for other services but not for spot markets
// In that case, trying to retrieve the market ticker for that pair may fail with this specific type of exception
// Some exchanges support certain currency pairs for other
// services but not for spot markets. In that case, trying to
// retrieve the market ticker for that pair may fail with this
// specific type of exception
log.info("Currency pair " + cp + " not supported in Spot Markets: " + cpnve.getMessage());
} catch (Exception e) {
// Catch any other type of generic exception (IO, network level, rate limit reached, etc)
log.info("Exception encountered while retrieving rate for currency pair " + cp + ": " + e.getMessage());
// Catch any other type of generic exception (IO, network level,
// rate limit reached, etc)
log.info("Exception encountered while retrieving rate for currency pair " + cp + ": " +
e.getMessage());
}
});

Expand All @@ -142,12 +148,16 @@ protected Set<ExchangeRate> doGet(Class<? extends Exchange> exchangeClass) {
this.getName()
));
} catch (CurrencyPairNotValidException cpnve) {
// Some exchanges support certain currency pairs for other services but not for spot markets
// In that case, trying to retrieve the market ticker for that pair may fail with this specific type of exception
// Some exchanges support certain currency pairs for other
// services but not for spot markets. In that case, trying to
// retrieve the market ticker for that pair may fail with this
// specific type of exception
log.info("Currency pair " + cp + " not supported in Spot Markets: " + cpnve.getMessage());
} catch (Exception e) {
// Catch any other type of generic exception (IO, network level, rate limit reached, etc)
log.info("Exception encountered while retrieving rate for currency pair " + cp + ": " + e.getMessage());
// Catch any other type of generic exception (IO, network level,
// rate limit reached, etc)
log.info("Exception encountered while retrieving rate for currency pair " + cp + ": " +
e.getMessage());
}
});

Expand Down
38 changes: 23 additions & 15 deletions pricenode/src/main/java/bisq/price/spot/ExchangeRateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,46 +65,52 @@ public Map<String, Object> getAllMarketPrices() {
providers.forEach(p -> {
Set<ExchangeRate> exchangeRates = p.get();

// Specific metadata fields for specific providers are expected by the client, mostly for historical reasons
// Specific metadata fields for specific providers are expected by the client,
// mostly for historical reasons
// Therefore, add metadata fields for all known providers
// Rates are encapsulated in the "data" map below
metadata.putAll(getMetadata(p, exchangeRates));
});

return new LinkedHashMap<String, Object>() {{
putAll(metadata);
// Use a sorted list by currency code to make comparision of json data between different
// price nodes easier
// Use a sorted list by currency code to make comparision of json data between
// different price nodes easier
List<ExchangeRate> values = new ArrayList<>(aggregateExchangeRates.values());
values.sort(Comparator.comparing(ExchangeRate::getCurrency));
put("data", values);
}};
}

/**
* For each currency, create an aggregate {@link ExchangeRate} based on the currency's rates from all providers.
* If multiple providers have rates for the currency, then aggregate price = average of retrieved prices.
* If a single provider has rates for the currency, then aggregate price = the rate from that provider.
* For each currency, create an aggregate {@link ExchangeRate} based on the currency's
* rates from all providers. If multiple providers have rates for the currency, then
* aggregate price = average of retrieved prices. If a single provider has rates for
* the currency, then aggregate price = the rate from that provider.
*
* @return Aggregate {@link ExchangeRate}s based on info from all providers, indexed by currency code
* @return Aggregate {@link ExchangeRate}s based on info from all providers, indexed
* by currency code
*/
private Map<String, ExchangeRate> getAggregateExchangeRates() {
Map<String, ExchangeRate> aggregateExchangeRates = new HashMap<>();

// Query all known providers and collect all exchange rates, grouped by currency code
// Query all providers and collect all exchange rates, grouped by currency code
Map<String, List<ExchangeRate>> currencyCodeToExchangeRates = getCurrencyCodeToExchangeRates();

// For each currency code, calculate aggregate rate
currencyCodeToExchangeRates.forEach((currencyCode, exchangeRateList) -> {
ExchangeRate aggregateExchangeRate;
if (exchangeRateList.size() == 1) {
// If a single provider has rates for this currency, then aggregate = rate from that provider
// If a single provider has rates for this currency, then aggregate = rate
// from that provider
aggregateExchangeRate = exchangeRateList.get(0);
}
else if (exchangeRateList.size() > 1) {
// If multiple providers have rates for this currency, then aggregate = average of the rates
// If multiple providers have rates for this currency, then
// aggregate = average of the rates
OptionalDouble opt = exchangeRateList.stream().mapToDouble(ExchangeRate::getPrice).average();
double priceAvg = opt.orElseThrow(IllegalStateException::new); // List size > 1, so opt is always set
// List size > 1, so opt is always set
double priceAvg = opt.orElseThrow(IllegalStateException::new);

aggregateExchangeRate = new ExchangeRate(
currencyCode,
Expand All @@ -113,7 +119,8 @@ else if (exchangeRateList.size() > 1) {
"Bisq-Aggregate");
}
else {
// If the map was built incorrectly and this currency points to an empty list of rates, skip it
// If the map was built incorrectly and this currency points to an empty
// list of rates, skip it
return;
}
aggregateExchangeRates.put(aggregateExchangeRate.getCurrency(), aggregateExchangeRate);
Expand Down Expand Up @@ -145,9 +152,10 @@ private Map<String, List<ExchangeRate>> getCurrencyCodeToExchangeRates() {
private Map<String, Object> getMetadata(ExchangeRateProvider provider, Set<ExchangeRate> exchangeRates) {
Map<String, Object> metadata = new LinkedHashMap<>();

// In case a provider is not available we still want to deliver the data of the other providers, so we catch
// a possible exception and leave timestamp at 0. The Bisq app will check if the timestamp is in a tolerance
// window and if it is too old it will show that the price is not available.
// In case a provider is not available we still want to deliver the data of the
// other providers, so we catch a possible exception and leave timestamp at 0. The
// Bisq app will check if the timestamp is in a tolerance window and if it is too
// old it will show that the price is not available.
long timestamp = 0;
try {
timestamp = getTimestamp(provider, exchangeRates);
Expand Down
3 changes: 2 additions & 1 deletion pricenode/src/main/java/bisq/price/spot/providers/Binance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public Binance() {
@Override
public Set<ExchangeRate> doGet() {
// Supported fiat: EUR, NGN, RUB, TRY, ZAR
// Supported alts: BEAM, DASH, DCR, DOGE, ETC, ETH, LTC, NAV, PIVX, XMR, XZC, ZEC, ZEN
// Supported alts: BEAM, DASH, DCR, DOGE, ETC, ETH, LTC, NAV, PIVX, XMR, XZC, ZEC,
// ZEN
return doGet(BinanceExchange.class);
}
}
3 changes: 2 additions & 1 deletion pricenode/src/test/java/bisq/price/ExchangeTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ protected void doGet_successfulCall(ExchangeRateProvider exchangeProvider) {
Set<ExchangeRate> retrievedExchangeRates = exchangeProvider.doGet();

// Log the valid exchange rates which were retrieved
// Useful when running the tests, to easily identify which exchanges provide useful pairs
// Useful when running the tests, to easily identify which exchanges provide
// useful pairs
retrievedExchangeRates.forEach(e -> log.info("Found exchange rate " + e.toString()));

// Sanity checks
Expand Down
34 changes: 21 additions & 13 deletions pricenode/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ public void getAllMarketPrices_withMultipleProviders_overlappingCurrencyCodes()

doSanityChecksForRetrievedDataMultipleProviders(retrievedData, asList(dummyProvider1, dummyProvider2));

// At least one rate was provided by each provider in this service, so the timestamp
// (for both providers) should not be 0
// At least one rate was provided by each provider in this service, so the
// timestamp (for both providers) should not be 0
assertNotEquals(0L, retrievedData.get(dummyProvider1.getPrefix() + "Ts"));
assertNotEquals(0L, retrievedData.get(dummyProvider2.getPrefix() + "Ts"));
}
Expand All @@ -159,8 +159,10 @@ public void getAllMarketPrices_withMultipleProviders_overlappingCurrencyCodes()
* Performs generic sanity checks on the response format and contents.
*
* @param retrievedData Response data retrieved from the {@link ExchangeRateService}
* @param provider {@link ExchangeRateProvider} available to the {@link ExchangeRateService}
* @param numberOfCurrencyPairsOnExchange Number of currency pairs this exchange was initiated with
* @param provider {@link ExchangeRateProvider} available to the
* {@link ExchangeRateService}
* @param numberOfCurrencyPairsOnExchange Number of currency pairs this exchange was
* initiated with
*/
private void doSanityChecksForRetrievedDataSingleProvider(Map<String, Object> retrievedData,
ExchangeRateProvider provider,
Expand All @@ -169,7 +171,8 @@ private void doSanityChecksForRetrievedDataSingleProvider(Map<String, Object> re
doSanityChecksForRetrievedDataMultipleProviders(retrievedData, asList(provider));

// Check that the amount of provided exchange rates matches expected value
// For one provider, the amount of rates of that provider should be the total amount of rates in the response
// For one provider, the amount of rates of that provider should be the total
// amount of rates in the response
List<String> retrievedMarketPricesData = (List<String>) retrievedData.get("data");
assertEquals(numberOfCurrencyPairsOnExchange, retrievedMarketPricesData.size());
}
Expand Down Expand Up @@ -205,8 +208,9 @@ private void doSanityChecksForRetrievedDataMultipleProviders(Map<String, Object>
assertEquals(uniqueRates, totalRates, "Found duplicate rates in data field");

// There should be only one ExchangeRate per currency
// In other words, even if multiple providers return rates for the same currency, the ExchangeRateService
// should expose only one (aggregate) ExchangeRate for that currency
// In other words, even if multiple providers return rates for the same currency,
// the ExchangeRateService should expose only one (aggregate) ExchangeRate for
// that currency
Map<String, ExchangeRate> currencyCodeToExchangeRateFromService = retrievedRates.stream()
.collect(Collectors.toMap(
ExchangeRate::getCurrency, exchangeRate -> exchangeRate
Expand All @@ -229,25 +233,28 @@ private void doSanityChecksForRetrievedDataMultipleProviders(Map<String, Object>
}
}

// For each ExchangeRate which is covered by multiple providers, ensure the rate value is an average
// For each ExchangeRate which is covered by multiple providers, ensure the rate
// value is an average
currencyCodeToExchangeRatesFromProviders.forEach((currencyCode, exchangeRateList) -> {
ExchangeRate rateFromService = currencyCodeToExchangeRateFromService.get(currencyCode);
double priceFromService = rateFromService.getPrice();

OptionalDouble opt = exchangeRateList.stream().mapToDouble(ExchangeRate::getPrice).average();
double priceAvgFromProviders = opt.getAsDouble();

// Ensure that the ExchangeRateService correctly aggregates exchange rates from multiple providers
// If multiple providers contain rates for a currency, the service should return a single aggregate rate
// Ensure that the ExchangeRateService correctly aggregates exchange rates
// from multiple providers. If multiple providers contain rates for a
// currency, the service should return a single aggregate rate
// Expected value for aggregate rate = avg(provider rates)
// This formula works for one, as well as many, providers for a specific currency
// This formula works for any number of providers for a specific currency
assertEquals(priceFromService, priceAvgFromProviders, "Service returned incorrect aggregate rate");
});
}

/**
* @param numberOfRatesAvailable Number of exchange rates this provider returns
* @return Dummy {@link ExchangeRateProvider} providing rates for "numberOfRatesAvailable" random currency codes
* @return Dummy {@link ExchangeRateProvider} providing rates for
* "numberOfRatesAvailable" random currency codes
*/
private ExchangeRateProvider buildDummyExchangeRateProvider(int numberOfRatesAvailable) {
ExchangeRateProvider dummyProvider = new ExchangeRateProvider(
Expand All @@ -267,7 +274,8 @@ protected Set<ExchangeRate> doGet() {
// Simulate the required amount of rates
for (int i = 0; i < numberOfRatesAvailable; i++) {
exchangeRates.add(new ExchangeRate(
"DUM-" + getRandomAlphaNumericString(3), // random symbol, avoid duplicates
// random symbol, avoid duplicates
"DUM-" + getRandomAlphaNumericString(3),
RandomUtils.nextDouble(1, 1000), // random price
System.currentTimeMillis(),
getName())); // ExchangeRateProvider name
Expand Down

0 comments on commit 7976798

Please sign in to comment.