Azure / azure-functions-openapi-extension

This extension provides an Azure Functions app with Open API capability for better discoverability to consuming parties
https://www.nuget.org/packages/Microsoft.Azure.WebJobs.Extensions.OpenApi/
MIT License
367 stars 191 forks source link

OpenApiProperty Nullable does not seem to work for classes #200

Open shellicar opened 3 years ago

shellicar commented 3 years ago

Overview

I have an existing Function using the OpenAPI library that I was able to apply Nullable to for some string properties. However other properties such as classes do not generate the expected nullable section in swagger.json

While attempting to create a reproducible, I'm unable to see any nullable/required generated in the swagger for OpenAPI V2 (default), and it doesn't work with classes for OpenAPI V3.

Reproducible

Here is a simple Function. csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <AzureFunctionsVersion>v3</AzureFunctionsVersion>
    <Nullable>Enable</Nullable>
    <LangVersion>9.0</LangVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.OpenApi" Version="0.8.1-preview" />
    <PackageReference Include="Microsoft.NET.Sdk.Functions" Version="3.0.13" />
  </ItemGroup>
  <ItemGroup>
    <None Update="host.json">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
    <None Update="local.settings.json">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <CopyToPublishDirectory>Never</CopyToPublishDirectory>
    </None>
  </ItemGroup>
</Project>

Function1.cs

using System.Collections.Generic;
using System.Net;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Azure.WebJobs;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.AspNetCore.Http;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Abstractions;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Attributes;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Enums;
using Microsoft.Extensions.Logging;
using Microsoft.OpenApi.Models;

namespace FunctionApp2
{
    public class ChildModel
    {
        [OpenApiProperty(Nullable = false, Description = "required!")]
        public string RequiredData { get; set; } = string.Empty;

        [OpenApiProperty(Nullable = true, Description = "optional!")]
        public string? OptionalData { get; set; }
    }

    public class FooModel
    {
        [OpenApiProperty(Nullable = false)]
        public ChildModel RequiredChild { get; set; } = new ChildModel();

        [OpenApiProperty(Nullable = true)]
        public ChildModel? OptionalChild { get; set; }

        [OpenApiProperty(Nullable = false, Description = "required!")]
        public string RequiredData { get; set; } = string.Empty;

        [OpenApiProperty(Nullable = true, Description = "optional!")]
        public string? OptionalData { get; set; }
    }

    public class OpenApiConfigurationOptions : IOpenApiConfigurationOptions
    {
        public OpenApiInfo Info { get; set; } = new OpenApiInfo
        {
            Title = "Test API",
            Version = "1.0"
        };

        public List<OpenApiServer> Servers { get; set; } = new();

        public OpenApiVersionType OpenApiVersion { get; set; } = OpenApiVersionType.V3;
        public bool IncludeRequestingHostName { get; set; } = false;
    }

    public class FooFunction
    {
        [FunctionName("foo")]
        [OpenApiOperation("foo")]
        [OpenApiResponseWithBody(HttpStatusCode.OK, "application/json", typeof(FooModel))]
        public async Task<ActionResult<FooModel>> GetFoo(
            [HttpTrigger(AuthorizationLevel.Function, "get", Route = null)] HttpRequest req,
            ILogger log)
        {
            var model = new FooModel();
            return new OkObjectResult(model);
        }
    }
}

swagger.json

{
  "openapi": "3.0.1",
  "info": {
    "title": "Test API",
    "version": "1.0"
  },
  "servers": [
    {
      "url": "http://localhost:7071/api"
    }
  ],
  "paths": {
    "/foo": {
      "get": {
        "operationId": "foo",
        "responses": {
          "200": {
            "description": "Payload of FooModel",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/fooModel"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "childModel": {
        "type": "object",
        "properties": {
          "requiredData": {
            "type": "string",
            "description": "required!"
          },
          "optionalData": {
            "type": "string",
            "description": "optional!",
            "nullable": true
          }
        }
      },
      "fooModel": {
        "type": "object",
        "properties": {
          "requiredChild": {
            "$ref": "#/components/schemas/childModel"
          },
          "optionalChild": {
            "$ref": "#/components/schemas/childModel"
          },
          "requiredData": {
            "type": "string",
            "description": "required!"
          },
          "optionalData": {
            "type": "string",
            "description": "optional!",
            "nullable": true
          }
        }
      }
    }
  }
}

Generated FooModel using NSwag.

[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.5.2.0 (Newtonsoft.Json v12.0.0.0)")]
    public partial class FooModel 
    {
        [Newtonsoft.Json.JsonProperty("requiredChild", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public ChildModel? RequiredChild { get; set; }= default!;

        [Newtonsoft.Json.JsonProperty("optionalChild", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public ChildModel? OptionalChild { get; set; }= default!;

        /// <summary>required!</summary>
        [Newtonsoft.Json.JsonProperty("requiredData", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public string? RequiredData { get; set; }= default!;

        /// <summary>optional!</summary>
        [Newtonsoft.Json.JsonProperty("optionalData", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public string? OptionalData { get; set; }= default!;

        private System.Collections.Generic.IDictionary<string, object> _additionalProperties = new System.Collections.Generic.Dictionary<string, object>();

        [Newtonsoft.Json.JsonExtensionData]
        public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
        {
            get { return _additionalProperties; }
            set { _additionalProperties = value; }
        }

    }

Expected

I would expect "nullable": true in the "components.schemas.fooModel.optionalChild" node. And the generated code to have the same Required value as the "optionalData" property.

In the docs it's not clear that this will not do anything unless V3 is specified, or if this is intentional or not.

shellicar commented 3 years ago

It seems that the OpenApiProperty parameters are applied to the object schema, rather than the property.

e.g.:

public class FooModel
    {
        [OpenApiProperty(Nullable = false, Description = "required child")]
        public ChildModel RequiredChild { get; set; } = new();

        [OpenApiProperty(Nullable = true, Description = "optional child")]
        public ChildModel? OptionalChild { get; set; }

        [OpenApiProperty(Nullable = false, Description = "required data")]
        public string RequiredData { get; set; } = string.Empty;

        [OpenApiProperty(Nullable = true, Description = "optional data")]
        public string? OptionalData { get; set; }
    }
"components": {
    "schemas": {
      "childModel": {
        "type": "object",
        "properties": {
          "requiredData": {
            "type": "string",
            "description": "required data"
          },
          "optionalData": {
            "type": "string",
            "description": "optional data",
            "nullable": true
          }
        },
        "description": "required child"
      },
      "fooModel": {
        "type": "object",
        "properties": {
          "requiredChild": {
            "$ref": "#/components/schemas/childModel"
          },
          "optionalChild": {
            "$ref": "#/components/schemas/childModel"
          },
          "requiredData": {
            "type": "string",
            "description": "required data"
          },
          "optionalData": {
            "type": "string",
            "description": "optional data",
            "nullable": true
          }
        }
      }
    }
  }
kevinmillstx commented 3 years ago

There seem to be multiple issues with nullable properties. I've noticed [JsonIgnore] on a long? does not remove the property from the generated spec. Whereas, it does when it's not nullable. Also, that a DateTime? annotated with [DataType(DataType.Date)] will not output to spec with date format; still gets generated as date-time. Making it non-nullable will properly result in date format.

shellicar commented 3 years ago

Hi just wondering if there is any update/response to this as it currently generates incorrect OpenAPI specification.

justinyoo commented 3 years ago

@shellicar There are multiple questions mixed up in this one issue. For one thing is fixed in #208

It would be great if you split issues what you're trying to achieve.

shellicar commented 3 years ago

Hi, thanks for the reply.

Sorry for any confusion. I didn't intend for there to be multiple questions or issues. If you're referring to the comment about V2/V3 that was just something I discovered when investigating.

I don't think this is fixed by the linked issue, I believe these properties use "ObjectTypeVisitor".

Here is a hopefully simpler example to illustrate the issue.

Code

using System.Collections.Generic;
using System.Net;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Azure.WebJobs;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.AspNetCore.Http;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Abstractions;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Attributes;
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Enums;
using Microsoft.Extensions.Logging;
using Microsoft.OpenApi.Models;

namespace FunctionApp2
{
    public class MyModel
    {
        [OpenApiProperty(Nullable = true, Description = "optional data")]
        public string OptionalString { get; set; }

        [OpenApiProperty(Nullable = true, Description = "optional data")]
        public ChildModel OptionalData { get; set; }
    }

    public class OtherModel
    {
        [OpenApiProperty(Nullable = false, Description = "required data")]
        public string RequiredString { get; set; }

        [OpenApiProperty(Nullable = false, Description = "required data")]
        public ChildModel RequiredData { get; set; }
    }

    public class ChildModel
    {
        public string Data { get; set; }
    }

    public class OpenApiConfigurationOptions : IOpenApiConfigurationOptions
    {
        public OpenApiInfo Info { get; set; } = new OpenApiInfo
        {
            Title = "Test API",
            Version = "1.0"
        };

        public List<OpenApiServer> Servers { get; set; } = new();

        public OpenApiVersionType OpenApiVersion { get; set; } = OpenApiVersionType.V3;
        public bool IncludeRequestingHostName { get; set; } = false;
    }

    public class FooFunction
    {
        [FunctionName("foo")]
        [OpenApiOperation("foo")]
        [OpenApiResponseWithBody(HttpStatusCode.OK, "application/json", typeof(MyModel))]
        public async Task<ActionResult<MyModel>> GetFoo(
            [HttpTrigger(AuthorizationLevel.Function, "get", Route = null)] HttpRequest req,
            ILogger log)
        {
            var model = new MyModel();
            return new OkObjectResult(model);
        }

        [FunctionName("foo2")]
        [OpenApiOperation("foo2")]
        [OpenApiResponseWithBody(HttpStatusCode.OK, "application/json", typeof(OtherModel))]
        public async Task<ActionResult<OtherModel>> GetFoo2(
            [HttpTrigger(AuthorizationLevel.Function, "get", Route = null)] HttpRequest req,
            ILogger log)
        {
            var model = new OtherModel();
            return new OkObjectResult(model);
        }
    }
}

Generated swagger.json

{
    "openapi": "3.0.1",
    "info": {
      "title": "Test API",
      "version": "1.0"
    },
    "servers": [
      {
        "url": "http://localhost:7071/api"
      }
    ],
    "paths": {
      "/foo": {
        "get": {
          "operationId": "foo",
          "responses": {
            "200": {
              "description": "Payload of MyModel",
              "content": {
                "application/json": {
                  "schema": {
                    "$ref": "#/components/schemas/myModel"
                  }
                }
              }
            }
          }
        }
      },
      "/foo2": {
        "get": {
          "operationId": "foo2",
          "responses": {
            "200": {
              "description": "Payload of OtherModel",
              "content": {
                "application/json": {
                  "schema": {
                    "$ref": "#/components/schemas/otherModel"
                  }
                }
              }
            }
          }
        }
      }
    },
    "components": {
      "schemas": {
        "childModel": {
          "type": "object",
          "properties": {
            "data": {
              "type": "string"
            }
          },
          "description": "optional data"
        },
        "myModel": {
          "type": "object",
          "properties": {
            "optionalString": {
              "type": "string",
              "description": "optional data",
              "nullable": true
            },
            "optionalData": {
              "$ref": "#/components/schemas/childModel"
            }
          }
        },
        "otherModel": {
          "type": "object",
          "properties": {
            "requiredString": {
              "type": "string",
              "description": "required data"
            },
            "requiredData": {
              "$ref": "#/components/schemas/childModel"
            }
          }
        }
      }
    }
  }

Expected swagger.json

{
    "openapi": "3.0.1",
    "info": {
      "title": "Test API",
      "version": "1.0"
    },
    "servers": [
      {
        "url": "http://localhost:7071/api"
      }
    ],
    "paths": {
      "/foo": {
        "get": {
          "operationId": "foo",
          "responses": {
            "200": {
              "description": "Payload of MyModel",
              "content": {
                "application/json": {
                  "schema": {
                    "$ref": "#/components/schemas/myModel"
                  }
                }
              }
            }
          }
        }
      },
      "/foo2": {
        "get": {
          "operationId": "foo2",
          "responses": {
            "200": {
              "description": "Payload of OtherModel",
              "content": {
                "application/json": {
                  "schema": {
                    "$ref": "#/components/schemas/otherModel"
                  }
                }
              }
            }
          }
        }
      }
    },
    "components": {
      "schemas": {
        "childModel": {
          "type": "object",
          "properties": {
            "data": {
              "type": "string"
            }
          }
        },
        "myModel": {
          "type": "object",
          "properties": {
            "optionalString": {
              "type": "string",
              "description": "optional data",
              "nullable": true
            },
            "optionalData": {
              "$ref": "#/components/schemas/childModel",
              "description": "optional data",
              "nullable": true
            }
          }
        },
        "otherModel": {
          "type": "object",
          "properties": {
            "requiredString": {
              "type": "string",
              "description": "required data"
            },
            "requiredData": {
              "$ref": "#/components/schemas/childModel",
              "description": "required data"
            }
          }
        }
      }
    }
  }

Hopefully the difference between the generated and expected swagger.json illustrates the issue, but I'll explain it here.

The issue is that the nullable and description properties (from OpenApiProperty) on classes are not applied to the property object in the model. Instead they are applied to the schema object.

justinyoo commented 3 years ago

Thanks for clarification! I found a couple of issues here:

  1. Although your ChildModel has no decorator, the #/components/schema/childModel has a description defined by its parent class.

👉 It shouldn't be appearing on that location, and it's something I need to look at and fix.

  1. You expect $ref, description and nullable in one place. However, according to the spec v3.0.1, each property can have either schema object or reference object.

Alternatively, any time a Schema Object can be used, a Reference Object can be used in its place. This allows referencing definitions instead of defining them inline.

When it uses the reference object, it only accepts the $ref property.

This object cannot be extended with additional properties and any properties added SHALL be ignored.

👉 Therefore, as long as the $ref is used, the values defined in OpenApiProperty(...) won't be appearing.

Hope this makes sense to you.

flcdrg commented 2 years ago

@justinyoo Something I'm encountering which seems similar. In the code sample above, is it possible to make OptionalChild nullable?

        [OpenApiProperty(Nullable = true)]
        public ChildModel? OptionalChild { get; set; }

Doesn't seem to have any effect. And the code samples above show NSWag is generating code that forbids null (so the property is not optional).

If I manually edited the generated yaml, I can add a nullable: true under the ChildModel definition. Not clear how to do that with the attributes though.

Rutix commented 2 years ago

@justinyoo I understand you linking the standard but do you know of a way to mark it nullable then which is valid according to the spec? I see that other libraries use the allOf property to be able to set nullable. See you get something like:

        "myModel": {
            "type": "object",
            "properties": {
                "optionalString": {
                    "type": "string",
                    "description": "optional data",
                    "nullable": true
                },
                "optionalData": {
                    "allOf": [
                        {
                            "$ref": "#/components/schemas/childModel"
                        }
                    ],
                    "nullable": true
                }
            }
        }

Is there a way to do that for Azure Functions or a way to get nullable working?

rockgecko-development commented 2 years ago

Yes, allOf is the correct way to fix this issue - I see it is in the roadmap via #370 Not only does it allow you to reference child types that are occasionally nullable, it also allows them to have the correct description. Notice in this reply, the generated swagger has:

"childModel": {
          ...
          "description": "optional data"
        }

"optional data" just happens to be the description attached to the first usage of childModel. The second usage has [OpenApiProperty(Nullable = false, Description = "required data")], which is lost.

The generated output should be:

"myModel": {
            "type": "object",
            "properties": {
                "optionalString": {
                    "type": "string",
                    "description": "optional data",
                    "nullable": true
                },
                "optionalData": {
                    "allOf": [
                        {
                            "$ref": "#/components/schemas/childModel"
                        }
                    ],
                    "nullable": true,
                    "description": "optional data",
                }
            }
        },
 "otherModel": {
          "type": "object",
          "properties": {
            "requiredString": {
              "type": "string",
              "description": "required data"
            },
            "requiredData": {
               "allOf": [
                        {
                            "$ref": "#/components/schemas/childModel"
                        }
                    ],
                    "description": "required data",
              }
            }
          }
        },
"childModel": {
          "type": "object",
          "properties": {
            "data": {
              "type": "string"
            }
          },
          "description": "<description generated from the ChildModel class definition, eg if OpenApiPropertyAnnotation were allowed on classes>"
        },
Joost1982 commented 1 year ago

@justinyoo Something I'm encountering which seems similar. In the code sample above, is it possible to make OptionalChild nullable?

        [OpenApiProperty(Nullable = true)]
        public ChildModel? OptionalChild { get; set; }

Doesn't seem to have any effect. And the code samples above show NSWag is generating code that forbids null (so the property is not optional).

I think have the same problem. I have a class with a Dictionary property, which I annotated with the Nullable OpenApiProperty attribute:

    [JsonPropertyName("data")]
    [OpenApiProperty(Nullable = true, Description = "optional data")] 
    public Dictionary<string, object>? Data { get; set; }

But the generated yaml (v3) file shows this:

        data:
          type: object
          additionalProperties:
            type: object
          description: optional data

So the description part works, but not the part that should make it nullable (which does work for string and int properties). I expected something like:

        data:
          type: object
          additionalProperties:
            type: object
          description: optional data
          nullable: true

I can add the missing part manually, but I have a lot of objects with these kind of properties...

Ah I see there is already a PR that fixes this! https://github.com/Azure/azure-functions-openapi-extension/pull/504

-- update --

@justinyoo just released a new version (1.5.1) and this version indeed produces the expected result. Very nice!

RedZone908 commented 6 months ago

I banged my head against a wall for a while wondering why this wasn't working even though I had version 1.5.1 of the package... it turned out that my OpenApiVersion was set to V2 and the nullable property wasn't introduced until V3. So whoever else is experiencing this problem, do this for your open api configuration options:

public class OpenApiConfigurationOptions : DefaultOpenApiConfigurationOptions
{
        public override OpenApiInfo Info { get; set; } = new OpenApiInfo
        {
            Title = "Test API",
            Version = "1.0"
        };

        public override OpenApiVersionType OpenApiVersion { get; set; } = OpenApiVersionType.V3;
}

Side note, shouldn't this issue be closed? It's been resolved for a year hasn't it?

brunopiovan commented 1 month ago

I got around this annoying thing by writing a custom DocumentFilter:

public class NullableDocumentFilter : IDocumentFilter
{
    public void Apply(IHttpRequestDataObject req, OpenApiDocument document)
    {
        foreach (var schema in document.Components.Schemas)
        {
            ProcessProperties(schema.Value.Properties);
        }
    }

    private static void ProcessProperties(IDictionary<string, OpenApiSchema> properties)
    {
        foreach (var property in properties)
        {
            if (property.Value.Nullable && property.Value.Type == "object" && property.Value.Reference != null)
            {
                property.Value.AllOf =
                [
                    new OpenApiSchema()
                    {
                        Reference = property.Value.Reference,
                    },
                ];

                property.Value.Type = null;
                property.Value.Reference = null;

                property.Value.Properties.Clear();
            }
            else
            {
                ProcessProperties(property.Value.Properties);
            }
        }
    }
}

add it to your DocumentFilters collection and make sure you use OpenApiVersion = OpenApiVersionType.V3:

services.AddSingleton<IOpenApiConfigurationOptions>(_ => new DefaultOpenApiConfigurationOptions()
{
    //...
    OpenApiVersion = OpenApiVersionType.V3,
    DocumentFilters =
    [
        new NullableDocumentFilter()
    ],
});

and set your properties as nullable and decorate it like so:

[OpenApiProperty(Nullable = true)]
public YourType? MyType{ get; init; }

and now it should render:

{
  "myType": {
    "allOf": [
      {
        "$ref": "#/components/schemas/yourType"
      }
    ],
    "nullable": true
  }
}

instead of

{
  "myType": {
    "$ref": "#/components/schemas/yourType"
  }
}