jakartaee / jsonb-api

Jakarta JSON Binding
https://eclipse-ee4j.github.io/jsonb-api/
Other
79 stars 39 forks source link

provide fine grained control over (de)serializing fields/properties #270

Closed nimo23 closed 3 years ago

nimo23 commented 3 years ago

Please provide a configuration where I can setup which fields/properties should be (de)serialized in a more fine grained way:

Currently, I can use PropertyVisibilityStrategy - but it is an all or nothing configuration. For example:

public static final class OnlyFieldsVisibilityStrategy implements PropertyVisibilityStrategy {
    @Override
    public boolean isVisible(Field field) {
        return true;
    }

    @Override
    public boolean isVisible(Method method) {
        return false;
    }
}

Having the following class:

@JsonbVisibility(name="OnlyFieldsVisibilityStrategy")
class User {

// name will be (de)serialized, getName() will be ignored
private String name;

// this field should be omitted for (de)serializing, because it should use getter access
@JsonbTransient
private lastRefresh;

// This getter is omitted for (de)serializing, because field access is used
private String getName(){
    return name;
}
// I need to serialize this, but it will be ignored even if I explicitly declared it with @JsonbProperty
@JsonbProperty
private int getLastRefresh(){
    return LocalDateTime.now();
}

}

Unfortunately, Json-B does not (de)serialize explicitly marked properties with @JsonbPropertybecause of OnlyFieldsVisibilityStrategy (see User#getLastRefresh()). Json-B annotated fields or properties should have higher priority than the general configuration by PropertyVisibilityStrategy. But this is currently not the case. Would be nice to cover such issues. Maybe by using the following solution:

    PropertyVisibilityStrategy.getVisibilityChecker()
        .withFieldVisibility(Visibility.ANY)
    .withGetterVisibility(Visibility.ONLY_WITH_ANNOTATIONS);

The enums for field or getter Visibility can be passed to a PropertyVisibilityStrategy (like com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility). We should also have Visibility.ONLY_WITH_ANNOTATIONS which (de)serialize the field or getter only if it is explicitly annotated with a jsonb-annotation.

nimo23 commented 3 years ago

For example (for inspiration), with jackson we can do something like the following which disables the auto detection for (de)serializing properties/methods:

ObjectMapper objectMapper = new ObjectMapper();        
objectMapper.disable(MapperFeature.AUTO_DETECT_CREATORS,
            MapperFeature.AUTO_DETECT_FIELDS,
            MapperFeature.AUTO_DETECT_GETTERS,
            MapperFeature.AUTO_DETECT_IS_GETTERS);

It also works on class level:

@JsonAutoDetect(
    fieldVisibility = Visibility.NONE,
    setterVisibility = Visibility.NONE,
    getterVisibility = Visibility.NONE,
    isGetterVisibility = Visibility.NONE,
    creatorVisibility = Visibility.NONE
)

With the approach above, I can target specific properties or methods to (de)serialize explicitly with @JsonProperty. It seems such approach is actually not possible with json-b but desirable.

rmannibucau commented 3 years ago

@nimo23 with JSON-B you do:

@JsonbVisibility(MyImpl.class)
public class MyModel {/*...*/}

with MyImpl implementing PropertyVisibilityStrategy which is strictly equivalent to jackson feature. Spec provides a "most common use case" default but you can tune it at will (I know for example some impl will skip all fields named "$pc.*"). So overall yes, we can support @JsonbAutoDetect but it is just one particular case and the current API is way more flexible with a good default so question is should we add some configurable impl? Due to the number of case I see the configuration way more verbose than the actual needed implementations so I tend to think it is not that worth it in practise (and overall trivial to do on top of current API if really desired).

So overall such approach is already doable in jsonb since 1.0 even if the API is a bit different. What we can surely do to make it even simpler is to add default to the SPI methods to enable to implement it inline with records but duplicating the API or adding another default implementation looks a bit overkill to me, no?

nimo23 commented 3 years ago

So overall such approach is already doable in jsonb since 1.0 even if the API is a bit different.

Using jackson, I can do this:

@JsonAutoDetect(
    fieldVisibility = Visibility.NONE,
    getterVisibility = Visibility.NONE,
    isGetterVisibility = Visibility.NONE,
    creatorVisibility = Visibility.NONE
)
class User {

// this will be serialized
@JsonProperty
private String name;

// will not be serialized
private lastRefresh;

// will not be serialized
private String getName(){
    return name;
}
// this will be serialized
@JsonProperty
private int getLastRefresh(){
    return LocalDateTime.now();
}

}

Using json-b, I can do this (which seems to be equivalent to the above jackson-solution):

public final class DisableAutoDetectStrategy implements PropertyVisibilityStrategy {
@Override
public boolean isVisible(Field field) {
    return field.isAnnotationPresent(javax.json.bind.annotation.JsonbProperty.class);
}

@Override
public boolean isVisible(Method method) {
    return method.isAnnotationPresent(javax.json.bind.annotation.JsonbProperty.class);
}
}

@rmannibucau I think, you are right. Both solutions produce the same output. Or did I miss something in DisableAutoDetectStrategy?

nimo23 commented 3 years ago

but duplicating the API or adding another default implementation looks a bit overkill to me, no

Yes, you are right. I can do it already with PropertyVisibilityStrategy. No need for redundant annotations..