Extending BuilderBasedDeserializer

[vjkoskela]

We have a thread local builder implementation to try and reduce the impact of builder instances on the garbage collector. Much of our builder usage is through Jackson, and thus we are writing a BeanDeserializerModifier with the goal of extending the BuilderBasedDeserializer.

Ideally, we would like to reuse most of the existing functionality but the class is proving somewhat difficult to extend. First, the Object deserialize(JsonParser p, DeserializationContext ctxt) method is declared final (as are a few others). This prevents us from subclassing and overriding the method. Other methods cannot be delegated/overridden because they are protected or private (e.g. _deserializeUsingPropertyBased and vanillaDeserialize) -- however, we can work around the protected ones by placing our class in the same package.

Our code provides a build method which accepts builder class and a consumer of an instance of that builder. This encapsulates the lifecycle of the builder into the consumer and allows the code to re-use builder instances (such builder instances are required to have a reset method which is invoked automatically before the builder is consumed). Retrofitting the code into Jackson was not too bad for the vanilla case.

First, we replace the first branch in public final Object deserialize(JsonParser p, DeserializationContext ctxt):

        if (p.isExpectedStartObjectToken()) {
            final JsonToken t = p.nextToken();
            if (_vanillaProcessing) {
                return vanillaDeserializeAndFinishBuild(p, ctxt, t);
            }
        }

And here's our vanillaDeserializeAndFinishBuild implementation which mostly vanillaDeserialize from BuilderBaseDeserializer but also includes our build logic and thus skips the finishBuild call:

    private Object vanillaDeserializeAndFinishBuild(
            final JsonParser p,
            final DeserializationContext ctxt,
            final JsonToken t)
            throws IOException {

        return ThreadLocalBuilder.buildGeneric(
                _threadLocalBuilderClass,
                b -> {
                    Object bean = b;
                    try {
                        for (; p.getCurrentToken() != JsonToken.END_OBJECT; p.nextToken()) {
                            String propName = p.getCurrentName();
                            // Skip field name:
                            p.nextToken();
                            SettableBeanProperty prop = _beanProperties.find(propName);
                            if (prop != null) { // normal case
                                try {
                                    bean = prop.deserializeSetAndReturn(p, ctxt, bean);
                                } catch (Exception e) {
                                    wrapAndThrow(e, bean, propName, ctxt);
                                }
                            } else {
                                handleUnknownVanilla(p, ctxt, bean, propName);
                            }
                        }
                    } catch (final Exception e) {
                        throw new RuntimeException(e);
                    }});
    }

We are using Jackson 2.9.2 and it looks like the method is final in master as well. No real issue here, I think we can work around it. Mostly, wanted to point out our use case and enquire as to whether you had any tips for extending BuilderBasedDeserializer and whether some of the restrictions could be lifted in the future. If not, any help understanding what pitfalls you see that led to restricting those methods could help us structure our code better. Finally, we're open to contributing some code to make the class more extensible, but didn't want to presume what direction you had in mind so any tips in this regard are welcome as well.

[cowtowncoder]

Right, I wouldn't expect BuilderBasedDeserializer to be easy to extend as it is not really designed to be user extensible -- at most it and BeanDeserializer is designed to allow extension for components like Afterburner. This is not to say it should not be extended, just that it's not considered a public extension point.

Beyond this I am ok removing final modifiers in select places in general. With caveat that extension by sub-classing is quite fragile, esp. for types that are not designed to be extended.
But there are probably some methods I'd prefer keeping final as long as there are enough non-final ones that can be overridden.
protected methods should be fine to override by classes from other packages I think (unlike "package default" access).

Other possibility could be to just instead approach this by cut-n-pasting existing builder based deserializer (which extends BeanDeserializerBase which is slightly easier to extend as that is needed by as-array variant, too).

[vjkoskela]

Thanks for the prompt reply! Caveat understood and I think we'll assume the risk. I think the best entry point for us is removing the final on the public final Object deserialize(JsonParser p, DeserializationContext ctxt) method. This would allow us to subclass and override instead of resorting to delegation.

The protected versus package private is only an issue currently as we are delegating to an instance of BuilderBasedDeserializer to work around the final issue above. Under this approach we can't invoke the protected methods on the delegate instance unless our class in the same package (since we're not a subclass). Since you're ok with removing the final I don't think any changes are necessary in the scoping.

I've submitted a patch along these lines.

#1870

Please let me know if I've missed something.

[cowtowncoder]

Since PR was merged, I think I can close this issue. New issues to be created for new changes, ideas, additional work.