vhson / protobuf-net

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

MetaType.ConstructType does not respect surrogates. #185

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

Run the following code:

public interface I { int N { get; } }

public class O : I
{
  public O(int n) { N = n; }
  public int N { get; private set; }
}

[ProtoContract]
public class OS
{
  public static implicit operator O(OS o) 
  { return o == null ? null : new O(o.N); }
  public static implicit operator OS(O o)
  { return o == null ? null : new OS { N = o.N }; }
  [ProtoMember(1)] public int N { get; set; }
}

public class C
{
  public static implicit operator CS(C o)
  { return o == null ? null : new CS { Unknown = o.Unknown }; }
  public static implicit operator C(CS o)
  { return o == null ? null : new C { Unknown = o.Unknown }; }
  public void PopulateRun() { Unknown = new O(43); }
  public I Unknown { get; private set; }
}

[ProtoContract]
public class CS
{
  [ProtoMember(1)] public I Unknown { get; set; }
}

class Program
{
  static void Main()
  {
    var m = RuntimeTypeModel.Default;
    m.AutoCompile = false;
    m.Add(typeof(C), false).SetSurrogate(typeof(CS));
    m.Add(typeof(O), false).SetSurrogate(typeof(OS));
    m.Add(typeof(I), false).ConstructType = typeof(O);   // (*)
    //m.Add(typeof(I), false).AddSubType(1, typeof(O));  // (**)

    var c = new C();
    c.PopulateRun();
    using (var ms = new MemoryStream())
    {
      Serializer.Serialize(ms, c);
      ms.Position = 0;
      var c2 = Serializer.Deserialize<C>(ms);
      Debug.Assert(c.Unknown.N == c2.Unknown.N);
    }
  }
}

What is the expected output? What do you see instead?

The expected output is a successful execution, in reality the code fails with:

