pizheng/protobuf-net

Enable (de)serialization of object properties containing a List<T> value.

Opened this issue · 5 comments

This issue is linked to 
http://code.google.com/p/protobuf-net/issues/detail?id=175

Observe the following code:

  [DataContract]
  public class I
  {
    [DataMember(Order = 1)]
    public object Data { get; set; }
  }

  [ProtoContract]
  public class K
  {
    [ProtoMember(1)]
    public string State { get; set; }
  }

  class Program
  {
    static void Main()
    {
      var m = RuntimeTypeModel.Default;
      m.Add(typeof(object), false).AddSubType(1, typeof(List<K>));

      var o = new I { Data = new List<K> { new K { State = "ddd" } } };
      using (var ms = new MemoryStream())
      {
        Serializer.Serialize(ms, o);
        ms.Position = 0;
        var o2 = Serializer.Deserialize<I>(ms);
        Debug.Assert(((List<K>)o.Data).SequenceEqual((List<K>)o2.Data));
      }
    }
  }

The assertion fails. The casts are OK, it is just that the o2.Data is an empty 
list.

Note, that if instead of List<K> one uses K (i.e. a non collection object), 
then all works OK (after the relevant adaptation of the code).

Thanks.

Original issue reported on code.google.com by mark.kha...@gmail.com on 9 Jun 2011 at 7:52

Applying the following patch seems to solve the problem:

Index: Serializers/ListDecorator.cs
===================================================================
--- Serializers/ListDecorator.cs    (révision 407)
+++ Serializers/ListDecorator.cs    (copie de travail)
@@ -381,7 +381,6 @@
         public override object Read(object value, ProtoReader source)
         {
             int field = source.FieldNumber;
-            object origValue = value;
             if (value == null) value = Activator.CreateInstance(concreteType);
             bool isList = IsList && !SuppressIList;
             if (packedWireType != WireType.None && source.WireType == WireType.String)
@@ -424,7 +423,7 @@
                     } while (source.TryReadFieldHeader(field));
                 }
             }
-            return origValue == value ? null : value;
+            return value;
         }

     }


Frankly, I do not understand the purpose of origValue.

Original comment by mark.kha...@gmail.com on 12 Jun 2011 at 1:35

The purpose of origValue here is to avoid unnecessary assignments to fields and 
properties for lists that haven't changed. In addition to preserving the v1 
behaviour, this also avoids a bug in the .NET 3.5 version of EntitySet 
(although that aspect could arguably be limited to the SuppressIList scenario)

Original comment by marc.gravell on 21 Jun 2011 at 8:05

But it breaks my scenario.

Original comment by mark.kha...@gmail.com on 21 Jun 2011 at 10:52

Given a choice of "don't break your scenario" and "have the rest of the code 
work reliably", I'm going to choose the latter. Now, I might be able to get the 
list-as-a-subclass working (although it is not an intended use-case), but if so 
it will be a little more subtle than just returning null/not-null.


Original comment by marc.gravell on 21 Jun 2011 at 11:00

Understood. Looking forward for this change.
Thanks.

Original comment by mark.kha...@gmail.com on 21 Jun 2011 at 11:02