RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.81k stars 1.3k forks source link

Generate a Rest Client with XML Deserialization and not JSON #2082

Open kchtun opened 5 years ago

kchtun commented 5 years ago

I am interfacing with an "external web API server" that returns only xml content-type. I have generated a rest client with NswagStudio but an exception is always thrown because of the response type which is XML and the deserialisation type which is JSON.

Is there any way to generate a client with XML Deserialisation?

RicoSuter commented 5 years ago

The client is mostly optimized for json. I think it does not yet support xml serialization...

DanielSmon commented 5 years ago

Hey guys, I'm facing this same issue at the moment as well. I'm running on dotnet core (still only 2.1).

While researching my options I took a look at the various csharp client generators in openapi-generator as well, but they don't support XML either. The openapi-generator also don't parse Enums nicely (they come out as string properties), so NSwag is a winner here for me as it generates proper enums. There are some other differences but from what I can tell NSwag should meet our requirements once I switch over the seralisation to XML.

I did a few quick hacks on a generated client to see what's involved in adding XML support after generation. In my case I know I won't need to regenerate my client for a while. So I'm weighing up whether I just continue with the XML conversion in my generated client, or whether I start implementing XML support in a fork of NSWag. Unfortunately I'm pretty time-poor right at the minute.

At this stage I think I might do both (manually update the client, and start on XML support). But the XML support will be slow (until it becomes a priority :) ).

In terms of implementation, I think what would be nice to support both XML and JSON out of the box and use standard Content negotiation to determine which serialisation to use at runtime. The default (preferred) content type could therefore be set to application/json by putting it first in the list of Accept header values.

One other thing worth mentioning is the use of System.Collections.Generic.ICollection<T> which the XmlSerializer complains about. It throws an System.NotSupportedException for interfaces so I ended up changing the property definitions to use the concrete implementation of System.Collections.ObjectModel.Collection<T> in my generated client instead of interfaces. This is probably the biggest hurdle to XML as it would be a fairly major change. @RicoSuter, what are your thoughts on this if we were to proceed?

