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

OpenAPI different default content type for pojo return and primitives #36198

Merged

Conversation

phillip-kruger
Copy link
Member

Fix #34700

Draft for now until OpenAPI release

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/openapi area/smallrye area/vertx labels Sep 28, 2023
@phillip-kruger phillip-kruger force-pushed the openapi-default-content-type branch 2 times, most recently from 91257a7 to efa0338 Compare October 1, 2023 04:34
@phillip-kruger phillip-kruger marked this pull request as ready for review October 1, 2023 04:34
@phillip-kruger phillip-kruger added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 1, 2023
@quarkus-bot

This comment has been minimized.

Signed-off-by: Phillip Kruger <phillip.kruger@gmail.com>
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 2, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@phillip-kruger phillip-kruger merged commit 9fc4b2a into quarkusio:main Oct 2, 2023
50 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Oct 2, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 2, 2023
Comment on lines +167 to +168
System.setProperty(io.smallrye.openapi.api.constants.OpenApiConstants.DEFAULT_PRODUCES_PRIMITIVES, "plain/text");
System.setProperty(io.smallrye.openapi.api.constants.OpenApiConstants.DEFAULT_CONSUMES_PRIMITIVES, "plain/text");
Copy link
Member

Choose a reason for hiding this comment

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

This should be "text/plain"

Copy link
Member

Choose a reason for hiding this comment

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

Created #36247

@rsvoboda
Copy link
Member

rsvoboda commented Oct 2, 2023

@phillip-kruger also shouldn't this have breaking change or noteworthy label as this changes the default?

@@ -216,7 +215,9 @@ SyntheticBeanBuildItem initBasicAuth(
&& !buildTimeConfig.auth.basic.orElse(false)) {
//if not explicitly enabled we make this a default bean, so it is the fallback if nothing else is defined
configurator.defaultBean();
securityInformationProducer.produce(SecurityInformationBuildItem.BASIC());
if (buildTimeConfig.auth.basic.isPresent() && buildTimeConfig.auth.basic.get()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

line 215 !buildTimeConfig.auth.basic.orElse(false) means that

quarkus.http.auth.basic is missing => inside IF body
quarkus.http.auth.basic=false => inside IF body
quarkus.http.auth.basic=true => don't go in

while line 218 says

buildTimeConfig.auth.basic.isPresent() => quarkus.http.auth.basic=false || quarkus.http.auth.basic=true
buildTimeConfig.auth.basic.get() => quarkus.http.auth.basic=true

so in order to have produced BASIC here it means that quarkus.http.auth.basic must be both true and false

@phillip-kruger I don't know what was intention of this condition, can you please fix it? Thank you

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.

OpenAPI generation content-type mismatch
4 participants