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

ParameterNamesModule: Conflicting getter definitions when using @JsonProperty annotation which overrides constructor parameter name #123

Open
davidboden opened this issue Jul 22, 2019 · 1 comment

Comments

@davidboden
Copy link

The intention of the example below is that the bean's value is serialised as both ownerUid and accountOwnerUid and that deserialisation uses the ownerUid value. The example can be made to work by removing the ParameterNameModules and relying on the @JsonProperty annotation. It appears to be rather a contrived example (it reflects some real examples, promise); I wanted to report it because I feel that it's fairly clear what the code should do and the behaviour is unexpected.

The error when running the test using Jackson 2.9.9.1 is:
com.fasterxml.jackson.databind.JsonMappingException: Conflicting getter definitions for property "ownerUid": com.example.rest.server.RestServerJacksonModuleTest$TestBeanWithAnnotationsAndDeprecation#getOwnerUid(0 params) vs com.example.rest.server.RestServerJacksonModuleTest$TestBeanWithAnnotationsAndDeprecation#getAccountOwnerUid(0 params)

@Test
void testDeserialisationWorksWithAnnotationsAndDeprecation() throws Exception {
  UUID ownerUid = UUID.randomUUID();
  TestBeanWithAnnotationsAndDeprecation testBeanIn = new TestBeanWithAnnotationsAndDeprecation("endTo", ownerUid);

  StringWriter sw = new StringWriter();
  ObjectMapper objectMapper = new ObjectMapper();
  objectMapper.registerModule(new ParameterNamesModule());
  objectMapper.writeValue(sw, testBeanIn);

  TestBeanWithAnnotationsAndDeprecation testBeanOut = objectMapper.readValue(new StringReader(sw.toString()), TestBeanWithAnnotationsAndDeprecation.class);

  assertThat(testBeanOut.getEndToEndId()).isEqualTo("endTo");
  assertThat(testBeanOut.getAccountOwnerUid()).isEqualTo(ownerUid);
  assertThat(testBeanOut.getOwnerUid()).isEqualTo(ownerUid);
}

static class TestBeanWithAnnotationsAndDeprecation {
  private final String endToEndId;
  private final UUID accountOwnerUid;

  TestBeanWithAnnotationsAndDeprecation(
      @JsonProperty("endToEndId") String endToEndId,
      @JsonProperty("ownerUid") UUID accountOwnerUid
  ) {
    this.endToEndId = endToEndId;
    this.accountOwnerUid = accountOwnerUid;
  }

  public String getEndToEndId() {
    return endToEndId;
  }

  public UUID getAccountOwnerUid() {
    return accountOwnerUid;
  }

  @Deprecated
  public UUID getOwnerUid() {
    return accountOwnerUid;
  }
}

The difference between running the test with the parameter names module is that _renameProperties in POJOPropertiesCollector decides that the property needs to be renamed. The breakpoint in there is not hit without the parameter names module being registered with the ObjectMapper.

@cowtowncoder
Copy link
Member

Hmmh. Interesting.

I can see how/why property introspector gets confused: the problem comes from multiple factors...

  1. Implied name (ONLY found if parameter names module registered) for 2nd argument is accountOwnerUid
  2. There is also getAccountOwnerUid(), which is then linked with constructor argument
  3. Due to rename of (1), both of these essentially get renamed as "ownerUid"
  4. Alas, there is also the "real" ownerUid() getter, getOwnerUid()

Now: while I can see what intent is, question is whether it is possible to resolve it reliably without breaking something else.

Now... assuming @Deprecated means that that getter is NOT to be used, changing that to @JsonIgnore should resolve the problem as it will ignore just that accessor but not others, because of existence of @JsonProperty.

Alternatively, I think that adding @JsonProperty next to getAccountOwnerUid() (or, if I misunderstood, next to getOwnerUid()) would also resolve the situation as one accessor would be explicit, other not (just implicit based on name).

So: on short term I would add either @JsonIgnore or @JsonProperty to break the ambiguity (from Jackson POV).

I am open to suggestion on logic for other tie-breakers, but this seems like bit of a tricky case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants