Convert resteasy-classic to use @ConfigMapping #45788
Conversation
|
/cc @gsmet (hibernate-orm) |
|
@IvanPetkov23 hey. Why did you close this one? |
|
I found that there is commit that is unrelated to this PR. Also I found errors in my code. |
|
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. |
|
You can ping me on Zulip any time if you're struggling with it. |
IvanPetkov23
force-pushed
the
feature/resteasy-classic
branch
from
d6c0a8d to
30c418e
Compare
|
|
||
| @ConfigRoot(phase = ConfigPhase.BUILD_TIME) | ||
| public class ResteasyJsonConfig { |
There was a problem hiding this comment.
Missing the @ConfigMapping annotation
There was a problem hiding this comment.
What prefix should i put in @ConfigMapping, there wasn`t any name attribute in @configroot
There was a problem hiding this comment.
It should be quarkus.resteasy-json
| @ConfigRoot(name = "resteasy.vertx") | ||
| public class ResteasyVertxConfig { | ||
| @ConfigRoot | ||
| @ConfigMapping(prefix = "resteasy.vertx") |
There was a problem hiding this comment.
| @ConfigMapping(prefix = "resteasy.vertx") | |
| @ConfigMapping(prefix = "quarkus.resteasy.vertx") |
| @WithName("deny-unannotated-endpoints") | ||
| boolean denyJaxRs(); |
There was a problem hiding this comment.
| @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") |
There was a problem hiding this comment.
That is not same, it is not one to one. you need quarkus.security.jaxrs unless there is some new feature I missed
There was a problem hiding this comment.
Yes, correct.
We know it is annoying. Let me provide an explanation for it:
@ConfigRoot would automatically add quarkus to ConfigRoot#name and this was fine for a while. When we started to have external extensions and even Quarkiverse, it would mean that these extensions would be forced to use the quarkus prefix, even if they were not part of the core, so we ended up adding ConfigRoot#prefix, for extensions to override the quarkus prefix.
When we did the @ConfigMapping, we just created a simple parameter prefix so everyone could just set the namespace they want.
There was a problem hiding this comment.
I never realized that was the reason, just took it as "that's how it is". It makes sense, thanks.
|
|
||
| /** | ||
| * @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) |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Ideally, let's adjust the config in follow up PRs.
I know it might look tempting to do the changes right away but it might make things confusing or harder to revert if we need to revert a specific adjustment.
So let's do the simple conversion and let's apply the cleanup afterwards.
There was a problem hiding this comment.
Sure, if interested, @IvanPetkov23 can provide a follow-up PR once this one is merged. Otherwise I'll take care of it when there is a time.
Part of #45446