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

findImplicitPropertyName and setPropertyNamingStrategy change from 2.6.4 to 2.7.0 #1077

Closed
michaelhixson opened this issue Jan 12, 2016 · 8 comments

Comments

@michaelhixson
Copy link
Contributor

I use a custom AnnotationIntrospector that reads the java.beans.ConstructorProperties annotation on constructors. It figures out the mapping from JSON fields to constructor arguments. This code worked in 2.6.4 but broke in 2.7.0, with respect to its interaction with PropertyNamingStrategy.

Here's a sample unit test that used to pass but now fails:

@Test
public void testConstructorProperties() throws IOException {
  Assert.assertEquals(
      "{\"first_value\":7,\"second_value\":9}",
      new ObjectMapper()
          .registerModule(new ConstructorPropertiesModule())
          .setPropertyNamingStrategy(
              PropertyNamingStrategy.CAMEL_CASE_TO_LOWER_CASE_WITH_UNDERSCORES)
          .writeValueAsString(new Foo(7, 9)));
}

public static final class Foo {
  private final Integer firstValue;
  private final Integer secondValue;

  @java.beans.ConstructorProperties({"firstValue", "secondValue"})
  public Foo(Integer firstValue, Integer secondValue) {
    this.firstValue = firstValue;
    this.secondValue = secondValue;
  }

  public Integer getFirstValue() {
    return firstValue;
  }

  public Integer getSecondValue() {
    return secondValue;
  }
}

public static final class ConstructorPropertiesModule extends SimpleModule {
  @Override
  public void setupModule(SetupContext context) {
    super.setupModule(context);
    context.insertAnnotationIntrospector(new NopAnnotationIntrospector() {
      @Override
      public boolean hasCreatorAnnotation(Annotated a) {
        return a instanceof AnnotatedConstructor
            && a.hasAnnotation(ConstructorProperties.class);
      }

      @Override
      public String findImplicitPropertyName(AnnotatedMember member) {
        if (!(member instanceof AnnotatedParameter)) {
          return null;
        }
        AnnotatedParameter parameter = (AnnotatedParameter) member;
        AnnotatedWithParams owner = parameter.getOwner();
        if (!(owner instanceof AnnotatedConstructor)) {
          return null;
        }
        AnnotatedConstructor constructor = (AnnotatedConstructor) owner;
        ConstructorProperties properties =
            constructor.getAnnotation(ConstructorProperties.class);
        if (properties == null) {
          return null;
        }
        int index = parameter.getIndex();
        String[] parameterNames = properties.value();
        if (index < 0 || index >= parameterNames.length) {
          return null;
        }
        return parameterNames[index];
      }
    });
  }
}

In case it matters, the use case for that custom AnnotationIntrospector is enabling deserialization of Lombok @Value classes. https://projectlombok.org/features/Value.html

@michaelhixson
Copy link
Contributor Author

It looks like I can work around this by configuring MapperFeature.ALLOW_EXPLICIT_PROPERTY_RENAMING to be true, instead of the default false.

@cowtowncoder
Copy link
Member

Possibly same root cause as #1076, but will keep open, see if tackling that helps here.

@cowtowncoder
Copy link
Member

@michaelhixson Actually, I think behavior is as expected here: since annotations you specify are explicit naming, they are not (by default) sent to naming strategy for renaming. But you can change the behavior using MapperFeature.ALLOW_EXPLICIT_PROPERTY_RENAMING (as per #918), if you do want to force reprocessing.

@cowtowncoder
Copy link
Member

Also note #926 which gives more control via @JsonNaming.

@michaelhixson
Copy link
Contributor Author

@cowtowncoder I'm fine with closing this then, since there's a simple workaround for me.

@cowtowncoder
Copy link
Member

@michaelhixson Ok thanks. I can see why behavior might be unexpected, too, since there is slight semantic of difference between @JsonProperty and @ ConstructorProperties (or it could be viewed at least -- one is for naming of json side, other could be viewed as naming of bean properties).
Anyway, the fact that Jackson considers them both explicit names to use externally needs to be documented. With that, I'll close this.

Thank you for reporting this: it is possible others may bump into it as well.

@michaelhixson
Copy link
Contributor Author

@cowtowncoder Your comment confused me. You have built-in support for @ConstructorProperties?! My custom AnnotationIntrospector is useless now?

sees #905...

Hooray!

@cowtowncoder
Copy link
Member

@michaelhixson Ah. Sorry, I thought you were involved in that issue... :)

It was #905, and yes, is now included in 2.7.

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