Support NRT and MSV in `required` and `nullable` #1185

maurei · 2022-09-06T15:40:18Z

This PR introduces improved support for required and nullable properties in OpenAPI schemas, by taking into account enabled/disabled nullable reference types and enabled/disabled model state validation. The table in Google Sheets (copy below) supersedes the original table from #1111.

Includes:

Use the native NullabilityInfoContext to check nullability information rather than relying on Swashbuckles work-around (supersedes Use native NullabilityInfoContext #1167).
Take into account if model state validation is enabled/disabled when generating OpenAPI schemas.
Small tweak in OpenApiTestContext to allow for retrieving a swagger file without having to write it to disk.

Closes #1111
Closes #1242

QUALITY CHECKLIST

Changes implemented in code
Complies with our contributing guidelines
Adapted tests
Documentation updated

…iTestContext to allow for this

codecov · 2022-09-06T22:01:30Z

Codecov Report

Merging #1185 (0dd49b0) into openapi (16a0026) will increase coverage by 0.06%.
The diff coverage is 95.77%.

@@             Coverage Diff             @@
##           openapi    #1185      +/-   ##
===========================================
+ Coverage    92.65%   92.72%   +0.06%     
===========================================
  Files          329      329              
  Lines         9863     9941      +78     
===========================================
+ Hits          9139     9218      +79     
+ Misses         724      723       -1

Files	Coverage Δ
...tNetCore.OpenApi.Client/Exceptions/ApiException.cs	`87.50% <ø> (ø)`
...piDotNetCore.OpenApi/ActionDescriptorExtensions.cs	`100.00% <100.00%> (ø)`
...enApi/JsonApiActionDescriptorCollectionProvider.cs	`96.00% <100.00%> (+0.05%)`	⬆️
...JsonApiMetadata/JsonApiEndpointMetadataProvider.cs	`96.00% <100.00%> (+1.00%)`	⬆️
...i/JsonApiMetadata/NonPrimaryDocumentTypeFactory.cs	`100.00% <100.00%> (ø)`
...OpenApi/JsonApiMetadata/RelationshipTypeFactory.cs	`100.00% <100.00%> (ø)`
...piDotNetCore.OpenApi/JsonApiOperationIdSelector.cs	`92.45% <ø> (ø)`
...iDotNetCore.OpenApi/ServiceCollectionExtensions.cs	`98.55% <100.00%> (+0.08%)`	⬆️
...enApi/SwaggerComponents/CachingSwaggerGenerator.cs	`100.00% <ø> (ø)`
...penApi/SwaggerComponents/EndpointOrderingFilter.cs	`100.00% <100.00%> (ø)`
... and 7 more

... and 1 file with indirect coverage changes

bkoelman

Very nice job! The implementation is straightforward and it's easy to verify the tests against what's in https://user-images.githubusercontent.com/9339132/177394179-2943249b-f5f1-4a72-adab-1c1d7ae85fc6.png. The code looks clean with good naming and structure.

The used models remind me of the Cow and Chicken series that my younger brother always watched! :)

src/JsonApiDotNetCore.OpenApi/SwaggerComponents/ResourceFieldObjectSchemaBuilder.cs

test/OpenApiTests/SchemaProperties/ModelStateValidationDisabledStartup.cs

...ApiTests/SchemaProperties/NullableReferenceTypesDisabled/ModelStateValidationEnabledTests.cs

test/OpenApiClientTests/SchemaProperties/NullableReferenceTypesEnabled/NullabilityTests.cs

test/OpenApiClientTests/SchemaProperties/NullableReferenceTypesDisabled/NullabilityTests.cs

...penApiClientTests/SchemaProperties/NullableReferenceTypesDisabled/RequiredAttributesTests.cs

src/JsonApiDotNetCore.OpenApi/SwaggerComponents/ResourceFieldObjectSchemaBuilder.cs

...penApiClientTests/SchemaProperties/NullableReferenceTypesDisabled/RequiredAttributesTests.cs

…nd-nullable-properties

Merge openapi branch into PR 1185: Support NRT and MSV in required and nullable

OpenApiTests/SchemaProperties test collection: * Allow for usage of OpenApiStartup directly * Remove unneeded adding of JsonConverter * Replace null checks with .ShouldHaveCount() calls * Adjust test names Misc: * Reverted .editorconfig changes and fixed ApiException constructor instead * Remove Debug statement

maurei · 2022-12-19T08:14:33Z

The review items I've reacted to are processed in my latest commit. The other ones are related to something I'm still investigating and will follow

OpenAPI NRT/MSV updates

…tribute

…erties" This reverts commit 66a2dc4, reversing changes made to c3c4844.

…inesAnalyzer v3.8.4

maurei added 3 commits September 6, 2022 13:14

Added tests for nullable and required properties in schema generation

99dcfa8

Added handling of modelstate validation in setting required attributes

2604f84

Not all swagger documents need to be saved to disk; changes in OpenAp…

f7194d9

…iTestContext to allow for this

maurei changed the base branch from master to openapi September 6, 2022 15:40

maurei changed the title ~~Openapi required and nullable properties~~ Support for handling of non-nullable reference types in required Sep 6, 2022

Added OpenApi client tests for nullable and required properties

449a079

