Fixes from PR review.
diff --git a/csharp/src/ProtocolBuffers.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/ProtocolBuffers.Test/WellKnownTypes/WrappersTest.cs
index 6c70601..ad88c4e 100644
--- a/csharp/src/ProtocolBuffers.Test/WellKnownTypes/WrappersTest.cs
+++ b/csharp/src/ProtocolBuffers.Test/WellKnownTypes/WrappersTest.cs
@@ -34,6 +34,7 @@
using Google.Protobuf.TestProtos;
using NUnit.Framework;
using System.Collections;
+using System.IO;
namespace Google.Protobuf.WellKnownTypes
{
@@ -232,9 +233,44 @@
Assert.IsTrue(dictionary.Contains(3));
}
- // Merging is odd with wrapper types, due to the way that default values aren't emitted in
- // the binary stream. In fact we cheat a little bit - a message with an explicitly present default
- // value will have that default value ignored.
+ [Test]
+ public void Oneof()
+ {
+ var message = new OneofWellKnownTypes { EmptyField = new Empty() };
+ // Start off with a non-wrapper
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.EmptyField, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+
+ message.StringField = "foo";
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.StringField, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+
+ message.StringField = "foo";
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.StringField, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+
+ message.DoubleField = 0.0f;
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.DoubleField, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+
+ message.DoubleField = 1.0f;
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.DoubleField, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+
+ message.ClearOneofField();
+ Assert.AreEqual(OneofWellKnownTypes.OneofFieldOneofCase.None, message.OneofFieldCase);
+ AssertOneofRoundTrip(message);
+ }
+
+ private void AssertOneofRoundTrip(OneofWellKnownTypes message)
+ {
+ // Normal roundtrip, but explicitly checking the case...
+ var bytes = message.ToByteArray();
+ var parsed = OneofWellKnownTypes.Parser.ParseFrom(bytes);
+ Assert.AreEqual(message, parsed);
+ Assert.AreEqual(message.OneofFieldCase, parsed.OneofFieldCase);
+ }
+
[Test]
[TestCase("x", "y", "y")]
[TestCase("x", "", "x")]
@@ -257,5 +293,34 @@
originalMessage.MergeFrom(mergingMessage.ToByteArray());
Assert.AreEqual(expected, originalMessage.StringField);
}
+
+ // Merging is odd with wrapper types, due to the way that default values aren't emitted in
+ // the binary stream. In fact we cheat a little bit - a message with an explicitly present default
+ // value will have that default value ignored.
+ [Test]
+ public void MergingCornerCase()
+ {
+ var message = new TestWellKnownTypes { Int32Field = 5 };
+
+ // Create a byte array which has the data of an Int32Value explicitly containing a value of 0.
+ // This wouldn't normally happen.
+ byte[] bytes;
+ var wrapperTag = WireFormat.MakeTag(TestWellKnownTypes.Int32FieldFieldNumber, WireFormat.WireType.LengthDelimited);
+ var valueTag = WireFormat.MakeTag(Int32Value.ValueFieldNumber, WireFormat.WireType.Varint);
+ using (var stream = new MemoryStream())
+ {
+ var coded = CodedOutputStream.CreateInstance(stream);
+ coded.WriteTag(wrapperTag);
+ coded.WriteLength(2); // valueTag + a value 0, each one byte
+ coded.WriteTag(valueTag);
+ coded.WriteInt32(0);
+ coded.Flush();
+ bytes = stream.ToArray();
+ }
+
+ message.MergeFrom(bytes);
+ // A normal implementation would have 0 now, as the explicit default would have been overwritten the 5.
+ Assert.AreEqual(5, message.Int32Field);
+ }
}
}
diff --git a/csharp/src/ProtocolBuffers/Collections/MapField.cs b/csharp/src/ProtocolBuffers/Collections/MapField.cs
index ead4573..f5e4fd3 100644
--- a/csharp/src/ProtocolBuffers/Collections/MapField.cs
+++ b/csharp/src/ProtocolBuffers/Collections/MapField.cs
@@ -51,7 +51,7 @@
public sealed class MapField<TKey, TValue> : IDeepCloneable<MapField<TKey, TValue>>, IFreezable, IDictionary<TKey, TValue>, IEquatable<MapField<TKey, TValue>>, IDictionary
{
// TODO: Don't create the map/list until we have an entry. (Assume many maps will be empty.)
- private bool allowNullValues;
+ private readonly bool allowNullValues;
private bool frozen;
private readonly Dictionary<TKey, LinkedListNode<KeyValuePair<TKey, TValue>>> map =
new Dictionary<TKey, LinkedListNode<KeyValuePair<TKey, TValue>>>();
diff --git a/csharp/src/ProtocolBuffers/FieldCodec.cs b/csharp/src/ProtocolBuffers/FieldCodec.cs
index caf0328..8546278 100644
--- a/csharp/src/ProtocolBuffers/FieldCodec.cs
+++ b/csharp/src/ProtocolBuffers/FieldCodec.cs
@@ -161,20 +161,30 @@
null); // Default value for the wrapper
}
- // Helper code to create codecs for wrapper types. Somewhat ugly with all the
+ /// <summary>
+ /// Helper code to create codecs for wrapper types.
+ /// </summary>
+ /// <remarks>
+ /// Somewhat ugly with all the static methods, but the conversions involved to/from nullable types make it
+ /// slightly tricky to improve. So long as we keep the public API (ForClassWrapper, ForStructWrapper) in place,
+ /// we can refactor later if we come up with something cleaner.
+ /// </remarks>
private static class WrapperCodecs
{
+ // All the field numbers are the same (1).
+ private const int WrapperValueFieldNumber = Google.Protobuf.WellKnownTypes.Int32Value.ValueFieldNumber;
+
private static readonly Dictionary<Type, object> Codecs = new Dictionary<Type, object>
{
- { typeof(bool), ForBool(WireFormat.MakeTag(1, WireFormat.WireType.Varint)) },
- { typeof(int), ForInt32(WireFormat.MakeTag(1, WireFormat.WireType.Varint)) },
- { typeof(long), ForInt64(WireFormat.MakeTag(1, WireFormat.WireType.Varint)) },
- { typeof(uint), ForUInt32(WireFormat.MakeTag(1, WireFormat.WireType.Varint)) },
- { typeof(ulong), ForUInt64(WireFormat.MakeTag(1, WireFormat.WireType.Varint)) },
- { typeof(float), ForFloat(WireFormat.MakeTag(1, WireFormat.WireType.Fixed32)) },
- { typeof(double), ForDouble(WireFormat.MakeTag(1, WireFormat.WireType.Fixed64)) },
- { typeof(string), ForString(WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited)) },
- { typeof(ByteString), ForBytes(WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited)) }
+ { typeof(bool), ForBool(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Varint)) },
+ { typeof(int), ForInt32(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Varint)) },
+ { typeof(long), ForInt64(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Varint)) },
+ { typeof(uint), ForUInt32(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Varint)) },
+ { typeof(ulong), ForUInt64(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Varint)) },
+ { typeof(float), ForFloat(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Fixed32)) },
+ { typeof(double), ForDouble(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.Fixed64)) },
+ { typeof(string), ForString(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.LengthDelimited)) },
+ { typeof(ByteString), ForBytes(WireFormat.MakeTag(WrapperValueFieldNumber, WireFormat.WireType.LengthDelimited)) }
};
/// <summary>
diff --git a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.h b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.h
index 5e2c5bc..6e2414a 100644
--- a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.h
+++ b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.h
@@ -83,4 +83,3 @@
} // namespace google
#endif // GOOGLE_PROTOBUF_COMPILER_CSHARP_WRAPPER_FIELD_H__
-
