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.

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

Adapt Jackson2ObjectMapperBuilder documentation to mention ParameterNamesModule registration #31959

Closed
lbenedetto opened this issue Jan 6, 2024 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@lbenedetto
Copy link

lbenedetto commented Jan 6, 2024

After updating to 3.2.x, deserialization seems to be broken in my project. Here is some simple code to reproduce.

Given a class:

import java.beans.ConstructorProperties;
import org.springframework.lang.NonNull;
import lombok.EqualsAndHashCode;
import lombok.Value;

@Value
@EqualsAndHashCode(callSuper = false)
public class TestClass {

  @ConstructorProperties("value")
  public TestClass(String somethingElse) {
    System.out.println("Got value: " + somethingElse);
    this.value = somethingElse;
  }

  @NonNull
  String value;
}

And a unit test:

  @Test
  void testCountryId() throws Exception {
    // Given
    var object = new TestClass("test");
    var mapper = new Jackson2ObjectMapperBuilder().build();

    // When
    var toString = mapper.writeValueAsString(object);
    var fromString = mapper.readValue(toString, TestClass.class);

    // Then
    then(fromString).isEqualTo(object);
  }

The test passes in both versions (in my actual project it fails because the value cannot be null), but

When running the unit test with Spring 3.2, it will print:

Got value: test
Got value: null

However, when running the unit test with Spring 3.1, it will print:

Got value: test
Got value: test

If I change the constructor parameter name from somethingElse to value then 3.2 prints the same as 3.1.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 6, 2024
@sdeleuze sdeleuze self-assigned this Jan 6, 2024
@bclozel
Copy link
Member

bclozel commented Jan 6, 2024

I think this is due to the addition of the parameter names module in #27511.

Rewriting the test without involving Spring at all shows the same behavior:

@Test
void testCountryId() throws Exception {
    // Given
    var object = new TestClass("test");
    var mapper = new ObjectMapper();
    mapper.registerModule(new ParameterNamesModule());

    // When
    var toString = mapper.writeValueAsString(object);
    var fromString = mapper.readValue(toString, TestClass.class);

    // Then
    then(fromString).isEqualTo(object);
    assertThat(fromString.getValue()).isEqualTo("test");
}

The behavior is described in Jackson's documentation for the module:

if class Person has a single argument constructor, its argument needs to be annotated with @JsonProperty("propertyName"). This is to preserve legacy behavior, see FasterXML/jackson-databind#1498

Updating the class accordingly makes things work:

@Value
@EqualsAndHashCode(callSuper = false)
public class TestClass {

    @ConstructorProperties("value")
    public TestClass(@JsonProperty("value") String somethingElse) {
        Assert.notNull(somethingElse, "constructor value should not be null");
        this.value = somethingElse;
    }

    @NonNull
    String value;
}

While this behavior change comes from a Spring Framework update, I believe the behavior itself comes from Jackson and there's nothing we can do (besides removing the default registration of that module?).

@sdeleuze sdeleuze changed the title Jackson2ObjectMapperBuilder misbehaving after updating to 3.2.x Jackson2ObjectMapperBuilder misbehaving after updating to 3.2.x Jan 8, 2024
@sdeleuze sdeleuze changed the title Jackson2ObjectMapperBuilder misbehaving after updating to 3.2.x Adapt Jackson2ObjectMapperBuilder documentation to mention ParameterNamesModuleregistration Jan 8, 2024
@sdeleuze sdeleuze added type: documentation A documentation task in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 8, 2024
@sdeleuze sdeleuze added this to the 6.1.3 milestone Jan 8, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 8, 2024

Thanks @bclozel for your analysis.

This change of behavior is indeed a side effect of the Jackson module registration, but on Spring side that was not properly documented. That's now the case in 2 places:

  • Jackson2ObjectMapperBuilder Javadoc now documents such registration properly.
  • The upgrade side-effect is now properly documented here.

As a consequence, I have turned this issue into a documentation one.

@sdeleuze sdeleuze changed the title Adapt Jackson2ObjectMapperBuilder documentation to mention ParameterNamesModuleregistration Adapt Jackson2ObjectMapperBuilder documentation to mention ParameterNamesModule registration Jan 8, 2024
@lbenedetto
Copy link
Author

lbenedetto commented Jan 9, 2024

I created a bug report with jackson and they seem to have accepted it as an actual issue on their end.
It seems that ParameterNamesModule causes @ConstructorProperties annotations to be ignored when they shouldn't be.
FasterXML/jackson-databind#4310

@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 9, 2024

Interesting, thanks for the follow up, hopefully this will be fixed on Jackson side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants