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

OpenAPI: Add type hints on 'id' parameters and properties #1595

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

bkoelman
Copy link
Member

This PR adds type hints to the string-based id parameters and properties.

The resulting experience is less awesome than I hoped. Effectively, it only makes a difference in NSwag/Kiota when TId is a guid (uuid in OpenAPI). This is because client generators don't generate a numeric type when the type is string, despite a type hint such as int64. That's understandable, because it would produce an invalid JSON:API request (sending "id": 3 would fail because the value must be a string).

The type hints do show up in SwaggerUI for all TId types, which could be helpful.

image

A corner case surfaced in NSwag when using Guid as the id type and /GenerateNullableReferenceTypes is turned on, but GenerateOptionalPropertiesAsNullable and GenerateOptionalParameters are off. In that case, the generated type for the Id property becomes Guid (non-nullable), so omitting it from the request body sends Guid.Empty, which produces an error. This is a problem when ClientIdGenerationMode is set to Allowed, because there's no way to not send the ID. To fix that, TId must be Guid?, so a nullable type gets generated. However this is such a corner case that I've updated the existing tests to run with GenerateOptionalPropertiesAsNullable and GenerateOptionalParameters turned on, which I expect users to do anyway.

Once we update to NSwag v14, its MSBuild properties enable us to set the switches we need/recommend from the JsonApiDotNetCore.OpenApi.Client.NSwag NuGet package via an imported .props file, so the limitation is even less likely to surface.

Closes #1572.

QUALITY CHECKLIST

Copy link

codecov bot commented Jul 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.45%. Comparing base (92e5590) to head (a18666c).

Additional details and impacted files
@@             Coverage Diff             @@
##           openapi    #1595      +/-   ##
===========================================
+ Coverage    91.42%   91.45%   +0.02%     
===========================================
  Files          412      413       +1     
  Lines        13442    13479      +37     
  Branches      2102     2104       +2     
===========================================
+ Hits         12290    12327      +37     
  Misses         747      747              
  Partials       405      405              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bkoelman bkoelman marked this pull request as ready for review July 14, 2024 12:02
@bkoelman bkoelman merged commit 47a1589 into openapi Jul 14, 2024
16 checks passed
@bkoelman bkoelman deleted the openapi-type-hints branch July 14, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant