Skip to content

Conversation

@mauve
Copy link
Contributor

@mauve mauve commented Jun 6, 2024

Adds async document, operation, parameter and request body filters.

Fixes: #1661

@mauve
Copy link
Contributor Author

mauve commented Jun 6, 2024

Hi @martincostello, Thanks for taking over maintainership of the project ❤️.

Is there a way for me to trigger the checks?

@martincostello
Copy link
Collaborator

Is there a way for me to trigger the checks?

No, it's a built-in feature of GitHub Actions for first time contributors to a repo - someone with write access needs to approve them to run until your first contribution is merged.

@mauve mauve force-pushed the mauve/async-filters branch from 4823ccc to a328799 Compare June 6, 2024 12:58
@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2024

Codecov Report

Attention: Patch coverage is 96.48241% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.43%. Comparing base (12245c0) to head (fb66275).

Files Patch % Lines
...washbuckle.AspNetCore.Swagger/SwaggerMiddleware.cs 80.76% 5 Missing ⚠️
...re.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs 98.72% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2938      +/-   ##
==========================================
+ Coverage   90.13%   90.43%   +0.29%     
==========================================
  Files          72       72              
  Lines        2736     2832      +96     
  Branches      433      443      +10     
==========================================
+ Hits         2466     2561      +95     
- Misses        270      271       +1     
Flag Coverage Δ
Linux 90.43% <96.48%> (+0.29%) ⬆️
Windows 90.21% <96.48%> (+0.08%) ⬆️
macOS 90.43% <96.48%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@mauve
Copy link
Contributor Author

mauve commented Jun 6, 2024

@martincostello The tests are now green :) Let me know if you want any further changes.

@martincostello
Copy link
Collaborator

Thanks - I'll take a look at this in more detail tomorrow 👍

@mauve
Copy link
Contributor Author

mauve commented Jun 10, 2024

@martincostello Hi, have you had chance to look at it?

@martincostello
Copy link
Collaborator

Sorry, not yet, but it's on my TODO list.

@martincostello
Copy link
Collaborator

Hi @mauve - I checked out your pull request locally and had a look around the code. I've done some refactoring in this commit that aims to reduce the duplication in SwaggerGenerator: martincostello@01cf4d5

What are your thoughts on the approach?

- Do not assume the `ISwaggerProvider` registration implements `IAsyncSwaggerProvider`.
- Implement sync filters in terms of async operations.
- Add CancellationToken parameter to async filters.
- Make test filters also async.
- Fix a number of analyser suggestions.
@mauve
Copy link
Contributor Author

mauve commented Jun 11, 2024

Great! Unsure what the process should be but I just cherry-picked your commit to the top of my PR.

@martincostello
Copy link
Collaborator

Thanks - I'll take a proper look at this again either later this afternoon, or more likely, tomorrow 👍

@mauve
Copy link
Contributor Author

mauve commented Jun 14, 2024

@martincostello I have removed the CancellationToken from GetSwaggerAsync, however I left them in the newly added interfaces so that we don't need to break that interface when we introduce CancellationToken again.

@martincostello martincostello added this to the v6.7.0 milestone Jun 17, 2024
This was referenced Oct 22, 2025
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.

Make IDocumentFilter async compatible

3 participants