Skip to content

Commit

Permalink
Polish style
Browse files Browse the repository at this point in the history 
 - Wrap comments at 90 chars per bisq-network/style#5
 - Wrap code at 120 chars per bisq-network/style#3
 - Remove unused imports
 - Remove extra newlines
 - Format code where appropriate
 - Remove unused Javadoc tags, e.g. @return, @param
 - End Javadoc summary sentence with a period where missing
 - Remove HTML formatting in Javadoc, e.g. extra <br>s
  • Loading branch information
@cbeams
cbeams committed May 11, 2020
1 parent 45636f1 commit f8c9c72
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 60 deletions.
8 changes: 4 additions & 4 deletions pricenode/src/main/java/bisq/price/mining/FeeRateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import java.time.Instant;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -39,8 +38,8 @@ class FeeRateService {
private final List<FeeRateProvider> providers;

/**
* Construct an {@link FeeRateService} with a list of all
* {@link FeeRateProvider} implementations discovered via classpath scanning.
* Construct a {@link FeeRateService} with a list of all {@link FeeRateProvider}
* implementations discovered via classpath scanning.
*
* @param providers all {@link FeeRateProvider} implementations in ascending
* order of precedence
Expand Down Expand Up @@ -76,7 +75,8 @@ public Map<String, Object> getFees() {
averageFeeRate = Math.min(averageFeeRate, BitcoinFeeRateProvider.MAX_FEE_RATE);

// Prepare response: Add timestamp of now
// Since this is an average, the timestamp is associated with when the moment in time when the avg was computed
// Since this is an average, the timestamp is associated with when the moment in
// time when the avg was computed
metadata.put("bitcoinFeesTs", Instant.now().getEpochSecond());

// Prepare response: Add the fee average
Expand Down
17 changes: 8 additions & 9 deletions pricenode/src/main/java/bisq/price/mining/providers/BitcoinFeeRateProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,19 @@
import org.springframework.stereotype.Component;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.RestClientException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.util.UriComponentsBuilder;

import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;

import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;

/**
* Provider that specifically interprets the mempool.space API format to retrieve a mining fee estimate. <br/><br/>
* Other {@link FeeRateProvider}s can be created for other APIs.
* Provider that specifically interprets the mempool.space API format to retrieve a mining
* fee estimate. Other {@link FeeRateProvider}s can be created for other APIs.
*/
public abstract class BitcoinFeeRateProvider extends FeeRateProvider {

Expand All @@ -55,8 +53,8 @@ public abstract class BitcoinFeeRateProvider extends FeeRateProvider {
private static final int DEFAULT_MAX_BLOCKS = 2;
private static final int DEFAULT_REFRESH_INTERVAL = 2;

// Keys of properties defining the available API endpoints
// To enable them, simply enable and adjust the corresponding lines in application.properties
// Keys of properties defining the available API endpoints. To enable them, simply
// uncomment and adjust the corresponding lines in application.properties
private static final String API_ENDPOINT_HOSTNAME_KEY_1 = "service.mining.feeEstimate.apiEndpointHostname.1";
private static final String API_ENDPOINT_HOSTNAME_KEY_2 = "service.mining.feeEstimate.apiEndpointHostname.2";
private static final String API_ENDPOINT_HOSTNAME_KEY_3 = "service.mining.feeEstimate.apiEndpointHostname.3";
Expand All @@ -80,8 +78,8 @@ public BitcoinFeeRateProvider(Environment env) {
}

protected FeeRate doGet() {
// Default value is the minimum rate
// If the connection to the fee estimate provider fails, we fall back to this value
// Default value is the minimum rate. If the connection to the fee estimate
// provider fails, we fall back to this value.
long estimatedFeeRate = MIN_FEE_RATE;
try {
estimatedFeeRate = getEstimatedFeeRate();
Expand Down Expand Up @@ -121,7 +119,8 @@ private Stream<Map.Entry<String, Long>> getFeeRatePredictions() {
}

/**
* @return Hostname of the fee estimation API endpoint. No prefix (https://), no suffix (trailing slashes, etc)
* Return the hostname of the fee estimation API endpoint. No prefix (https://), no
* suffix (trailing slashes, etc).
*/
protected abstract String getMempoolApiHostname();

Expand Down
43 changes: 19 additions & 24 deletions pricenode/src/test/java/bisq/price/mining/FeeRateServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import static org.junit.jupiter.api.Assertions.assertNotEquals;

/**
* Tests the {@link bisq.price.mining.FeeRateService}, which can aggregate data from several {@link FeeRateProvider}s
* <br/><br/>
* Tests the {@link bisq.price.mining.FeeRateService}, which can aggregate data from
* several {@link FeeRateProvider}s.
* @see bisq.price.mining.providers.BitcoinFeeRateProviderTest
*/
public class FeeRateServiceTest extends TestBase {
Expand All @@ -42,7 +42,8 @@ public void getFees_noWorkingProvider() {

Map<String, Object> retrievedData = service.getFees();

// Even with no working providers, we expect the service to return pre-configured minimum fee rate
// Even with no working providers, we expect the service to return pre-configured
// minimum fee rate
doSanityChecksForRetrievedData(retrievedData, BitcoinFeeRateProvider.MIN_FEE_RATE);
}

Expand All @@ -56,7 +57,8 @@ public void getFees_singleProvider_feeBelowMin() {

Map<String, Object> retrievedData = service.getFees();

// When the provider returns a value below the expected min, the service should return the min
// When the provider returns a value below the expected min, the service should
// return the min
doSanityChecksForRetrievedData(retrievedData, BitcoinFeeRateProvider.MIN_FEE_RATE);
}

Expand All @@ -70,18 +72,18 @@ public void getFees_singleProvider_feeAboveMax() {

Map<String, Object> retrievedData = service.getFees();

// When the provider returns a value above the expected max, the service should return the max
// When the provider returns a value above the expected max, the service should
// return the max
doSanityChecksForRetrievedData(retrievedData, BitcoinFeeRateProvider.MAX_FEE_RATE);
}

@Test
public void getFees_multipleProviders() {
// 3 providers, returning 1xMIN, 2xMIN, 3xMIN
FeeRateService service = new FeeRateService(
asList(
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 1),
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 2),
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 3)));
FeeRateService service = new FeeRateService(asList(
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 1),
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 2),
buildDummyReachableBitcoinFeeRateProvider(BitcoinFeeRateProvider.MIN_FEE_RATE * 3)));

Map<String, Object> retrievedData = service.getFees();

Expand All @@ -91,14 +93,11 @@ public void getFees_multipleProviders() {

/**
* Performs a few basic sanity checks on the returned data object
*
* @param retrievedData
* @param expectedFeeRate
*/
private void doSanityChecksForRetrievedData(Map<String, Object> retrievedData, long expectedFeeRate) {
// Check if the response has the expected format
// Since the timestamp is that of the average (not that of the individual fee rates reported
// by the individual providers), we always expect a non-zero timestamp
// Check if the response has the expected format. Since the timestamp is that of
// the average (not that of the individual fee rates reported by the individual
// providers), we always expect a non-zero timestamp
assertNotEquals(0L, retrievedData.get("bitcoinFeesTs"));

Map<String, String> retrievedDataMap = (Map<String, String>) retrievedData.get("dataMap");
Expand All @@ -108,8 +107,6 @@ private void doSanityChecksForRetrievedData(Map<String, Object> retrievedData, l

/**
* Simulates a reachable provider, which successfully returns an API response
* @param feeRate
* @return
*/
private BitcoinFeeRateProvider buildDummyReachableBitcoinFeeRateProvider(long feeRate) {
GenericXmlApplicationContext ctx = new GenericXmlApplicationContext();
Expand All @@ -128,13 +125,11 @@ protected FeeRate doGet() {
}

/**
* Simulates an unreachable provider, which for whatever reason cannot deliver a response to the API.<br/><br/>
* Reasons for that could be: host went offline, connection timeout, connection cannot be established (expired
* certificate), etc.
*
* @return
* Simulates an unreachable provider, which for whatever reason cannot deliver a
* response to the API. Reasons for that could be: host went offline, connection
* timeout, connection cannot be established (expired certificate), etc.
*/
private BitcoinFeeRateProvider buildDummyUnreachableBitcoinFeeRateProvider() throws RestClientException{
private BitcoinFeeRateProvider buildDummyUnreachableBitcoinFeeRateProvider() throws RestClientException {
GenericXmlApplicationContext ctx = new GenericXmlApplicationContext();
BitcoinFeeRateProvider dummyProvider = new BitcoinFeeRateProvider.First(ctx.getEnvironment()) {
@Override
Expand Down
6 changes: 3 additions & 3 deletions pricenode/src/test/java/bisq/price/mining/providers/BitcoinFeeRateProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
import static org.junit.Assert.assertTrue;

/**
* Tests specific to a {@link BitcoinFeeRateProvider} which queries one API endpoint<br/><br/>
*
* For tests related to managing parallel fee API endpoints, see {@link bisq.price.mining.FeeRateServiceTest}
* Tests specific to a {@link BitcoinFeeRateProvider} which queries one API endpoint. For
* tests related to managing parallel fee API endpoints, see
* {@link bisq.price.mining.FeeRateServiceTest}
*/
public class BitcoinFeeRateProviderTest {

Expand Down
37 changes: 21 additions & 16 deletions pricenode/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,20 @@
public class ExchangeRateServiceTest extends TestBase {

/**
* Logback version of the Slfj logger used by {@link ExchangeRateService}. This
* allows us to test if specific messages were logged <br/><br/>
*
* Logback version of the Slf4j logger used by {@link ExchangeRateService}. This
* allows us to test if specific messages were logged.
* See https://stackoverflow.com/a/52229629
*/
private static Logger exchangeRateServiceLogger;
private static final String LIST_APPENDER_NAME = "testListAppender";

@BeforeAll
static void setup () {
static void setup() {
// Get the logger object for logs in ExchangeRateService
exchangeRateServiceLogger = (Logger) LoggerFactory.getLogger(ExchangeRateService.class);

// Initiate and append a ListAppender, which allows us to programmatically inspect log messages
// Initiate and append a ListAppender, which allows us to programmatically inspect
// log messages
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
listAppender.setName(LIST_APPENDER_NAME);
listAppender.start();
Expand All @@ -75,7 +75,8 @@ public void getAllMarketPrices_withNoExchangeRates_logs_Exception() {

Map<String, Object> retrievedData = service.getAllMarketPrices();

doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider.getPrefix(), numberOfCurrencyPairsOnExchange);
doSanityChecksForRetrievedDataSingleProvider(
retrievedData, dummyProvider.getPrefix(), numberOfCurrencyPairsOnExchange);

// No exchange rates provided by this exchange, two things should happen
// A) the timestamp should be set to 0
Expand All @@ -86,7 +87,8 @@ public void getAllMarketPrices_withNoExchangeRates_logs_Exception() {
assertEquals(0L, retrievedData.get(dummyProvider.getPrefix() + "Ts"));

// B) Check that an error is logged
// Log msg has the format: java.lang.IllegalStateException: No exchange rate data found for ExchangeName-JzfP1
// Log msg has the format: java.lang.IllegalStateException: No exchange rate data
// found for ExchangeName-JzfP1
List<ILoggingEvent> logsList = ((ListAppender) exchangeRateServiceLogger.getAppender(LIST_APPENDER_NAME)).list;
assertEquals(Level.ERROR, logsList.get(0).getLevel());
assertTrue(logsList.get(0).getMessage().endsWith("No exchange rate data found for " + dummyProvider.getName()));
Expand All @@ -100,7 +102,8 @@ public void getAllMarketPrices_withSingleExchangeRate() {

Map<String, Object> retrievedData = service.getAllMarketPrices();

doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider.getPrefix(), numberOfCurrencyPairsOnExchange);
doSanityChecksForRetrievedDataSingleProvider(
retrievedData, dummyProvider.getPrefix(), numberOfCurrencyPairsOnExchange);

// One rate was provided by this provider, so the timestamp should not be 0
assertNotEquals(0L, retrievedData.get(dummyProvider.getPrefix() + "Ts"));
Expand All @@ -118,13 +121,14 @@ public void getAllMarketPrices_withMultipleProviders() {
doSanityChecksForRetrievedDataMultipleProviders(retrievedData,
asList(dummyProvider1.getPrefix(), dummyProvider2.getPrefix()));

// One rate was provided by each provider in this service, so the timestamp (for both providers) should not be 0
// 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"));
}

/**
* Performs generic sanity checks on the response format and contents
* Performs generic sanity checks on the response format and contents.
*
* @param retrievedData Response data retrieved from the {@link ExchangeRateService}
* @param providerPrefix {@link ExchangeRateProvider#getPrefix()}
Expand All @@ -143,17 +147,18 @@ private void doSanityChecksForRetrievedDataSingleProvider(Map<String, Object> re
}

/**
* Performs generic sanity checks on the response format and contents
* Performs generic sanity checks on the response format and contents.
*
* @param retrievedData Response data retrieved from the {@link ExchangeRateService}
* @param providerPrefixes List of all {@link ExchangeRateProvider#getPrefix()} the {@link ExchangeRateService} uses
* @param providerPrefixes List of all {@link ExchangeRateProvider#getPrefix()} the
* {@link ExchangeRateService} uses
*/
private void doSanityChecksForRetrievedDataMultipleProviders(Map<String, Object> retrievedData,
List<String> providerPrefixes) {
List<String> providerPrefixes) {
// Check the correct amount of entries were present in the service response:
// The timestamp and the count fields are per provider, so N providers means N times those fields
// timestamp (x N) + count (x N) + price data (stored as a list under the key "data")
// So expected size is Nx2 + 1
// The timestamp and the count fields are per provider, so N providers means N
// times those fields timestamp (x N) + count (x N) + price data (stored as a list
// under the key "data"). So expected size is Nx2 + 1.
int n = providerPrefixes.size();
assertEquals(n * 2 + 1, retrievedData.size());
for (String providerPrefix : providerPrefixes) {
Expand Down
5 changes: 1 addition & 4 deletions pricenode/src/test/java/bisq/price/util/VersionControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,11 @@

import java.io.IOException;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertTrue;



public class VersionControllerTest {
public class VersionControllerTest {

@Test
public void getVersion() throws IOException {
Expand Down

0 comments on commit f8c9c72

Please sign in to comment.