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

remove the sample Swagger UI request body if present fix #12412 #12927

Conversation

PierreBesson
Copy link
Contributor

@PierreBesson PierreBesson commented Nov 1, 2020

This is my workaround for #12412. It does not remove the Swagger UI additionalProps: "string" content but it patches it when detected so that requests to management endpoints from Swagger UI work out of the box (no need to remove the sample request anymore).


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@pascalgrimaud
Copy link
Member

seriously, the patch was so easy ! thanks a lot @PierreBesson
I'll be able to use it in my real projects :)

@PierreBesson PierreBesson force-pushed the fix-12412-swagger-ui-management-default-body branch from a2cbc85 to c81355c Compare November 1, 2020 16:51
@cbornet
Copy link
Member

cbornet commented Nov 1, 2020

Can you also check that the request method is a GET ?
I don't understand why Spring Boot actuator passes GET params in the body. This is clearly discouraged by the HTTP RFC...

@ndywicki
Copy link
Contributor

ndywicki commented Nov 3, 2020

Good job @PierreBesson 👍
I will test the modification.
I see there is another topic on Open API v3 migration #12125 but it is in the V7 roadmap :)

@@ -102,6 +102,10 @@
if (authToken) {
req.headers['Authorization'] = "Bearer " + authToken;
}
// Remove the sample Swagger UI request body if present
if (req.body === '{"additionalProp1":"string","additionalProp2":"string","additionalProp3":"string"}') {
Copy link
Member

Choose a reason for hiding this comment

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

you need to add this part above too, for oauth2 configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -102,6 +102,10 @@
if (authToken) {
req.headers['Authorization'] = "Bearer " + authToken;
}
// Remove the sample Swagger UI request body if present
if (req.body === '{"additionalProp1":"string","additionalProp2":"string","additionalProp3":"string"}') {
Copy link
Member

Choose a reason for hiding this comment

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

same, you need to add this part above too, for oauth2 configuration

@@ -98,6 +98,10 @@
if (authToken) {
req.headers['Authorization'] = "Bearer " + authToken;
}
// Remove the sample Swagger UI request body if present
if (req.body === '{"additionalProp1":"string","additionalProp2":"string","additionalProp3":"string"}') {
Copy link
Member

Choose a reason for hiding this comment

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

same, you need to add this part above too, for oauth2 configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added it for other auth methods as well. Good catch @pascalgrimaud !

@PierreBesson PierreBesson force-pushed the fix-12412-swagger-ui-management-default-body branch from c81355c to 856bce5 Compare November 9, 2020 19:45
@PierreBesson
Copy link
Contributor Author

Can you also check that the request method is a GET ?
I don't understand why Spring Boot actuator passes GET params in the body. This is clearly discouraged by the HTTP RFC...

Done, I only do it in case of GET. I agree that no one should pass a body to a GET request but just in case someone does it, it will still be supported by our Swagger UI if the content is not {"additionalProp1":"string","additionalProp2":"string","additionalProp3":"string"}.

@pascalgrimaud pascalgrimaud merged commit 2862092 into jhipster:main Nov 9, 2020
@pascalgrimaud pascalgrimaud added this to the 7.0.0-beta.0 milestone Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants