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.

Sign up for GitHub

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

Jump to bottom

Move standard ObjectMapper customization to dedicated Customizer #40819

Merged
merged 1 commit into from
May 31, 2024
Merged

Move standard ObjectMapper customization to dedicated Customizer #40819

merged 1 commit into from
May 31, 2024

Conversation

FWest98
Copy link
Contributor

@FWest98 FWest98 commented May 23, 2024

This change moved the standard ObjectMapper customization behaviour to its own dedicated ObjectMapperCustomizer with maximum priority. This way, these default (config-driven) customizations are available to anyone injecting all Customizers in their own code when creating custom ObjectMapper instances - such as ones based on YAML or other formats.

The current code has these customizations tied to the instantiation of a default ObjectMapper - thus unavailable to anyone constructing a YAMLMapper, CsvMapper, or any of the other custom data formats. Other customizations of the ObjectMapper offered by Quarkus (such as OTel-specific ones) are offered through the ObjectMapperCustomizer interface. This MR makes the situation more consistent, allowing users to inherit all Quarkus customizations.

The new customizer has the highest priority to ensure that the customizations remain applied in the same order.

@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label May 23, 2024
@gsmet gsmet requested a review from geoand May 28, 2024 08:20
@geoand
Copy link
Contributor

geoand commented May 28, 2024

Makes sense!

Can you please squash the commits?

@FWest98
Move standard ObjectMapper customization to dedicated Customizer
5e259f1 
This will allow users to produce their own custom ObjectMappers with the same base behaviour
as the one offered by Quarkus.
@FWest98 FWest98 force-pushed the fw/defaultmappercustomizer branch from cc0d36d to 5e259f1 Compare May 28, 2024 13:38
FWest98
FWest98 commented May 28, 2024
View reviewed changes
extensions/jackson/deployment/src/main/java/io/quarkus/jackson/deployment/JacksonProcessor.java
@@ -111,6 +112,8 @@ public class JacksonProcessor {
@BuildStep
void unremovable(Capabilities capabilities, BuildProducer<UnremovableBeanBuildItem> producer,
BuildProducer<AdditionalBeanBuildItem> additionalProducer) {
additionalProducer.produce(AdditionalBeanBuildItem.unremovableOf(ConfigurationCustomizer.class));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the correct way of doing it? Or should I use the producer instead of the additionalProducer?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should work just fine :)

@FWest98
Copy link
Contributor Author

FWest98 commented May 28, 2024 •
edited
Loading

@geoand I managed to fix my testing issue and squashed the commits!

geoand
geoand approved these changes May 31, 2024
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 31, 2024
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented May 31, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5e259f1.

✅ 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.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

📦 integration-tests/rest-client

✖ io.quarkus.it.rest.client.wronghost.ExternalWrongHostUsingVerifyHostTestCase.restClient - History

  • Read timed out - java.net.SocketTimeoutException
java.net.SocketTimeoutException: Read timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:288)
	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:314)
	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)
	at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:808)
	at java.base/java.net.Socket$SocketInputStream.read(Socket.java:966)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:161)
	at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

@geoand geoand merged commit 7c51431 into quarkusio:main May 31, 2024
51 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 31, 2024
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

Assignees
No one assigned
Labels
area/jackson Issues related to Jackson (JSON library) triage/flaky-test
Projects
None yet
Milestone
3.12.0.CR1
Development

Successfully merging this pull request may close these issues.
2 participants
@FWest98 @geoand