Switch out TypeConverter to allow serialization.

[JimBobSquarePants]

If we attempt to serialize a class using JSON.NET that has a custom TypeConverter bound the serialization will fail. This can have serious implications when using ApiControllers. See #124

This pull request switches out the TypeConverter base class and TypeConverterAttribute class for a custom IDittoTypeConverter and DittoTypeConverterAttribute types.

Unfortunately it seems that we will not be able to maintain backwards compatibility.

Answers on a postcard!

[leekelleher]

@JimBobSquarePants I'll review this tomorrow, (as I'm at a meetup tonight). I have a couple of ideas that I'd like to try out.

[JimBobSquarePants]

Great. Looking forward to seeing what you can come up with.

[JimBobSquarePants]

Looking at @walleyuan's comment and blog for issue #124 It looks like I can scrap all this. The following code allows full serialization/deserialization.

IEnumerable<Type> registerTypes = PluginManager.Current.ResolveTypes<MYBASECLASS>();

foreach (Type type in registerTypes)
{
    // Ensure that Json.NET can serialize/deserialize our types.
    TypeDescriptor.AddAttributes(type, new TypeConverterAttribute(typeof(JsonConverter)));
}

[leekelleher]

I didn't get chance to look at this yet.

My understanding was that whenever someone (library or dev) wanted to find an associated TypeConverter for an object-type, they'd call TypeDescriptor.GetConverter, which would look for the TypeConverterAttribute (or if implemented ICustomTypeDescriptor - gets the details from there).

Since within Ditto we don't use TypeDescriptor.GetConverter, we check for the TypeConverterAttribute directly, (see here), we could introduce our own DittoTypeConverterAttribute. This would do exactly the same as TypeConverterAttribute, but standalone, e.g. .NET or other code libraries would be aware of it and try to associate the custom TypeConverter.

This way we wouldn't need to introduce our own IDittoTypeConverter, we'd still be using regular .NET TypeConverter classes, just having an additional way to register them with an object-type.

Does that make sense? Any flaws to the idea?

[leekelleher]

@JimBobSquarePants The unit-test that you added CanSerializeModelWithTypeAttribute, I've tried adding it to the develop branch (locally) and it passes fine.

I'm wondering if we can come up with a unit-test that fails the JSON serialization? Then we can work out a fix from that.

[JimBobSquarePants]

Yeah, I'll have a go.

[leekelleher]

Hey @JimBobSquarePants, how would you like to proceed on this one?

[leekelleher]

@JimBobSquarePants I'm closing this one off. I wont delete the branch yet, just in case you wanted to revisit it?