Skip to content

Conversation

@michael-simons
Copy link
Contributor

@michael-simons michael-simons commented Nov 26, 2025

Jackson 3 does not detected package private constructors anyore in case they have more than one argument. This prevents the Restclient / Resttemplate and their test derivates deserializing metrics endpoints as they could do in Spring Boot 3.x with Jackson 2.

This changes the isolated mapper to the behaviour of Jackson 2, so that the mapping works out of the box again.

Here is an example that worked out of the box with Spring Boot 3:

var metrics = restTemplate.getForObject("/actuator/metrics/org.neo4j.jdbc.connections",
				MetricsEndpoint.MetricDescriptor.class);
assertThat(metrics.getMeasurements()).hasSize(1);
assertThat(metrics.getMeasurements().get(0).getValue()).isGreaterThanOrEqualTo(6.0);

This fails equally with Boot 4 using TestRestTemplate, but RestTestClient too.

I raised an issue with Jackson, and this change of behaviour in Jackson is intended, FasterXML/jackson-databind#5427 (comment) nevertheless painful in Boot / Testing consumption of metrics and I would like to suggest the inclusion of "non private" constructors for creators like I did here in my change.

Edit: Here's a bit of making a case why this change would improve quality of life: neo4j/neo4j-jdbc@b1dfa19#diff-3513ca16e1531d8f2d75d3c8b6bf1140c99ba7ad144bea71e7e9fdee1c86535a (this was before I learned about the creator visibility)

…ackage private constructors.

Jackson 3 does not detected package private constructors anyore in case they have more than one argument. This prevents the Restclient / Resttemplate and their test derivates deserializing metrics endpoints as they could do in Spring Boot 3.x with Jackson 2.

This changes the isolated mapper to the behaviour of Jackson 2, so that the mapping works out of the box again.

Signed-off-by: Michael Simons <michael@simons.ac>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 26, 2025
@wilkinsona
Copy link
Member

Thanks, @michael-simons.

The proposed change will configure the JsonMapper that's used on the server-side to serialise endpoint responses to JSON. It's unlikely that this JsonMapper will be used by a RestClient or RestTemplate as they typically use the application's main JsonMapper. As such, I'm not sure the change will help.

I wonder if tools.jackson.databind.cfg.MapperBuilder.configureForJackson2() could tune the visibility? You could then set spring.jackson.use-jackson-2-defaults to true and I think things would work as they did with Boot 3.x.

Somewhat related, we intentionally made the constructors package-private to try and limit the creation of those objects to the endpoints themselves. I've just learned through this PR that Jackson 2 makes it easy to work around that, but the intention remains that those types aren't really intended to be used/created on the client side.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Nov 26, 2025
@michael-simons
Copy link
Contributor Author

Aaah, ok. If that's not their intended use case, I can live with that.

However what is true is that the ObjectMapper instance picked up by the testing infrastructure is picking up the endpoint specific one.

If you look at my specific changes in this class, you notice that I need to configure

@TestPropertySource(properties = { "management.endpoints.jackson.isolated-json-mapper=false" })

to make the template "do the right thing" and use the mixins

neo4j/neo4j-jdbc@b1dfa19#diff-3513ca16e1531d8f2d75d3c8b6bf1140c99ba7ad144bea71e7e9fdee1c86535a

or the changes via the customizer

neo4j/neo4j-jdbc@2b372f0

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 26, 2025
@wilkinsona
Copy link
Member

wilkinsona commented Nov 26, 2025

If you look at my specific changes in this class, you notice that I need to configure … to make the template "do the right thing"

I did not expect that and I think you've found a bug. It's necessary because EndpointJsonMapperWebMvcConfigurer is configuring the JacksonJsonHttpMessageConverter to use the Actuator-specific JsonMapper for types that implement OperationResponseBody. It's doing that assuming that the JacksonJsonHttpMessageConverter is server-side only but in fact it's shared with the client-side too. I wonder if we should define the message converter beans as prototypes so there's better separation between client and server.

@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress and removed status: feedback-provided Feedback has been provided labels Nov 26, 2025
@michael-simons
Copy link
Contributor Author

I'm happy that this investigation is useful, then :) Thank you for all that valuable feedback.

@bclozel
Copy link
Member

bclozel commented Nov 26, 2025

After discussing this today with the team, I have created #48310. I think this should mostly solve this problem as the actuator converter should only be contributed to the server side.

This will unfortunately break your test case @michael-simons. Thanks a lot for this report, this helps uncovering an important flaw in our current arrangement. Hopefully your integration tests can be easily updated to not depend on those Boot types directly.

I guess this PR is superseded by that other new issue? Am I missing something?

@wilkinsona
Copy link
Member

Yeah, I think we can close this one. Once #48310 has been fixed, customizations of the app's main JsonMapper would be sufficient, without the need to disable the Actuator's isolated mapper.

@wilkinsona wilkinsona closed this Nov 27, 2025
@wilkinsona wilkinsona added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Nov 27, 2025
@michael-simons
Copy link
Contributor Author

@sdeleuze Thanks, for both the feedback and the heads up. I will be able to query the metrics endpoints by other means, no worries. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: superseded An issue that has been superseded by another

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants