Cannot access attributes from Converter

[jakub-bochenski]

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

I tried this, but the result is cached, meaning that the attribute value is always the same:

.setHandlerInstantiator(object : HandlerInstantiatorAdapter() {

                override fun converterInstance(
                    config: MapperConfig<*>,
                    annotated: Annotated,
                    implClass: Class<*>
                ): Converter<*, *> {
                    if (implClass == MyConverter::class.java) {
                        return MyConverter {
                            config.attributes.getAttribute("test") as String
                        }
                    }
                    return implClass.newInstance() as Converter<*, *>
                }
            })

Describe the solution you'd like

Ideally Converter would have access to the MapperConfig in the convert() method.

Maybe there could be a new interface to opt-in for this, since the signature change would be breaking.
(E.g. like ResolvableSerializer, but it would have to be called per-usage to work around caching).

Caching serializes on a per-config basis would allow the above code to work correctly.

Usage example

mapper.writer().withAttribute("myAttr", "Hello!").writeValueAsString(something);

Additional context

Currently you have to rely on ThreadLocal or write a full custom Serializer to have access to context values. It would be nice if you could use attributes in a Converter since it's much simpler to implement than a Serializer

[JooHyukKim]

Sounds reasonable 🤔

[jakub-bochenski]

Actually maybe just adding a new method in converter could do the trick (plus updating the relevant calls)?

public interface Converter<IN, OUT> {
    
    default OUT convert(IN var1, MapperConfig<?> var2) {
        return convert(var1);
    }

    OUT convert(IN var1);

    JavaType getInputType(TypeFactory var1);

    JavaType getOutputType(TypeFactory var1);

    public abstract static class None implements Converter<Object, Object> {
        public None() {
        }
    }
}

[cowtowncoder]

@jakub-bochenski Yeah, this is a gap, missing info.
Unfortunately changing signature is a majorly backwards-incompatible change so this would not be possible for Jackson 2.x.
It could be done for 3.0.

However, I don't think MapperConfig (or its subtypes, Serialization- / DeserializationConfig) allow access to dynamic (per-call) attributes. Instead, DatabindContext (super-type of DeserializationContext / SerializerProvider) should be passed, I think.

[cowtowncoder]

I like the idea of improvement btw, even tho I think it can't go in 2.x.

But since we are trying to get 3.0.0-rc1 ready, this would be great to get done, being API change.

[jakub-bochenski]

Unfortunately changing signature is a majorly backwards-incompatible change so this would not be possible for Jackson 2.x.

@cowtowncoder but it's not a breaking change if done as in #4915 (comment)

Old implementation will continue working because of the default implementation

[cowtowncoder]

@jakub-bochenski Fair enough. Although, upgrading modules that call Converter method(s) might be tricky -- theoretically problematic since old calls would directly call now-deprecated method. Although it is unclear if many (or any?) modules actually call Converter; jackson-databind might be the only caller.

But maybe it'd be worth tackling this for 2.x after all. Access to configuration is important and should be provided.

[jakub-bochenski]

However, I don't think MapperConfig (or its subtypes, Serialization- / DeserializationConfig) allow access to dynamic (per-call) attributes. Instead, DatabindContext (super-type of DeserializationContext / SerializerProvider) should be passed, I think.

The workaround I have now is I've copy-pasted the StdDelegatingSerializer and changed it's methods like this:

   @Override
    public void serialize(Object value, JsonGenerator gen, SerializerProvider provider) throws IOException
    {
        Object delegateValue = convertValue(value, provider);

I'm using this to set the value before serializing the object:

mapper.writer().withAttribute("myAttr", "Hello!")

this seems to work. Haven't tested it for concurrency though.

TBH I'm not sure when should I use the "per-call" attributes vs the "shared" attributes. If I set them on the one-time writer() what is the difference?

[cowtowncoder]

TBH I'm not sure when should I use the "per-call" attributes vs the "shared" attributes. If I set them on the one-time writer() what is the difference?

They are logically merged: so you can set "default" attributes on ObjectMapper, but then overrides on ObjectReader / ObjectWriter. Latter has precedence over former.
Division is necessary since ObjectMappers are sort-of-immutable (in 2.x; fully immutable in 3.0), config-then-use, so attributes cannot be changed on mapper for different calls.

[jakub-bochenski]

@cowtowncoder not sure if this is the right place for this discussion, but your explanation still leaves me confused :D

What I observe is that mapper.writer().withAttribute("myAttr", "Hello!") will result in the writer having a new instance of ContextAttributes, copied from the mapper instance with the shared attributes set to myAttr -> Hello.
The mapper just keeps it's original ContextAttributes instance that remains unchanged.

So I don't really see where the per-call attributes come into the picture.

[cowtowncoder]

Term "shared" in there is confusing, but basically you can both:

1. Add different attributes for ObjectReader / ObjectWriter and (I think)
2. Potentially modify Attribute values during read/write processing (to "share" attribute values across (De)Serializers

Use of "shared" is unfortunate since one could argue that mapper default set of attributes are actually shared, and mutable per-instance copy is NOT shared.

This if my memory is correct: I haven't looked into this part of code in a while.

[JooHyukKim]

@jakub-bochensk @cowtowncoder Just so we are all on same page, is this approach in our mind?
#4975

[cowtowncoder]

@JooHyukKim I'd have to think about this more -- but yes the basic idea would be like that. But we'd definitely have to consider compatibility concerns carefully, whether to even consider this for 2.x, or only 3.0 (master). I haven't thought this through.

(and as I mentioned elsewhere, I'll be out for a week, will then look back into this and other issues).