domaindrivendev / Swashbuckle.WebApi

Seamlessly adds a swagger to WebApi projects!
BSD 3-Clause "New" or "Revised" License
3.07k stars 678 forks source link

Wrong Xml example data generation #1050

Open dlazzarino opened 7 years ago

dlazzarino commented 7 years ago

Hi all, i have a problem with auto-generated Xml example value on Swagger-UI, the xml structure is invalid and can't be used to test api

Steps to reproduce the problem:

public class Foo
    {
        public string Name { get; set; }
        public int Code { get; set; }
        public IList<FooItem> Items { get; set; }
    }

    public class FooItem
    {
        public string ItemName { get; set; }
        public int ItemCode { get; set; }
    }

and the Xml sample data generated from Swagger UI

<?xml version="1.0"?>
<Foo>
  <Name>string</Name>
  <Code>1</Code>
  <Items>
    <ItemName>string</ItemName>
    <ItemCode>1</ItemCode>
  </Items>
</Foo>

Any suggestions?

Thanks in advance, Diego

SwashbuckleXml.zip

grmihel2 commented 7 years ago

Encountering the exact same problem. I wish to support both JSON and XML. Json works fine, but the XML seems to have issues with the array types (using ICollection in my data model). Running Swashbuckle 5.5.3 btw

heldersepu commented 7 years ago

I think this is a Swagger UI issue not in Swashbuckle... The UI gets all it needs from the /swagger/docs/v1

heldersepu commented 7 years ago

Here is the version that Swashbuckle is using: https://github.com/swagger-api/swagger-ui/blob/64dc3060b3700b12e466f8d67b7d7ec3574b015f/src/main/javascript/view/partials/signature.js#L973

And I think the latest is quite different I do not see those XML functions anymore, so upgrading to latest might/could fix this...

Take a look at the live Demo: http://petstore.swagger.io/? I think that /pet/findByStatus has the same issue you describe here

danpetitt commented 7 years ago

I think this is the issue I have just logged on Swagger UI XML Example for Arrays does not display

danpetitt commented 7 years ago

This is a Swashbuckle issue; i have almost resolved but but i am stuck so i think someone with a bit more experience with the codebase needs to look.

Basically there are two issues:

  1. The definition needs to have an xml: { name: "typeName" } property so that it knows what name to output the tag
  2. When there is an array, you need an additional property on xml: xml: { name: "typeName", wrapped: true } so it knows its a contained type

