swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.8k stars 6.02k forks source link

[CSharp Client] does not support Content-Types "application/soap+xml" and "application/xml" #6591

Open dnn01 opened 6 years ago

dnn01 commented 6 years ago
Description

I generated a CSharp client from the attached Json file and the content type is correctly identified as application/soap+xml. However, the client does not appear to support this content type.

To perform the XML POST, the call to serialize the body is:

       if (body != null && body.GetType() != typeof(byte[]))
       {
           localVarPostBody = Configuration.ApiClient.Serialize(body); // http body (model) parameter
        }
       else
       {
           localVarPostBody = body; // byte array
       }

The serialize method in the API client is:

public String Serialize(object obj)
        {
            try
            {
              return obj != null ? JsonConvert.SerializeObject(obj) : null;
            }
            catch (Exception e)
            {
                throw new ApiException(500, e.Message);
            }
        }

In the prepare request method:

if (postBody != null) // http body (model or byte[]) parameter
            {
                if (postBody.GetType() == typeof(String))
                {
                   request.AddParameter("application/json", postBody, ParameterType.RequestBody);
                }
                else if (postBody.GetType() == typeof(byte[]))
                {
                    request.AddParameter(contentType, postBody, ParameterType.RequestBody);
                }
            }

            return request;
Swagger-codegen version

Version 2.2.3 (swagger-codegen-cli-2.2.3.jar) Also tried Version 2.3.0 (swagger-codegen-cli-2.3.0-20170923.081757-145.jar) with the same result

Swagger declaration file content or url

The json file is: (Log4NetService.zip)

Note: this Swagger definition was created from the WSDL of the backend service using apiconnect-wsdl

Command line used for generation

java -jar swagger-codegen-cli-2.2.3.jar generate -i Log4NetService.json -l csharp

Steps to reproduce
  1. Generate the Csharp code using the json file attached
  2. Navigate to the method AppendLoggingEventWithHttpInfo() in the Api.DefaultAPI class
  3. Note that the call localVarPostBody = Configuration.ApiClient.Serialize(body) to the Serialize method does not pass in content type, even though it is available in localVarHttpContentType
  4. Navigate to the the method Serialize() in the Client.ApiClient class
  5. Note that the method only supports json
Related issues/PRs

Default Java client does not support Content-Type "application/xml": https://github.com/swagger-api/swagger-codegen/issues/3870

wing328 commented 6 years ago

Java client supports XML via the following option:

    withXml
        whether to include support for application/xml content type and include XML annotations in the model (works with libraries that provide support for JSON and XML) (Default: false)

We can work with you to add the XML support to C# API client.

dnn01 commented 6 years ago

Thank you, that sounds great. What are our next steps?

wing328 commented 6 years ago

Here are a few good starting points:

https://github.com/swagger-api/swagger-codegen/blob/3cb36738b1789ccf99082fa6ee3f4df2e491bdee/modules/swagger-codegen/src/main/resources/csharp/ApiClient.mustache#L374-L378 needs to be updated as we now assume the payload must be a JSON string.

Same for ApiClient.mustache#L392-L395 as the object needs to be serialized into XML instead of JSON.

wing328 commented 6 years ago

Doing a search for "withXML" in https://github.com/swagger-api/swagger-codegen/blob/6e7ad13e1b327b45ce9cdafdfb6c1b46fe8b8b32/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractJavaCodegen.java#L49 with show you the code on how to add the "withXML" CLI option to a generator.

dnn01 commented 6 years ago

I searched all files within the folder swagger-codegen\modules\swagger-codegen\src\main for the string "withXml" and (in addition to AbstractJavaCodegen.java) found it in several files, but can't figure out how swagger-codegen\modules\swagger-codegen\src\main\resources\Java\ApiClient.mustache, reads the withXml flag from AbstractJavaCodegen.java. I am inexperienced in Mustache and have limited Java experience, which could be why. I do see withXml used in swagger-codegen\modules\swagger-codegen\src\main\resources\Java\libraries\resttemplate\ApiClient.mustache, but can't make the connection as to what happens with that.

