aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Cai <maxtroy@google.com>2013-10-15 18:11:56 +0100
committerMax Cai <maxtroy@google.com>2013-10-15 18:11:56 +0100
commit334a7d1117a576bc6010c14677fb6444639c1dda (patch)
tree08e02cb00d3c80f193e51917fab0017b88093539
parent63e819adbb4d2d8215f5d3d8297dc50bf2fe3329 (diff)
downloadexternal_protobuf-334a7d1117a576bc6010c14677fb6444639c1dda.zip
external_protobuf-334a7d1117a576bc6010c14677fb6444639c1dda.tar.gz
external_protobuf-334a7d1117a576bc6010c14677fb6444639c1dda.tar.bz2
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
-rw-r--r--java/pom.xml1
-rw-r--r--java/src/test/java/com/google/protobuf/NanoTest.java101
-rw-r--r--src/google/protobuf/compiler/javanano/javanano_enum_field.cc28
-rw-r--r--src/google/protobuf/compiler/javanano/javanano_message_field.cc22
-rw-r--r--src/google/protobuf/compiler/javanano/javanano_primitive_field.cc31
-rw-r--r--src/google/protobuf/unittest_repeated_merge_nano.proto47
6 files changed, 193 insertions, 37 deletions
diff --git a/java/pom.xml b/java/pom.xml
index 1f43670..dcc1cfa 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -145,6 +145,7 @@
<arg value="../src/google/protobuf/unittest_multiple_nano.proto" />
<arg value="../src/google/protobuf/unittest_multiple_nameclash_nano.proto" />
<arg value="../src/google/protobuf/unittest_enum_class_nano.proto" />
+ <arg value="../src/google/protobuf/unittest_repeated_merge_nano.proto" />
</exec>
<exec executable="../src/protoc">
<arg value="--javanano_out=store_unknown_fields=true:target/generated-test-sources" />
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index dbc0ddd..d4e9dfc 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -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 };
+ 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) {
diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
index cdb3d09..fe67d76 100644
--- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
@@ -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");
}
}
diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
index 04f1c14..027428f 100644
--- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
@@ -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::
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
index 8097be8..0562a6e 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
@@ -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");
}
}
diff --git a/src/google/protobuf/unittest_repeated_merge_nano.proto b/src/google/protobuf/unittest_repeated_merge_nano.proto
new file mode 100644
index 0000000..4f03224
--- /dev/null
+++ b/src/google/protobuf/unittest_repeated_merge_nano.proto
@@ -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;
+
+}