From c6c1f9731bf24f09b5bf8da3da8d58df03603223 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 9 Mar 2017 18:54:16 -0800 Subject: [PATCH] Fixed #54 (manual merge of patch) --- .../dataformat/protobuf/ByteAccumulator.java | 22 ++-- .../protobuf/ProtobufGenerator.java | 40 +++++- .../protobuf/RoundtripNestedMessageTest.java | 122 ++++++++++++++++++ release-notes/VERSION | 4 + 4 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/RoundtripNestedMessageTest.java diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ByteAccumulator.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ByteAccumulator.java index 0b93c6edf..d0eed3f46 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ByteAccumulator.java +++ b/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; } diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java index fb584b7ec..b64978e3a 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java +++ b/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; diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/RoundtripNestedMessageTest.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/RoundtripNestedMessageTest.java new file mode 100644 index 000000000..48bed012d --- /dev/null +++ b/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); + } +} diff --git a/release-notes/VERSION b/release-notes/VERSION index c655e2580..ec00ab6c0 100644 --- a/release-notes/VERSION +++ b/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