maurei force-pushed the openapi-required-and-nullable-properties branch from a75fc23 to 916c753 Compare September 6, 2022 18:52

maurei changed the title ~~Support for handling of non-nullable reference types in required~~ Support for handling of non-nullable reference types in required Sep 6, 2022

Use NullabilityInfoContext for nullability information

c55a2cb

maurei force-pushed the openapi-required-and-nullable-properties branch from 916c753 to c55a2cb Compare September 6, 2022 19:20

maurei changed the title ~~Support for handling of non-nullable reference types in required~~ Support NRT and MSV in required and nullable Sep 7, 2022

maurei marked this pull request as ready for review September 7, 2022 05:29

maurei requested a review from bkoelman September 7, 2022 05:35

bkoelman requested changes Sep 8, 2022

View reviewed changes

bkoelman added 2 commits September 8, 2022 02:28

Merge branch 'merge-master-v503-into-openapi' into openapi-required-a…

31638b8

…nd-nullable-properties

Post-merge fixes

fc15912

This comment was marked as resolved.

Sign in to view

bkoelman added 2 commits December 15, 2022 11:27

Merge branch 'openapi' into openapi-required-and-nullable-properties

7288b8a

Post-merge fixes

d7c8b4b

bkoelman mentioned this pull request Dec 15, 2022

Merge openapi branch into PR 1185: Support NRT and MSV in required and nullable #1229

Merged

bkoelman and others added 3 commits December 15, 2022 17:37

Fixed: do not share NullabilityInfoContext, it is not thread-safe

d72e43e

Merge pull request #1229 from json-api-dotnet/merge-openapi-into-1185

526d03f

Merge openapi branch into PR 1185: Support NRT and MSV in required and nullable

maurei added 2 commits December 19, 2022 10:16

remove redundant new lines in eof added by cleanupcode

7266bc6

Improved naming in OpenApiTests/SchemaProperties

945a803

maurei force-pushed the openapi-required-and-nullable-properties branch 2 times, most recently from d462a9d to 589ad48 Compare December 20, 2022 12:26

This comment was marked as resolved.

Sign in to view

bkoelman added 26 commits January 11, 2023 00:04

Rename base type to avoid OpenApiClientTests.OpenApiClientTests

6881208

Adapt to existing naming convention

cc3815c

Remove redundant assertions, fix formatting

4ef8214

Correct test names

dcdf5a5

Centralize code for property assignment in tests

2d7f8b9

Merge pull request #1244 from json-api-dotnet/openapi-nrt-msv-updates

7364e94

OpenAPI NRT/MSV updates

Merge branch 'openapi' into openapi-required-and-nullable-properties

7c6f9eb

Apply Resharper hint: convert switch statement to expression

41d6d67

Simplify expressions

bd31216

Simplify exception assertions

6a87751

Use string interpolation

5c85c3a

Corrections in openapi documentation

b08ab9b

Simplify code

f7786bb

Remove redundant suppression

c3c4844

Merge branch 'master' into openapi-required-and-nullable-properties

66a2dc4

Combine OpenAPI client tests for create resource with null/default at…

f46db8a

…tribute

Fixup OpenAPI example and docs

63bf071

Revert "Merge branch 'master' into openapi-required-and-nullable-prop…

ce8045d

…erties" This reverts commit 66a2dc4, reversing changes made to c3c4844.

Workaround for running OpenAPI tests on Windows

fac0471

Merge branch 'openapi' into openapi-required-and-nullable-properties

dd667be

Address failing InspectCode

9194487

Remove redundant calls

85d608b

Remove redundant tests

007ff30

Move types out of the wrong namespace

817aeb3

Merge branch 'openapi' into openapi-required-and-nullable-properties

fb9b12a

Remove redundant suppressions in openapi after update to CSharpGuidel…

0dd49b0

…inesAnalyzer v3.8.4

bkoelman approved these changes Oct 9, 2023

View reviewed changes

bkoelman merged commit c48dea3 into openapi Oct 9, 2023
15 checks passed

bkoelman deleted the openapi-required-and-nullable-properties branch October 9, 2023 19:15

bkoelman mentioned this pull request Dec 6, 2023

Partial post/patch support microsoft/kiota#3846

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support NRT and MSV in `required` and `nullable` #1185

Support NRT and MSV in `required` and `nullable` #1185

maurei commented Sep 6, 2022 •

edited by bkoelman

Loading

codecov bot commented Sep 6, 2022 •

edited

Loading

bkoelman left a comment

This comment was marked as resolved.

maurei commented Dec 19, 2022 •

edited

Loading

This comment was marked as resolved.

Support NRT and MSV in required and nullable #1185

Support NRT and MSV in required and nullable #1185

Conversation

maurei commented Sep 6, 2022 • edited by bkoelman Loading

QUALITY CHECKLIST

codecov bot commented Sep 6, 2022 • edited Loading

Codecov Report

bkoelman left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

maurei commented Dec 19, 2022 • edited Loading

This comment was marked as resolved.

Support NRT and MSV in `required` and `nullable` #1185

Support NRT and MSV in `required` and `nullable` #1185

maurei commented Sep 6, 2022 •

edited by bkoelman

Loading

codecov bot commented Sep 6, 2022 •

edited

Loading

maurei commented Dec 19, 2022 •

edited

Loading