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

@JsonProperty field names not respected in superclass fields #151

Closed
marshallpierce opened this issue May 10, 2018 · 19 comments
Closed

@JsonProperty field names not respected in superclass fields #151

marshallpierce opened this issue May 10, 2018 · 19 comments

Comments

@marshallpierce
Copy link

Repro: https://bitbucket.org/marshallpierce/repro-jackson-kotlin-inheritance/src/master/

Put briefly, serializing this:

open class SuperClass(@JsonProperty("annotationName") val someCrazyFieldName: String)
class ChildClass(s: String) : SuperClass(s)

results in the following, which uses the field name instead of what's defined in the annotation

{"someCrazyFieldName":"asdf"}
@cowtowncoder
Copy link
Member

and the usual question to help @apatrida: is this with 2.9.5?

@marshallpierce
Copy link
Author

Yes, jackson 2.9.5 and kotlin 1.2.41.

@arekolek
Copy link

arekolek commented Jul 4, 2018

@marshallpierce it looks like it works if you use @get:JsonProperty("annotationName") instead

@apatrida
Copy link
Member

As @arekolek says, target the annotation to the getter (@cowtowncoder another one where the annotations end up on the wrong thing since Kotlin has more annotation targets than Java)

@or-dvir
Copy link

or-dvir commented Jul 16, 2018

not fixed in 2.9.6

@apatrida why has this issue been closed? are you not going to fix it?

@arekolek
Copy link

@or-dvir
Just using:

open class SuperClass(@get:JsonProperty("annotationName") val someCrazyFieldName: String)
class ChildClass(s: String) : SuperClass(s)

works fine

@or-dvir
Copy link

or-dvir commented Jul 17, 2018

@arekolek i am getting an error. the @get: has a red underline saying expected annotation identifier after ':'

this is my code

open class TvShowEntity(@ColumnInfo(name = "stillWatchingDb")
                        @get:@JsonProperty("stillWatching")
                        var _isStillWatching: Boolean = true)

@arekolek
Copy link

arekolek commented Jul 17, 2018 via email

@or-dvir
Copy link

or-dvir commented Jul 17, 2018

face palm
its working now. thanks

@apatrida
Copy link
Member

Still fixed :-D

@or-dvir
Copy link

or-dvir commented Aug 5, 2018

ok guys... this is NOT a proper solution, but a workaround which can cause problems (admittedly, due to human error).

story time:

i got stuck on a problem for a few days now where when i deserialized an object (not even using jackson, im talking about a simple getSerializableExtra of the intent class), it would not keep its original (inherited) values. i tried many different solutions but none worked and those that did, were not good enough (too much boilerplate/duplication of code etc).
after multiple days and a lot of frustration, i finally realized that actually it IS a problem with this library and the inherited values were not properly initialized in the first place. this is when i turned to google (yet again) and got to this issue page. the second i started reading the description i thought ot myself "wait a second, this sound familiar" and turns out not only have i read this before, but i actively participated in this thread lol...

the point of this story is that this solu.... i mean workaround, is an unnecessary extra step which could very easily be forgotten - and if indeed forgotten, could cause trouble.

for the creators of this library (or anyone willing to dive into this and submit a pull request), PLEASE look into this and remove this unnecessary extra step

@fransflippo
Copy link

I agree the proposed solution is not intuitive. Why would the @JsonProperty annotation on the val work on a normal class but not on a superclass? I feel this needs a proper fix.

@TomWolk
Copy link

TomWolk commented Dec 23, 2020

I would like to have a fix as well

@nicolasmafra
Copy link

I am using java and jackson version 2.12.4 and still encounter this same problem.
Annotating the getter is not a good approach because I am using lombok.

@dinomite
Copy link
Member

@nickmafra @or-dvir A good first step towards getting this fixed would be to make a pull request that uses @marshallpierce's provided reproduction to create a failing test alongside the other issues we have https://github.com/FasterXML/jackson-module-kotlin/tree/2.13/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/failing

@dinomite dinomite reopened this Sep 21, 2021
@cowtowncoder
Copy link
Member

And just to make sure: test reproductions can not use Lombok directly -- they must use results of Lombok's processing, but Jackson does not (and can not) know anything about that processing: databind and Kotlin modules simple see whatever Lombok's bytecode processor gives. So using Lombok itself is fine but test reproductions cannot use it.

@dinomite
Copy link
Member

Agreed. The example at the top of this issue is perfect because it has no dependencies beyond those that j-m-k itself uses https://bitbucket.org/marshallpierce/repro-jackson-kotlin-inheritance/src/master/

@k163377
Copy link
Contributor

k163377 commented Mar 3, 2023

There are significant difficulties in fixing this problem.

First, the annotations given to constructor parameters on Kotlin are not carried over to the constructors of subclasses.
This means that this annotation cannot be referenced during processing of the subclass.
Therefore, to solve this problem, we need to reflect the annotations collected by traversing the parent class to the class being processed.

Although it is theoretically possible to do this, but it would create other problems, such as the following

  • More reflection processing is required at initialization time for all classes
  • Need to implement very complex and difficult-to-specify processing
    • Determining which constructors to target in the base class (it is difficult to know in reflection which constructors in the base class are being called)
    • Determination of parameters to be processed (not only names in Kotlin, but also JsonProperty, etc. need to be considered?)

I agree that this behavior is not good.
However, I think it is difficult to implement a feature that causes so many problems when workarounds exist and only occur in limited circumstances.
It may be possible to simplify the implementation by limiting the situation, but in that case, I think it should be implemented as custom by individual users.

For those who want to implement
Here is an example of an implementation that copies annotations from another constructor.
https://github.com/ProjectMapK/jackson-module-kogera/blob/develop/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinClassIntrospector.kt

@k163377
Copy link
Contributor

k163377 commented Apr 2, 2023

Similar issues are summarized in #658.
This issue is closed.

@k163377 k163377 closed this as completed Apr 2, 2023
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

10 participants