ProtoBuf.ProtoException was unhandled
  Message=No parameterless constructor found for O
  Source=protobuf-net
  StackTrace:
       at ProtoBuf.Meta.TypeModel.ThrowCannotCreateInstance(Type type) in z:\Work\protobuf-net-v2\protobuf-net\Meta\TypeModel.cs:line 933
       at ProtoBuf.Serializers.TypeSerializer.CreateInstance(ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\TypeSerializer.cs:line 210
       at ProtoBuf.Serializers.TypeSerializer.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\TypeSerializer.cs:line 186
       at ProtoBuf.Meta.RuntimeTypeModel.Deserialize(Int32 key, Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Meta\RuntimeTypeModel.cs:line 370
       at ProtoBuf.ProtoReader.ReadTypedObject(Object value, Int32 key, ProtoReader reader, Type type) in z:\Work\protobuf-net-v2\protobuf-net\ProtoReader.cs:line 512
       at ProtoBuf.ProtoReader.ReadObject(Object value, Int32 key, ProtoReader reader) in z:\Work\protobuf-net-v2\protobuf-net\ProtoReader.cs:line 501
       at ProtoBuf.Serializers.SubItemSerializer.ProtoBuf.Serializers.IProtoSerializer.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\SubItemSerializer.cs:line 58
       at ProtoBuf.Serializers.TagDecorator.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\TagDecorator.cs:line 61
       at ProtoBuf.Serializers.PropertyDecorator.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\PropertyDecorator.cs:line 53
       at ProtoBuf.Serializers.TypeSerializer.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\TypeSerializer.cs:line 165
       at ProtoBuf.Serializers.SurrogateSerializer.Read(Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Serializers\SurrogateSerializer.cs:line 59
       at ProtoBuf.Meta.RuntimeTypeModel.Deserialize(Int32 key, Object value, ProtoReader source) in z:\Work\protobuf-net-v2\protobuf-net\Meta\RuntimeTypeModel.cs:line 370
       at ProtoBuf.Meta.TypeModel.DeserializeCore(ProtoReader reader, Type type, Object value, Boolean noAutoCreate) in z:\Work\protobuf-net-v2\protobuf-net\Meta\TypeModel.cs:line 481
       at ProtoBuf.Meta.TypeModel.Deserialize(Stream source, Object value, Type type) in z:\Work\protobuf-net-v2\protobuf-net\Meta\TypeModel.cs:line 420
       at ProtoBuf.Serializer.Deserialize[T](Stream source) in z:\Work\protobuf-net-v2\protobuf-net\Serializer.cs:line 58
       at HelloProtoBuf.Program.Main() in C:\Work\VS\HelloProtoBuf\Program.cs:line 57
       at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
       at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
       at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
       at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean ignoreSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
       at System.Threading.ThreadHelper.ThreadStart()
  InnerException: 

What version of the product are you using? On what operating system?

v2, build 407

Please provide any additional information below.

Note, that AddSubType does respect the surrogates, i.e. if we change the code 
by commenting (*) and uncommenting (**) - the code will work as expected.

Original issue reported on code.google.com by mark.kha...@gmail.com on 13 Jun 2011 at 8:57

GoogleCodeExporter commented 9 years ago
I would not expect it to. In the scenario presented, it is serializing an I, 
based only on what it knows about I. And it knows *nothing* about I (it doesn't 
have any serialization members). You have supplied a default concrete 
implementation to use when the value is null, but that type is non-creatable.

Even if this worked, it would deserialize with N as 0. I can, however, perhaps 
fix the handling a *bit* such that it creates an OS, converts it to null, then 
applies the (non-existant) serialization rules of I to this new value.

Original comment by marc.gravell on 13 Jun 2011 at 10:11

GoogleCodeExporter commented 9 years ago
Marc, thanks for the details explanation, although I did not fully grasp it. As 
the user of the API, I do not quite understand the semantic difference between

m.Add(typeof(I), false).ConstructType = typeof(O);

and

m.Add(typeof(I), false).AddSubType(1, typeof(O));

Except, that there may be many subtypes, but only a single construct type and 
that these statements are mutually exclusive. 

If I am wrong (and I guess I am), then what is exactly ConstructType for?

Original comment by mark.kha...@gmail.com on 13 Jun 2011 at 11:50

GoogleCodeExporter commented 9 years ago
Maybe I'll rename it if it is confusing (with suitable [Obsolete(...)] etc).

The use-case there is when you want to specify a default implementation of an 
interface, most likely because it is interface based but you want to specify 
the actual model to use. The serialization is still based entirely off the 
interface in this case, it has simply managed to create an object.

The difference with AddSubType is that here the serializer is considering the 
subtypes *as active in the serialization*; for example, considering any 
serialization members specific to the concrete type (not necessarily specified 
on the interface).

Original comment by marc.gravell on 13 Jun 2011 at 12:24

GoogleCodeExporter commented 9 years ago
I can see that AddSubType and ConstructType are not mutually exclusive, meaning 
API allows to write something like this:

m.Add(typeof(I), false).AddSubType(1, typeof(O)).ConstructType = typeof(O2);

Is it meaningful?

Original comment by mark.kha...@gmail.com on 13 Jun 2011 at 12:34

GoogleCodeExporter commented 9 years ago
Yes; the `ConstructType` has a use in that when specified it will be used (in 
place of `I`) when no sub-type is specified (which is possible considering the 
two ends of data need not have exactly the same version, etc). If `I` is an 
abstract base-class or interface, or you simply have a preferred default 
implementation, then this should allow it to function as preferred.

Original comment by marc.gravell on 15 Jun 2011 at 10:24

GoogleCodeExporter commented 9 years ago
But you are going to serialize 'I' according to the signature of 'I', rather
than its ConstructType, which is different from the Subtypes, where you
serialize according to the particular subtype declaration, am I right?

Original comment by mark.kha...@gmail.com on 15 Jun 2011 at 10:56

GoogleCodeExporter commented 9 years ago
This only applies during deserialization, but yes; it will deserialize 
according to `I` rules.

Original comment by marc.gravell on 15 Jun 2011 at 11:33

GoogleCodeExporter commented 9 years ago
May I ask why? After all, you could deserialize according to the
ConstructType, respecting the surrogate logic, of course. Both approaches
are valid, aren't they? May be both should be supported?

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

GoogleCodeExporter commented 9 years ago
What data would it be deserializing? If the sender had knowingly serialized 
values from the concrete type, it would have to be mapped as a sub-type 
(=sub-message), otherwise there is a vast problem of field conflicts. To be 
encoded as a sub-message, I need a field-marker (the number in the AddSubType 
call). ConstructType is *only* intended for a default implementation of an 
otherwise abstract type.

If you need serialization *of that type* (which isn't the expected type, I), 
then it will need to be properly defined as a sub-type.

Original comment by marc.gravell on 15 Jun 2011 at 12:26

GoogleCodeExporter commented 9 years ago
I think I understand finally. protobuf-net must maintain the cross platform
nature of the protocol buffers specification. Meaning, the receiving end
must rely exclusively on the proto spec, which mandates that interface
serialized according to a concrete type must have this concrete type
described in the same proto spec and include a field number for it. Hence
ConstructType cannot be used. Is my understanding correct?

Original comment by mark.kha...@gmail.com on 15 Jun 2011 at 12:34

GoogleCodeExporter commented 9 years ago
The google spec *has no notion at all* of interfaces, concrete types, abstract 
types, inheritance, etc. I use a few tricks here to provide additional features 
*within* the spec, but my bigger point is simply around the intent of 
ConstructType. To provide safety from field collisions, I need some sub-message 
separation between data of different types (even if for the same object). 
ConstructType does not provide a sub-message, as it has no "field". If it *had* 
a field, it would be completely indistinguishable from AddSubType - so if that 
is the intent, just use AddSubType.

Original comment by marc.gravell on 15 Jun 2011 at 1:14