zhejiushizhuce / protobuf-net

Automatically exported from code.google.com/p/protobuf-net
0 stars 0 forks source link

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

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
But it breaks my scenario.

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

GoogleCodeExporter commented 9 years ago
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

GoogleCodeExporter commented 9 years ago
Understood. Looking forward for this change.
Thanks.

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