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

Convert resteasy-classic to use @ConfigMapping #45788

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

IvanPetkov23
Copy link
Contributor

Part of #45446

Copy link

quarkus-bot bot commented Jan 22, 2025

/cc @gsmet (hibernate-orm)

@IvanPetkov23 IvanPetkov23 marked this pull request as draft January 22, 2025 12:45
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jan 22, 2025
@IvanPetkov23 IvanPetkov23 deleted the feature/resteasy-classic branch January 22, 2025 13:50
@gsmet
Copy link
Member

gsmet commented Jan 22, 2025

@IvanPetkov23 hey. Why did you close this one?

@IvanPetkov23
Copy link
Contributor Author

I found that there is commit that is unrelated to this PR. Also I found errors in my code.

@gsmet
Copy link
Member

gsmet commented Jan 22, 2025

OK, keep in mind that you can rebase your PR interactively to drop a commit, and also amend a previous commit and then force push to the branch.

https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History has some nice information about it.

Now it's no biggie but it might be helpful in the future.

@gsmet
Copy link
Member

gsmet commented Jan 22, 2025

You can ping me on Zulip any time if you're struggling with it.

@IvanPetkov23 IvanPetkov23 restored the feature/resteasy-classic branch January 23, 2025 06:48
@IvanPetkov23 IvanPetkov23 reopened this Jan 23, 2025
@quarkus-bot quarkus-bot bot removed the triage/invalid This doesn't seem right label Jan 23, 2025
@IvanPetkov23 IvanPetkov23 force-pushed the feature/resteasy-classic branch from d6c0a8d to 30c418e Compare January 23, 2025 14:03

@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
public class ResteasyJsonConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the @ConfigMapping annotation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What prefix should i put in @ConfigMapping, there wasn`t any name attribute in @configroot

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be quarkus.resteasy-json

@ConfigRoot(name = "resteasy.vertx")
public class ResteasyVertxConfig {
@ConfigRoot
@ConfigMapping(prefix = "resteasy.vertx")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@ConfigMapping(prefix = "resteasy.vertx")
@ConfigMapping(prefix = "quarkus.resteasy.vertx")

Comment on lines +20 to +21
@WithName("deny-unannotated-endpoints")
boolean denyJaxRs();
Copy link
Contributor

@gastaldi gastaldi Jan 23, 2025

Choose a reason for hiding this comment

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

Suggested change
@WithName("deny-unannotated-endpoints")
boolean denyJaxRs();
@WithDefault("false")
boolean denyUnannotatedEndpoints();

@ConfigRoot(name = "security.jaxrs", phase = ConfigPhase.BUILD_AND_RUN_TIME_FIXED)
public class JaxRsSecurityConfig {
@ConfigRoot(phase = ConfigPhase.BUILD_AND_RUN_TIME_FIXED)
@ConfigMapping(prefix = "security.jaxrs")
Copy link
Member

Choose a reason for hiding this comment

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

That is not same, it is not one to one. you need quarkus.security.jaxrs unless there is some new feature I missed


/**
* @author Michal Szynkiewicz, [email protected]
*/
@ConfigRoot(name = "security.jaxrs", phase = ConfigPhase.BUILD_AND_RUN_TIME_FIXED)
public class JaxRsSecurityConfig {
@ConfigRoot(phase = ConfigPhase.BUILD_AND_RUN_TIME_FIXED)
Copy link
Member

Choose a reason for hiding this comment

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

Interestingly, we don't need this during the runtime, it could be moved to the deployment module and same change can be done in Quarkus REST. I don't think anyone has use case for this at runtime, it is too late for security...

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.

4 participants