Fixed: crash when restoring certain backups with poorly formatted amount strings

Added some GncXmlHelper tests

Author: codinguser

CHANGELOG.md

@@ -1,5 +1,9 @@
 Change Log
 ===============================================================================
+Version 1.6.4 *(2015-08-xx)*
+----------------------------
+* Fixed: Isolated crash during backup restoration
+
 Version 1.6.3 *(2015-08-09)*
 ----------------------------
 * Fixed: Transfer account ignored when saving transaction with one split (after opening split editor)

app/build.gradle

@@ -5,7 +5,7 @@ apply plugin: 'crashlytics'
 
 def versionMajor = 1
 def versionMinor = 6
-def versionPatch = 3
+def versionPatch = 4
 def versionBuild = 0
 
 def buildTime() {

app/src/main/java/org/gnucash/android/export/xml/GncXmlHelper.java

@@ -19,9 +19,7 @@
 
 import android.support.annotation.NonNull;
 
-import org.gnucash.android.model.Money;
-import org.gnucash.android.model.Split;
-import org.gnucash.android.model.TransactionType;
+import org.gnucash.android.ui.transaction.TransactionFormFragment;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -162,41 +160,8 @@ public static long parseDate(String dateString) throws ParseException {
     }
 
     /**
-     * Parses the amounts in template transaction splits.
-     * <p>GnuCash desktop formats the amounts based on the locale of the computer. That poses a problem here as the input can vary per user.<br/>
-     *     The solution is to parse the string irrespective of comma or thousands separators as follows:
-     *     <ol>
-     *         <li>Find the last non-numeric character and split the string at that point</li>
-     *         <li>If the length of the 2nd part is &gt;= 2, then it is a thousands separator, else it is decimal</li>
-     *         <li>Append the two parts again together accordingly</li>
-     *         <li>If no non-numeric character was found, then just return a new {@link BigDecimal}</li>
-     *     </ol>
-     * </p>
-     * @param amountString String value of the amount.
-     * @return BigDecimal representation of the amount
-     * @see #formatTemplateSplitAmount(BigDecimal)
-     */
-    public static BigDecimal parseTemplateSplitAmount(@NonNull String amountString){
-        Pattern pattern = Pattern.compile(".*\\D");
-        Matcher matcher = pattern.matcher(amountString);
-        if (matcher.find()){
-            int index = matcher.end();
-            String wholeNum = amountString.substring(0, index).replaceAll("\\D", "");
-            String decimal = amountString.substring(index);
-            String parsedAmountString;
-            if (decimal.length() > 2){ //then it is just another thousands separator, just add it back
-                parsedAmountString = wholeNum + decimal;
-            } else { //add it as a decimal
-                parsedAmountString = wholeNum + "." + decimal;
-            }
-            return new BigDecimal(parsedAmountString);
-        } else {//an amount string with no commas or periods
-            return new BigDecimal(amountString);
-        }
-    }
-
-    /**
-     * Parses amount strings from GnuCash XML into {@link java.math.BigDecimal}s
+     * Parses amount strings from GnuCash XML into {@link java.math.BigDecimal}s.
+     * The amounts are formatted as 12345/4100
      * @param amountString String containing the amount
      * @return BigDecimal with numerical value
      * @throws ParseException if the amount could not be parsed
@@ -207,9 +172,11 @@ public static BigDecimal parseSplitAmount(String amountString) throws ParseExcep
         {
             throw new ParseException("Cannot parse money string : " + amountString, 0);
         }
-        BigInteger numerator = new BigInteger(amountString.substring(0, pos));
-        int scale = amountString.length() - pos - 2;
-        return new BigDecimal(numerator, scale);
+
+        int scale = amountString.length() - pos - 2; //do this before, because we could modify the string
+        String numerator = TransactionFormFragment.stripCurrencyFormatting(amountString.substring(0, pos));
+        BigInteger numeratorInt = new BigInteger(numerator);
+        return new BigDecimal(numeratorInt, scale);
     }
 
     /**
@@ -224,7 +191,8 @@ public static String formatSplitAmount(BigDecimal amount, Currency trxCurrency){
         BigDecimal denom = new BigDecimal(denomInt);
         String denomString = Integer.toString(denomInt);
 
-        return amount.multiply(denom).stripTrailingZeros().toPlainString() + "/" + denomString;
+        String numerator = TransactionFormFragment.stripCurrencyFormatting(amount.multiply(denom).stripTrailingZeros().toPlainString());
+        return numerator + "/" + denomString;
     }
 
     /**

app/src/test/java/org/gnucash/android/test/unit/export/GncXmlHelperTest.java

@@ -0,0 +1,46 @@
+package org.gnucash.android.test.unit.export;
+
+import org.gnucash.android.export.xml.GncXmlHelper;
+import org.junit.Test;
+
+import java.math.BigDecimal;
+import java.text.ParseException;
+import java.util.Currency;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test the helper methods used for generating GnuCash XML
+ */
+public class GncXmlHelperTest {
+
+    /**
+     * Tests the parsing of split amounts
+     */
+    @Test
+    public void testParseSplitAmount() throws ParseException {
+        String splitAmount = "12345/100";
+        BigDecimal amount = GncXmlHelper.parseSplitAmount(splitAmount);
+        assertThat(amount.toPlainString()).isEqualTo("123.45");
+
+        amount = GncXmlHelper.parseSplitAmount("1.234,50/100");
+        assertThat(amount.toPlainString()).isEqualTo("1234.50");
+    }
+
+    @Test(expected = ParseException.class)
+    public void shouldFailToParseWronglyFormattedInput() throws ParseException {
+        GncXmlHelper.parseSplitAmount("123.45");
+    }
+
+    @Test
+    public void testFormatSplitAmount(){
+        BigDecimal bigDecimal = new BigDecimal("45.90");
+        String amount = GncXmlHelper.formatSplitAmount(bigDecimal, Currency.getInstance("USD"));
+        assertThat(amount).isEqualTo("4590/100");
+
+
+        bigDecimal = new BigDecimal("350");
+        amount = GncXmlHelper.formatSplitAmount(bigDecimal, Currency.getInstance("EUR"));
+        assertThat(amount).isEqualTo("35000/100");
+    }
+}