jefffhaynes / BinarySerializer

A declarative serialization framework for controlling formatting of data at the byte and bit level using field bindings, converters, and code.
MIT License
292 stars 62 forks source link

Problem updating to new version #136

Closed ricebus closed 5 years ago

ricebus commented 5 years ago

Hi, I'm upgrading from 7.5.7 to 8.4.2 but now something's broken.

{System.InvalidOperationException: There was an error reflecting member 'Fields' ---> System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: Lists must define one and only one generic argument in the list object hierarchy.
   at BinarySerialization.Graph.TypeGraph.ListTypeNode.GetChildType()
   at BinarySerialization.Graph.TypeGraph.CollectionTypeNode.Construct()
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes, StackCrawlMark& stackMark)
   at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at BinarySerialization.Graph.TypeGraph.ContainerTypeNode.GenerateChild(Type parentType, MemberInfo memberInfo)
   --- End of inner exception stack trace ---
   at BinarySerialization.Graph.TypeGraph.ContainerTypeNode.GenerateChild(Type parentType, MemberInfo memberInfo)
   at System.Linq.Enumerable.SelectEnumerableIterator`2.ToArray()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.OrderedEnumerable`1.ToList()
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at BinarySerialization.Graph.TypeGraph.ObjectTypeNode.GenerateChildren(Type parentType)
   at BinarySerialization.Graph.TypeGraph.ObjectTypeNode.Construct(Boolean isSubType)}

This is the code it's refering to:

        [FieldOrder(8)]
        [FieldLength("Size", ConverterType = typeof(HeaderSizeToFieldLengthConverter), BindingMode = BindingMode.OneWay)]
        public ObservableCollection<NcpField> Fields { get; set; }

No idea what's wrong. Help? :)

jefffhaynes commented 5 years ago

Can you post enough code to reproduce? Thanks

ricebus commented 5 years ago

https://github.com/ricebus/NcpSharp/archive/master.zip

Thats a lot of code, but basically I run IqExample. The serializer itself is under NcpSharp.Base

jefffhaynes commented 5 years ago

I don't think I have access to it but let me try something on my end first...

ricebus commented 5 years ago

you're right, it's because the rep is private. try this: http://s000.tinyupload.com/index.php?file_id=82276215263186930404

jefffhaynes commented 5 years ago

So the issue is with the ObservableCollection but the reason is a little complicated. In general I would say you should separate UI stuff from protocol stuff but here's the issue and I'm open to suggestions for trying to address it:

Basically when serializing a list of something the serializer looks at the list of generic arguments and says, ok, what is this a list of? In this case it sees that the collection contains a list of NcpField items. However, then it looks at the base of ObservableCollection, which is Collection and which is itself a generic type. In this case Collection also defines a generic type of NcpField. Unfortunately, the serializer now has no way of knowing if that's the same generic type defined in the derived class, because that's an implementation detail. So at this point it either has to assume that it is the same type or bail and say, I can't resolve this.

If we assume that they are in fact the same type then it does solve your issue but it requires making a somewhat unfounded assumption in the serializer. However, if using List instead of ObvservableCollection is an option for you I would encourage you to consider that as it's probably the cleaner solution.

ricebus commented 5 years ago

1) Why did it work before? 2) I'm using ObservableCollection not for UI purpose but so that I can recalculate Checksum/Size automatically for the packets... 3) Maybe some flag to make it an explict decision?

jefffhaynes commented 5 years ago

BinarySerializer handles checksums now so you could consider that :)

At some point I believe I added the check because examples were cropping up where it was impossible to deduce intent. Again, the use of generic arguments is really a bit of a trick anyway where the serializer is assuming you mean a collection of items of type X. Assuming anything beyond that is starting to get a bit dodgy. I suppose the counterpoint is that we're already assuming something anyway...

Let me think about it.

ricebus commented 5 years ago

How about length/size calculation? it's not quite the same as checksum. I need the generics mainly for that. I want to be able to add members to the List and the packet length gets updated automatically..

jefffhaynes commented 5 years ago

Can you use a FieldLength binding?

ricebus commented 5 years ago

I thought that only works one way on deserialization no? and there's no way to put converters on it

jefffhaynes commented 5 years ago

It works in both directions and you should be able to use converters, yes

jefffhaynes commented 5 years ago

On serialization it will compute the length and assign that to the source field

ricebus commented 5 years ago

You know what, let me check. I did that a long time ago and I think I had a good reason back then.. Let me try to brute force the s**t out of this

jefffhaynes commented 5 years ago

Ok let me know if I can help

ricebus commented 5 years ago

My hierarchy is NcpPacket contains several NcpFields which in turn contains several NcpParameters.

        public object ConvertBack(object value, object parameter, BinarySerializationContext context)
        {
            // DURING SERIALIZATION
            uint size = NcpPacket.HEADER_SIZE + NcpPacket.FOOTER_SIZE;

            NcpPacket packet = (NcpPacket)context.Parent;

            foreach (NcpField field in packet.Fields)
                size += field.Size;

            return size;
            //throw new NotImplementedException();
        }

field.Size is always zero, are you sure it's getting updated?

ricebus commented 5 years ago

Oh, now I understand, you're not actually updating the size fields when you calculate them, so the values aren't saved anywhere but the serializer, right?

jefffhaynes commented 5 years ago

Correct. The first thing the serializer does is create a copy of your object graph. The original graph is kept completely intact.

ricebus commented 5 years ago

So it's not the same behavior as an ObservableCollection. I made it work by updating manually on ConvertBack.

jefffhaynes commented 5 years ago

It's not the same but if you're using the checksum as part of the serialization then it should work. If you're using the checksum for something else then it isn't part of serialization and I'll go back to my original statement that you should probably consider separating the concerns.

jefffhaynes commented 5 years ago

ok, upon further reflection I decided there was no real reason to prevent this. fixed in 8.4.3