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

Swagger plugins support #3056

Merged
merged 10 commits into from
Sep 1, 2024
Merged

Conversation

jvmlet
Copy link
Contributor

@jvmlet jvmlet commented Aug 29, 2024

Pull Request

The issue or feature being addressed

Closes #3049

Copy link
Collaborator

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

Please add tests.

src/Swashbuckle.AspNetCore.SwaggerUI/SwaggerUIOptions.cs Outdated Show resolved Hide resolved
src/Swashbuckle.AspNetCore.SwaggerUI/SwaggerUIOptions.cs Outdated Show resolved Hide resolved
src/Swashbuckle.AspNetCore.SwaggerUI/SwaggerUIOptions.cs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/Swashbuckle.AspNetCore.SwaggerUI/index.js Outdated Show resolved Hide resolved
test/WebSites/CustomUIConfig/Startup.cs Outdated Show resolved Hide resolved
test/WebSites/CustomUIConfig/Startup.cs Outdated Show resolved Hide resolved
test/WebSites/CustomUIConfig/wwwroot/ext/custom-plugin.js Outdated Show resolved Hide resolved
test/WebSites/CustomUIConfig/wwwroot/ext/custom-plugin.js Outdated Show resolved Hide resolved
@martincostello
Copy link
Collaborator

I also don't see tests for request/response interceptors.

[Fact]
public async Task IndexUrl_ReturnsInterceptors_IfConfigured()
{
var client = new TestSite(typeof(CustomUIConfig.Startup)).BuildClient();
var response = await client.GetAsync("/swagger/index.js");
var content = await response.Content.ReadAsStringAsync();
Assert.Contains("\"RequestInterceptorFunction\":", content);
Assert.Contains("\"ResponseInterceptorFunction\":", content);
}

@jvmlet
Copy link
Contributor Author

jvmlet commented Aug 29, 2024

Please add tests.

There is no infrastructure in integration tests to evaluate the index.html in browser (like Selenium) and I also don't see tests for request/response interceptors.

@martincostello
Copy link
Collaborator

martincostello commented Aug 29, 2024

#3056 (comment)

We don't test with Selenium, we test that the desired rendering effect is in the HTML as in the examples of existing tests I've directed you to.

@jvmlet
Copy link
Contributor Author

jvmlet commented Aug 29, 2024

I also don't see tests for request/response interceptors.

[Fact]
public async Task IndexUrl_ReturnsInterceptors_IfConfigured()
{
var client = new TestSite(typeof(CustomUIConfig.Startup)).BuildClient();
var response = await client.GetAsync("/swagger/index.js");
var content = await response.Content.ReadAsStringAsync();
Assert.Contains("\"RequestInterceptorFunction\":", content);
Assert.Contains("\"ResponseInterceptorFunction\":", content);
}

Great, I would expect to see test that actually asserts the js function functionality , not the template. With plugins it always there ....

@martincostello
Copy link
Collaborator

I would expect to see test that actually asserts the js function functionality

That would be testing swagger-ui itself, which isn't our code.

@jvmlet
Copy link
Contributor Author

jvmlet commented Aug 29, 2024

I would expect to see test that actually asserts the js function functionality

That would be testing swagger-ui itself, which isn't our code.

Then how do you suggest to test this line ? Without Selenium I can't assert that the plugin was loaded ...

README.md Outdated Show resolved Hide resolved
src/Swashbuckle.AspNetCore.SwaggerUI/index.js Outdated Show resolved Hide resolved
@martincostello
Copy link
Collaborator

Then how do you suggest to test this line ? Without Selenium I can't assert that the plugin was loaded ...

Do what the tests I already directed you to do - check that the text of the configured plugins is in the rendered document.

README.md Outdated Show resolved Hide resolved
@jvmlet
Copy link
Contributor Author

jvmlet commented Aug 29, 2024

Please add tests.

added

@martincostello martincostello added this to the v6.8.0 milestone Sep 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.53%. Comparing base (90f302d) to head (b7b55d4).
Report is 13 commits behind head on master.

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3056      +/-   ##
==========================================
+ Coverage   90.49%   90.53%   +0.03%     
==========================================
  Files          76       76              
  Lines        3063     3095      +32     
  Branches      496      504       +8     
==========================================
+ Hits         2772     2802      +30     
- Misses        291      293       +2     
Flag Coverage Δ
Linux 90.53% <100.00%> (+0.03%) ⬆️
Windows 90.53% <100.00%> (+0.03%) ⬆️
macOS 90.53% <100.00%> (+0.03%) ⬆️

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.

@martincostello martincostello enabled auto-merge (squash) September 1, 2024 08:14
@martincostello martincostello merged commit 20c3ecb into domaindrivendev:master Sep 1, 2024
9 checks passed
@AkselKP
Copy link

AkselKP commented Sep 18, 2024

@jvmlet Did you test this "manually"? It looks like the plugins code is executed twice...? Is this behavior within swagger-ui and out of our control?
Using your example:
image

FYI @martincostello

@jvmlet
Copy link
Contributor Author

jvmlet commented Sep 18, 2024

This is what swagger-ui does ... Not in the app's control.
@martincostello , when do you plan to release next version ?

@martincostello
Copy link
Collaborator

#3060 (comment)

@martincostello
Copy link
Collaborator

Can someone confirm whether or not the double execution is an issue or not? I don't want to publish 6.8.0 if it turns out there's a bug with this we need to fix.

@jvmlet
Copy link
Contributor Author

jvmlet commented Sep 19, 2024

This is not double execution , this is double instantiation.
This is what swagger-ui does, regardless of the source definition of the plugin (swashbuckle or end-user manual code writing ....)

renovate bot referenced this pull request in orso-co/Orso.Arpa.Api Sep 27, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[Swashbuckle.AspNetCore](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore)
| `6.7.3` -> `6.8.0` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Swashbuckle.AspNetCore/6.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Swashbuckle.AspNetCore/6.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Swashbuckle.AspNetCore/6.7.3/6.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Swashbuckle.AspNetCore/6.7.3/6.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>domaindrivendev/Swashbuckle.AspNetCore
(Swashbuckle.AspNetCore)</summary>

###
[`v6.8.0`](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/releases/tag/v6.8.0)

##### What's Changed

- Added dependency injection/easy registration for async filters by
[@&#8203;tofi92](https://redirect.github.com/tofi92) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3030](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3030)
- Improve IncludeXmlComments performance (2) by
[@&#8203;mus65](https://redirect.github.com/mus65) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3044](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3044)
- Apply ParameterFilters and RequestBodyFilters for WithOpenApi
endpoints by
[@&#8203;jgarciadelanoceda](https://redirect.github.com/jgarciadelanoceda)
in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3059](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3059)
- Swagger plugins support by
[@&#8203;jvmlet](https://redirect.github.com/jvmlet) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3056](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3056)
- Add benchmarks project by
[@&#8203;martincostello](https://redirect.github.com/martincostello) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3050](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3050)
- Fix NuGet badge by
[@&#8203;martincostello](https://redirect.github.com/martincostello) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3064](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3064)
- SwaggerGen - Improved handling of dictionaries with enum key by
[@&#8203;flarestudiopl](https://redirect.github.com/flarestudiopl) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3068](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3068)
- Fix typo by
[@&#8203;tom-star119](https://redirect.github.com/tom-star119) in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3073](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3073)
- Do not fill the RequestBody description with the first parameter of a…
by
[@&#8203;jgarciadelanoceda](https://redirect.github.com/jgarciadelanoceda)
in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3076](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3076)

##### New Contributors

- [@&#8203;tofi92](https://redirect.github.com/tofi92) made their first
contribution in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3030](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3030)
- [@&#8203;mus65](https://redirect.github.com/mus65) made their first
contribution in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3044](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3044)
- [@&#8203;jvmlet](https://redirect.github.com/jvmlet) made their first
contribution in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3056](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3056)
- [@&#8203;flarestudiopl](https://redirect.github.com/flarestudiopl)
made their first contribution in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3068](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3068)
- [@&#8203;tom-star119](https://redirect.github.com/tom-star119) made
their first contribution in
[https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3073](https://redirect.github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/3073)

**Full Changelog**:
domaindrivendev/Swashbuckle.AspNetCore@v6.7.3...v6.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,every
weekend,before 5am every weekday" in timezone Europe/Berlin, Automerge -
At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/orso-co/Orso.Arpa.Api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC45Ny4wIiwidXBkYXRlZEluVmVyIjoiMzguOTcuMCIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AiLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

[Feature request]: Expose plugins settings in swagger ui config API
4 participants