Result StackTrace:  
at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel model, String ns, ImportContext context, String dataType, XmlAttributes a, Boolean repeats, Boolean openModel, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportElement(TypeModel model, XmlRootAttribute root, String defaultNamespace, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(Type type, XmlRootAttribute root, String defaultNamespace)
   at System.Xml.Serialization.XmlSerializer..ctor(Type type, String defaultNamespace)
   at System.Xml.Serialization.XmlSerializer..ctor(Type type)
   at MyProject.UnitTests.MyClient.erializationTests.CanSerialize() in C:\source\repos\MyProject.UnitTests\SerializationTests.cs:line 34
--- End of stack trace from previous location where exception was thrown ---
----- Inner Stack Trace -----
   at System.Xml.Serialization.XmlReflectionImporter.InitializeStructMembers(StructMapping mapping, StructModel model, Boolean openModel, String typeName, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportStructLikeMapping(StructModel model, String ns, Boolean openModel, XmlAttributes a, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel model, String ns, ImportContext context, String dataType, XmlAttributes a, Boolean repeats, Boolean openModel, RecursionLimiter limiter)
----- Inner Stack Trace -----
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel model, String ns, ImportContext context, String dataType, XmlAttributes a, Boolean repeats, Boolean openModel, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportAccessorMapping(MemberMapping accessor, FieldModel model, XmlAttributes a, String ns, Type choiceIdentifierType, Boolean rpc, Boolean openModel, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportFieldMapping(StructModel parent, FieldModel model, XmlAttributes a, String ns, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.InitializeStructMembers(StructMapping mapping, StructModel model, Boolean openModel, String typeName, RecursionLimiter limiter)
----- Inner Stack Trace -----
   at System.Xml.Serialization.StructModel.CheckSupportedMember(TypeDesc typeDesc, MemberInfo member, Type type)
   at System.Xml.Serialization.StructModel.GetPropertyModel(PropertyInfo propertyInfo)
   at System.Xml.Serialization.StructModel.GetFieldModel(MemberInfo memberInfo)
   at System.Xml.Serialization.XmlReflectionImporter.InitializeStructMembers(StructMapping mapping, StructModel model, Boolean openModel, String typeName, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportStructLikeMapping(StructModel model, String ns, Boolean openModel, XmlAttributes a, RecursionLimiter limiter)
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel model, String ns, ImportContext context, String dataType, XmlAttributes a, Boolean repeats, Boolean openModel, RecursionLimiter limiter)
----- Inner Stack Trace -----
Result Message: 
System.InvalidOperationException : There was an error reflecting type 'MyProject.MyClient.WorkspaceCreationRequest'.
---- System.InvalidOperationException : There was an error reflecting property 'PartyDetails'.
-------- System.InvalidOperationException : There was an error reflecting type 'MyProject.MyClient.PartyDetails'.
------------ System.InvalidOperationException : Cannot serialize member 'MyProject.MyClient.PartyDetails.Party' of type 'System.Collections.Generic.ICollection`1[[MyProject.MyClient.Anonymous12, MyProject.MyClient, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]', see inner exception for more details.
---------------- System.NotSupportedException : Cannot serialize member MyProject.MyClient.PartyDetails.Party of type System.Collections.Generic.ICollection`1[[MyProject.MyClient.Anonymous12, MyProject.MyClient, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]] because it is an interface.

Cheers!

DanielSmon commented 5 years ago

Just reading #2036. Perhaps an extensible Input/Output formatter style approach similar to ASP.NET Core would be a better approach here. That pattern is widely understood as well.

Then custom formatters could be registered via the partial implementation.

RicoSuter commented 5 years ago

There are settings to change ICollection to Collection...

DanielSmon commented 5 years ago

Thanks, that will simplify things! Will RTFM and see what else I'm missing :)

RicoSuter commented 5 years ago

That was not my final answer to this issue... will add more soon 🙂

RicoSuter commented 5 years ago

There are many dimensions to this issues:

  1. NJsonSchema.Generation: Generate XML information when generating JSON Schema/Swagger specs - @emilw did a lot in this area and I think this should be completed: https://github.com/RicoSuter/NJsonSchema/pulls?q=is%3Apr+is%3Aclosed+author%3Aemilw
  2. NJsonSchema.CodeGeneration.CSharp: Generate XML annotations (e.g. XmlElementAttribute, etc.) in the CSharp DTO generator based on the XML information in the schemas - not sure whether this is already there - we need to check that...
  3. NSwag.CodeGeneration.CSharp: When a response/request is not JSON but XML we need to use the XML serializer in the generated code to serialize/deserialize (which will pick up the attributes)
    • Maybe set the Accept header so that the correct/expected format is received (prefer JSON, fallback to XML - because XML support is probably not that good in Swagger/OpenAPI)
    • At the moment we have something like
if JSON then code for JSON 
else FileResponse

we need to change the output to

if JSON then code for JSON 
else if XML then code for XML 
else FileResponse

I think/hope this issue is only about 3. and the rest is already implemented...

Just reading #2036. Perhaps an extensible Input/Output formatter style approach similar to ASP.NET Core would be a better approach here. That pattern is widely understood as well.

I think we shouldn't implement this externally (as proposed in #2036) but the generated clients should be completely self-contained and not require external code - #2036 would only be needed for special customizations (e.g. MessagePack serialization which is not directly supported by Swagger) and/or to make the generated clients easier to understand (but still generating a default implementation).

What do you think?

emilw commented 5 years ago

Hi, The background for only doing "half" of the implementation e.g. supporting C# to Swagger spec was to be able to handle "legacy" middlewares that only could handle XML but still could consume the Swagger spec. This is quite common as many enterprises still sits on old middlewares where XML still is king. In my head i did not see the use case where the server would generate a Swagger spec that only supported XML, so can you add some additional background to that @DanielSmon ? What servers/rest/http-rpc:s do you have in mind that do not support JSON?

@RicoSuter , regarding your bullet number 1 i see it as finalized for C# to Swagger. It's the other way that is not done in my head.

DanielSmon commented 5 years ago

Hey guys, thanks for the replies.

@RicoSuter, your points 2 and 3 look good to me - that's what I had to do to hack my generated client to get it to work as a test.

I understand and agree that generated clients should be self-contained. I was merely thinking of a change to make it a bit more extensible so that if someone needs some other serialiser, they're free to implement it without having to modify the generator further. So for example, instead of this:

if JSON then code for JSON 
else if XML then code for XML 
else FileResponse

we were to do something like this:

formatter = select formatter for current content-type
if formatter is found, use formatter
else FileResponse

Then we'd just move the "code for JSON" and "code for XML" to be an implementation of the formatter interface. That way, generated clients would have the JSON and XML formatters loaded by default so the out of box experience would "just work" for JSON and XML. I..e they wouldn't require external code.

But if someone wanted another formatter for another content type, they could implement their own formatter and load it somewhere using the partial class so it would persist when the client is re-generated. The only problem with this approach is if people need additional attributes for their custom formatter, but I think that's outside of the scope of this (at least there's an entrypoint to have a formatter and they can perform whatever logic they need in there). It's only a little more complex than just adding if JSON, else if XML..., but lets people extend it more easily for edge cases.

@emilw, yeah unfortunately I'm integrating with a 3rd party that has implemented their API just like that. They only support XML, but thankfully have an OpenAPI spec (complete with Swagger UI). It's a financial/related institution to do with property transactions here in Australia. I don't know why they decided to implement it like this, but they have! :)

In fact, their OpenAPI spec is in YAML, so I have to convert it to JSON because most tooling doesn't play nicely with YAML out of the box. That reminds me, I think NSwag is meant to support YAML but I couldn't get it to work. I'm not terribly fussed about that though and I haven't had time to investigate which is why I haven't raised an issue here yet. Plus there are some other minor manual tweaks I have to make to their YAML to get it to work anyway (a few typos and inaccessible external references). Overall it's pretty good though - certainly far better than not having the spec!

DanielSmon commented 5 years ago

Hi guys,

Sorry for the absence. I haven't forgotten about this as we still have this requirement. It's just a bit lower on our priority list right now. I might have some help soon though. So will let you know when we are able to make a proper start on this.

Cheers, Daniel

MikaelUmaN commented 5 years ago

In our org we have a requirement to use more sophisticated serialization techniques such as msgpack and compression for performance.

So I would also love an extensible solution where the client comes with json as default, but then enable you to add options regarding content, very much like AspNetCore does.

As it stands now, I would probably generate a client using nswag and then either find a way to create a script that does replaces json with msgpack or hand-code the changes.

RicoSuter commented 5 years ago

ATM the easiest solution would be to replace this template:

https://github.com/RicoSuter/NSwag/blob/master/src/NSwag.CodeGeneration.CSharp/Templates/Client.Class.ReadObjectResponse.liquid

RicoSuter commented 5 years ago

The final solution would be to

RicoSuter commented 5 years ago

Create a PR: https://github.com/RicoSuter/NSwag/pull/2248

Please review.

anth12 commented 5 years ago

I would like to see support for any serialization/transport type; json, xml, protobuf, message pack... I'm after a swagger client generator that can handle protobuf & json...

Akorna commented 4 years ago

@RicoSuter Is there any news for the open PR?

janseris commented 10 months ago

Hi, any news? I need XML for cases when dictionary key is a class