Can you please point me in the right direction?

Thanks.

wing328 commented 6 years ago

@dnn01 I've added the "withXml" option to the branch "csharp_with_xml" (https://github.com/swagger-api/swagger-codegen/tree/csharp_with_xml)

Please check it out when you've time.

dnn01 commented 6 years ago

@wing328,

Here's what I did:

  1. Built the csharp_with_xml branch using mvn clean package
  2. Grabbed a copy of modules\swagger-codegen-cli\target\swagger-codegen-cli.jar
  3. Ran the commandjava -jar swagger-codegen-cli.jar generate -i swagger.json -l csharp --additional-properties withXml=true

Please let me know if I missed anything.

I do not see any XML support in the APIClient and I believe that is because that support needs to be added to APIClient.mustache, correct?

Thanks

wing328 commented 6 years ago

Yup, you will need to use {{#withXml}} ... {{/withXml}} or {{^withXml}} ... {{/withXml}} in the mustache template to add new code on handling XML payload.

jimschubert commented 6 years ago

@wing328 doesn't that ({{#withXml}} ... {{/withXml}} or {{^withXml}} ... {{/withXml}}) generate a JSON or XML client? I would think it would make more sense to support this by default based on the application/xml content-type header, and change the client code to a switch. For instance.:

var contentType = // get response.Headers.ContentType or whatever
// at this point, it must be a model (json)
try
{
    switch(contentType) {
       case "application/xml":
            XmlDocument doc = new XmlDocument();
            doc.LoadXml(response.Content);
            var json = JsonConvert.SerializeXmlNode(doc, Formatting.None, true);
            return JsonConvert.DeserializeObject(json, type, serializerSettings);
       break;
       default:
             return JsonConvert.DeserializeObject(response.Content, type, serializerSettings);
    }
}
catch (Exception e)
{
    throw new ApiException(500, e.Message);
}

That said, I seem to recall there being issues with JsonConvert.SerializeXmlNode requiring some additional annotation that I can't remember off the top of my head (probably related to xml attributes).

More information on this is available here: https://www.newtonsoft.com/json/help/html/ConvertingJSONandXML.htm

We could also look into Web API's content negotiator, and see if it's reusable or if there's an alternative serialization framework that supports many content types.

wing328 commented 6 years ago

I would think it would make more sense to support this by default based on the application/xml content-type header, and change the client code to a switch. For instance.:

Thanks for bringing that up. I thought about that too. My assumption is that the API client needs to deal with either JSON or XML payload but not both at the same time. I also recall seeing some feedbacks about avoiding unnecessary code in the auto-generated API clients so some developers may not want to see XML-related code if they only need to deal with JSON in their APIs.

wing328 commented 6 years ago

@dnn01 just checking. Do you need anything from us (the community) to add the XML support?

jimschubert commented 6 years ago

@wing328 ah, yeah if it's a matter of keeping the generated code lean, that makes sense. However, it's common for APIs to return json and XML from endpoints so I think we need both at the same time.

We could use the withXml block and just put an if statement with an early return. I don't think we'd need a negated block.

dnn01 commented 6 years ago

@wing328 Yes, I could use some help. The snag I ran into is that the services behind the Swagger API are SOAP and I need to serialize to a SOAP-encoded format.

I tried SoapFormatter, but that appears to be deprecated. Additionally, it does not support generic types. Another option I tried was using XMLSerializer and XMLTypeMapping, but it appears that the class needs to be created using XSD, which I don't believe Swagger supports yet.

Short of creating the SOAP message manually, I have run out of options, so any help or suggestions will be appreciated.

Thanks

wing328 commented 6 years ago

@dnn01 sorry I'm not familiar with XML processing in C# either.

Let's see if someone from the community can help on this.

jonsa commented 5 years ago

This issue just bit me when trying to integrate a system that only returns XML. Is there a reason for the home-brewed deserialization in ApiClient instead of using the deserializing power of RestSharp which is used to make the request?