Example:

            "responses":{
               "200":{
                  "description":"Returns an array of channel objects",
                  "schema":{
                     "type":"array",
                     "items":{
                        "$ref":"#/definitions/Channel"
                     }
                  }
               },

Should actually be:

            "responses":{
               "200":{
                  "description":"Returns an array of channel objects",
                  "schema":{
                     "type":"array",
                     "items":{
                        "$ref":"#/definitions/Channel"
                     }
                     xml: {
                        "name": "Channel",
                        "wrapped": true
                      }
                  }
               },

And:

"definitions":{
  "Channel":{
     "type":"object",
     "properties":{
        "id":{
           "format":"int32",
           "type":"integer"
        },
        "name":{
           "type":"string"
        },
        "active":{
           "format":"int32",
           "type":"integer"
        },
        "defaultTerritoryID":{
           "format":"int32",
           "type":"integer"
        }
     }
  }
}

Should actually be:

"definitions":{
  "Channel":{
     "type":"object",
     "properties":{
        "id":{
           "format":"int32",
           "type":"integer"
        },
        "name":{
           "type":"string"
        },
        "active":{
           "format":"int32",
           "type":"integer"
        },
        "defaultTerritoryID":{
           "format":"int32",
           "type":"integer"
        }
     },
     "xml":{
        "name":"Channel"
     }
  }
}

When i apply a fix, some tests work, some need adjusting, others fail as the 'name' seems to be either a system type name or a class name (so could be Int32 or Channel); so I worked around that but then 'enums' pose a problem and they dont come in as a SystemType but the Name is a 'PrimitiveEnum' type.

I can do a 'breaking' PR if someone wants to take a look

heldersepu commented 7 years ago

@coderangerdan Just paste a link to your branch of this fork reproducing this ...

danpetitt commented 7 years ago

@heldersepu Inline Models Fix

Another problem/issue with this is that if you respond with an IEnumerable, there is no 'type' name associated with it so you cant give it a proper name (often it would be a plural of the class being responded, but that may not always be the case). So i was thinking that perhaps the response attribute has a 'name' property so this can be defined; i dont think this would be against the normal behaviour and is in keeping with the description property.

For example, I should be able to respond with: [SwaggerResponse( HttpStatusCode.OK, Description = "Returns an array of channel objects", Type = typeof( IEnumerable<Channel> ), TypeName = "MyChannels" )]

Which should write out:

"responses":{
   "200":{
      "description":"Returns an array of channel objects",
      "schema":{
         "type":"array",
         "items":{
            "$ref":"#/definitions/Channel"
         }
         xml: {
            "name": "MyChannels",
            "wrapped": true
          }
      }
   }

"definitions":{
  "Channel":{
     "type":"object",
     "properties":{
        "id":{
           "format":"int32",
           "type":"integer"
        },
        "name":{
           "type":"string"
        },
        "active":{
           "format":"int32",
           "type":"integer"
        },
        "defaultTerritoryID":{
           "format":"int32",
           "type":"integer"
        }
     },
     "xml":{
        "name":"Channel"
     }
  }
}

What do you think?

danpetitt commented 7 years ago

I have implemented this and fixed the tests; branch updated. Not sure if this is a good fix or not ... or even all-encompassing (i.e. request models etc)

heldersepu commented 7 years ago

that TypeName is that part of the swagger definitions?

danpetitt commented 7 years ago

No, I added it to the SwaggerResponse attribute, like 'description' is already there; that way you can control the xml.name (which is part of swagger definition) when there is no way of determining that at runtime: IEnumerable<Account> is what exactly? In this case we want to use the 'name' Accounts but you cant determine that; in fact you may want your root XML Node to be AccountArray; with this solution, you now can

heldersepu commented 7 years ago

Now I understand better what you are trying to accomplish... Code looks good.

aspnetlabs commented 6 years ago

Probable workaround for XML, for array properties, add this schema filter.

    /// <summary>
    /// Custom schema filter.
    /// </summary>
    public class CustomSchemaFilter : ISchemaFilter
    {
        /// <summary>
        /// Apply custom schema filter.
        /// </summary>
        /// <param name="model"></param>
        /// <param name="context"></param>
        public void Apply(Schema model, SchemaFilterContext context)
        {
            //This code generates, Swagger format for Complex object.
            if (model.Type == "object")
            {
                foreach (var property in model.Properties)
                {
                    //Array property, which wraps its elements
                    if (property.Value.Type == "array")
                    {
                        property.Value.Xml = new Xml()
                        {
                            Name = $"{property.Key}",
                            Wrapped = true,
                        };

                        //Schema for array elements.
                        property.Value.Items = new Schema()
                        {
                            AllOf = new List<Schema>() { new Schema() { Ref = property.Value.Items.Ref } },
                            //For complex type, Type is null, for simple type i.e string, type is string.
                            Type = property.Value.Items?.Type ?? "object",

                            Xml = new Xml()
                            {
                                Name =
                                property.Value.Items.Type != null ?
                                //string for simple type.
                                 property.Value.Items.Type =="string"? property.Value.Items.Type : property.Value.Items.Format.Replace("32","").Replace("64","")  : 
                                //Singular property name i.e if parent propery is Accounts, the item will have Account.
                                //Where Account is a complex type object.
                                property.Key.Substring(0, property.Key.Length - 1), 
                            }
                        };
                    }
                }
            }
        } 
    }
simonfanz commented 6 years ago

Just hit this issue and after much frustration arrived here. What's the best approach to fix?

simonfanz commented 6 years ago

The example from aspnetlabs only partially worked, but did get me on track to get this, which seems to work other than ignoring xml attributes on the classes:

internal class ApplySchemaVendorExtensions : ISchemaFilter
    {
        public void Apply(Schema schema, SchemaRegistry schemaRegistry, Type type)
        {
            // Fix issues with xml array examples not generating correctly
            if (!type.IsValueType)
            {
                schema.xml = new Xml { name = type.Name };
                if(schema.properties != null)
                {
                    foreach (var property in schema.properties)
                    {
                        //Array property, which wraps its elements
                        if (property.Value.type == "array")
                        {
                            property.Value.xml = new Xml
                            {
                                name = $"{property.Key}",
                                wrapped = true
                            };
                        }
                    }
                }
            }
    }
}

Not really familiar enough with swagger\swagger-ui to know what the flow on effects of this will be though...

GeorgyOkov commented 5 years ago

Does anyone know how to fix this for version Swashbuckle v5 (open-api 3)?

GeorgyOkov commented 5 years ago

One remark:

context.SystemType.IsValueType

System.String is not a value type

Result is:

It must be checked for string too.

Let me know if you have any questions.

Best regards,

Georgy Okov

Track-POD Inc

Skype. gokov_iss

Viber/Phone: +37 529 7047011

www.track-pod.com http://www.track-pod.com/

From: Robert McKee notifications@github.com Sent: Tuesday, February 19, 2019 3:05 AM To: domaindrivendev/Swashbuckle Swashbuckle@noreply.github.com Cc: GeorgyOkov georgy@track-pod.com; Mention mention@noreply.github.com Subject: Re: [domaindrivendev/Swashbuckle] Wrong Xml example data generation (#1050)

@GeorgyOkov https://github.com/GeorgyOkov this seems to work for Swashbuckle v5:

internal class ApplySchemaVendorExtensions : ISchemaFilter { public void Apply(OpenApiSchema schema, SchemaFilterContext context) { if (context.SystemType.IsValueType) { return; }

       schema.Xml = new OpenApiXml { Name = context.SystemType.Name };
       if (schema.Properties == null)
       {
           return;
       }

       foreach (var property in schema.Properties.Where(property => property.Value.Type == "array"))
       {
           property.Value.Xml = new OpenApiXml
           {
               Name = property.Key,
               Wrapped = true
           };
       }
   }

}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/domaindrivendev/Swashbuckle/issues/1050#issuecomment-464927292 , or mute the thread https://github.com/notifications/unsubscribe-auth/Ap-ruaXaO-LMz_mM209CZ5nYEsP6z1k_ks5vOz-YgaJpZM4M9uBg . https://github.com/notifications/beacon/Ap-ruQJ3KCoCcH-2qSi0rYS2doKXiRNwks5vOz-YgaJpZM4M9uBg.gif

kingmotley commented 5 years ago

@GeorgyOkov I noticed the same issue along with a host of other issues after I posted that code. That was a pretty straight forward translation of the code that @simonfanz posted.

Unfortunately it seems to have some unintended side effects so I removed the post until I can resolve them.

euroform-pa commented 5 years ago

Added the demo project to reproduce the issue with Swashbuckle for Xml sample generation issue. Demo project Kindly suggest the solution to view correct Xml sample generation.

euroform-pa commented 5 years ago

Added the demo project to reproduce the issue with Swashbuckle for Xml sample generation issue. Demo project Kindly suggest the solution to view correct Xml sample generation.

Any update on this?

joegithub commented 5 years ago

@GeorgyOkov I've had success with the following: Note that this is for v5.0.0-rc4

public class CustomXmlSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        if (context.ApiModel.Type.IsValueType)
        {
            return;
        }

        if (schema.Type == "string")
        {
            return;
        }            

        schema.Xml = new OpenApiXml
        {
            Name = context.ApiModel.Type.Name
        };

        if (schema.Properties == null)
        {
            return;
        }

        foreach (var property in schema.Properties.Where(x => x.Value.Type == "array"))
        {
            property.Value.Xml = new OpenApiXml
            {
                Name = $"{property.Key}",
                Wrapped = true
            };
        }
    }
}
euroform-pa commented 5 years ago

@joegithub @GeorgyOkov Finally I got it working with Xml. Following is the CustomXmlSchemaFilter that handles return of collection, collection property in POCO.

public class CustomXmlSchemaFilter : ISchemaFilter
    {
        private const string _SCHEMA_ARRAY_TYPE = "array";
        private const string _SCHEMA_STRING_TYPE = "string";
        private const string _PREFIX_ARRAY = "ArrayOf";

        public void Apply(OpenApiSchema schema, SchemaFilterContext context)
        {
            if (context.ApiModel.Type.IsValueType)
                return;

            if (schema.Type == _SCHEMA_STRING_TYPE)
                return;

            schema.Xml = new OpenApiXml
            {
                Name = context.ApiModel.Type.Name
            };

            if (schema.Type == _SCHEMA_ARRAY_TYPE && schema.Items.Reference != null)
            {
                schema.Xml = new OpenApiXml
                {
                    Name = $"{_PREFIX_ARRAY}{schema.Items.Reference.Id}",
                    Wrapped = true,
                };
            }

            if (schema.Properties == null)
            {
                return;
            }

            foreach (var property in schema.Properties.Where(x => x.Value.Type == _SCHEMA_ARRAY_TYPE))
            {
                property.Value.Items.Xml = new OpenApiXml
                {
                    Name = property.Value.Items.Type,
                };
                property.Value.Xml = new OpenApiXml
                {
                    Name = property.Key,
                    Wrapped = true
                };
            }
        }
    }

Specify to use this Schema filter in Swagger via following:

services.AddSwaggerGen(c =>
{
    ...

    c.SchemaFilter<CustomXmlSchemaFilter>();
});

Hope this helps.

cobrul commented 3 years ago

The solution above doesn't work for an return of type string[] or IEnumerable, unless you add also name for items which are not reference:

public class CustomXmlSchemaFilter : ISchemaFilter
{
    private const string _SCHEMA_ARRAY_TYPE = "array";
    private const string _SCHEMA_STRING_TYPE = "string";
    private const string _PREFIX_ARRAY = "ArrayOf";

    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        if (context.Type.IsValueType)
            return;

        if (schema.Type == _SCHEMA_STRING_TYPE)
            return;

        schema.Xml = new OpenApiXml
        {
            Name = context.Type.Name
        };

        if (schema.Type == _SCHEMA_ARRAY_TYPE && schema.Items.Reference != null)
        {
            schema.Xml = new OpenApiXml
            {
                Name = $"{_PREFIX_ARRAY}{schema.Items.Reference.Id}",
                Wrapped = true,
            };
        }
        // add this for string[]
        if (schema.Type == _SCHEMA_ARRAY_TYPE && schema.Items.Type != null)
        {
            schema.Xml = new OpenApiXml
            {
                Name = $"{_PREFIX_ARRAY}{schema.Items.Type}",
                Wrapped = true,
            };
            schema.Items.Xml = new OpenApiXml
            {
                Name = schema.Items.Type
            };
        }

        if (schema.Properties == null)
        {
            return;
        }

        foreach (var property in schema.Properties.Where(x => x.Value.Type == _SCHEMA_ARRAY_TYPE))
        {
            property.Value.Items.Xml = new OpenApiXml
            {
                Name = property.Value.Items.Type,
            };
            property.Value.Xml = new OpenApiXml
            {
                Name = property.Key,
                Wrapped = true
            };
        }
    }
}