Change method sendMany to receive main parameters explicit and optional as Map. Reinforce the validations of all parameters. Modified documentation to the new variables.

charlycrs · charlycrs · commit 938d1ead9896 · 2019-11-04T20:16:21.000-06:00
diff --git a/bitgo-java-api/src/main/java/com/bitso/bitgo/v2/BitGoClient.java b/bitgo-java-api/src/main/java/com/bitso/bitgo/v2/BitGoClient.java
@@ -5,6 +5,7 @@
 import com.bitso.bitgo.v2.entity.WalletTransferResponse;
 
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -37,24 +38,26 @@ Optional<String> login(String email, String password, String otp, boolean extens
     Optional<Wallet> getWalletByAddress(String coin, String waddress) throws IOException;
 
     /**
-     * Invokes the sendmany method see https://www.bitgo.com/api/v2/?shell#send-transaction-to-many
-     * Required parameters are:
-     * coin                        tbtc for test bitcoin, see full list at https://www.bitgo.com/api/v2/#coin-digital-currency-support
-     * walletId                    The ID of the source wallet.
-     * walletPass                  The wallet passphrase.
-     * recipients                  A map with the recipients' addresses as keys and the corresponding
-     *                                    amounts as values. Amounts are in satoshis
-     * sequenceId                  A unique identifier for this transaction (optional).
      *
-     * Optional parameters are:
-     * message                     Notes about the transaction (optional).
+     * @param coin                  tbtc for test bitcoin, see full list at https://www.bitgo.com/api/v2/#coin-digital-currency-support
+     * @param walletId              The ID of the source wallet.
+     * @param walletPass            The wallet passphrase.
+     * @param recipients            A map with the recipients' addresses as keys and the corresponding
+     *                              amounts as values. Amounts are in satoshis
+     * @param sequenceId            A unique identifier for this transaction (optional).
+     * @param optionalParameters    A map with optional extra parameters.
+     * Some of the known optional parameters are:
+     * message                     Notes about the transaction.
      * fee                         Fee (in satoshis), leave null for autodetect. Do not specify unless you are sure it is sufficient.
      * feeTxConfirmTarget          Calculate fees per kilobyte, targeting transaction confirmation in this number of blocks. Default: 2, Minimum: 2, Maximum: 20
      * minConfirms                 only choose unspent inputs with a certain number of confirmations. We recommend setting this to 1 and using enforceMinConfirmsForChange
      * enforceMinConfirmsForChange Defaults to false. When constructing a transaction, minConfirms will only be enforced for unspents not originating from the wallet
      * @return A SendCoinsResponse, or empty if there was a problem (although more likely in case of a problem it will throw).
+     * @throws IOException
      */
-    Optional<SendCoinsResponse> sendMany(Map<String, Object> parameters) throws IOException;
+    Optional<SendCoinsResponse> sendMany(String coin, String walletId, String walletPass,
+                                         Map<String, BigDecimal> recipients, String sequenceId,
+                                         Map<String, Object> optionalParameters) throws IOException;
 
     Optional<Map<String, Object>> getCurrentUserProfile() throws IOException;
 
diff --git a/bitgo-java-api/src/main/java/com/bitso/bitgo/v2/BitGoClientImpl.java b/bitgo-java-api/src/main/java/com/bitso/bitgo/v2/BitGoClientImpl.java
@@ -8,6 +8,7 @@
 import com.bitso.bitgo.v2.entity.WalletTransferResponse;
 import com.fasterxml.jackson.core.type.TypeReference;
 import lombok.Getter;
+import lombok.NonNull;
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 
@@ -104,26 +105,25 @@ public Optional<Wallet> getWalletByAddress(String coin, String waddress) throws
     }
 
     @Override
