Support for abstract base classes

[Tornhoof]

Motivation: I'm trying to annotate my DTOs with the bond attributes instead of using those partial classes the compiler creates. Issue: One of my classes derives from an abstract base class with a shared property I want to annotate too Expected Result: No exception Actual result: Serialization fails with the attempt to create an abstract type:

System.MissingMethodException
Cannot create an abstract class.
   at System.RuntimeTypeHandle.CreateInstance(RuntimeType type, Boolean publicOnly, Boolean noCheck, Boolean& canBeCached, RuntimeMethodHandleInternal& ctor, Boolean& bNeedSecurityCheck)
   at System.RuntimeType.CreateInstanceSlow(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
   at System.Activator.CreateInstance(Type type, Boolean nonPublic)
   at System.Activator.CreateInstance(Type type)
   at Bond.Reflection.GetDefaultValue(ISchemaField schemaField) in S:\bondlab\nuget\bond\cs\src\core\Reflection.cs:line 983
   at Bond.Schema`1.Cache.GetDefaultValue(ISchemaField schemaField) in S:\bondlab\nuget\bond\cs\src\core\Schema.cs:line 178
   at Bond.Schema`1.Cache.<>c.<GetFields>b__7_0(ISchemaField field) in S:\bondlab\nuget\bond\cs\src\core\Schema.cs:line 167
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at Bond.Schema`1.Cache..ctor(Type type) in S:\bondlab\nuget\bond\cs\src\core\Schema.cs:line 69
   at Bond.Schema`1..cctor() in S:\bondlab\nuget\bond\cs\src\core\Schema.cs:line 37

I'm not using any default values and then you apparently create an instance of the type to get the default value. Which is fine, but you create an instance of the declaring type which is in this case the abstract base class, obviously this does not work. This is always wrong in my opinion, because you can always override the property's default value in the constructor of the derived type. You should create the derived derived instead. I can't think of any valid scenario where you want to create the base type instead of the derived type.

Repro: https://gist.github.com/Tornhoof/ca88705265266725deba7a24044bd692

[chwarr]

Thanks for the minimal repro and excellent bug report.

Briefly, this is happening due to the recursive way that serialization/deserialization is implemented. When creating the serializer/deserializer, bases are handled by recursively invoking the creation of a serializer/deserializer. There's currently no handling for abstract bases, as the Bond type system--as opposed to the C# type system--has no notion of abstract structs. So, that's the bug. On to the fix...

Can you explain in some more detail how you'd like to use abstract bases if they someone worked? I'm particularly interested in how you'd like to handle deserialization.

Abstract base structs get pretty philosophical pretty quickly. I've got some thoughts about them, but I need to write them up more coherently, and seeing how you'd envision using them will help.

Thanks.

[Tornhoof]

The reason for the abstract classes is in this case is a rather simple one, for our event sourcing system I'm trying to evaluate if it's possible to replace the current json storage format with a more concise one, simply to reduce the overhead and the bond binary format is roughly half the size. So I evaluated protobuf, wire, avro and now bond and my changes to our codebase were as minimal as possible, so I was stuck with the abstract base class. As previously mentioned, I'm not defining a new transfer schema, but instead trying to reuse the existing objects. Back to solving the problem: I totally understand that bond has no concept of abstract classes, so the fix is only for the edge case usage (e.g. not using the bond compiler), assuming the creation of the abstract class for both serialization and deserialization is the only problem, which it looks like. We also can't change the behaviour of the default value logic as this would be a breaking change.

1. We can add a new bond attribute which then bypasses the Activate.CreateInstance to get the default value, you would annotate the default constructor of your class and the check would simply skip the last part of the 'Bond.Reflection.GetDefaultValue' method then.
2. We can use a more elaborate approach to find out if a constructor has side effects, Roger Alsing's Wire protocol for Akka.net solved that by looking at the il bytes of the constructor (if the length of the il bytes is <=8 bytes the constructor is "empty", see https://rogeralsing.com/2016/08/16/wire-writing-one-of-the-fastest-net-serializers/ (fast creation of empty objects for more details). Again you would skip the last part of the 'Bond.Reflection.GetDefaultValue' method.

Especially the second solution assumes that side-effect free constructors are the normal case (which probably it is, especially for transfer objects), so you can only use abstract base classes with empty constructors. I think this is a valid limitation we can accept. The second solution is the easier one for the user as he does not need to understand what an empty constructor is and how to annotate them. The IL size method might collide with certain code quality rules though, that's why I thought about the attribute solution.

Again this is only for the edge case where you don't want to use the bond compiler, but instead annotate your classes manually, Marc Gravell's protobuf-net is fairly successful with this approach, so I tried the same with bond.

[chwarr]

Thanks for the details and the pointers. I'm in the middle of some higher priority work, so it will be a few days before I can get back to you.

[ia64mail]

Is there any progress so far? I've faced with the same problem in my project.

[chwarr]

Nothing to report.