Annotation given to constructor parameters containing value class as argument does not work

[k163377]

Currently, annotations given to constructor parameters containing value class as an argument are ignored.

data class Data(@JsonProperty("prop") val ui: UInt = UInt.MAX_VALUE)

println(jacksonObjectMapper().writeValueAsString(Data())) // -> {"ui":4294967295}

The cause is that annotations are only given to parameters of the synthetic constructor (ignored by Jackson).

// Excerpts from the decompile results
public final class Data {
   private Data(int ui) {
      this.ui = ui;
   }

   // $FF: synthetic method
   public Data(@JsonProperty("prop") int ui, DefaultConstructorMarker $constructor_marker) {
      this(ui);
   }
}

About workaround

Please change to give annotations to getter, field, etc.

data class Data(@get:JsonProperty("prop") val ui: UInt = UInt.MAX_VALUE) // -> {"prop":4294967295}

About fix

An experimental project, jackson-module-kogera, fixed this problem by introducing the following

https://github.com/ProjectMapK/jackson-module-kogera/blob/develop/src/main/kotlin/io/github/projectmapk/jackson/module/kogera/KotlinClassIntrospector.kt

Please note that this code appears to be unreviewed and fragile.
Also, currently jackson-module-kotlin does not support deserialization of value class(#650).

[markitovtr1]

As I commented in the issue above (#748 ), it appears that this happens to all other properties in the constructor as well.

I made a small reproduction here

[k163377]

@cowtowncoder
Let me discuss this issue with you.

In the sample code below, I want to do something like copying annotations from the lower constructor to the upper constructor.

// Excerpts from the decompile results
public final class Data {
   private Data(int ui) {
      this.ui = ui;
   }

   // $FF: synthetic method
   public Data(@JsonProperty("prop") int ui, DefaultConstructorMarker $constructor_marker) {
      this(ui);
   }
}

This is because the bytecode generated by Kotlin only annotates the lower synthetic constructor, while Jackson can only process the upper constructor.

I have prototyped the following code using ClassIntrospector to achieve this (sorry, the link is written in Kotlin).
https://github.com/ProjectMapK/jackson-module-kogera/blob/develop/src/main/kotlin/io/github/projectmapk/jackson/module/kogera/KotlinClassIntrospector.kt

The main point is that I try to reproduce the BasicClassIntrospector parsing process as much as possible while overriding the forSerialization/Deserialization and only copying annotations if the class being processed is defined in Kotlin.
This copying is done by adding annotations to the corresponding synthetic constructor parameters to the resultant Map processed by Jackson.

On the other hand, I have the following concerns about this implementation.

1. it is difficult to keep up with changes in the BasicClassIntrospector implementation
2. since ClassIntrospector can only be set, it cannot coexist with other ClassIntrospectors

Do you have a better idea on how to implement this, or even how to solve this problem?

[cowtowncoder]

@k163377 yes, I can see challenges there.

But one clarifying question: is the need for copying annotations across Constructors within same class? (I think so, just confirming).
(as opposed to super-type to sub-type).

I guess in some way this would seem to indicate need for some kind of handler which would be given a set of constructors, with annotations, but allowing modifying that set of annotations. This would occur for single classes at a time, during basic annotation introspection -- so within [Basic]ClassIntrospector processing.
This would be something Kotlin module would register handler for, in case of Kotlin classes.
I do not see an alternate way, and conversely adding a handled might be simple enough.

Actually I guess handler should be able to post-process any BeanDescriptions... or, maybe, some intermediate collected information.
But not be too specific to Constructors.

[k163377]

@cowtowncoder

is the need for copying annotations across Constructors within same class?

Yes, that is correct (if Kotlin does not add any options).

This would occur for single classes at a time, during basic annotation introspection

Does this mean adding functionality to AnnotationIntrospector?
If so, I think this problem can be solved in a smarter way.

[cowtowncoder]

@cowtowncoder

is the need for copying annotations across Constructors within same class?

Yes, that is correct (if Kotlin does not add any options).

This would occur for single classes at a time, during basic annotation introspection

Does this mean adding functionality to AnnotationIntrospector? If so, I think this problem can be solved in a smarter way.

No, like I said above it'd be for ClassIntrospector extension (for BasicClassIntrospector) which handles low-level merging of annotations without any knowledge of semantics.

AnnotationIntrospector only operates on merged/flattened annotations and should never combine different annotations either within hierarchy, within class, or across accessors (Methods, Fields). Former is done by ClassIntrospector, and latter (combining b/w accessors) by POJOPropertyBuilder (orchestrated by POJOPropertiesCollector).

[k163377]

@cowtowncoder
My biggest concern is that only one ClassIntrospector can be set for an ObjectMapper.
This means that any ClassIntrospector set by a KotlinModule will either override the user setting or be overwritten by the user setting.

AnnotationIntrospector only operates on merged/flattened annotations and should never combine different annotations either within hierarchy, within class, or across accessors (Methods, Fields).

Personally, I feel that the operations being performed here could be interpreted as processing for individual AnnotatedParameter or AnnotatedConstructor.

[cowtowncoder]

@k163377 The idea would be to add handler used by ClassIntrospector, not that it would be replaced. Conceptually this is a logical place where it should be done, but perhaps it could be done elsewhere. Problem however is that it is not (if I understand correctly) matter of just selecting right AnnotatedConstructor to use, but that of copying/associating annotations from one not to use (generated) to one that is to be used (public, visible).

AnnotatedParameter and AnnotatedConstructor are dumb value objects and cannot really have any logic like this; it needs to be something that creates/modifies these value objects instead.

[k163377]

@cowtowncoder

The idea would be to add handler used by ClassIntrospector, not that it would be replaced.

I feel there are some misunderstandings, so let me confirm a couple of things.
First, as shown below, if a custom ClassIntrospector is set, it will overwrite the existing one.
https://github.com/FasterXML/jackson-databind/blob/ba4c7183e2b5a14fb20bab139bfb38cccb814dd8/src/main/java/com/fasterxml/jackson/databind/cfg/BaseSettings.java#L238-L245

It is not possible to get an instance of ClassIntrospector already set up from Module.SetupContext, so it is not possible to extend it with such an instance.
https://www.javadoc.io/doc/com.fasterxml.jackson.core/jackson-databind/latest/com/fasterxml/jackson/databind/Module.SetupContext.html

Are you talking about introducing a AnnotationIntrospectorPair like mechanism for the entire ClassIntrospector or within it?

Personally, I feel that the safest approach is to add the following interface to AnnotationIntrospector.

public class AI extends NopAnnotationIntrospector {
    public void modifyIntrospected(AnnotatedMember m) {
        if (m instanceof AnnotatedConstructor) {
            AnnotatedConstructor ac = (AnnotatedConstructor) m;

            for (int i = 0; i < ac.getParameterCount(); i++) {
                AnnotationMap am = ac.getParameterAnnotations(i);
                am.add(...);
            }
        } else if (m instanceof AnnotatedParameter) {
            AnnotatedParameter ap = (AnnotatedParameter) m;
            AnnotationMap am = ap.getAllAnnotations();
            am.add(...);
        }
    }
}

[cowtowncoder]

@cowtowncoder

The idea would be to add handler used by ClassIntrospector, not that it would be replaced.

I feel there are some misunderstandings, so let me confirm a couple of things. First, as shown below, if a custom ClassIntrospector is set, it will overwrite the existing one. https://github.com/FasterXML/jackson-databind/blob/ba4c7183e2b5a14fb20bab139bfb38cccb814dd8/src/main/java/com/fasterxml/jackson/databind/cfg/BaseSettings.java#L238-L245

Yes. That is why I said the we need a new handler called by ClassIntrospector (or passed to it, to use).
Not something to override in ClassIntrospector.
I agree that sub-classing ClassIntrospector is not going to work.

[k163377]

Thank you.

I do not understand how this will be implemented, but I am relieved to know that at least the concerns I mentioned have been taken into consideration.

[cowtowncoder]

@k163377 Fair. I do not know exactly how, either, but I know that the place where action would make most sense -- regardless of how it can be customized -- is ClassIntrospector because that's what it does; it figures out accessors, flattens annotations and so on.
Conversely other points are less optimal because ones that handle ClassIntrospector produced entities -- AnnotatedClass, BeanDescription -- are then missing original information wrt where specific annotations come from.

I am definitely open to suggestion on how to allow customization, and how configuration gets piped through. But this is the place where I feel it should happen, and if we can make it so, would work very well: implementation of whatever modifier is provided could be quite simple.