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

Fix up nullability and primitive type handing in schema generation #56372

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Jun 21, 2024

Addresses feedback from #56330 (comment) and #56318

@captainsafia captainsafia requested a review from a team as a code owner June 21, 2024 23:34
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Jun 21, 2024
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Jun 21, 2024
@@ -11,6 +11,21 @@ namespace Microsoft.AspNetCore.OpenApi;

internal static class JsonTypeInfoExtensions
{
private static readonly List<Type> _exemptPrimitives =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these from an OpenAPI spec? Otherwise there are probably other primitives, e.g. nint, byte

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these and a few more other primitive types. Lemme know what you think of the list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any of Guid, Uri, TimeSpan, DateOnly and TimeOnly? Just comparing to the list we have in Swashbuckle and they're the only ones that are different to this set. They basically end up as a string with a format of some kind in the schema.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes! I recall you sharing this in the other PR. I've updated this set so it's consistent between the two. I wasn't able to find the logic in NSwag for this but this probably a set that we should be consist with throughout the implementations. 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd forgotten about Version 😄

Did you want TimeSpan too, or was there a reason to skip that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet another reason to make this public 😓 Added that as well. Let me know if there is anything else I missed. I'd like to get this PR in soon to ease some of the build failures it is causing but happy to add things in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new set seems to cover all the common primitives/cases I'm aware of 👍

@captainsafia captainsafia force-pushed the safia/fixup-schemas branch from cc14932 to d596b7a Compare June 26, 2024 14:03
@captainsafia captainsafia force-pushed the safia/fixup-schemas branch from d596b7a to ca073bf Compare June 26, 2024 14:52
@captainsafia captainsafia merged commit 4c8b5fe into main Jun 26, 2024
26 checks passed
@captainsafia captainsafia deleted the safia/fixup-schemas branch June 26, 2024 17:07
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview7 milestone Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

object property causes build crash when generating OpenAPI document at build time
4 participants