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

[Bug]: Required and nullable properties are not nullable in the OpenAPI JSON #2877

Closed
cremor opened this issue May 14, 2024 · 11 comments · Fixed by #2879
Closed

[Bug]: Required and nullable properties are not nullable in the OpenAPI JSON #2877

cremor opened this issue May 14, 2024 · 11 comments · Fixed by #2879
Labels

Comments

@cremor
Copy link
Contributor

cremor commented May 14, 2024

Describe the bug

Properties with the required modifier are always generated as not nullable (and in the case of strings also with minLength: 1), even if they are nullable.

Expected behavior

Nullable properties should be nullable in the OpenAPI JSON.

Example:
grafik

Actual behavior

Nullable properties are always not-nullable in the OpenAPI JSON if they are also required.

Example:
grafik

Steps to reproduce

  1. Create a new .NET 8 ASP.NET Core Web API project.
  2. Change the WeatherForecast model like shown below.
  3. Change the controller GET method to return null; so the project compiles.
  4. Change Program.cs to have the following code:
            builder.Services.AddSwaggerGen(c =>
            {
                c.SupportNonNullableReferenceTypes();
                c.UseAllOfToExtendReferenceSchemas();
            });
  1. Check the generated OpenAPI JSON or Swagger UI.
    public class WeatherForecast
    {
        public string? StringNullable { get; set; }
        public string StringNotNullable { get; set; }
        public required string? StringRequiredNullable { get; set; }
        public required string StringRequiredNotNullable { get; set; }
        public int? IntNullable { get; set; }
        public int IntNotNullable { get; set; }
        public required int? IntRequiredNullable { get; set; }
        public required int IntRequiredNotNullable { get; set; }
        public Other? OtherNullable { get; set; }
        public Other OtherNotNullable { get; set; }
        public required Other? OtherRequiredNullable { get; set; }
        public required Other OtherRequiredNotNullable { get; set; }
    }

    public class Other
    {
    }

Exception(s) (if any)

No response

Swashbuckle.AspNetCore version

6.6.1

.NET Version

8.0.204

Anything else?

This is a new problem in Swashbuckle 6.6.1 (6.6.0). In 6.5.0 the required keyword didn't have any effect.
I assume this was caused by @keahpeters in #2810

required doesn't mean that a not-null value is required. It just means that any value (which can be null) is required. In other words: It just means that the request body needs to contain the property.

Related to #2036, but that issue has so many comments already that I'm not sure if it requests something more than I do here.
Should be added to #2793

@cremor cremor added the bug label May 14, 2024
@martincostello
Copy link
Collaborator

Thanks for the report - this looks like an oversight in the changes for required.

@cremor
Copy link
Contributor Author

cremor commented May 14, 2024

I've updated the issue description so that SupportNonNullableReferenceTypes and UseAllOfToExtendReferenceSchemas is used in the repro steps and the actual output.

@martincostello
Copy link
Collaborator

@cremor If you can validate the changes from #2879 fix your issue using a CI version from our MyGet feed, we can look at shipping a fix tomorrow if it's all good.

@cremor
Copy link
Contributor Author

cremor commented May 14, 2024

I can check it tomorrow around 7:00 CEST.

@martincostello
Copy link
Collaborator

Thanks!

@cremor
Copy link
Contributor Author

cremor commented May 15, 2024

I can confirm that this is fixed in 6.6.2-preview.334, thanks!

@cremor
Copy link
Contributor Author

cremor commented May 20, 2024

we can look at shipping a fix tomorrow if it's all good.

@martincostello I know that the 6.6.2 milestone got much bigger since you wrote this, but what's the current plan for shipping it?

If you need help, is there a way I can? I already reviewed the two open PRs in the milestone.

@martincostello
Copy link
Collaborator

I'm waiting for reviews from one of the other contributors with write access. Once all the PRs associated with the milestone are merged (there's 2), I'll do the release.

@eluchsinger
Copy link

eluchsinger commented May 29, 2024

Hi thanks for the fix.

I'm using v 6.6.2 and it seems to fix some cases, but for me at least not everything.

For example, I have a required DateTimeOffset? SubmitDate and it gets generated to submitDate: Date

Source

image

Generated

image

Weirdly enough, the searchterm field is also affected and it's a string. But I've seen other strings behave differently.

When I remove the required, the submitDate is nullable. This behavior does not exist with Swashbuckle.AspNetCore 6.5.0.

@martincostello
Copy link
Collaborator

Could you create a new issue please with a reproducible code sample, and then we can look into that.

@cremor
Copy link
Contributor Author

cremor commented May 29, 2024

You should check the OpenAPI JSON file that Swashbuckle creates, not some generated code that gets generated from another library. If it's nullable (and required) in the OpenAPI JSON then it's the fault of the code generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants