Prohibit null values in map fields

On deserialization, missing values for message types
are replaced with a "default" message.

5 files changed, 51 insertions, 77 deletions

diff --git a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
index ba82c0e8..9c845907 100644
--- a/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
+++ b/csharp/src/Google.Protobuf.Test/Collections/MapFieldTest.cs
@@ -56,7 +56,7 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
-        public void NullValues()
+        public void NullValuesProhibited()
         {
             TestNullValues<int?>(0);
             TestNullValues("");
@@ -65,19 +65,12 @@ namespace Google.Protobuf.Collections
 
         private void TestNullValues<T>(T nonNullValue)
         {
-            var map = new MapField<int, T>(false);
+            var map = new MapField<int, T>();
             var nullValue = (T) (object) null;
             Assert.Throws<ArgumentNullException>(() => map.Add(0, nullValue));
             Assert.Throws<ArgumentNullException>(() => map[0] = nullValue);
             map.Add(1, nonNullValue);
             map[1] = nonNullValue;
-
-            // Doesn't throw...
-            map = new MapField<int, T>(true);
-            map.Add(0, nullValue);
-            map[0] = nullValue;
-            map.Add(1, nonNullValue);
-            map[1] = nonNullValue;
         }
 
         [Test]
@@ -161,27 +154,6 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
-        public void EqualityHandlesNullValues()
-        {
-            var map1 = new MapField<string, ForeignMessage>();
-            map1.Add("a", new ForeignMessage { C = 10 });
-            map1.Add("b", null);
-
-            var map2 = new MapField<string, ForeignMessage>();
-            map2.Add("a", new ForeignMessage { C = 10 });
-            map2.Add("b", null);
-
-            EqualityTester.AssertEquality(map1, map2);
-            // Check the null value isn't ignored entirely...
-            Assert.IsTrue(map1.Remove("b"));
-            EqualityTester.AssertInequality(map1, map2);
-            map1.Add("b", new ForeignMessage());
-            EqualityTester.AssertInequality(map1, map2);
-            map1["b"] = null;
-            EqualityTester.AssertEquality(map1, map2);
-        }
-
-        [Test]
         public void Add_Dictionary()
         {
             var map1 = new MapField<string, string>
@@ -454,30 +426,6 @@ namespace Google.Protobuf.Collections
         }
 
         [Test]
-        public void AllowNullValues_Property()
-        {
-            // Non-message reference type values are non-nullable by default, but can be overridden
-            Assert.IsFalse(new MapField<int, string>().AllowsNullValues);
-            Assert.IsFalse(new MapField<int, string>(false).AllowsNullValues);
-            Assert.IsTrue(new MapField<int, string>(true).AllowsNullValues);
-
-            // Non-nullable value type values are never nullable
-            Assert.IsFalse(new MapField<int, int>().AllowsNullValues);
-            Assert.IsFalse(new MapField<int, int>(false).AllowsNullValues);
-            Assert.Throws<ArgumentException>(() => new MapField<int, int>(true));
-
-            // Message type values are nullable by default, but can be overridden
-            Assert.IsTrue(new MapField<int, TestAllTypes>().AllowsNullValues);
-            Assert.IsFalse(new MapField<int, TestAllTypes>(false).AllowsNullValues);
-            Assert.IsTrue(new MapField<int, TestAllTypes>(true).AllowsNullValues);
-
-            // Nullable value type values are nullable by default, but can be overridden
-            Assert.IsTrue(new MapField<int, int?>().AllowsNullValues);
-            Assert.IsFalse(new MapField<int, int?>(false).AllowsNullValues);
-            Assert.IsTrue(new MapField<int, int?>(true).AllowsNullValues);
-        }
-
-        [Test]
         public void KeysReturnsLiveView()
         {
             var map = new MapField<string, string>();
diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs
index 1163f524..cda7f885 100644
--- a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs
+++ b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs
@@ -221,7 +221,7 @@ namespace Google.Protobuf
                 },

                 MapInt32ForeignMessage = {

                     { 0, new ForeignMessage { C = 10 } },

-                    { 5, null },

+                    { 5, new ForeignMessage() },

                 },

                 MapInt32Enum = {

                     { 1, MapEnum.MAP_ENUM_BAR },

@@ -269,6 +269,40 @@ namespace Google.Protobuf
         }

 

         [Test]

+        public void MapWithOnlyKey_PrimitiveValue()

+        {

+            // Hand-craft the stream to contain a single entry with just a key.

+            var memoryStream = new MemoryStream();

+            var output = new CodedOutputStream(memoryStream);

+            output.WriteTag(TestMap.MapInt32DoubleFieldNumber, WireFormat.WireType.LengthDelimited);

+            int key = 10;

+            output.WriteLength(1 + CodedOutputStream.ComputeInt32Size(key));

+            output.WriteTag(1, WireFormat.WireType.Varint);

+            output.WriteInt32(key);

+            output.Flush();

+

+            var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());

+            Assert.AreEqual(0.0, parsed.MapInt32Double[key]);

+        }

+

+        [Test]

+        public void MapWithOnlyKey_MessageValue()

+        {

+            // Hand-craft the stream to contain a single entry with just a key.

+            var memoryStream = new MemoryStream();

+            var output = new CodedOutputStream(memoryStream);

+            output.WriteTag(TestMap.MapInt32ForeignMessageFieldNumber, WireFormat.WireType.LengthDelimited);

+            int key = 10;

+            output.WriteLength(1 + CodedOutputStream.ComputeInt32Size(key));

+            output.WriteTag(1, WireFormat.WireType.Varint);

+            output.WriteInt32(key);

+            output.Flush();

+

+            var parsed = TestMap.Parser.ParseFrom(memoryStream.ToArray());

+            Assert.AreEqual(new ForeignMessage(), parsed.MapInt32ForeignMessage[key]);

+        }

+

+        [Test]

         public void MapIgnoresExtraFieldsWithinEntryMessages()

         {

             // Hand-craft the stream to contain a single entry with three fields

diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
index 2cf2d5fc..ace70b00 100644
--- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
+++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
@@ -196,13 +196,6 @@ namespace Google.Protobuf
         }
 
         [Test]
-        public void NullValueForMessage()
-        {
-            var message = new TestMap { MapInt32ForeignMessage = { { 10, null } } };
-            AssertJson("{ 'mapInt32ForeignMessage': { '10': null } }", JsonFormatter.Default.Format(message));
-        }
-
-        [Test]
         [TestCase("a\u17b4b", "a\\u17b4b")] // Explicit
         [TestCase("a\u0601b", "a\\u0601b")] // Ranged
         [TestCase("a\u0605b", "a\u0605b")] // Passthrough (note lack of double backslash...)
diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs
index c00b0f41..dfada6bd 100644
--- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs
+++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestWellKnownTypes.cs
@@ -2080,7 +2080,7 @@ namespace Google.Protobuf.TestProtos {
     public const int DoubleFieldFieldNumber = 10;
     private static readonly pbc::MapField<int, double?>.Codec _map_doubleField_codec
         = new pbc::MapField<int, double?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<double>(18), 82);
-    private readonly pbc::MapField<int, double?> doubleField_ = new pbc::MapField<int, double?>(true);
+    private readonly pbc::MapField<int, double?> doubleField_ = new pbc::MapField<int, double?>();
     public pbc::MapField<int, double?> DoubleField {
       get { return doubleField_; }
     }
@@ -2089,7 +2089,7 @@ namespace Google.Protobuf.TestProtos {
     public const int FloatFieldFieldNumber = 11;
     private static readonly pbc::MapField<int, float?>.Codec _map_floatField_codec
         = new pbc::MapField<int, float?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<float>(18), 90);
-    private readonly pbc::MapField<int, float?> floatField_ = new pbc::MapField<int, float?>(true);
+    private readonly pbc::MapField<int, float?> floatField_ = new pbc::MapField<int, float?>();
     public pbc::MapField<int, float?> FloatField {
       get { return floatField_; }
     }
@@ -2098,7 +2098,7 @@ namespace Google.Protobuf.TestProtos {
     public const int Int64FieldFieldNumber = 12;
     private static readonly pbc::MapField<int, long?>.Codec _map_int64Field_codec
         = new pbc::MapField<int, long?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<long>(18), 98);
-    private readonly pbc::MapField<int, long?> int64Field_ = new pbc::MapField<int, long?>(true);
+    private readonly pbc::MapField<int, long?> int64Field_ = new pbc::MapField<int, long?>();
     public pbc::MapField<int, long?> Int64Field {
       get { return int64Field_; }
     }
@@ -2107,7 +2107,7 @@ namespace Google.Protobuf.TestProtos {
     public const int Uint64FieldFieldNumber = 13;
     private static readonly pbc::MapField<int, ulong?>.Codec _map_uint64Field_codec
         = new pbc::MapField<int, ulong?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<ulong>(18), 106);
-    private readonly pbc::MapField<int, ulong?> uint64Field_ = new pbc::MapField<int, ulong?>(true);
+    private readonly pbc::MapField<int, ulong?> uint64Field_ = new pbc::MapField<int, ulong?>();
     public pbc::MapField<int, ulong?> Uint64Field {
       get { return uint64Field_; }
     }
@@ -2116,7 +2116,7 @@ namespace Google.Protobuf.TestProtos {
     public const int Int32FieldFieldNumber = 14;
     private static readonly pbc::MapField<int, int?>.Codec _map_int32Field_codec
         = new pbc::MapField<int, int?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<int>(18), 114);
-    private readonly pbc::MapField<int, int?> int32Field_ = new pbc::MapField<int, int?>(true);
+    private readonly pbc::MapField<int, int?> int32Field_ = new pbc::MapField<int, int?>();
     public pbc::MapField<int, int?> Int32Field {
       get { return int32Field_; }
     }
@@ -2125,7 +2125,7 @@ namespace Google.Protobuf.TestProtos {
     public const int Uint32FieldFieldNumber = 15;
     private static readonly pbc::MapField<int, uint?>.Codec _map_uint32Field_codec
         = new pbc::MapField<int, uint?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<uint>(18), 122);
-    private readonly pbc::MapField<int, uint?> uint32Field_ = new pbc::MapField<int, uint?>(true);
+    private readonly pbc::MapField<int, uint?> uint32Field_ = new pbc::MapField<int, uint?>();
     public pbc::MapField<int, uint?> Uint32Field {
       get { return uint32Field_; }
     }
@@ -2134,7 +2134,7 @@ namespace Google.Protobuf.TestProtos {
     public const int BoolFieldFieldNumber = 16;
     private static readonly pbc::MapField<int, bool?>.Codec _map_boolField_codec
         = new pbc::MapField<int, bool?>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForStructWrapper<bool>(18), 130);
-    private readonly pbc::MapField<int, bool?> boolField_ = new pbc::MapField<int, bool?>(true);
+    private readonly pbc::MapField<int, bool?> boolField_ = new pbc::MapField<int, bool?>();
     public pbc::MapField<int, bool?> BoolField {
       get { return boolField_; }
     }
@@ -2143,7 +2143,7 @@ namespace Google.Protobuf.TestProtos {
     public const int StringFieldFieldNumber = 17;
     private static readonly pbc::MapField<int, string>.Codec _map_stringField_codec
         = new pbc::MapField<int, string>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForClassWrapper<string>(18), 138);
-    private readonly pbc::MapField<int, string> stringField_ = new pbc::MapField<int, string>(true);
+    private readonly pbc::MapField<int, string> stringField_ = new pbc::MapField<int, string>();
     public pbc::MapField<int, string> StringField {
       get { return stringField_; }
     }
@@ -2152,7 +2152,7 @@ namespace Google.Protobuf.TestProtos {
     public const int BytesFieldFieldNumber = 18;
     private static readonly pbc::MapField<int, pb::ByteString>.Codec _map_bytesField_codec
         = new pbc::MapField<int, pb::ByteString>.Codec(pb::FieldCodec.ForInt32(8), pb::FieldCodec.ForClassWrapper<pb::ByteString>(18), 146);
-    private readonly pbc::MapField<int, pb::ByteString> bytesField_ = new pbc::MapField<int, pb::ByteString>(true);
+    private readonly pbc::MapField<int, pb::ByteString> bytesField_ = new pbc::MapField<int, pb::ByteString>();
     public pbc::MapField<int, pb::ByteString> BytesField {
       get { return bytesField_; }
     }
diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs
index c87ceb2f..b72ef982 100644
--- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs
+++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs
@@ -151,6 +151,8 @@ namespace Google.Protobuf.WellKnownTypes
         [Test]
         public void MapWrappersSerializeDeserialize()
         {
+            // Note: no null values here, as they are prohibited in map fields
+            // (despite being representable).
             var message = new MapWellKnownTypes
             {
                 BoolField = { { 10, false }, { 20, true } },
@@ -158,13 +160,12 @@ namespace Google.Protobuf.WellKnownTypes
                     { -1, ByteString.CopyFrom(1, 2, 3) },
                     { 10, ByteString.CopyFrom(4, 5, 6) },
                     { 1000, ByteString.Empty },
-                    { 10000, null }
                 },
                 DoubleField = { { 1, 12.5 }, { 10, -1.5 }, { 20, 0d } },
                 FloatField = { { 2, 123.25f }, { 3, -20f }, { 4, 0f } },
                 Int32Field = { { 5, int.MaxValue }, { 6, int.MinValue }, { 7, 0 } },
                 Int64Field = { { 8, long.MaxValue }, { 9, long.MinValue }, { 10, 0L } },
-                StringField = { { 11, "First" }, { 12, "Second" }, { 13, "" }, { 14, null } },
+                StringField = { { 11, "First" }, { 12, "Second" }, { 13, "" } },
                 Uint32Field = { { 15, uint.MaxValue }, { 16, uint.MinValue }, { 17, 0U } },
                 Uint64Field = { { 18, ulong.MaxValue }, { 19, ulong.MinValue }, { 20, 0UL } },
             };
@@ -224,13 +225,11 @@ namespace Google.Protobuf.WellKnownTypes
         [Test]
         public void Reflection_MapFields()
         {
-            // Just a single example... note that we can't have a null value here
-            var message = new MapWellKnownTypes { Int32Field = { { 1, 2 }, { 3, null } } };
+            // Just a single example... note that we can't have a null value here despite the value type being int?
+            var message = new MapWellKnownTypes { Int32Field = { { 1, 2 } } };
             var fields = MapWellKnownTypes.Descriptor.Fields;
             var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].Accessor.GetValue(message);
             Assert.AreEqual(2, dictionary[1]);
-            Assert.IsNull(dictionary[3]);
-            Assert.IsTrue(dictionary.Contains(3));
         }
 
         [Test]