Fix repeated field merging semantics.
The public doc states that repeated fields are simply concatenated and doesn't impose a different semantics for packed fields. This CL fixes this for packed fields and adds tests covering all cases. Also fixed a bit of missed null-repeated-field treatments. Change-Id: Ie35277bb1a9f0b8171dc9d07b6adf9b9d3308de2
This commit is contained in:
parent
80dcae757e
commit
cd3c68b255
@ -49,6 +49,7 @@ import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas;
|
||||
import com.google.protobuf.nano.NanoOuterClass;
|
||||
import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano;
|
||||
import com.google.protobuf.nano.NanoReferenceTypes;
|
||||
import com.google.protobuf.nano.TestRepeatedMergeNano;
|
||||
import com.google.protobuf.nano.UnittestImportNano;
|
||||
import com.google.protobuf.nano.UnittestMultipleNano;
|
||||
import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano;
|
||||
@ -2685,33 +2686,125 @@ public class NanoTest extends TestCase {
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
message = new TestAllTypesNano();
|
||||
message.repeatedNestedEnum = null;
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
message = new TestAllTypesNano();
|
||||
message.repeatedBytes = null;
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
message = new TestAllTypesNano();
|
||||
message.repeatedNestedMessage = null;
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
message.repeatedPackedInt32 = null;
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
message.repeatedPackedNestedEnum = null;
|
||||
MessageNano.toByteArray(message); // should not NPE
|
||||
message.toString(); // should not NPE
|
||||
|
||||
// Create a second message to merge into message.
|
||||
TestAllTypesNano secondMessage = new TestAllTypesNano();
|
||||
secondMessage.repeatedInt32 = new int[] {1, 2, 3};
|
||||
secondMessage.repeatedNestedEnum = new int[] {
|
||||
TestAllTypesNano.FOO, TestAllTypesNano.BAR
|
||||
};
|
||||
secondMessage.repeatedBytes = new byte[][] {{1, 2}, {3, 4}};
|
||||
TestAllTypesNano.NestedMessage nested =
|
||||
new TestAllTypesNano.NestedMessage();
|
||||
nested.bb = 55;
|
||||
secondMessage.repeatedNestedMessage =
|
||||
new TestAllTypesNano.NestedMessage[] {nested};
|
||||
secondMessage.repeatedPackedInt32 = new int[] {1, 2, 3};
|
||||
secondMessage.repeatedPackedNestedEnum = new int[] {
|
||||
TestAllTypesNano.FOO, TestAllTypesNano.BAR
|
||||
};
|
||||
|
||||
// Should not NPE
|
||||
message.mergeFrom(CodedInputByteBufferNano.newInstance(
|
||||
MessageNano.toByteArray(secondMessage)));
|
||||
assertEquals(3, message.repeatedInt32.length);
|
||||
assertEquals(3, message.repeatedInt32[2]);
|
||||
assertEquals(2, message.repeatedNestedEnum.length);
|
||||
assertEquals(TestAllTypesNano.FOO, message.repeatedNestedEnum[0]);
|
||||
assertEquals(2, message.repeatedBytes.length);
|
||||
assertEquals(4, message.repeatedBytes[1][1]);
|
||||
assertEquals(1, message.repeatedNestedMessage.length);
|
||||
assertEquals(55, message.repeatedNestedMessage[0].bb);
|
||||
assertEquals(3, message.repeatedPackedInt32.length);
|
||||
assertEquals(2, message.repeatedPackedInt32[1]);
|
||||
assertEquals(2, message.repeatedPackedNestedEnum.length);
|
||||
assertEquals(TestAllTypesNano.BAR, message.repeatedPackedNestedEnum[1]);
|
||||
}
|
||||
|
||||
public void testRepeatedMerge() throws Exception {
|
||||
// Check that merging repeated fields cause the arrays to expand with
|
||||
// new data.
|
||||
TestAllTypesNano first = new TestAllTypesNano();
|
||||
first.repeatedInt32 = new int[] {1, 2, 3};
|
||||
TestAllTypesNano second = new TestAllTypesNano();
|
||||
second.repeatedInt32 = new int[] {4, 5};
|
||||
MessageNano.mergeFrom(first, MessageNano.toByteArray(second));
|
||||
assertEquals(5, first.repeatedInt32.length);
|
||||
assertEquals(1, first.repeatedInt32[0]);
|
||||
assertEquals(4, first.repeatedInt32[3]);
|
||||
|
||||
first = new TestAllTypesNano();
|
||||
first.repeatedNestedEnum = new int[] {TestAllTypesNano.BAR};
|
||||
second = new TestAllTypesNano();
|
||||
second.repeatedNestedEnum = new int[] {TestAllTypesNano.FOO};
|
||||
MessageNano.mergeFrom(first, MessageNano.toByteArray(second));
|
||||
assertEquals(2, first.repeatedNestedEnum.length);
|
||||
assertEquals(TestAllTypesNano.BAR, first.repeatedNestedEnum[0]);
|
||||
assertEquals(TestAllTypesNano.FOO, first.repeatedNestedEnum[1]);
|
||||
|
||||
first = new TestAllTypesNano();
|
||||
first.repeatedNestedMessage = new TestAllTypesNano.NestedMessage[] {
|
||||
new TestAllTypesNano.NestedMessage()
|
||||
};
|
||||
first.repeatedNestedMessage[0].bb = 3;
|
||||
second = new TestAllTypesNano();
|
||||
second.repeatedNestedMessage = new TestAllTypesNano.NestedMessage[] {
|
||||
new TestAllTypesNano.NestedMessage()
|
||||
};
|
||||
second.repeatedNestedMessage[0].bb = 5;
|
||||
MessageNano.mergeFrom(first, MessageNano.toByteArray(second));
|
||||
assertEquals(2, first.repeatedNestedMessage.length);
|
||||
assertEquals(3, first.repeatedNestedMessage[0].bb);
|
||||
assertEquals(5, first.repeatedNestedMessage[1].bb);
|
||||
|
||||
first = new TestAllTypesNano();
|
||||
first.repeatedPackedSfixed64 = new long[] {-1, -2, -3};
|
||||
second = new TestAllTypesNano();
|
||||
second.repeatedPackedSfixed64 = new long[] {-4, -5};
|
||||
MessageNano.mergeFrom(first, MessageNano.toByteArray(second));
|
||||
assertEquals(5, first.repeatedPackedSfixed64.length);
|
||||
assertEquals(-1, first.repeatedPackedSfixed64[0]);
|
||||
assertEquals(-4, first.repeatedPackedSfixed64[3]);
|
||||
|
||||
first = new TestAllTypesNano();
|
||||
first.repeatedPackedNestedEnum = new int[] {TestAllTypesNano.BAR};
|
||||
second = new TestAllTypesNano();
|
||||
second.repeatedPackedNestedEnum = new int[] {TestAllTypesNano.FOO};
|
||||
MessageNano.mergeFrom(first, MessageNano.toByteArray(second));
|
||||
assertEquals(2, first.repeatedPackedNestedEnum.length);
|
||||
assertEquals(TestAllTypesNano.BAR, first.repeatedPackedNestedEnum[0]);
|
||||
assertEquals(TestAllTypesNano.FOO, first.repeatedPackedNestedEnum[1]);
|
||||
|
||||
// Now test repeated merging in a nested scope
|
||||
TestRepeatedMergeNano firstContainer = new TestRepeatedMergeNano();
|
||||
firstContainer.contained = new TestAllTypesNano();
|
||||
firstContainer.contained.repeatedInt32 = new int[] {10, 20};
|
||||
TestRepeatedMergeNano secondContainer = new TestRepeatedMergeNano();
|
||||
secondContainer.contained = new TestAllTypesNano();
|
||||
secondContainer.contained.repeatedInt32 = new int[] {30};
|
||||
MessageNano.mergeFrom(firstContainer, MessageNano.toByteArray(secondContainer));
|
||||
assertEquals(3, firstContainer.contained.repeatedInt32.length);
|
||||
assertEquals(20, firstContainer.contained.repeatedInt32[1]);
|
||||
assertEquals(30, firstContainer.contained.repeatedInt32[2]);
|
||||
}
|
||||
|
||||
private void assertHasWireData(MessageNano message, boolean expected) {
|
||||
|
@ -269,24 +269,32 @@ GenerateMergingCode(io::Printer* printer) const {
|
||||
" arrayLength++;\n"
|
||||
"}\n"
|
||||
"input.rewindToPosition(startPos);\n"
|
||||
"this.$name$ = new $type$[arrayLength];\n"
|
||||
"for (int i = 0; i < arrayLength; i++) {\n"
|
||||
" this.$name$[i] = input.readInt32();\n"
|
||||
"int i = this.$name$ == null ? 0 : this.$name$.length;\n"
|
||||
"int[] newArray = new int[i + arrayLength];\n"
|
||||
"if (i != 0) {\n"
|
||||
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"}\n"
|
||||
"for (; i < newArray.length; i++) {\n"
|
||||
" newArray[i] = input.readInt32();\n"
|
||||
"}\n"
|
||||
"this.$name$ = newArray;\n"
|
||||
"input.popLimit(limit);\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
"int arrayLength = com.google.protobuf.nano.WireFormatNano.getRepeatedFieldArrayLength(input, $tag$);\n"
|
||||
"int i = this.$name$.length;\n"
|
||||
"int arrayLength = com.google.protobuf.nano.WireFormatNano.\n"
|
||||
" getRepeatedFieldArrayLength(input, $tag$);\n"
|
||||
"int i = this.$name$ == null ? 0 : this.$name$.length;\n"
|
||||
"int[] newArray = new int[i + arrayLength];\n"
|
||||
"System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"this.$name$ = newArray;\n"
|
||||
"for (; i < this.$name$.length - 1; i++) {\n"
|
||||
" this.$name$[i] = input.readInt32();\n"
|
||||
"if (i != 0) {\n"
|
||||
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"}\n"
|
||||
"for (; i < newArray.length - 1; i++) {\n"
|
||||
" newArray[i] = input.readInt32();\n"
|
||||
" input.readTag();\n"
|
||||
"}\n"
|
||||
"// Last one without readTag.\n"
|
||||
"this.$name$[i] = input.readInt32();\n");
|
||||
"newArray[i] = input.readInt32();\n"
|
||||
"this.$name$ = newArray;\n");
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -235,34 +235,36 @@ GenerateMergingCode(io::Printer* printer) const {
|
||||
" .getRepeatedFieldArrayLength(input, $tag$);\n"
|
||||
"int i = this.$name$ == null ? 0 : this.$name$.length;\n"
|
||||
"$type$[] newArray = new $type$[i + arrayLength];\n"
|
||||
"if (this.$name$ != null) {\n"
|
||||
" System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"if (i != 0) {\n"
|
||||
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"}\n"
|
||||
"this.$name$ = newArray;\n"
|
||||
"for (; i < this.$name$.length - 1; i++) {\n"
|
||||
" this.$name$[i] = new $type$();\n");
|
||||
"for (; i < newArray.length - 1; i++) {\n"
|
||||
" newArray[i] = new $type$();\n");
|
||||
|
||||
if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
|
||||
printer->Print(variables_,
|
||||
" input.readGroup(this.$name$[i], $number$);\n");
|
||||
" input.readGroup(newArray[i], $number$);\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
" input.readMessage(this.$name$[i]);\n");
|
||||
" input.readMessage(newArray[i]);\n");
|
||||
}
|
||||
|
||||
printer->Print(variables_,
|
||||
" input.readTag();\n"
|
||||
"}\n"
|
||||
"// Last one without readTag.\n"
|
||||
"this.$name$[i] = new $type$();\n");
|
||||
"newArray[i] = new $type$();\n");
|
||||
|
||||
if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
|
||||
printer->Print(variables_,
|
||||
"input.readGroup(this.$name$[i], $number$);\n");
|
||||
"input.readGroup(newArray[i], $number$);\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
"input.readMessage(this.$name$[i]);\n");
|
||||
"input.readMessage(newArray[i]);\n");
|
||||
}
|
||||
|
||||
printer->Print(variables_,
|
||||
"this.$name$ = newArray;\n");
|
||||
}
|
||||
|
||||
void RepeatedMessageFieldGenerator::
|
||||
|
@ -519,35 +519,40 @@ GenerateMergingCode(io::Printer* printer) const {
|
||||
" arrayLength++;\n"
|
||||
"}\n"
|
||||
"input.rewindToPosition(startPos);\n"
|
||||
"this.$name$ = new $type$[arrayLength];\n"
|
||||
"for (int i = 0; i < arrayLength; i++) {\n"
|
||||
" this.$name$[i] = input.read$capitalized_type$();\n"
|
||||
"int i = this.$name$ == null ? 0 : this.$name$.length;\n"
|
||||
"$type$[] newArray = new $type$[i + arrayLength];\n"
|
||||
"if (i != 0) {\n"
|
||||
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"}\n"
|
||||
"for (; i < newArray.length; i++) {\n"
|
||||
" newArray[i] = input.read$capitalized_type$();\n"
|
||||
"}\n"
|
||||
"this.$name$ = newArray;\n"
|
||||
"input.popLimit(limit);\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
"int arrayLength = com.google.protobuf.nano.WireFormatNano\n"
|
||||
" .getRepeatedFieldArrayLength(input, $tag$);\n"
|
||||
"int i = this.$name$.length;\n");
|
||||
"int i = this.$name$ == null ? 0 : this.$name$.length;\n");
|
||||
|
||||
if (GetJavaType(descriptor_) == JAVATYPE_BYTES) {
|
||||
printer->Print(variables_,
|
||||
"byte[][] newArray = new byte[i + arrayLength][];\n"
|
||||
"System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"this.$name$ = newArray;\n");
|
||||
"byte[][] newArray = new byte[i + arrayLength][];\n");
|
||||
} else {
|
||||
printer->Print(variables_,
|
||||
"$type$[] newArray = new $type$[i + arrayLength];\n"
|
||||
"System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"this.$name$ = newArray;\n");
|
||||
"$type$[] newArray = new $type$[i + arrayLength];\n");
|
||||
}
|
||||
printer->Print(variables_,
|
||||
"for (; i < this.$name$.length - 1; i++) {\n"
|
||||
" this.$name$[i] = input.read$capitalized_type$();\n"
|
||||
"if (i != 0) {\n"
|
||||
" java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
|
||||
"}\n"
|
||||
"for (; i < newArray.length - 1; i++) {\n"
|
||||
" newArray[i] = input.read$capitalized_type$();\n"
|
||||
" input.readTag();\n"
|
||||
"}\n"
|
||||
"// Last one without readTag.\n"
|
||||
"this.$name$[i] = input.read$capitalized_type$();\n");
|
||||
"newArray[i] = input.read$capitalized_type$();\n"
|
||||
"this.$name$ = newArray;\n");
|
||||
}
|
||||
}
|
||||
|
||||
|
47
src/google/protobuf/unittest_repeated_merge_nano.proto
Normal file
47
src/google/protobuf/unittest_repeated_merge_nano.proto
Normal file
@ -0,0 +1,47 @@
|
||||
// Protocol Buffers - Google's data interchange format
|
||||
// Copyright 2008 Google Inc. All rights reserved.
|
||||
// http://code.google.com/p/protobuf/
|
||||
//
|
||||
// Redistribution and use in source and binary forms, with or without
|
||||
// modification, are permitted provided that the following conditions are
|
||||
// met:
|
||||
//
|
||||
// * Redistributions of source code must retain the above copyright
|
||||
// notice, this list of conditions and the following disclaimer.
|
||||
// * Redistributions in binary form must reproduce the above
|
||||
// copyright notice, this list of conditions and the following disclaimer
|
||||
// in the documentation and/or other materials provided with the
|
||||
// distribution.
|
||||
// * Neither the name of Google Inc. nor the names of its
|
||||
// contributors may be used to endorse or promote products derived from
|
||||
// this software without specific prior written permission.
|
||||
//
|
||||
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
|
||||
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
|
||||
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
|
||||
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
|
||||
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
|
||||
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
|
||||
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
|
||||
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
|
||||
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
|
||||
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
// Author: maxtroy@google.com (Max Cai)
|
||||
|
||||
package protobuf_unittest;
|
||||
|
||||
import "google/protobuf/unittest_nano.proto";
|
||||
|
||||
option java_package = "com.google.protobuf.nano";
|
||||
option java_multiple_files = true;
|
||||
|
||||
// A container message for testing the merging of repeated fields at a
|
||||
// nested level. Other tests will be done using the repeated fields in
|
||||
// TestAllTypesNano.
|
||||
message TestRepeatedMergeNano {
|
||||
|
||||
optional TestAllTypesNano contained = 1;
|
||||
|
||||
}
|
Loading…
Reference in New Issue
Block a user