PgClient Money does not preserve sign if integral part is zero (#1362)

Fixes #1360

Signed-off-by: Thomas Segismont <[email protected]>

Author: tsegismont

vertx-pg-client/src/main/java/io/vertx/pgclient/data/Money.java

@@ -11,92 +11,79 @@
 package io.vertx.pgclient.data;
 
 import java.math.BigDecimal;
-import java.math.BigInteger;
+import java.text.DecimalFormat;
+import java.util.Objects;
 
 /**
  * The PostgreSQL <a href="https://www.postgresql.org/docs/9.1/datatype-money.html">MONEY</> type.
- *
- * This has the {@link #getIntegerPart() integer part} and {@link #getDecimalPart() decimal part} of the value without loss of information.
- *
- * {@link #bigDecimalValue()} returns the value without loss of information
- * {@link #doubleValue()} ()} returns the value possible loss of information
+ * <p>
+ * {@link #bigDecimalValue()} returns the value without loss of information.
+ * {@link #doubleValue()} returns the value possible loss of information.
  */
 public class Money {
 
-  private long integerPart;
-  private int decimalPart;
+  private BigDecimal value;
 
+  /**
+   * @deprecated as of 4.5, use {@link #Money(Number)} instead
+   */
+  @Deprecated
   public Money(long integerPart, int decimalPart) {
-    setIntegerPart(integerPart);
-    setDecimalPart(decimalPart);
+    this(new BigDecimal(integerPart + "." + new DecimalFormat("00").format(decimalPart)));
   }
 
   public Money(Number value) {
-    if (value instanceof Double || value instanceof Float) {
-      value = BigDecimal.valueOf((double) value);
+    this.value = (value instanceof BigDecimal ? (BigDecimal) value : new BigDecimal(String.valueOf(value))).stripTrailingZeros();
+    if (this.value.toBigInteger().abs().longValue() > Long.MAX_VALUE / 100) {
+      throw new IllegalArgumentException("Value is too big: " + value);
     }
-    if (value instanceof BigDecimal) {
-      BigInteger bd = ((BigDecimal) value).multiply(new BigDecimal(100)).toBigInteger();
-      setIntegerPart(bd.divide(BigInteger.valueOf(100)).longValueExact());
-      setDecimalPart(bd.remainder(BigInteger.valueOf(100)).abs().intValueExact());
-    } else {
-      setIntegerPart(value.longValue());
+    if (this.value.scale() > 2) {
+      throw new IllegalArgumentException("Value has more than two decimal digits: " + value);
     }
   }
 
   public Money() {
+    value = BigDecimal.ZERO;
   }
 
+  /**
+   * @deprecated as of 4.5, use {@link #bigDecimalValue()} instead
+   */
+  @Deprecated
   public long getIntegerPart() {
-    return integerPart;
+    return value.toBigInteger().longValue();
   }
 
+  /**
+   * @deprecated as of 4.5, use {@link #bigDecimalValue()} instead
+   */
+  @Deprecated
   public int getDecimalPart() {
-    return decimalPart;
+    return value.remainder(BigDecimal.ONE).movePointRight(value.scale()).abs().intValue();
   }
 
   /**
-   * Set the integer part of the monetary value.
-   *
-   * <p> This value must belong to the range {@code ]Long.MAX_VALUE / 100, Long.MIN_VALUE / 100[}
-   *
-   * @param part the integer part of the value
-   * @return this object
+   * @deprecated as of 4.5, create another instance instead
    */
+  @Deprecated
   public Money setIntegerPart(long part) {
-    if (part > Long.MAX_VALUE / 100 || part < Long.MIN_VALUE / 100) {
-      throw new IllegalArgumentException();
-    }
-    integerPart = part;
+    value = new Money(part, value.remainder(BigDecimal.ONE).abs().intValue()).bigDecimalValue();
     return this;
   }
 
   /**
-   * Set the decimal part of the monetary value.
-   *
-   * <p> This value must belong to the range {@code [0, 100]}
-   *
-   * @param part decimal part
-   * @return this object
+   * @deprecated as of 4.5, create another instance instead
    */
+  @Deprecated
   public Money setDecimalPart(int part) {
-    if (part > 99 || part < 0) {
-      throw new IllegalArgumentException();
-    }
-    decimalPart = part;
+    value = new Money(value.longValue(), part).bigDecimalValue();
     return this;
   }
 
   /**
    * @return the monetary amount as a big decimal without loss of information
    */
   public BigDecimal bigDecimalValue() {
-    BigDecimal value = new BigDecimal(integerPart).multiply(BigDecimal.valueOf(100));
-    if (integerPart >= 0) {
-      value = value.add(BigDecimal.valueOf(decimalPart));
-    } else {
-      value = value.subtract(BigDecimal.valueOf(decimalPart));
-    }
     return value;
   }
 
@@ -111,17 +98,17 @@ public double doubleValue() {
   public boolean equals(Object o) {
     if (this == o) return true;
     if (o == null || getClass() != o.getClass()) return false;
-    Money that = (Money) o;
-    return decimalPart == that.decimalPart && integerPart == that.integerPart;
+    Money money = (Money) o;
+    return Objects.equals(value, money.value);
   }
 
   @Override
   public int hashCode() {
-    return ((Long)integerPart).hashCode() ^ ((Integer)decimalPart).hashCode();
+    return Objects.hash(value);
   }
 
   @Override
   public String toString() {
-    return "Money(" + integerPart + "." + decimalPart + ")";
+    return "Money(" + new DecimalFormat("#0.##").format(value) + ")";
   }
 }

vertx-pg-client/src/main/java/io/vertx/pgclient/impl/codec/DataTypeCodec.java

@@ -33,6 +33,7 @@
 import io.vertx.sqlclient.data.Numeric;
 import io.vertx.sqlclient.impl.codec.CommonCodec;
 
+import java.math.BigDecimal;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -1455,19 +1456,12 @@ private static void binaryEncodeInet(Inet value, ByteBuf buff) {
   }
 
   private static void binaryEncodeMoney(Money money, ByteBuf buff) {
-    long integerPart = money.getIntegerPart();
-    long value;
-    if (integerPart >= 0) {
-      value = money.getIntegerPart() * 100 + money.getDecimalPart();
-    } else {
-      value = money.getIntegerPart() * 100 - money.getDecimalPart();
-    }
-    binaryEncodeINT8(value, buff);
+    binaryEncodeINT8(money.bigDecimalValue().movePointRight(2).longValue(), buff);
   }
 
   private static Money binaryDecodeMoney(int index, int len, ByteBuf buff) {
     long value = binaryDecodeINT8(index, len, buff);
-    return new Money(value / 100, Math.abs(((int)value % 100)));
+    return new Money(BigDecimal.valueOf(value, 2));
   }
 
   private static String binaryDecodeTsQuery(int index, int len, ByteBuf buff) {

vertx-pg-client/src/test/java/io/vertx/pgclient/data/MoneyTest.java

@@ -0,0 +1,58 @@
+package io.vertx.pgclient.data;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+import java.math.BigDecimal;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(Parameterized.class)
+public class MoneyTest {
+
+  @Parameters
+  public static Object[][] data() {
+    return new Object[][]{{"-1234.56", -1234L, 56}, {"-0.12", 0L, 12}, {"0.12", 0L, 12}, {"1234.56", 1234L, 56}};
+  }
+
+  @Parameter
+  public String strValue;
+  @Parameter(1)
+  public long integralPart;
+  @Parameter(2)
+  public int fractionalPart;
+  private BigDecimal value;
+  private Money money;
+
+  @Before
+  public void setUp() {
+    value = new BigDecimal(strValue);
+    money = new Money(value);
+  }
+
+  @Test
+  public void testBigDecimalValue() {
+    assertEquals(value, money.bigDecimalValue());
+  }
+
+  @Test
+  public void testIntegerPart() {
+    assertEquals(integralPart, money.getIntegerPart());
+  }
+
+  @Test
+  public void testDecimalPart() {
+    assertEquals(fractionalPart, money.getDecimalPart());
+  }
+
+  @Test
+  @SuppressWarnings("deprecation")
+  public void setParts() {
+    assertEquals(new Money(3.24D), money.setIntegerPart(3).setDecimalPart(24));
+    assertEquals(new Money(-173.01D), money.setIntegerPart(-173).setDecimalPart(1));
+  }
+}