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

Add support for JsonPolymorphic and JsonDerivedType attributes to Swashbuckle.AspNetCore.Annotations for .NET7 and later #2671

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

schnerring
Copy link

@schnerring schnerring commented Jun 16, 2023

Beginning with .NET 7, System.Text.Json supports polymorphic type hierarchy serialization and deserialization with attribute annotations.

This PR adds support for JsonPolymorphic and JsonDerivedType attributes to Swashbuckle.AspNetCore.Annotations for .NET 7 and later.

Fixes #2571

@schnerring schnerring changed the title Add stj polymorphic attr Add support for JsonPolymorphic and JsonDerivedType attributes to Swashbuckle.AspNetCore.Annotations for .NET7 and later Jun 16, 2023
@schnerring schnerring force-pushed the add-stj-polymorphic-attr branch 2 times, most recently from 929b810 to 8995799 Compare June 16, 2023 15:21
@schnerring
Copy link
Author

schnerring commented Jun 16, 2023

To fix the appveyor build, tests should be upgraded to net7 first. I'll rebase after related PR has been merged: #2672

@tomohisa
Copy link

This change would be great! @schnerring
Looking forward to use it in Swashbuckle.AspNetCore.Annotations

@martincostello
Copy link
Collaborator

Thanks for contributing - if you'd like to continue with this pull request, please rebase against the default branch to pick up our new CI. We'd also need tests for these new scenarios.

Copy link
Contributor

This pull request is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further changes are made.

@github-actions github-actions bot added the stale Stale issues or pull requests label Jun 13, 2024
@schnerring
Copy link
Author

schnerring commented Jun 21, 2024

I rebased the PR against the current master branch.

The "test" (more of a sample) of the obsolete functionality is in here:

test/WebSites/NswagClientExample/Controllers/AnimalsController.cs

I've adjusted this to also include the new attributes.

Integration tests would be nice, but I'm not sure how to do that from scratch and would appreciate some input. We basically need to test if the OpenAPI spec is generated correctly. Is there a test harness inside the code base I could reuse?

@martincostello
Copy link
Collaborator

Integration tests would be nice, but I'm not sure how to do that from scratch and would appreciate some input. We basically need to test if the OpenAPI spec is generated correctly. Is there a test harness inside the code base I could reuse?

If you look at the Swashbuckle.AspNetCore.IntegrationTests project, it validates things through the sites in the WebSites folder. There are also verification tests associated with generated schemas here.

@github-actions github-actions bot removed the stale Stale issues or pull requests label Jun 22, 2024
@ferhatmacak
Copy link

any progress?

@wont-work
Copy link

to anyone interested, it's possible to "manually" do what this PR does on your own code, until it gets merged: https://codeberg.org/KittyShopper/outpost/src/commit/3d8f0af997d3963ba552efe7ef9aec9b81d61a05/Outpost/Core/Util/OpenApi.cs#L135-L155

@schnerring
Copy link
Author

any progress?

As I'm currently not using Swashbuckle and I'm busy with other things, I don't really have the time to dig into the code base and figure out how to test this. Any help would be appreciated.

@martincostello
Copy link
Collaborator

The preferred approach is to add a controller/endpoint to one of the test websites and then add appropriate verification tests to the integration test project to verify that the expected OpenAPI document is produced. There's various examples of that approach here.

@schnerring schnerring force-pushed the add-stj-polymorphic-attr branch 2 times, most recently from 41527d6 to 7196c12 Compare September 1, 2024 21:41
@schnerring
Copy link
Author

schnerring commented Sep 1, 2024

Sorry @martincostello, I didn't want you to have to repeat yourself. You already told me earlier where to find the integration tests. But as far as I can tell, the old polymorphism feature is also untested, so there is no way for me to copy any tests and work from there. I also added a (failing) test case for the NSwagClientExample to test if a valid Swagger doc is generated, but I have given up after an hour of debugging.

However, I implemented your other code review suggestions and rebased it to the current master. So maybe someone with more Swashbuckle expertise can pick up this PR in the future.

@Laurianti
Copy link

to anyone interested, it's possible to "manually" do what this PR does on your own code, until it gets merged: https://codeberg.org/KittyShopper/outpost/src/commit/3d8f0af997d3963ba552efe7ef9aec9b81d61a05/Outpost/Core/Util/OpenApi.cs#L135-L155

Yes, it works, could you fix and merge the PR please?

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

Successfully merging this pull request may close these issues.

System.Text.JSON polymorphic behaviour not supported
6 participants