-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Ensure that custom Jackson modules work in dev-mode #44373
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
144 changes: 144 additions & 0 deletions
144
...java/io/quarkus/resteasy/reactive/jackson/deployment/test/CustomModuleLiveReloadTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| package io.quarkus.resteasy.reactive.jackson.deployment.test; | ||
|
|
||
| import static io.restassured.RestAssured.given; | ||
| import static org.hamcrest.Matchers.containsString; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| import jakarta.inject.Singleton; | ||
| import jakarta.ws.rs.GET; | ||
| import jakarta.ws.rs.Path; | ||
| import jakarta.ws.rs.Produces; | ||
| import jakarta.ws.rs.core.MediaType; | ||
|
|
||
| import org.jboss.shrinkwrap.api.asset.StringAsset; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonGenerator; | ||
| import com.fasterxml.jackson.core.JsonParser; | ||
| import com.fasterxml.jackson.core.JsonToken; | ||
| import com.fasterxml.jackson.databind.DeserializationContext; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.SerializerProvider; | ||
| import com.fasterxml.jackson.databind.deser.std.StdDeserializer; | ||
| import com.fasterxml.jackson.databind.module.SimpleModule; | ||
| import com.fasterxml.jackson.databind.ser.std.StdSerializer; | ||
|
|
||
| import io.quarkus.jackson.ObjectMapperCustomizer; | ||
| import io.quarkus.test.QuarkusDevModeTest; | ||
| import io.vertx.core.json.JsonArray; | ||
|
|
||
| public class CustomModuleLiveReloadTest { | ||
|
|
||
| @RegisterExtension | ||
| static final QuarkusDevModeTest TEST = new QuarkusDevModeTest() | ||
| .withApplicationRoot((jar) -> jar | ||
| .addClasses(Resource.class, StringAndInt.class, StringAndIntSerializer.class, | ||
| StringAndIntDeserializer.class, Customizer.class) | ||
| .addAsResource(new StringAsset("index content"), "META-INF/resources/index.html")); | ||
|
|
||
| @Test | ||
| void test() { | ||
| assertResponse(); | ||
|
|
||
| // force reload | ||
| TEST.addResourceFile("META-INF/resources/index.html", "html content"); | ||
|
|
||
| assertResponse(); | ||
| } | ||
|
|
||
| private static void assertResponse() { | ||
| given().accept("application/json").get("test/array") | ||
| .then() | ||
| .statusCode(200) | ||
| .body(containsString("first:1"), containsString("second:2")); | ||
| } | ||
|
|
||
| @Path("test") | ||
| public static class Resource { | ||
|
|
||
| @Path("array") | ||
| @GET | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public JsonArray array() { | ||
| var array = new JsonArray(); | ||
| array.add(new StringAndInt("first", 1)); | ||
| array.add(new StringAndInt("second", 2)); | ||
| return array; | ||
| } | ||
| } | ||
|
|
||
| public static class StringAndInt { | ||
| private final String stringValue; | ||
| private final int intValue; | ||
|
|
||
| public StringAndInt(String s, int i) { | ||
| this.stringValue = s; | ||
| this.intValue = i; | ||
| } | ||
|
|
||
| public static StringAndInt parse(String value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| int dot = value.indexOf(':'); | ||
| if (-1 == dot) { | ||
| throw new IllegalArgumentException(value); | ||
| } | ||
| try { | ||
| return new StringAndInt(value.substring(0, dot), Integer.parseInt(value.substring(dot + 1))); | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException(value, e); | ||
| } | ||
| } | ||
|
|
||
| public String format() { | ||
| return this.stringValue + ":" + intValue; | ||
| } | ||
| } | ||
|
|
||
| public static class StringAndIntSerializer extends StdSerializer<StringAndInt> { | ||
|
|
||
| public StringAndIntSerializer() { | ||
| super(StringAndInt.class); | ||
| } | ||
|
|
||
| @Override | ||
| public void serialize(StringAndInt value, JsonGenerator gen, SerializerProvider provider) throws IOException { | ||
| if (value == null) | ||
| gen.writeNull(); | ||
| else { | ||
| gen.writeString(value.format()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public static class StringAndIntDeserializer extends StdDeserializer<StringAndInt> { | ||
|
|
||
| public StringAndIntDeserializer() { | ||
| super(StringAndInt.class); | ||
| } | ||
|
|
||
| @Override | ||
| public StringAndInt deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { | ||
| if (p.currentToken() == JsonToken.VALUE_STRING) { | ||
| return StringAndInt.parse(p.getText()); | ||
| } else if (p.currentToken() == JsonToken.VALUE_NULL) { | ||
| return null; | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| @Singleton | ||
| public static class Customizer implements ObjectMapperCustomizer { | ||
| @Override | ||
| public void customize(ObjectMapper objectMapper) { | ||
| var m = new SimpleModule("test"); | ||
| m.addSerializer(StringAndInt.class, new StringAndIntSerializer()); | ||
| m.addDeserializer(StringAndInt.class, new StringAndIntDeserializer()); | ||
| objectMapper.registerModule(m); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the
QuarkusJacksonJsonCodecalways be loaded by Vert.x? Because if so, I wonder if we should populate a new mapper directly instead of resetting it.That being said I'm not entirely sure we could safely drop some of the guards but wanted to push the idea in case you didn't think about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the method is probably confusing here - what it does is actually create a new
ObjectMapperused by Vert.x. Do you want me to change the method as to make it easier for us to understand next time we stumble upon the code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I understand what the method does.
What I was wondering is if the
reset()couldpopulate()the default mapper instead of populating it. Maybe in a synchronized method so that we could drop the volatile.That would make sense only if the codec is always initialized though. Otherwise we might pay the initialization cost for nothing.
It might be a bad idea though, I will let you decide what's best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to avoid that because populating means extracting from Arc, which I am not 100% sure would give the proper bean at this point.
Whereas the way it's done now, the loading of the mapper happens in the same "timing" as if there no reload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sure, it makes perfect sense. We can revisit if we see it creates some performance issues but in this case, we will have to be very careful as to how operations are ordered.