Merge pull request #4942 from chimp1984/refactor-http-client

Refactor http client
bisq-network · Dec 14, 2020 · 2bad7fc · 2bad7fc
2 parents f4df51c + ad2a329
commit 2bad7fc
Show file tree

Hide file tree

Showing 14 changed files with 241 additions and 99 deletions.
diff --git a/core/src/main/java/bisq/core/btc/BitcoinModule.java b/core/src/main/java/bisq/core/btc/BitcoinModule.java
@@ -26,7 +26,6 @@
 import bisq.core.btc.wallet.BtcWalletService;
 import bisq.core.btc.wallet.NonBsqCoinSelector;
 import bisq.core.btc.wallet.TradeWalletService;
-import bisq.core.provider.PriceNodeHttpClient;
 import bisq.core.provider.ProvidersRepository;
 import bisq.core.provider.fee.FeeProvider;
 import bisq.core.provider.fee.FeeService;
@@ -95,8 +94,6 @@ protected void configure() {
         bind(BtcNodes.class).in(Singleton.class);
         bind(Balances.class).in(Singleton.class);
 
-        bind(PriceNodeHttpClient.class).in(Singleton.class);
-
         bind(ProvidersRepository.class).in(Singleton.class);
         bind(FeeProvider.class).in(Singleton.class);
         bind(PriceFeedService.class).in(Singleton.class);

diff --git a/core/src/main/java/bisq/core/notifications/MobileNotificationService.java b/core/src/main/java/bisq/core/notifications/MobileNotificationService.java
@@ -256,6 +256,11 @@ private void doSendMessage(String iv,
                                boolean useSound,
                                Consumer<String> resultHandler,
                                Consumer<Throwable> errorHandler) throws Exception {
+        if (httpClient.hasPendingRequest()) {
+            log.warn("We have a pending request open. We ignore that request. httpClient {}", httpClient);
+            return;
+        }
+
         String msg;
         if (mobileModel.getOs() == null)
             throw new RuntimeException("No mobileModel OS set");
@@ -297,7 +302,7 @@ private void doSendMessage(String iv,
         String threadName = "sendMobileNotification-" + msgAsHex.substring(0, 5) + "...";
         ListenableFuture<String> future = executorService.submit(() -> {
             Thread.currentThread().setName(threadName);
-            String result = httpClient.requestWithGET(param, "User-Agent",
+            String result = httpClient.get(param, "User-Agent",
                     "bisq/" + Version.VERSION + ", uid:" + httpClient.getUid());
             log.info("sendMobileNotification result: " + result);
             checkArgument(result.equals(SUCCESS), "Result was not 'success'. result=" + result);

diff --git a/...sq/core/provider/PriceNodeHttpClient.java → ...ava/bisq/core/provider/FeeHttpClient.java b/...sq/core/provider/PriceNodeHttpClient.java → ...ava/bisq/core/provider/FeeHttpClient.java
@@ -21,12 +21,14 @@
 import bisq.network.http.HttpClientImpl;
 
 import javax.inject.Inject;
+import javax.inject.Singleton;
 
 import javax.annotation.Nullable;
 
-public class PriceNodeHttpClient extends HttpClientImpl {
+@Singleton
+public class FeeHttpClient extends HttpClientImpl {
     @Inject
-    public PriceNodeHttpClient(@Nullable Socks5ProxyProvider socks5ProxyProvider) {
+    public FeeHttpClient(@Nullable Socks5ProxyProvider socks5ProxyProvider) {
         super(socks5ProxyProvider);
     }
 }
diff --git a/core/src/main/java/bisq/core/provider/PriceHttpClient.java b/core/src/main/java/bisq/core/provider/PriceHttpClient.java
@@ -0,0 +1,34 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.provider;
+
+import bisq.network.Socks5ProxyProvider;
+import bisq.network.http.HttpClientImpl;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+import javax.annotation.Nullable;
+
+@Singleton
+public class PriceHttpClient extends HttpClientImpl {
+    @Inject
+    public PriceHttpClient(@Nullable Socks5ProxyProvider socks5ProxyProvider) {
+        super(socks5ProxyProvider);
+    }
+}
diff --git a/core/src/main/java/bisq/core/provider/fee/FeeProvider.java b/core/src/main/java/bisq/core/provider/fee/FeeProvider.java
@@ -17,10 +17,12 @@
 
 package bisq.core.provider.fee;
 
+import bisq.core.provider.FeeHttpClient;
 import bisq.core.provider.HttpClientProvider;
-import bisq.core.provider.PriceNodeHttpClient;
 import bisq.core.provider.ProvidersRepository;
 
+import bisq.network.http.HttpClient;
+
 import bisq.common.app.Version;
 import bisq.common.util.Tuple2;
 
@@ -40,12 +42,12 @@
 public class FeeProvider extends HttpClientProvider {
 
     @Inject
-    public FeeProvider(PriceNodeHttpClient httpClient, ProvidersRepository providersRepository) {
+    public FeeProvider(FeeHttpClient httpClient, ProvidersRepository providersRepository) {
         super(httpClient, providersRepository.getBaseUrl(), false);
     }
 
     public Tuple2<Map<String, Long>, Map<String, Long>> getFees() throws IOException {
-        String json = httpClient.requestWithGET("getFees", "User-Agent", "bisq/" + Version.VERSION);
+        String json = httpClient.get("getFees", "User-Agent", "bisq/" + Version.VERSION);
 
         LinkedTreeMap<?, ?> linkedTreeMap = new Gson().fromJson(json, LinkedTreeMap.class);
         Map<String, Long> tsMap = new HashMap<>();
@@ -64,4 +66,8 @@ public Tuple2<Map<String, Long>, Map<String, Long>> getFees() throws IOException
         }
         return new Tuple2<>(tsMap, map);
     }
+
+    public HttpClient getHttpClient() {
+        return httpClient;
+    }
 }
diff --git a/core/src/main/java/bisq/core/provider/fee/FeeRequest.java b/core/src/main/java/bisq/core/provider/fee/FeeRequest.java
@@ -45,7 +45,7 @@ public FeeRequest() {
     public SettableFuture<Tuple2<Map<String, Long>, Map<String, Long>>> getFees(FeeProvider provider) {
         final SettableFuture<Tuple2<Map<String, Long>, Map<String, Long>>> resultFuture = SettableFuture.create();
         ListenableFuture<Tuple2<Map<String, Long>, Map<String, Long>>> future = executorService.submit(() -> {
-            Thread.currentThread().setName("FeeRequest-" + provider.toString());
+            Thread.currentThread().setName("FeeRequest @ " + provider.getHttpClient().getBaseUrl());
             return provider.getFees();
         });
 

diff --git a/core/src/main/java/bisq/core/provider/fee/FeeService.java b/core/src/main/java/bisq/core/provider/fee/FeeService.java
@@ -137,6 +137,11 @@ public void requestFees(Runnable resultHandler) {
     }
 
     public void requestFees(@Nullable Runnable resultHandler, @Nullable FaultHandler faultHandler) {
+        if (feeProvider.getHttpClient().hasPendingRequest()) {
+            log.warn("We have a pending request open. We ignore that request. httpClient {}", feeProvider.getHttpClient());
+            return;
+        }
+
         long now = Instant.now().getEpochSecond();
         // We all requests only each 2 minutes
         if (now - lastRequest > MIN_PAUSE_BETWEEN_REQUESTS_IN_MIN * 60) {

diff --git a/core/src/main/java/bisq/core/provider/price/PriceFeedService.java b/core/src/main/java/bisq/core/provider/price/PriceFeedService.java
@@ -21,7 +21,7 @@
 import bisq.core.locale.TradeCurrency;
 import bisq.core.monetary.Altcoin;
 import bisq.core.monetary.Price;
-import bisq.core.provider.PriceNodeHttpClient;
+import bisq.core.provider.PriceHttpClient;
 import bisq.core.provider.ProvidersRepository;
 import bisq.core.trade.statistics.TradeStatistics3;
 import bisq.core.user.Preferences;
@@ -101,7 +101,7 @@ public class PriceFeedService {
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     @Inject
-    public PriceFeedService(@SuppressWarnings("SameParameterValue") PriceNodeHttpClient httpClient,
+    public PriceFeedService(PriceHttpClient httpClient,
                             @SuppressWarnings("SameParameterValue") ProvidersRepository providersRepository,
                             @SuppressWarnings("SameParameterValue") Preferences preferences) {
         this.httpClient = httpClient;
@@ -194,17 +194,17 @@ private void request(boolean repeatRequests) {
             if (throwable instanceof PriceRequestException) {
                 String baseUrlOfFaultyRequest = ((PriceRequestException) throwable).priceProviderBaseUrl;
                 String baseUrlOfCurrentRequest = priceProvider.getBaseUrl();
-                if (baseUrlOfFaultyRequest != null && baseUrlOfCurrentRequest.equals(baseUrlOfFaultyRequest)) {
-                    log.warn("We received an error: baseUrlOfCurrentRequest={}, baseUrlOfFaultyRequest={}",
-                            baseUrlOfCurrentRequest, baseUrlOfFaultyRequest);
+                if (baseUrlOfCurrentRequest.equals(baseUrlOfFaultyRequest)) {
+                    log.warn("We received an error: baseUrlOfCurrentRequest={}, baseUrlOfFaultyRequest={}, error={}",
+                            baseUrlOfCurrentRequest, baseUrlOfFaultyRequest, throwable.toString());
                     retryWithNewProvider();
                 } else {
                     log.debug("We received an error from an earlier request. We have started a new request already so we ignore that error. " +
                                     "baseUrlOfCurrentRequest={}, baseUrlOfFaultyRequest={}",
                             baseUrlOfCurrentRequest, baseUrlOfFaultyRequest);
                 }
             } else {
-                log.warn("We received an error with throwable={}", throwable);
+                log.warn("We received an error with throwable={}", throwable.toString());
                 retryWithNewProvider();
             }
 
@@ -393,6 +393,11 @@ private boolean applyPriceToConsumer() {
     }
 
     private void requestAllPrices(PriceProvider provider, Runnable resultHandler, FaultHandler faultHandler) {
+        if (httpClient.hasPendingRequest()) {
+            log.warn("We have a pending request open. We ignore that request. httpClient {}", httpClient);
+            return;
+        }
+
         priceRequest = new PriceRequest();
         SettableFuture<Tuple2<Map<String, Long>, Map<String, MarketPrice>>> future = priceRequest.requestAllPrices(provider);
         Futures.addCallback(future, new FutureCallback<>() {

diff --git a/core/src/main/java/bisq/core/provider/price/PriceProvider.java b/core/src/main/java/bisq/core/provider/price/PriceProvider.java
@@ -58,7 +58,7 @@ public Tuple2<Map<String, Long>, Map<String, MarketPrice>> getAll() throws IOExc
         if (P2PService.getMyNodeAddress() != null)
             hsVersion = P2PService.getMyNodeAddress().getHostName().length() > 22 ? ", HSv3" : ", HSv2";
 
-        String json = httpClient.requestWithGET("getAllMarketPrices", "User-Agent", "bisq/"
+        String json = httpClient.get("getAllMarketPrices", "User-Agent", "bisq/"
                 + Version.VERSION + hsVersion);
 
 

diff --git a/core/src/main/java/bisq/core/provider/price/PriceRequest.java b/core/src/main/java/bisq/core/provider/price/PriceRequest.java
@@ -50,7 +50,7 @@ public SettableFuture<Tuple2<Map<String, Long>, Map<String, MarketPrice>>> reque
         String baseUrl = provider.getBaseUrl();
         SettableFuture<Tuple2<Map<String, Long>, Map<String, MarketPrice>>> resultFuture = SettableFuture.create();
         ListenableFuture<Tuple2<Map<String, Long>, Map<String, MarketPrice>>> future = executorService.submit(() -> {
-            Thread.currentThread().setName("PriceRequest-" + baseUrl);
+            Thread.currentThread().setName("PriceRequest @ " + baseUrl);
             return provider.getAll();
         });
 

diff --git a/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofRequest.java b/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofRequest.java
@@ -197,6 +197,11 @@ public void requestFromService(Consumer<Result> resultHandler, FaultHandler faul
             return;
         }
 
+        if (httpClient.hasPendingRequest()) {
+            log.warn("We have a pending request open. We ignore that request. httpClient {}", httpClient);
+            return;
+        }
+
         // Timeout handing is delegated to the connection timeout handling in httpClient.
 
         ListenableFuture<Result> future = executorService.submit(() -> {
@@ -206,7 +211,7 @@ public void requestFromService(Consumer<Result> resultHandler, FaultHandler faul
                     "&viewkey=" + model.getTxKey() +
                     "&txprove=1";
             log.info("Param {} for {}", param, this);
-            String json = httpClient.requestWithGET(param, "User-Agent", "bisq/" + Version.VERSION);
+            String json = httpClient.get(param, "User-Agent", "bisq/" + Version.VERSION);
             try {
                 String prettyJson = new GsonBuilder().setPrettyPrinting().create().toJson(new JsonParser().parse(json));
                 log.info("Response json from {}\n{}", this, prettyJson);

diff --git a/p2p/src/main/java/bisq/network/http/HttpClient.java b/p2p/src/main/java/bisq/network/http/HttpClient.java
@@ -26,17 +26,19 @@ public interface HttpClient {
 
     void setIgnoreSocks5Proxy(boolean ignoreSocks5Proxy);
 
-    String requestWithGET(String param,
-                          @Nullable String headerKey,
-                          @Nullable String headerValue) throws IOException;
+    String get(String param,
+               @Nullable String headerKey,
+               @Nullable String headerValue) throws IOException;
 
-    String requestWithGETNoProxy(String param,
-                                 @Nullable String headerKey,
-                                 @Nullable String headerValue) throws IOException;
+    String post(String param,
+                @Nullable String headerKey,
+                @Nullable String headerValue) throws IOException;
 
     String getUid();
 
     String getBaseUrl();
 
+    boolean hasPendingRequest();
+
     void shutDown();
 }