Skip to content

JOHNZON-431: Fix detection of class level annotations #141

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.

Sign up for GitHub

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

Open
ArneLimburg wants to merge 1 commit into
base: master
Choose a base branch
from
+60 −4
Open

JOHNZON-431: Fix detection of class level annotations #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbAccessMode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ final boolean record = isRecord(clazz) || Meta.getAnnotation(clazz, JohnzonRecor
.orElseGet(() -> naming.translateName(parameter.getName())) :
naming.translateName(parameter.getName()));

final JsonbTypeAdapter adapter = getAnnotation(parameter, JsonbTypeAdapter.class);
final JsonbDateFormat dateFormat = getAnnotation(parameter, JsonbDateFormat.class);
final JsonbNumberFormat numberFormat = getAnnotation(parameter, JsonbNumberFormat.class);
final JsonbTypeAdapter adapter = getAnnotationOnParameterOrParameterType(parameter, JsonbTypeAdapter.class);
final JsonbDateFormat dateFormat = getAnnotationOnParameterOrParameterType(parameter, JsonbDateFormat.class);
final JsonbNumberFormat numberFormat = getAnnotationOnParameterOrParameterType(parameter, JsonbNumberFormat.class);
final JohnzonConverter johnzonConverter = getAnnotation(parameter, JohnzonConverter.class);
final JsonbTypeDeserializer deserializer = getAnnotation(parameter, JsonbTypeDeserializer.class);
final JsonbTypeDeserializer deserializer = getAnnotationOnParameterOrParameterType(parameter, JsonbTypeDeserializer.class);
if (adapter == null && dateFormat == null && numberFormat == null && johnzonConverter == null && deserializer == null) {
converters[i] = defaultConverters.get(parameter.getType());
itemConverters[i] = null;
Expand Down Expand Up @@ -1054,6 +1054,14 @@ private static <T extends Annotation> T getAnnotation(final Parameter param, fin
return Meta.findMeta(param.getAnnotations(), api);
}

private static <T extends Annotation> T getAnnotationOnParameterOrParameterType(final Parameter param, final Class<T> api) {
final T annotation = getAnnotation(param, api);
if (annotation != null) {
return annotation;
}
return Meta.getAnnotation(param.getType(), api);

@rmannibucau rmannibucau Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we filter api? this one is more specific for params so guess the fallback should be at higher level

@ArneLimburg ArneLimburg Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Note that the test is green, as soon, as I define a setter for "InnerObject" in "OuterObject" in the test.

Currently classes, that are only used as constructor parameters (and there is i.e. no setter with that class as parameter) are completely ignored by the rest of the code.

Grabbing them at a more general location would be somewhere deep in org.apache.johnzon.mappings.Mapper (probably in createClassMapping). That refactoring would have much more impact i.e. in scanning performance.

Shall I go that path (i.e. not only scanning fields, getters and setters there, but also constructor parameters)?

@rmannibucau rmannibucau Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of your change is that getAnnotation(parameter) does match a class annotation, this should go on getAnnotation(Type). Several annotations can't be set on types and since johnzon enables programmatic decoration this enables broken/undefined usages so I would fix the annotation read per annotation type and not in the parameter lookup method.

note that it is fine to add a getAnnotationOnParameterOrClass method to ease that but still called per api allowing it

hope it makes sense

@ArneLimburg ArneLimburg Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the getAnnotation(parameter) is private and just used at that location where the bug is.

So do you mean, I should just rename getAnnotation(...) to getAnnotationOnParameterOrParameterType(...) ?

@rmannibucau rmannibucau Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArneLimburg not sure what you mean, you mean it is a bug we can't read a @JsonbProperty from a type? my point was more to add a method in the spirit of org.apache.johnzon.mapper.access.Meta#getIndirectAnnotation which does "read at that location (param) else this one (type)" with meta annotation support and use it only for the annotations allowing it like JsonbTypeAdapter, JsonbDateFormat, JsonbNumberFormat and JsonbTypeDeserializer but not JsonbProperty and JohnzonConverter (they all use the same getAnnotation).

@ArneLimburg ArneLimburg Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I got your point and directly implemented it. Feel free to have a look

}

@Override
public void clean(final Class<?> value) {
if (Cleanable.class.isInstance(visibility)) {
Expand Down
48 changes: 48 additions & 0 deletions johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
import org.junit.Test;

import jakarta.json.bind.adapter.JsonbAdapter;
import jakarta.json.bind.annotation.JsonbCreator;
import jakarta.json.bind.annotation.JsonbProperty;
import jakarta.json.bind.annotation.JsonbTypeAdapter;

import static org.junit.Assert.assertEquals;

Expand All @@ -43,6 +46,18 @@ public void testInheritedAdapterRecognized() {
assertEquals("\"#336699\"", json);
}

@Test
public void adapterAtConstructorParameterRecognized() {
// given
String json = "{\"innerValue\": \"value\"}";

// when
OuterObject object = jsonbRule.fromJson(json, OuterObject.class);

// then
assertEquals("value", object.innerObject.value);
}

interface ValueType<T> {
T value();
}
Expand Down Expand Up @@ -76,4 +91,37 @@ public ColorId adaptFromJson(String s) throws Exception {
return new ColorId(s);
}
}

public static class OuterObject {

private InnerObject innerObject;

@JsonbCreator
public OuterObject(@JsonbProperty("innerValue") InnerObject object) {
innerObject = object;
}
}

@JsonbTypeAdapter(InnerObjectAdapter.class)
static class InnerObject {

private String value;

public InnerObject(String value) {
this.value = value;
}
}

public static class InnerObjectAdapter implements JsonbAdapter<InnerObject, String> {

@Override
public String adaptToJson(InnerObject object) throws Exception {
return object.value;
}

@Override
public InnerObject adaptFromJson(String value) throws Exception {
return new InnerObject(value);
}
}
}
Loading