Skip to content

Allow BeanPropertyWriter Sub-classes to Override get() (remove final) #3343

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

Closed
alzimmermsft opened this issue Dec 7, 2021 · 4 comments
Closed
Assignees
Labels
2.19 Issues planned at 2.19 or later
Milestone

Comments

@alzimmermsft
Copy link

alzimmermsft commented Dec 7, 2021

Is your feature request related to a problem? Please describe.

In BeanPropertyWriter the get method is final and a few of the serialization methods inline the call to get making it much more difficult to create sub-classes of BeanPropertyWriter that attempt to optimize the get operation. An example of this can be seen in the Blackbird module which requires the entire serializeAsField method to be overloaded (https://github.com/FasterXML/jackson-modules-base/blob/2.14/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/ser/ObjectPropertyWriter.java#L45) which then enforces additional constraints that the overload needs to handle complete serialization.

Describe the solution you'd like

I think it would be useful for sub-classes of BeanPropertyWriter to be able to override get functionality and allow BeanPropertyWriter to handle serialization based on its configuration (if serialization isn't also overridden). This would allow for further enhancements to the Blackbird module where it could become a simple optimized value getter using MethodHandle based reflection and pass the retrieved back to the BeanPropertyWriter. Overall, this would enable Blackbird to remove its restrictions on which types it is allowed to handle.

Usage example

BeanPropertyWriter.java

public void serializeAsField(Object bean, JsonGenerator gen,
        SerializerProvider prov) throws Exception {
    final Object value = getValue(bean);

    // Null handling is bit different, check that first
    if (value == null) {
        if (_nullSerializer != null) {
            gen.writeFieldName(_name);
            _nullSerializer.serialize(null, gen, prov);
        }
        return;
    }
    // then find serializer to use
    JsonSerializer<Object> ser = _serializer;
    if (ser == null) {
        Class<?> cls = value.getClass();
        PropertySerializerMap m = _dynamicSerializers;
        ser = m.serializerFor(cls);
        if (ser == null) {
            ser = _findAndAddDynamic(m, cls, prov);
        }
    }
    // and then see if we must suppress certain values (default, empty)
    if (_suppressableValue != null) {
        if (MARKER_FOR_EMPTY == _suppressableValue) {
            if (ser.isEmpty(prov, value)) {
                return;
            }
        } else if (_suppressableValue.equals(value)) {
            return;
        }
    }
    // For non-nulls: simple check for direct cycles
    if (value == bean) {
        // four choices: exception; handled by call; pass-through or write null
        if (_handleSelfReference(bean, gen, prov, ser)) {
            return;
        }
    }
    gen.writeFieldName(_name);
    if (_typeSerializer == null) {
        ser.serialize(value, gen, prov);
    } else {
        ser.serializeWithType(value, gen, prov, _typeSerializer);
    }
}

// Basically the same as the existing get() as this changes the scope from "public final" to "protected"
protected Object getValue(Object bean) throws Exception {
    return get(bean);
}

Blackbird Optimized Getters

@Override
protected Object getValue(Object bean) throws Exception {
  // Insert optimized MethodHandle/Callsite retrieval here
}
@alzimmermsft alzimmermsft added the to-evaluate Issue that has been received but not yet evaluated label Dec 7, 2021
@alzimmermsft alzimmermsft changed the title Allows BeanPropertyWriter Sub-classes to Override Get or Implement New Get Allow BeanPropertyWriter Sub-classes to Override Get or Implement New Get Dec 7, 2021
@cowtowncoder cowtowncoder added 2.19 Issues planned at 2.19 or later and removed to-evaluate Issue that has been received but not yet evaluated labels Apr 9, 2025
@cowtowncoder cowtowncoder self-assigned this Apr 9, 2025
@cowtowncoder cowtowncoder changed the title Allow BeanPropertyWriter Sub-classes to Override Get or Implement New Get Allow BeanPropertyWriter Sub-classes to Override get() (remove final) Apr 9, 2025
@cowtowncoder cowtowncoder added this to the 2.19.0 milestone Apr 9, 2025
@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 9, 2025

Making get() non-final is easy enough to do, will change for 2.19(.0) (post -rc2).

cowtowncoder added a commit that referenced this issue Apr 9, 2025
@cowtowncoder
Copy link
Member

@stevenschlansker Do you think this change for 2.19.0 could help reduce code in Blackbird, as suggested?

@stevenschlansker
Copy link

stevenschlansker commented Apr 11, 2025

I don't plan to make any changes to blackbird currently, and the next revision from me would probably be for 3.0+
But I agree in principle, if a class may be overridden, it is nice to leave the possibility open for subclasses.
My understanding is that nowadays the JVM knows if a method is "effectively final" for optimization purposes, even if the final modifier is missing.

@cowtowncoder
Copy link
Member

Yes, I think effect of final on non-polymorphic cases has been not been sigificant in years.

Np with not changing, just thought I'd mention in case you did changes things in 2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.19 Issues planned at 2.19 or later
Projects
None yet
Development

No branches or pull requests

3 participants