Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for F# types in XmlSerializer #2767

Open
cpx86 opened this issue Jun 29, 2015 · 7 comments
Open

Support for F# types in XmlSerializer #2767

cpx86 opened this issue Jun 29, 2015 · 7 comments
Milestone

Comments

@cpx86
Copy link

cpx86 commented Jun 29, 2015

Recently I've experimented with using NServiceBus from F#, and intuitively, it feels like a very nice combination. There are however, some limitations currently which makes the code less idiomatic. One of these is the serialization of F# records and discriminated unions.

The BinaryFormatter and Json.NET >= 6.0.3 have support for this out-of-the-box; the XmlSerializer however does not. I've written some proof-of-concept code in NServiceBus.Core and so far it appears that this would be relatively trivial to implement. Mostly it just needs to traverse the type definitions in a slightly different way, by checking for a custom attribute and using non-public fields. Records and unions have a very distinct "signature" compared to regular classes, so I think it could be implemented without making a big mess of the "normal"/C# serialization code.

Before I delve any further into implementing this fully though, I just want to check: Is this something that could be of interest to pull into the core library? I can think of several reasons personally why I would like it, but maybe there are good reasons for why it shouldn't be done (or hasn't already). :)

@cpx86
Copy link
Author

cpx86 commented Jul 7, 2015

Impatience and vacation prompted me to go ahead and implement support for record and union types :)
https://github.com/cpx/NServiceBus/commit/fe3ea49fd9d070124bd7dd633a03535fa0e2e7e7 (based on master)
https://github.com/cpx/NServiceBus/commit/0a38d984ca6c138795733aebc4b702ecb0409f62 (based on develop)

The implementation uses the F# reflection API for inspecting and constructing F# types where possible (my initial approach was to only use standard .NET reflection but it quickly got messy). In the serializer and deserializer, I've basically just added two new special cases (IsRecord and IsUnion) that are executed before falling back to treating the data/object as a regular C# class

Tests are written in a separate F# assembly and covers serialization and deserialization separately. I thought this made for a pretty nice approach compared to serializing and deserializing in the same test since it allows you to see what the XML should look like for each test case.

I have NOT performance tested this yet, since it was all written on my laptop. I don't think it will have any big impact, but I'll do a perf test once I have access to my regular dev box.

@danielmarbach
Copy link
Contributor

@cpx Thanks for your effort. How about providing it as a community package as a plugin serializer?

@cpx86
Copy link
Author

cpx86 commented Jul 15, 2015

@danielmarbach That is an option I considered at first - however, I feel that it would pretty much defeat the purpose. As a community package, the serializer would leave no guarantees of being compatible with the built-in XmlSerializer even for normal C# types. So for example if you have an existing solution using the XmlSerializer you would probably want to make that switch across all your buses; and most people, unless they have a huge love for XML, would probably just switch to a Json.NET-based serializer instead of some potentially crummy, third-party, custom-written XML serializer.

From a different perspective, F# is a "first-class" language in the CLR, that is becoming quite popular, so ensuring NServiceBus can be used from both C# and F# makes sense, I think.

@danielmarbach
Copy link
Contributor

@Particular/nservicebus-maintainers Should we discuss this? Since this hinders adoption for FSharp users. The drawback of the proposed solution is that it references FSharp.Core which has major implications on all our users.

@timbussmann
Copy link
Contributor

I'm 👍 for F# support but not if it requires us to reference FSharp.Core, then it needs to be handled in a dedicated package. Given the support of multi-deserializers in v6, it doesn't seem too dramatic to use the json.net serializer as a "workaround". I don't think this has a priority which would make that package happen anytime soon, so I'd leave that to the community for now. (+1 on closing this)

@bording
Copy link
Member

bording commented Mar 11, 2016

I took a quick look at how Json.NET added support for discriminated unions, since it doesn't have a reference to FSharp.Core.

The general approach appears to be that it has some special code to figure out if the type being serialized/deserialized is a discriminated union, and if it is, it assumes that FSharp.Core has been referenced by the calling code. It then goes and loads the assembly via reflection to get to everything it needs from it.

Doing something similar would let us support it in our XmlSerializer without needing to reference FSharp.Core. I don't see us doing this directly, but what if we received a PR for it that modeled the approach Json.NET took?

@danielmarbach
Copy link
Contributor

If we can do this in a performant and backwards compatible way I'll be the last stopping the line on this one. So then 👍 on bringing this in by community PR.

@andreasohlund andreasohlund modified the milestone: Future Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants