Fix #17, now BigInteger, BigDecimal round-trip correctly

Author: cowtowncoder

cbor/release-notes/VERSION

@@ -6,6 +6,7 @@ Project: jackson-dataformat-cbor
 
 2.8.0 (not yet released)
 
+#17: Support parsing of `BigInteger`, `BigDecimal`, not just generating
 #18: Fail to report error for trying to write field name outside Object (root level)
 
 2.7.3 (16-Mar-2016)

cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java

@@ -780,7 +780,13 @@ public void writeNumber(BigInteger v) throws IOException
             return;
         }
         _verifyValueWrite("write number");
-        
+        _write(v);
+    }
+
+    // Main write method isolated so that it can be called directly
+    // in cases where that is needed (to encode BigDecimal)
+    protected void _write(BigInteger v) throws IOException
+    {
         /* Supported by using type tags, as per spec: major type for tag '6';
          * 5 LSB either 2 for positive bignum or 3 for negative bignum.
          * And then byte sequence that encode variable length integer.
@@ -796,7 +802,7 @@ public void writeNumber(BigInteger v) throws IOException
         _writeLengthMarker(PREFIX_TYPE_BYTES, len);
         _writeBytes(data, 0, len);
     }
-    
+
     @Override
     public void writeNumber(double d) throws IOException
     {
@@ -852,36 +858,25 @@ public void writeNumber(BigDecimal dec) throws IOException
         _verifyValueWrite("write number");
         /* Supported by using type tags, as per spec: major type for tag '6';
          * 5 LSB 4.
-         * And then a two-int array, with mantissa and exponent
+         * And then a two-element array; integer exponent, and int/bigint mantissa
          */
-        _writeByte(BYTE_TAG_BIGFLOAT);
+        // 12-May-2016, tatu: Before 2.8, used "bigfloat", but that was incorrect...
+        _writeByte(BYTE_TAG_DECIMAL_FRACTION);
         _writeByte(BYTE_ARRAY_2_ELEMENTS);
 
         int scale = dec.scale();
         _writeIntValue(scale);
-
         /* Hmmmh. Specification suggest use of regular integer for mantissa.
-         * But... it may or may not fit. Let's try to do that, if it works;
-         * if not, use byte array.
+         * But if it doesn't fit, use "bignum" 
          */
         BigInteger unscaled = dec.unscaledValue();
-        byte[] data = unscaled.toByteArray();
-        if (data.length <= 4) {
-            int v = data[0]; // let it be sign extended on purpose
-            for (int i = 1; i < data.length; ++i) {
-                v = (v << 8) + (data[i] & 0xFF);
-            }
-            _writeIntValue(v);
-        } else if (data.length <= 8) {
-            long v = data[0]; // let it be sign extended on purpose
-            for (int i = 1; i < data.length; ++i) {
-                v = (v << 8) + (data[i] & 0xFF);
-            }
-            _writeLongValue(v);
+        int bitLength = unscaled.bitLength();
+        if (bitLength <= 31) {
+            _writeIntValue(unscaled.intValue());
+        } else if (bitLength <= 63) {
+            _writeLongValue(unscaled.longValue());
         } else {
-            final int len = data.length;
-            _writeLengthMarker(PREFIX_TYPE_BYTES, len);
-            _writeBytes(data, 0, len);
+            _write(unscaled);
         }
     }
 
@@ -1307,8 +1302,7 @@ private final void _writeIntValue(int i) throws IOException
     {
         int marker;
         if (i < 0) {
-            i += 1;
-            i = -1;
+            i = -i - 1;
             marker = PREFIX_TYPE_INT_NEG;
         } else {
             marker = PREFIX_TYPE_INT_POS;
@@ -1321,7 +1315,7 @@ private final void _writeLongValue(long l) throws IOException
         _ensureRoomForOutput(9);
         if (l < 0) {
             l += 1;
-            l = -1;
+            l = -l;
             _outputBuffer[_outputTail++] = (PREFIX_TYPE_INT_NEG + 27);
         } else {
             _outputBuffer[_outputTail++] = (PREFIX_TYPE_INT_POS + 27);

cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java

@@ -15,6 +15,8 @@
 import com.fasterxml.jackson.core.util.ByteArrayBuilder;
 import com.fasterxml.jackson.core.util.TextBuffer;
 
+import static com.fasterxml.jackson.dataformat.cbor.CBORConstants.*;
+
 public final class CBORParser extends ParserMinimalBase
 {
     private final static byte[] NO_BYTES = new byte[0];
@@ -713,7 +715,7 @@ public JsonToken nextToken() throws IOException
             _typeByte = ch;
             _tokenIncomplete = true;
             if (_tagValue >= 0) {
-                return _nextBinaryWithTag(_tagValue);
+                return _handleTaggedBinary(_tagValue);
             }
             return (_currToken = JsonToken.VALUE_EMBEDDED_OBJECT);
 
@@ -723,12 +725,14 @@ public JsonToken nextToken() throws IOException
             return (_currToken = JsonToken.VALUE_STRING);
 
         case 4: // Array
-            _currToken = JsonToken.START_ARRAY;
             {
                 int len = _decodeExplicitLength(lowBits);
+                if (_tagValue >= 0) {
+                    return _handleTaggedArray(_tagValue, len);
+                }
                 _parsingContext = _parsingContext.createChildArrayContext(len);
             }
-            return _currToken;
+            return (_currToken = JsonToken.START_ARRAY);
 
         case 5: // Object
             _currToken = JsonToken.START_OBJECT;
@@ -816,14 +820,13 @@ protected String _numberToName(int ch, boolean neg) throws IOException
         return String.valueOf(1);
     }
 
-    protected JsonToken _nextBinaryWithTag(int tag) throws IOException
+    protected JsonToken _handleTaggedBinary(int tag) throws IOException
     {
         // For now all we should get is BigInteger
-        
         boolean neg;
-        if (tag == CBORConstants.TAG_BIGNUM_POS) {
+        if (tag == TAG_BIGNUM_POS) {
             neg = false;
-        } else  if (tag == CBORConstants.TAG_BIGNUM_NEG) {
+        } else  if (tag == TAG_BIGNUM_NEG) {
             neg = true;
         } else {
             // 12-May-2016, tatu: Since that's all we know, let's otherwise
@@ -834,12 +837,68 @@ protected JsonToken _nextBinaryWithTag(int tag) throws IOException
         // First: get the data
         _finishToken();
 
-        _numberBigInt = new BigInteger(_binaryValue);
-        _numTypesValid |= NR_BIGINT;
-        
+        BigInteger nr = new BigInteger(_binaryValue);
+        if (neg) {
+            nr = nr.negate();
+        }
+        _numberBigInt = nr;
+        _numTypesValid = NR_BIGINT;
         return (_currToken = JsonToken.VALUE_NUMBER_INT);
     }
 
+    protected JsonToken _handleTaggedArray(int tag, int len) throws IOException
+    {
+        // For simplicity, let's create matching array context -- in perfect
+        // world that wouldn't be necessarily, but in this one there are
+        // some constraints that make it necessary
+        _parsingContext = _parsingContext.createChildArrayContext(len);
+
+        // BigDecimal is the only thing we know for sure
+        if (tag != CBORConstants.TAG_DECIMAL_FRACTION) {
+            return (_currToken = JsonToken.START_ARRAY);
+        }
+        _currToken = JsonToken.START_ARRAY;
+
+        // but has to have length of 2; otherwise we have a problem...
+        if (len != 2) {
+            _reportError("Unexpected array size ("+len+") for tagged 'bigfloat' value; should have exactly 2 number elements");
+        }
+        // and then use recursion to get values
+        JsonToken t = nextToken();
+        // First: exponent, which MUST be a simple integer value
+        if (t != JsonToken.VALUE_NUMBER_INT) {
+            _reportError("Unexpected token ("+t+") as the first part of 'bigfloat' value: should get VALUE_NUMBER_INT");
+        }
+        int exp = getIntValue();
+
+        t = nextToken();
+        // Should get an integer value; int/long/BigInteger
+        if (t != JsonToken.VALUE_NUMBER_INT) {
+            _reportError("Unexpected token ("+t+") as the second part of 'bigfloat' value: should get VALUE_NUMBER_INT");
+        }
+        
+        BigDecimal dec;
+
+        switch (getNumberType()) {
+        case INT:
+        case LONG:
+            dec = BigDecimal.valueOf(getLongValue(), exp);
+            break;
+        case BIG_INTEGER:
+        default:
+            dec = new BigDecimal(getBigIntegerValue(), exp);
+            break;
+        }
+
+        t = nextToken();
+        if (t != JsonToken.END_ARRAY) {
+            _reportError("Unexpected token ("+t+") after 2 elements of 'bigfloat' value");
+        }
+        _numberBigDecimal = dec;
+        _numTypesValid = NR_BIGDECIMAL;
+        return (_currToken = JsonToken.VALUE_NUMBER_FLOAT);
+    }
+    
     // base impl is fine:
     //public String getCurrentName() throws IOException
 

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/BigNumbersTest.java

@@ -0,0 +1,84 @@
+package com.fasterxml.jackson.dataformat.cbor;
+
+import java.io.ByteArrayOutputStream;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.dataformat.cbor.*;
+
+// tests for [cbor#17]
+public class BigNumbersTest extends CBORTestBase
+{
+    public void testBigDecimal() throws Exception
+    {
+        _testBigDecimal(BigDecimal.ONE);
+        _testBigDecimal(BigDecimal.ZERO);
+        _testBigDecimal(BigDecimal.TEN);
+        _testBigDecimal(BigDecimal.ONE.scaleByPowerOfTen(-1));
+        _testBigDecimal(BigDecimal.ONE.scaleByPowerOfTen(-3));
+        _testBigDecimal(BigDecimal.ONE.scaleByPowerOfTen(-100));
+        _testBigDecimal(BigDecimal.ONE.scaleByPowerOfTen(3));
+        _testBigDecimal(BigDecimal.ONE.scaleByPowerOfTen(137));
+
+        _testBigDecimal(new BigDecimal("0.01"));
+        _testBigDecimal(new BigDecimal("0.33"));
+        _testBigDecimal(new BigDecimal("1.1"));
+        _testBigDecimal(new BigDecimal("900.373"));
+
+        BigDecimal bd = new BigDecimal("12345.667899024");
+        _testBigDecimal(bd);
+        _testBigDecimal(bd.negate());
+
+        // ensure mantissa is beyond long; more than 22 digits or so
+        bd = new BigDecimal("1234567890.12345678901234567890");
+        _testBigDecimal(bd);
+        _testBigDecimal(bd.negate());
+    }
+
+    private void _testBigDecimal(BigDecimal expValue) throws Exception
+    {
+        final ByteArrayOutputStream sourceBytes = new ByteArrayOutputStream();
+        final CBORGenerator sourceGen = cborGenerator(sourceBytes);
+        sourceGen.writeStartObject();
+        sourceGen.writeFieldName("a");
+        sourceGen.writeNumber(expValue);
+        sourceGen.writeEndObject();
+        sourceGen.close();
+
+        byte[] b = sourceBytes.toByteArray();
+        
+        // but verify that the original content can be parsed
+        CBORParser parser = cborParser(b);
+        assertToken(JsonToken.START_OBJECT, parser.nextToken());
+        assertToken(JsonToken.FIELD_NAME, parser.nextToken());
+        assertEquals("a", parser.getCurrentName());
+        assertToken(JsonToken.VALUE_NUMBER_FLOAT, parser.nextToken());
+        assertEquals(expValue, parser.getDecimalValue());
+        assertToken(JsonToken.END_OBJECT, parser.nextToken());
+        parser.close();
+    }
+
+    public void testBigInteger() throws Exception
+    {
+        _testBigInteger(BigInteger.TEN);
+        _testBigInteger(BigInteger.TEN.negate());
+    }
+
+    private void _testBigInteger(BigInteger expValue) throws Exception
+    {
+        final ByteArrayOutputStream sourceBytes = new ByteArrayOutputStream();
+        final CBORGenerator sourceGen = cborGenerator(sourceBytes);
+        sourceGen.writeNumber(expValue);
+        sourceGen.close();
+
+        // but verify that the original content can be parsed
+        CBORParser parser = cborParser(sourceBytes.toByteArray());
+        assertToken(JsonToken.VALUE_NUMBER_INT, parser.nextToken());
+        assertEquals(expValue, parser.getBigIntegerValue());
+        assertNull(parser.nextToken());
+        parser.close();
+
+        // but wait... there's more. Negative variants
+    }
+}

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/GeneratorSimpleTest.java

@@ -383,11 +383,10 @@ public void testCopyCurrentEventWithTag() throws Exception {
         // copyCurrentEvent doesn't preserve fixed arrays, so we can't
         // compare with the source bytes.
         Assert.assertArrayEquals(new byte[] {
-                CBORConstants.BYTE_TAG_BIGFLOAT,
-                CBORConstants.BYTE_ARRAY_INDEFINITE,
+                CBORConstants.BYTE_TAG_DECIMAL_FRACTION,
+                CBORConstants.BYTE_ARRAY_2_ELEMENTS,
                 0,
                 1,
-                CBORConstants.BYTE_BREAK
             },
             targetBytes.toByteArray());
     }
@@ -409,11 +408,10 @@ public void testCopyCurrentSturctureWithTag() throws Exception {
         // copyCurrentEvent doesn't preserve fixed arrays, so we can't
         // compare with the source bytes.
         Assert.assertArrayEquals(new byte[] {
-                CBORConstants.BYTE_TAG_BIGFLOAT,
-                CBORConstants.BYTE_ARRAY_INDEFINITE,
+                CBORConstants.BYTE_TAG_DECIMAL_FRACTION,
+                CBORConstants.BYTE_ARRAY_2_ELEMENTS,
                 0,
                 1,
-                CBORConstants.BYTE_BREAK
             },
             targetBytes.toByteArray());
     }