Add a RESTEasy Multipart extension #11893
Conversation
boring-cyborg
bot
added
area/core
area/dependencies
|
I'll pass it on to @gastaldi for the time being. Tag me in if needed :) |
|
@evialle if you could put together a small reproducer showing that your filter solves the issue, that would be helpful. Maybe I'm missing something obvious. |
gsmet
force-pushed
the
resteasy-multipart
branch
from
175d966 to
53dc0c7
Compare
|
@asoldano could you have a look at the principle of that one? I'm changing the default for In 2020, I think the default should be UTF-8. |
gsmet
force-pushed
the
resteasy-multipart
branch
from
53dc0c7 to
75ab3c1
Compare
|
@geoand interested in your feedback on this one. |
|
I just have one question: Would it make sense to not have a separate extension, but simply enable the filter conditionally at build time if the multipart dependency is present? |
That makes sense, as long as the necessary changes are restricted to just a filter setting 👍 |
|
It’s a very common usage. To be honest I wanted an extension for that one since a few months. Having an extension exposes it and I think it’s the right thing to do. The nice perk is that it won’t break existing applications not using the extension. |
|
Funny you mention that, I had the same impression in #879 😉 |
|
Ah ah good one, you got me here :). I think I totally underestimated back in the time the number of people using multipart. We have a good reason to do it now so I would say let’s do it. |
|
This time @gastaldi gets to say: "I told you so" 😆 |
|
I'm +1 for this |
| import io.quarkus.runtime.annotations.ConfigPhase; | ||
| import io.quarkus.runtime.annotations.ConfigRoot; | ||
|
|
||
| @ConfigRoot(name = "resteasy.multipart", phase = ConfigPhase.RUN_TIME) |
There was a problem hiding this comment.
Shouldn't this be BUILD_AND_RUN_TIME_FIXED since it is not supposed to change during runtime?
There was a problem hiding this comment.
Hmmm, isn't the bean instantiated at runtime? So you could change the value with a -D... in another environment for instance.
| * Note that the default value is UTF-8 which is different from RESTEasy's default value US-ASCII. | ||
| */ | ||
| @ConfigItem(defaultValue = "UTF-8") | ||
| public Charset defaultCharset; |
There was a problem hiding this comment.
I know strong typing is important but why a java.nio.charset.Charset if you only use the name?
There was a problem hiding this comment.
Mostly to be consistent with the other places. And this way it gets somehow validated by the converter.
|
I think this is ready and I answered all questions? @gastaldi can I get your approval if you're good with it? |
|
Thanks! |
Fixes #10323 by adding a new
quarkus-resteasy-multipart.Fixes #880
When using the extension, the default charset is UTF-8 instead of US-ASCII as it looks like a much saner default. It's configurable via Quarkus configuration properties.