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

Potentially breaking annotation introspection change in 2.7.x #1076

Closed
danielnorberg opened this issue Jan 12, 2016 · 13 comments
Closed

Potentially breaking annotation introspection change in 2.7.x #1076

danielnorberg opened this issue Jan 12, 2016 · 13 comments

Comments

@danielnorberg
Copy link
Contributor

jackson-databind 2.7.x seems to have changed how AnnotationIntrospector#findImplicitPropertyName() interacts with @JsonProperty annotations in abstract types.

I've attempted to put together a minimal repro here:

https://github.com/danielnorberg/jackson-27-annotation-introspection-breakage-repro

https://github.com/danielnorberg/jackson-27-annotation-introspection-breakage-repro/blob/master/src/main/java/test/Repro.java

The above code works when using jackson <= 2.6.4.

I'm not sure if maybe the issue is that I'm misusing AnnotationIntrospector#findImplicitPropertyName(), but what I'm trying to achieve is to both derive property names automatically from method names and allow users to explicitly override property names using @JsonProperty in the auto-matter library.

https://github.com/danielnorberg/auto-matter/blob/master/jackson/src/test/java/io/norberg/automatter/jackson/Foo.java

https://github.com/danielnorberg/auto-matter#jackson-json-support

@michaelhixson
Copy link
Contributor

I feel like this is the same underlying issue as #1077, but I created the separate issue just in case.

@cowtowncoder
Copy link
Member

I can not think of any related intended changes. Did this problem not occur with 2.7.0 release candidates?

@cowtowncoder
Copy link
Member

Looks like it failed with 2.7.0-rc1 as well (and probably rc2, rc3). It does look like a bug from the repro class; will try to see what causes.

@michaelhixson
Copy link
Contributor

@danielnorberg Do you get the behavior you were expecting if, where you create the ObjectMapper, you add this?

.configure(MapperFeature.ALLOW_EXPLICIT_PROPERTY_RENAMING, true)

If so, maybe the incompatibility was introduced in da579e3

@danielnorberg
Copy link
Contributor Author

@michaelhixson no, it still blows up even if I set ALLOW_EXPLICIT_PROPERTY_RENAMING to true.

@danielnorberg
Copy link
Contributor Author

@cowtowncoder Yeah, I was planning to bisect to try and find the breaking change, but haven't had time to do that yet. Thanks for taking a look! Love jackson btw, not sure what I would do without it =)

@danielnorberg
Copy link
Contributor Author

@michaelhixson @cowtowncoder By bisecting I've narrowed it down to 3243c9f. 525bf3e passes.

@cowtowncoder
Copy link
Member

@danielnorberg Appreciate your help! I hope to have a look at this later tonight.

@danielnorberg
Copy link
Contributor Author

@cowtowncoder Seems like SimpleType#getInterfaces(Class<?> cls) does not return the interfaces that a class implements when constructed by SimpleType#construct(Class<?> cls).

https://github.com/danielnorberg/jackson-27-annotation-introspection-breakage-repro/blob/master/src/main/java/test/ClassUtilTest.java

@cowtowncoder
Copy link
Member

@danielnorberg Ah! Yes, you should not be calling that method -- instead, use method constructType() from DeserializationContext / SerializerProvider.

I should probably mark that method @deprecated; it can not be implemented in a way that introspection would work. But it should be made clear that method is not to be called unless caller knows exactly what it is doing.

cowtowncoder added a commit that referenced this issue Jan 13, 2016
…ss)` deprecated (was not yet, for some reason, should have been)
@danielnorberg
Copy link
Contributor Author

@cowtowncoder That seems to resolve my issue. Thanks!

@cowtowncoder
Copy link
Member

@danielnorberg Thank you for doing the detective work here -- might have taken a while for me actually, as I wouldn't have realized the root cause immediately. I will update javadocs, and deprecate that factory method, since it will not work with 2.7, unlike earlier.

@danielnorberg
Copy link
Contributor Author

@cowtowncoder 🍻

aryehpro added a commit to aryehpro/jackson-databind that referenced this issue Jan 25, 2016
# By Tatu Saloranta (113) and others
# Via Tatu Saloranta
* 'master' of https://github.com/FasterXML/jackson-databind: (124 commits)
  Minor addition related to FasterXML#1087: resolve context type, assuming type bindings from that are expected to work.
  Add unit test for FasterXML#999
  minor warnings cleanup
  Add Javadoc badge with automatic version detection
  Fix FasterXML#1083
  Add failing test for FasterXML#1083
  add a unit test to verify that Object Id works via AtomicReference too
  Minor javadoc improvement wrt FasterXML#1076, making `SimpleType.construct(Class)` deprecated (was not yet, for some reason, should have been)
  Fix FasterXML#1078
  Fix FasterXML#1079
  [maven-release-plugin] prepare for next development iteration
  [maven-release-plugin] prepare release jackson-databind-2.7.0
  prepare for 2.7.0 final
  Fix FasterXML#1073
  Try to reproduce FasterXML#1074
  javadoc trimming
  Try to reproduce FasterXML#825 again, still passes.
  minor improvement to ensure base64 encoding uses configured setting
  Undo part of change done for StringDeserializer; caused issues with XML handling
  further minor cleanups to cleanup
  ...
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

3 participants