Fixed #54 (manual merge of patch)

Author: cowtowncoder

protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ByteAccumulator.java

@@ -30,19 +30,25 @@ public class ByteAccumulator
 
     protected int _segmentBytes;
 
+    /**
+     * Used to cache start pointer for nested message's parent
+     * 
+     * @since 2.8.8
+     */
+    protected int _parentStart;
+
     public ByteAccumulator(ByteAccumulator p, int typedTag,
-            byte[] prefixBuffer, int prefixOffset)
+            byte[] prefixBuffer, int prefixOffset, int parentStart)
     {
-        _parent = p;
-        _typedTag = typedTag;
-        _prefixBuffer = prefixBuffer;
-        _prefixOffset = prefixOffset;
+        this(p, typedTag, prefixBuffer, prefixOffset);
+        _parentStart = parentStart;
     }
 
-    public ByteAccumulator(ByteAccumulator p,
-            byte[] prefixBuffer, int prefixOffset) {
+    public ByteAccumulator(ByteAccumulator p, int typedTag,
+            byte[] prefixBuffer, int prefixOffset)
+    {
         _parent = p;
-        _typedTag = -1;
+        _typedTag = typedTag;
         _prefixBuffer = prefixBuffer;
         _prefixOffset = prefixOffset;
     }

protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java

@@ -1709,8 +1709,8 @@ private final void _startBuffering(int typedTag) throws IOException
                 _output.write(_currBuffer, start, len);
             }
         }
+        _buffered = new ByteAccumulator(_buffered, typedTag, _currBuffer, ptr, _currStart);
         _currStart = _currPtr = ptr + 10;
-        _buffered = new ByteAccumulator(_buffered, typedTag, _currBuffer, ptr);
     }
 
     /**
@@ -1734,24 +1734,28 @@ private final void _startBuffering() throws IOException
                 _output.write(_currBuffer, _currStart, len);
             }
         }
-        
+        _buffered = new ByteAccumulator(_buffered, -1, _currBuffer, ptr, _currStart);
         _currStart = _currPtr = ptr + 5;
-        _buffered = new ByteAccumulator(_buffered, _currBuffer, ptr);
     }
 
     private final void _finishBuffering() throws IOException
     {
         final int start = _currStart;
-        final int currLen = _currPtr - start;
+        final int newStart = _currPtr;        
+        final int currLen = newStart - start;
 
         ByteAccumulator acc = _buffered;
+        final ByteAccumulator child = _buffered;
         acc = acc.finish(_output, _currBuffer, start, currLen);
         _buffered = acc;
         if (acc == null) {
             _currStart = 0;
             _currPtr = 0;
         } else {
-            _currStart = _currPtr;
+            // 08-Mar-2017, tatu: for [dataformats-binary#54]
+            // need to reposition buffer, otherwise will overwrite
+            final int parentStart = child._parentStart;
+            _flushBuffer(parentStart, child._prefixOffset - parentStart, newStart);
         }
     }
 
@@ -1788,6 +1792,32 @@ protected final void _ensureMore() throws IOException
         _currBuffer = ProtobufUtil.allocSecondary(_currBuffer);
     }
 
+    /**
+     * Flushes current buffer either to output or (if buffered) ByteAccumulator (append)
+     * and sets current start position to current pointer.
+     *
+     * @since 2.8.8
+     */
+    protected final void _flushBuffer(final int start, final int currLen, final int newStart) throws IOException
+    {
+        ByteAccumulator acc = _buffered;
+        if (acc == null) {
+            // without accumulation, we know buffer is free for reuse
+            if (currLen > 0) {
+                _output.write(_currBuffer, start, currLen);
+            }
+            _currStart = 0;
+            _currPtr = 0;
+            return;
+        }
+        // but with buffered, need to append, allocate new buffer (since old
+        // almost certainly contains buffered data)
+        if (currLen > 0) {
+            acc.append(_currBuffer, start, currLen);
+        }
+        _currPtr = _currStart = newStart;
+    }
+
     protected void _complete() throws IOException
     {
         _complete = true;

protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/RoundtripNestedMessageTest.java

@@ -0,0 +1,122 @@
+package com.fasterxml.jackson.dataformat.protobuf;
+
+import java.io.IOException;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.fasterxml.jackson.dataformat.protobuf.ProtobufMapper;
+import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema;
+import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader;
+
+public class RoundtripNestedMessageTest extends ProtobufTestBase
+{
+    private static final String VALUE_A = "value";
+    private static final String VALUE_B = "value-b";
+    private static final String VALUE_C = "valc";
+    private static final String VALUE_SUB_A = "a-value!";
+
+    private static final String PROTO = //
+            "message TestObject {\n" + //
+                      "optional string a = 1;\n" + //
+                      "optional TestSub b = 2;\n" + //
+                      "}\n" + //
+                      "message TestSub {;\n" + //
+                      "optional string c = 2;\n" + //
+                      "optional string b = 3;\n" + //
+                      "optional TestSubSub d = 4;\n" + //
+                      "}\n" + //
+                      "message TestSubSub {;\n" + //
+                      "optional string a = 1;\n" + //
+                      "}\n"; //
+
+    static class TestObject {
+       String a;
+       TestSub b;
+
+       public String getA() {
+            return a;
+       }
+
+       public void setA(String a) {
+            this.a = a;
+       }
+
+       public TestSub getB() {
+            return b;
+       }
+
+       public void setB(TestSub b) {
+            this.b = b;
+       }
+    }
+
+    // ordering would be needed prior to fix for [#59]
+    //@com.fasterxml.jackson.annotation.JsonPropertyOrder({"d", "b", "c"})
+    static class TestSub {
+        String b;
+        String c;
+        TestSubSub d;
+
+        public String getB() {
+            return b;
+        }
+
+        public void setB(String b) {
+            this.b = b;
+        }
+
+        public String getC() {
+            return c;
+        }
+
+        public void setC(String c) {
+            this.c = c;
+        }
+
+        public TestSubSub getD() {
+            return d;
+        }
+
+        public void setD(TestSubSub d) {
+            this.d = d;
+        }
+    }
+
+    public static class TestSubSub {
+        String a;
+        
+        public String getA() {
+            return a;
+        }
+        
+        public void setA(String a) {
+            this.a = a;
+        }
+    }
+
+    @Test
+    public void testNestedRoundtrip() throws IOException
+    {
+       TestObject testClass = new TestObject();
+       ProtobufMapper om = new ProtobufMapper();
+       ProtobufSchema s = ProtobufSchemaLoader.std.parse(PROTO);
+       testClass.a = VALUE_A;
+       testClass.b = new TestSub();
+       testClass.b.b = VALUE_B;
+       testClass.b.c = VALUE_C;
+       // if this following row is commented out, test succeeds with old code
+       testClass.b.d = new TestSubSub();
+       testClass.b.d.a = VALUE_SUB_A;
+
+       byte[] proto = om.writer(s)
+               .writeValueAsBytes(testClass);
+        TestObject res = om.readerFor(TestObject.class).with(s)
+               .readValue(proto);
+
+       Assert.assertEquals(VALUE_A, res.a);
+       Assert.assertEquals(VALUE_C, res.b.c);
+       Assert.assertEquals(VALUE_B, res.b.b);
+       Assert.assertEquals(VALUE_SUB_A, res.b.d.a);
+    }
+}

release-notes/VERSION

@@ -11,6 +11,10 @@ Modules:
 
 2.9.0 (not yet released)
 
+2.8.8 (not yet released)
+
+#54 (protobuf): Some fields are left null
+
 2.8.7 (21-Feb-2017)
 
 #34 (avro): Reading Avro with specified reader and writer schemas