Tweaks and more tests for maps
- Change the default message hash code to 1 to be consistent with other code
- Change the empty list/map hash code to 0 as "empty map" is equivalent to "no map"
- Removed map fields from unittest_proto3.proto
- Created map_unittest_proto3.proto which is like map_unittest.proto but proto3-only
- Fixed factory methods in FieldCodec highlighted by using all field types :)
- Added tests for map serialization:
- Extra fields within entries
- Entries with value then key
- Non-contiguous entries for the same map
- Multiple entries for the same key
Changes to generated code coming in next commit
diff --git a/csharp/generate_protos.sh b/csharp/generate_protos.sh
index 60bc281..e861f16 100755
--- a/csharp/generate_protos.sh
+++ b/csharp/generate_protos.sh
@@ -43,10 +43,12 @@
rm src/google/protobuf/descriptor_proto_file.proto
$PROTOC -Isrc --csharp_out=csharp/src/ProtocolBuffers.Test/TestProtos \
+ src/google/protobuf/map_unittest_proto3.proto \
src/google/protobuf/unittest_proto3.proto \
src/google/protobuf/unittest_import_proto3.proto \
src/google/protobuf/unittest_import_public_proto3.proto
+
$PROTOC -Icsharp/protos/extest --csharp_out=csharp/src/ProtocolBuffers.Test/TestProtos \
csharp/protos/extest/unittest_issues.proto
diff --git a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
index efeb7a9..f866866 100644
--- a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
+++ b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs
@@ -1,5 +1,4 @@
using System;
-using System.Configuration;
using System.IO;
using Google.Protobuf.TestProtos;
using NUnit.Framework;
@@ -157,63 +156,204 @@
Assert.AreEqual(message, parsed);
}
+ // Note that not every map within map_unittest_proto3 is used. They all go through very
+ // similar code paths. The fact that all maps are present is validation that we have codecs
+ // for every type.
[Test]
public void RoundTrip_Maps()
{
- var message = new TestAllTypes
+ var message = new TestMap
{
- MapBoolToEnum = {
- { false, TestAllTypes.Types.NestedEnum.BAR},
- { true, TestAllTypes.Types.NestedEnum.BAZ}
+ MapBoolBool = {
+ { false, true },
+ { true, false }
},
- MapInt32ToBytes = {
+ MapInt32Bytes = {
{ 5, ByteString.CopyFrom(6, 7, 8) },
{ 25, ByteString.CopyFrom(1, 2, 3, 4, 5) },
{ 10, ByteString.Empty }
},
- MapStringToNestedMessage = {
- { "", new TestAllTypes.Types.NestedMessage { Bb = 10 } },
- { "null value", null },
+ MapInt32ForeignMessage = {
+ { 0, new ForeignMessage { C = 10 } },
+ { 5, null },
+ },
+ MapInt32Enum = {
+ { 1, MapEnum.MAP_ENUM_BAR },
+ { 2000, MapEnum.MAP_ENUM_FOO }
}
};
byte[] bytes = message.ToByteArray();
- TestAllTypes parsed = TestAllTypes.Parser.ParseFrom(bytes);
+ TestMap parsed = TestMap.Parser.ParseFrom(bytes);
Assert.AreEqual(message, parsed);
}
[Test]
public void MapWithEmptyEntry()
{
- var message = new TestAllTypes
+ var message = new TestMap
{
- MapInt32ToBytes = { { 0, ByteString.Empty } }
+ MapInt32Bytes = { { 0, ByteString.Empty } }
};
byte[] bytes = message.ToByteArray();
- Assert.AreEqual(3, bytes.Length); // Tag for field entry (2 bytes), length of entry (0; 1 byte)
+ Assert.AreEqual(2, bytes.Length); // Tag for field entry (1 byte), length of entry (0; 1 byte)
- var parsed = TestAllTypes.Parser.ParseFrom(bytes);
- Assert.AreEqual(1, parsed.MapInt32ToBytes.Count);
- Assert.AreEqual(ByteString.Empty, parsed.MapInt32ToBytes[0]);
+ var parsed = TestMap.Parser.ParseFrom(bytes);
+ Assert.AreEqual(1, parsed.MapInt32Bytes.Count);
+ Assert.AreEqual(ByteString.Empty, parsed.MapInt32Bytes[0]);
}
-
+
[Test]
public void MapWithOnlyValue()
{
// Hand-craft the stream to contain a single entry with just a value.
var memoryStream = new MemoryStream();
var output = CodedOutputStream.CreateInstance(memoryStream);
- output.WriteTag(TestAllTypes.MapStringToNestedMessageFieldNumber, WireFormat.WireType.LengthDelimited);
- var nestedMessage = new TestAllTypes.Types.NestedMessage { Bb = 20 };
+ output.WriteTag(TestMap.MapInt32ForeignMessageFieldNumber, WireFormat.WireType.LengthDelimited);
+ var nestedMessage = new ForeignMessage { C = 20 };
// Size of the entry (tag, size written by WriteMessage, data written by WriteMessage)
output.WriteRawVarint32((uint)(nestedMessage.CalculateSize() + 3));
output.WriteTag(2, WireFormat.WireType.LengthDelimited);
output.WriteMessage(nestedMessage);
output.Flush();
- var parsed = TestAllTypes.Parser.ParseFrom(memoryStream.ToArray());
- Assert.AreEqual(nestedMessage, parsed.MapStringToNestedMessage[""]);
+ var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());
+ Assert.AreEqual(nestedMessage, parsed.MapInt32ForeignMessage[0]);
+ }
+
+ [Test]
+ public void MapIgnoresExtraFieldsWithinEntryMessages()
+ {
+ // Hand-craft the stream to contain a single entry with three fields
+ var memoryStream = new MemoryStream();
+ var output = CodedOutputStream.CreateInstance(memoryStream);
+
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+
+ var key = 10; // Field 1
+ var value = 20; // Field 2
+ var extra = 30; // Field 3
+
+ // Each field can be represented in a single byte, with a single byte tag.
+ // Total message size: 6 bytes.
+ output.WriteRawVarint32(6);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value);
+ output.WriteTag(3, WireFormat.WireType.Varint);
+ output.WriteInt32(extra);
+ output.Flush();
+
+ var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());
+ Assert.AreEqual(value, parsed.MapInt32Int32[key]);
+ }
+
+ [Test]
+ public void MapFieldOrderIsIrrelevant()
+ {
+ var memoryStream = new MemoryStream();
+ var output = CodedOutputStream.CreateInstance(memoryStream);
+
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+
+ var key = 10;
+ var value = 20;
+
+ // Each field can be represented in a single byte, with a single byte tag.
+ // Total message size: 4 bytes.
+ output.WriteRawVarint32(4);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key);
+ output.Flush();
+
+ var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());
+ Assert.AreEqual(value, parsed.MapInt32Int32[key]);
+ }
+
+ [Test]
+ public void MapNonContiguousEntries()
+ {
+ var memoryStream = new MemoryStream();
+ var output = CodedOutputStream.CreateInstance(memoryStream);
+
+ // Message structure:
+ // Entry for MapInt32Int32
+ // Entry for MapStringString
+ // Entry for MapInt32Int32
+
+ // First entry
+ var key1 = 10;
+ var value1 = 20;
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+ output.WriteRawVarint32(4);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key1);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value1);
+
+ // Second entry
+ var key2 = "a";
+ var value2 = "b";
+ output.WriteTag(TestMap.MapStringStringFieldNumber, WireFormat.WireType.LengthDelimited);
+ output.WriteRawVarint32(6); // 3 bytes per entry: tag, size, character
+ output.WriteTag(1, WireFormat.WireType.LengthDelimited);
+ output.WriteString(key2);
+ output.WriteTag(2, WireFormat.WireType.LengthDelimited);
+ output.WriteString(value2);
+
+ // Third entry
+ var key3 = 15;
+ var value3 = 25;
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+ output.WriteRawVarint32(4);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key3);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value3);
+
+ output.Flush();
+ var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());
+ var expected = new TestMap
+ {
+ MapInt32Int32 = { { key1, value1 }, { key3, value3 } },
+ MapStringString = { { key2, value2 } }
+ };
+ Assert.AreEqual(expected, parsed);
+ }
+
+ [Test]
+ public void DuplicateKeys_LastEntryWins()
+ {
+ var memoryStream = new MemoryStream();
+ var output = CodedOutputStream.CreateInstance(memoryStream);
+
+ var key = 10;
+ var value1 = 20;
+ var value2 = 30;
+
+ // First entry
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+ output.WriteRawVarint32(4);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value1);
+
+ // Second entry - same key, different value
+ output.WriteTag(TestMap.MapInt32Int32FieldNumber, WireFormat.WireType.LengthDelimited);
+ output.WriteRawVarint32(4);
+ output.WriteTag(1, WireFormat.WireType.Varint);
+ output.WriteInt32(key);
+ output.WriteTag(2, WireFormat.WireType.Varint);
+ output.WriteInt32(value2);
+ output.Flush();
+
+ var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());
+ Assert.AreEqual(value2, parsed.MapInt32Int32[key]);
}
[Test]
diff --git a/csharp/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj b/csharp/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj
index d11e259..c3fd8ba 100644
--- a/csharp/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj
+++ b/csharp/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj
@@ -78,6 +78,7 @@
<Compile Include="GeneratedMessageTest.cs" />
<Compile Include="Collections\MapFieldTest.cs" />
<Compile Include="Collections\RepeatedFieldTest.cs" />
+ <Compile Include="TestProtos\MapUnittestProto3.cs" />
<Compile Include="TestProtos\UnittestImportProto3.cs" />
<Compile Include="TestProtos\UnittestImportPublicProto3.cs" />
<Compile Include="TestProtos\UnittestIssues.cs" />
diff --git a/csharp/src/ProtocolBuffers/Collections/MapField.cs b/csharp/src/ProtocolBuffers/Collections/MapField.cs
index 6834cfb..9eed833 100644
--- a/csharp/src/ProtocolBuffers/Collections/MapField.cs
+++ b/csharp/src/ProtocolBuffers/Collections/MapField.cs
@@ -261,7 +261,7 @@
public override int GetHashCode()
{
var valueComparer = EqualityComparer<TValue>.Default;
- int hash = 19;
+ int hash = 0;
foreach (var pair in list)
{
hash ^= pair.Key.GetHashCode() * 31 + valueComparer.GetHashCode(pair.Value);
@@ -380,7 +380,6 @@
private readonly Codec codec;
internal TKey Key { get; set; }
internal TValue Value { get; set; }
- internal int Size { get; set; }
internal MessageAdapter(Codec codec)
{
diff --git a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
index ebfc522..4d4212c 100644
--- a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
+++ b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs
@@ -193,7 +193,7 @@
public override int GetHashCode()
{
- int hash = 23;
+ int hash = 0;
for (int i = 0; i < count; i++)
{
hash = hash * 31 + array[i].GetHashCode();
diff --git a/csharp/src/ProtocolBuffers/FieldCodec.cs b/csharp/src/ProtocolBuffers/FieldCodec.cs
index 931b54d..617af2f 100644
--- a/csharp/src/ProtocolBuffers/FieldCodec.cs
+++ b/csharp/src/ProtocolBuffers/FieldCodec.cs
@@ -33,11 +33,16 @@
return new FieldCodec<int>(input => input.ReadSInt32(), (output, value) => output.WriteSInt32(value), CodedOutputStream.ComputeSInt32Size, tag);
}
- public static FieldCodec<uint> ForFixedInt32(uint tag)
+ public static FieldCodec<uint> ForFixed32(uint tag)
{
return new FieldCodec<uint>(input => input.ReadFixed32(), (output, value) => output.WriteFixed32(value), CodedOutputStream.ComputeFixed32Size, tag);
}
+ public static FieldCodec<int> ForSFixed32(uint tag)
+ {
+ return new FieldCodec<int>(input => input.ReadSFixed32(), (output, value) => output.WriteSFixed32(value), CodedOutputStream.ComputeSFixed32Size, tag);
+ }
+
public static FieldCodec<uint> ForUInt32(uint tag)
{
return new FieldCodec<uint>(input => input.ReadUInt32(), (output, value) => output.WriteUInt32(value), CodedOutputStream.ComputeUInt32Size, tag);
@@ -53,11 +58,16 @@
return new FieldCodec<long>(input => input.ReadSInt64(), (output, value) => output.WriteSInt64(value), CodedOutputStream.ComputeSInt64Size, tag);
}
- public static FieldCodec<ulong> ForFixedInt64(uint tag)
+ public static FieldCodec<ulong> ForFixed64(uint tag)
{
return new FieldCodec<ulong>(input => input.ReadFixed64(), (output, value) => output.WriteFixed64(value), CodedOutputStream.ComputeFixed64Size, tag);
}
+ public static FieldCodec<long> ForSFixed64(uint tag)
+ {
+ return new FieldCodec<long>(input => input.ReadSFixed64(), (output, value) => output.WriteSFixed64(value), CodedOutputStream.ComputeSFixed64Size, tag);
+ }
+
public static FieldCodec<ulong> ForUInt64(uint tag)
{
return new FieldCodec<ulong>(input => input.ReadUInt64(), (output, value) => output.WriteUInt64(value), CodedOutputStream.ComputeUInt64Size, tag);
diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc
index 9e055f6..9e2fe9b 100644
--- a/src/google/protobuf/compiler/csharp/csharp_message.cc
+++ b/src/google/protobuf/compiler/csharp/csharp_message.cc
@@ -408,7 +408,7 @@
// Start with a non-zero value to easily distinguish between null and "empty" messages.
printer->Print(
"public override int GetHashCode() {\n"
- " int hash = 17;\n");
+ " int hash = 1;\n");
printer->Indent();
for (int i = 0; i < descriptor_->field_count(); i++) {
scoped_ptr<FieldGeneratorBase> generator(
diff --git a/src/google/protobuf/map_unittest_proto3.proto b/src/google/protobuf/map_unittest_proto3.proto
new file mode 100644
index 0000000..16be277
--- /dev/null
+++ b/src/google/protobuf/map_unittest_proto3.proto
@@ -0,0 +1,120 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc. All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// 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.
+
+// This file is mostly equivalent to map_unittest.proto, but imports
+// unittest_proto3.proto instead of unittest.proto, so that it only
+// uses proto3 messages. This makes it suitable for testing
+// implementations which only support proto3.
+// The TestRequiredMessageMap message has been removed as there are no
+// required fields in proto3.
+syntax = "proto3";
+
+option cc_enable_arenas = true;
+option csharp_namespace = "Google.Protobuf.TestProtos";
+
+import "google/protobuf/unittest_proto3.proto";
+
+// We don't put this in a package within proto2 because we need to make sure
+// that the generated code doesn't depend on being in the proto2 namespace.
+// In map_test_util.h we do "using namespace unittest = protobuf_unittest".
+package protobuf_unittest;
+
+// Tests maps.
+message TestMap {
+ map<int32 , int32 > map_int32_int32 = 1;
+ map<int64 , int64 > map_int64_int64 = 2;
+ map<uint32 , uint32 > map_uint32_uint32 = 3;
+ map<uint64 , uint64 > map_uint64_uint64 = 4;
+ map<sint32 , sint32 > map_sint32_sint32 = 5;
+ map<sint64 , sint64 > map_sint64_sint64 = 6;
+ map<fixed32 , fixed32 > map_fixed32_fixed32 = 7;
+ map<fixed64 , fixed64 > map_fixed64_fixed64 = 8;
+ map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
+ map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
+ map<int32 , float > map_int32_float = 11;
+ map<int32 , double > map_int32_double = 12;
+ map<bool , bool > map_bool_bool = 13;
+ map<string , string > map_string_string = 14;
+ map<int32 , bytes > map_int32_bytes = 15;
+ map<int32 , MapEnum > map_int32_enum = 16;
+ map<int32 , ForeignMessage> map_int32_foreign_message = 17;
+}
+
+message TestMapSubmessage {
+ TestMap test_map = 1;
+}
+
+message TestMessageMap {
+ map<int32, TestAllTypes> map_int32_message = 1;
+}
+
+// Two map fields share the same entry default instance.
+message TestSameTypeMap {
+ map<int32, int32> map1 = 1;
+ map<int32, int32> map2 = 2;
+}
+
+enum MapEnum {
+ MAP_ENUM_FOO = 0;
+ MAP_ENUM_BAR = 1;
+ MAP_ENUM_BAZ = 2;
+}
+
+message TestArenaMap {
+ map<int32 , int32 > map_int32_int32 = 1;
+ map<int64 , int64 > map_int64_int64 = 2;
+ map<uint32 , uint32 > map_uint32_uint32 = 3;
+ map<uint64 , uint64 > map_uint64_uint64 = 4;
+ map<sint32 , sint32 > map_sint32_sint32 = 5;
+ map<sint64 , sint64 > map_sint64_sint64 = 6;
+ map<fixed32 , fixed32 > map_fixed32_fixed32 = 7;
+ map<fixed64 , fixed64 > map_fixed64_fixed64 = 8;
+ map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
+ map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
+ map<int32 , float > map_int32_float = 11;
+ map<int32 , double > map_int32_double = 12;
+ map<bool , bool > map_bool_bool = 13;
+ map<int32 , MapEnum > map_int32_enum = 14;
+ map<int32 , ForeignMessage> map_int32_foreign_message = 15;
+}
+
+// Previously, message containing enum called Type cannot be used as value of
+// map field.
+message MessageContainingEnumCalledType {
+ enum Type {
+ TYPE_FOO = 0;
+ }
+ map<int32, MessageContainingEnumCalledType> type = 1;
+}
+
+// Previously, message cannot contain map field called "entry".
+message MessageContainingMapCalledEntry {
+ map<int32, int32> entry = 1;
+}
diff --git a/src/google/protobuf/unittest_proto3.proto b/src/google/protobuf/unittest_proto3.proto
index 41fa56d..f59d217 100644
--- a/src/google/protobuf/unittest_proto3.proto
+++ b/src/google/protobuf/unittest_proto3.proto
@@ -140,11 +140,6 @@
string oneof_string = 113;
bytes oneof_bytes = 114;
}
-
- // Sample maps
- map<string, NestedMessage> map_string_to_nested_message = 200;
- map<int32, bytes> map_int32_to_bytes = 201;
- map<bool, NestedEnum> map_bool_to_enum = 202;
}
// This proto includes a recusively nested message.
