AqlaSolutions/AqlaSerializer

Virtual members of AutoAddStrategy should be called only when it makes sense

Closed this issue · 7 comments

Hello!

The exception is thrown when I try to serialize 2D array (in that case it's double[,]):

Data of this type has inbuilt behaviour, and cannot be added to a model in this way: System.Double
Stacktrace:

>	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.FindOrAddAuto(System.Type type, bool demand, bool addWithContractOnly, bool addEvenIfAutoDisabled, out AqlaSerializer.Meta.MetaType metaType) Line 429	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.FindOrAddAuto(System.Type type, bool demand, bool addWithContractOnly, bool addEvenIfAutoDisabled) Line 405	C#
 	aqlaserializer.dll!AqlaSerializer.AutoAddStrategy.ApplyDefaultBehaviour(AqlaSerializer.Meta.MetaType metaType) Line 294	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.MetaType.ApplyDefaultBehaviour() Line 137	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.FindOrAddAuto(System.Type type, bool demand, bool addWithContractOnly, bool addEvenIfAutoDisabled, out AqlaSerializer.Meta.MetaType metaType) Line 498	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.FindOrAddAuto(System.Type type, bool demand, bool addWithContractOnly, bool addEvenIfAutoDisabled) Line 405	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.GetKey(System.Type type, bool demand, bool getBaseKey) Line 895	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.RuntimeTypeModel.GetKeyImpl(System.Type type) Line 885	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.TypeModel.GetKey(ref System.Type type) Line 1120	C#
 	aqlaserializer.dll!AqlaSerializer.Meta.TypeModel.SerializeWithLengthPrefix(System.IO.Stream dest, object value, System.Type type, AqlaSerializer.PrefixStyle style, int fieldNumber, AqlaSerializer.SerializationContext context) Line 713	C#

Any idea how it can be fixed?

Regards!

@aienabled, are you using Compatibility mode? Can you provide a test? I can't reproduce this.

@aienabled I pushed a possible fix though I still can't reproduce it. Please check it.

@AqlaSolutions, thank you for the quick reply!
No, I'm not using Compatibility mode.

It seems the issue happens because I'm using a custom AutoAddStrategy with custom CanAutoAddType(type) implementation. It was returning true for every value type, including double. So the serializer invoked CanAutoAddType(type), received true and the code went to the exception throwing section.

I was able to fix it by modifying CanAutoAddType(type) override to return false for every type.IsPrimitive.

I think to prevent this issue it might be better to check type.IsPrimitive before calling AutoAddStrategy.CanAutoAddType(type) because this call doesn't make any sense for the primitive types.

Regards!

@aienabled, you can make your custom AutoAddStrategy to check what the default AutoAddStrategy.CanAutoAddType implementation returns (possible only when it encounters unknown namespace?).

Also you can check RuntimeTypeModel.CheckTypeCanBeAdded, it doesn't use AutoAddStrategy logic. In the latest commit I modified CheckTypeCanBeAdded to specially check for builtin types.

Like

if (!RuntimeTypeModel.CheckTypeCanBeAdded(_model, type)) return false;
if (!type.Namespace.StartsWith("aienabled.Project") && !base.CanAutoAddType(type)) return false;
// return ...your logic...

@aienabled, ok, I'll change it to ensure that virtual members of AutoAddStrategy are called only when it makes sense. But for plain interface IAutoAddStrategy (without inheriting AutoAddStrategy) it's on your side.

fixed in master

Thank you! The fix works correctly.