-    public Optional<SendCoinsResponse> sendMany(Map<String, Object> parameters) throws IOException {
-        // Get all needed parameters
-        String coin = parameters.get("coin").toString();
-        String walletId = parameters.get("walletId").toString();
-        String walletPass = parameters.get("walletPass").toString();
-        Map<String, Object> targets = (Map<String, Object>)parameters.get("recipients");
-        HashMap<String, BigDecimal> recipients = new HashMap<String, BigDecimal>(targets.size());
-        for (String address : targets.keySet()) {
-            recipients.put(address, new BigDecimal(targets.get(address).toString()));
+    public Optional<SendCoinsResponse> sendMany(@NonNull String coin, @NonNull String walletId, @NonNull String walletPass,
+                                                @NonNull Map<String, BigDecimal> recipients, @NonNull String sequenceId,
+                                                Map<String, Object> optionalParameters) throws IOException {
+        // Validate all needed parameters
+        if (!coin.matches("^(btc|ltc|bch|tbtc|tltc|tbch)$")) {
+            throw new IOException("Invalid currency");
+        }
+        if (walletId.equals("")) {
+            throw new IOException("WalletId can't be empty");
+        }
+        if (walletPass.equals("")) {
+            throw new IOException("WalletPass can't be empty ");
+        }
+        if (sequenceId.equals("")) {
+            throw new IOException("SequenceId can't be empty");
+        }
+        if (recipients.size() == 0) {
+            throw new IOException("Transaction must have at least one address/amount tuple as destination");
         }
-        String sequenceId = parameters.get("sequenceId").toString();
-
-        // Check for optional parameters or default them to null
-        String message = parameters.containsKey("message") ? parameters.get("message").toString() : null;
-        BigDecimal fee = parameters.containsKey("fee") ? new BigDecimal(parameters.get("fee").toString()) : null;
-        BigDecimal feeTxConfirmTarget = parameters.containsKey("feeTxConfirmTarget") ?
-                new BigDecimal(parameters.get("feeTxConfirmTarget").toString()) : null;
-        int minConfirms = parameters.containsKey("minConfirms") ? Integer.parseInt(parameters.get("minConfirms").toString()) : null;
-        boolean enforceMinConfirmsForChange = parameters.containsKey("enforceMinConfirmsForChange") ?
-                Boolean.valueOf(parameters.get("enforceMinConfirmsForChange").toString()) : null;
 
         String url = baseUrl + SEND_MANY_URL.replace("$COIN", coin).replace("$WALLET", walletId);
         final List<Map<String, Object>> addr = new ArrayList<>(recipients.size());
@@ -135,25 +135,56 @@ public Optional<SendCoinsResponse> sendMany(Map<String, Object> parameters) thro
         }
         final Map<String, Object> data = new HashMap<>();
         data.put("recipients", addr);
-        if (message != null) {
-            data.put("message", message);
-        }
-        if (sequenceId != null) {
-            data.put("sequenceId", sequenceId);
-        }
-        if (fee != null) {
-            data.put("fee", fee.longValue());
-        }
-        if (feeTxConfirmTarget != null) {
-            data.put("feeTxConfirmTarget", feeTxConfirmTarget);
-        }
-        if (minConfirms > 0) {
-            data.put("minConfirms", minConfirms);
+        data.put("walletPassphrase", walletPass);
+        data.put("sequenceId", sequenceId);
+
+        // Check for optional parameters or default them to null
+        if (optionalParameters != null) {
+            Object message = optionalParameters.containsKey("message") ? optionalParameters.get("message") : null;
+            Object fee = optionalParameters.containsKey("fee") ? optionalParameters.get("fee") : null;
+            Object feeTxConfirmTarget = optionalParameters.containsKey("feeTxConfirmTarget") ?
+                    optionalParameters.get("feeTxConfirmTarget") : null;
+            Object minConfirms = optionalParameters.containsKey("minConfirms") ? optionalParameters.get("minConfirms") : null;
+            Object enforceMinConfirmsForChange = optionalParameters.containsKey("enforceMinConfirmsForChange") ?
+                    optionalParameters.get("enforceMinConfirmsForChange") : null;
+            if (message != null) {
+                if (message.getClass().getName().equals(String.class.getName())) {
+                    data.put("message", message.toString());
+                } else {
+                    throw new IOException("Message should be a String value");
+                }
+            }
+            if (fee != null) {
+                if (fee.getClass().getName().equals(BigDecimal.class.getName())) {
+                    data.put("fee", ((BigDecimal) fee).longValue());
+                } else {
+                    throw new IOException("Fee should be a BigDecimal value");
+                }
+            }
+            if (feeTxConfirmTarget != null) {
+                if (feeTxConfirmTarget.getClass().getName().equals(BigDecimal.class.getName())) {
+                    data.put("feeTxConfirmTarget", feeTxConfirmTarget);
+                } else {
+                    throw new IOException("FeeTxConfirmTarget should be a BigDecimal value");
+                }
+            }
+            if (minConfirms != null) {
+                if (minConfirms.getClass().getName().equals(Integer.class.getName()) && ((int) minConfirms > 0)) {
+                    data.put("minConfirms", minConfirms);
+                } else {
+                    throw new IOException("MinConfirms should be an Integer value higher than 0");
+                }
+            }
+            if (enforceMinConfirmsForChange != null) {
+                if (enforceMinConfirmsForChange.getClass().getName().equals(Boolean.class.getName())) {
+                    data.put("enforceMinConfirmsForChange", enforceMinConfirmsForChange);
+                } else {
+                    throw new IOException("EnforceMinConfirmsForChange should be a Boolean value");
+                }
+            }
         }
-        data.put("enforceMinConfirmsForChange", enforceMinConfirmsForChange);
 
         log.info("sendMany {}", data);
-        data.put("walletPassphrase", walletPass);
         HttpURLConnection conn = httpPost(url, data);
         if (conn == null) {
             return Optional.empty();
diff --git a/bitgo-java-api/src/test/java/com/bitso/bitgo/util/TestClientV2.java b/bitgo-java-api/src/test/java/com/bitso/bitgo/util/TestClientV2.java
@@ -6,7 +6,6 @@
 import com.bitso.bitgo.v2.entity.Wallet;
 import com.bitso.bitgo.v2.entity.WalletTransferResponse;
 import lombok.extern.slf4j.Slf4j;
-import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -53,15 +52,11 @@ public void testSendMany() throws IOException {
             targets.put(TLTC_TEST_FAUCET_ADDRESS, amount);
             amount = amount.add(initAmount);
         }
-        parameters.put("coin", COIN);
-        parameters.put("walletId", WALLET_ID);
-        parameters.put("walletPass", WALLET_PASSPHRASE);
-        parameters.put("recipients", targets);
         parameters.put("message", "test");
         parameters.put("minConfirms", 1);
         parameters.put("enforceMinConfirmsForChange", true);
-        parameters.put("sequenceId", ""); //Set it up for transaction to work
-        Optional<SendCoinsResponse> resp = client.sendMany(parameters);
+        String sequenceId = "btc35"; //Set it up for transaction to work
+        Optional<SendCoinsResponse> resp = client.sendMany(COIN, WALLET_ID, WALLET_PASSPHRASE, targets, sequenceId, parameters);
         Assert.assertTrue(resp.isPresent());
         Assert.assertNotNull(resp.get().getTx());
 
@@ -70,32 +65,40 @@ public void testSendMany() throws IOException {
     @Test
     @Ignore
     public void testSendManyFail() throws IOException {
-        Map<String, Object> parameters = new HashMap<String, Object>();
         int unlockResult = client.unlock("0000000", TimeUnit.HOURS.toSeconds(1));
         System.out.println(unlockResult);
         assertTrue(unlockResult != 400 && unlockResult != 401);
         Map<String, BigDecimal> targets = new HashMap<>();
         BigDecimal initAmount = new BigDecimal("0.001").movePointRight(8);
         BigDecimal amount = initAmount;
         targets.put(TLTC_TEST_FAUCET_ADDRESS, amount);
-        // avoid coin required parameter to test error handling
-        parameters.put("coin", "");
-        parameters.put("walletId", WALLET_ID);
-        parameters.put("walletPass", WALLET_PASSPHRASE);
-        parameters.put("recipients", targets);
-        parameters.put("message", "test");
-        parameters.put("minConfirms", 1);
-        parameters.put("enforceMinConfirmsForChange", true);
-        parameters.put("sequenceId", "btc34"); //Set it up for transaction to work
-        Optional<SendCoinsResponse> resp = client.sendMany(parameters);
-        // Coin is part of the URL of api, should not find the required page (also walletId is part of the URL)
-        Assert.assertEquals(404, resp.get().getResponseCode());
-        // Restart test swapping the wrong variable
-        parameters.put("coin", COIN);
-        parameters.put("sequenceId", "");
-        resp = client.sendMany(parameters);
-        Assert.assertEquals(400, resp.get().getResponseCode());
-        Assert.assertThat(resp.get().getError(), CoreMatchers.containsString("invalid sequence id"));
+        String sequenceId = "123";
+        // Send required COIN parameter as empty
+        try {
+            client.sendMany("", WALLET_ID, WALLET_PASSPHRASE, targets, sequenceId, null);
+        } catch (Exception e) {
+            Assert.assertEquals("Invalid currency", e.getMessage());
+        }
+        // Send COIN  parameter  misformed
+        try {
+            client.sendMany("abc", WALLET_ID, WALLET_PASSPHRASE, targets, sequenceId, null);
+        } catch (Exception e) {
+            Assert.assertEquals("Invalid currency", e.getMessage());
+        }
+        // Send required COIN parameter as null
+        try {
+            client.sendMany(null, WALLET_ID, WALLET_PASSPHRASE, targets, sequenceId, null);
+        } catch (Exception e) {
+            Assert.assertEquals("coin is marked @NonNull but is null", e.getMessage());
+        }
+        // Send an optional parameter with a wrong type
+        try {
+            Map<String, Object> parameters = new HashMap<String, Object>();
+            parameters.put("fee", "0");
+            client.sendMany(COIN, WALLET_ID, WALLET_PASSPHRASE, targets, sequenceId, parameters);
+        } catch (Exception e) {
+            Assert.assertEquals("Fee should be a BigDecimal value", e.getMessage());
+        }
     }
 
     @Test
