Skip to content

ObjectMapper.convertValue to Map leads to unexpected Map for type configured with @JsonSubTypes and @JsonValue #5035

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

Open
1 task done
blacelle opened this issue Mar 19, 2025 · 12 comments
Labels
to-evaluate Issue that has been received but not yet evaluated

Comments

@blacelle
Copy link

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

Given some type wrapping a String property, with a deserialization startegy configured through @JsonSubTypes and @JsonValue, ObjectMapper.convertToMap leads to an unexpected output Map.

Instead of producing a Map with the @JsonValue as value, we receive a List of 2 elements: the name as defined by @JsonSubTypes and the @JsonValue attribute.

If the @JsonValue attribute is an Object (instead of a String), we receive the expected value (not embedded in a List.

Follows some analysis for an unrelated issue in #5030

Version Information

2.18.2

Reproduction

package eu.solven.adhoc.column;

import java.util.Map;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

public class TestIntConstructor_WeirdSerialization {

	@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
			include = JsonTypeInfo.As.PROPERTY,
			property = "type",
			defaultImpl = AroundString.class)
	@JsonSubTypes({ @JsonSubTypes.Type(value = AroundString.class, name = "from") })
	public interface AroundSomething {
		Object getInner();
	}

	public static class AroundString implements AroundSomething {
		@JsonValue
		String inner;  // <-- Turning this to Object will make the test passes

		@Override
		public Object getInner() {
			return inner;
		}

		public void setInner(String inner) {
			this.inner = inner;
		}

	}

	public static class HasFromObject {
		AroundSomething c;

		public AroundSomething getC() {
			return c;
		}

		public void setC(AroundSomething c) {
			this.c = c;
		}
	}

	@Test
	public void testJackson_convertToWrappingPojo_string() throws JsonProcessingException {
		AroundString matcher = new AroundString();
		matcher.setInner("foo");

		HasFromObject wrapper = new HasFromObject();
		wrapper.setC(matcher);

		ObjectMapper objectMapper = new ObjectMapper();

		Map asMap = objectMapper.convertValue(wrapper, Map.class);
		Assertions.assertThat(asMap.toString()).isEqualTo("{c=foo}");
	}
}

Expected behavior

Always receive the @JsonValue attribute as Map value.

Additional context

No response

@blacelle blacelle added the to-evaluate Issue that has been received but not yet evaluated label Mar 19, 2025
@JooHyukKim
Copy link
Member

I suspect there seems to be misunderstanding of how convertValue() is expected to work.
It's simple serialize - deserialize combo.

Regardless, can you try separately serialize - deserialize combo and see if it works? --if so, should look into convertValue implementation

@cowtowncoder
Copy link
Member

Yes, and trying to use convertValue() on types with polymorphic handling is asking for trouble as well.

I am not sure I understand intended use case here.

@blacelle What exactly are you trying to do here?

@blacelle
Copy link
Author

blacelle commented Mar 21, 2025

I am not sure I understand intended use case here.

The actual fonctional case is the following:

  1. I got a deep tree of nested objects
  2. I want to convert this deep tree into JSON, but I'd like to modify it with non-trivial rules (e.g. stripping some empty arrays on not-managed types, could be any other rules)
  3. So I push the root object into .convertValue(..., Map.class)
  4. Modify the Map (e.g. stripping some empty arrays on not-managed types, could be any other rules)
  5. Call writeValueAsString over the Map.
  6. The issue happens because somewhere in the tree, I got an object like the reported AroundString

I suspect there seems to be misunderstanding of how convertValue() is expected to work.
It's simple serialize - deserialize combo.

As as Jackson user, I felt it worked this way, but I supposed the implementation might be different. The following testCase confirms something is unexpected (to me) with writeValueAsString as it actually returns "{"c":["from","foo"]}".

	@Test
	public void testJackson_writeAsString_string() throws JsonProcessingException {
		AroundString matcher = new AroundString();
		matcher.setInner("foo");

		HasFromObject wrapper = new HasFromObject();
		wrapper.setC(matcher);

		ObjectMapper objectMapper = new ObjectMapper();

		String asString = objectMapper.writeValueAsString(wrapper);
		Assertions.assertThat(asString).isEqualTo("\""{\"c\":\"foo\"}\""); // FAILs with `"{"c":["from","foo"]}"`
	}

If I remove @JsonTypeInfo and @JsonSubTypes, objectMapper.writeValueAsString(wrapper) now returns "{"c":"foo"}" which looks much better.

When I switch @JsonValue String inner; to @JsonValue Object inner;, I get "{"c":"foo"}" without removing @JsonTypeInfo and @JsonSubTypes

@cowtowncoder
Copy link
Member

What really might cause problems is @JsonValue with polymorphic handling -- interaction between delegated serializer of "serialize-as-this-instead" type, and TypeSerializer constructed for AroundString is probably getting confused.

I am not sure we can ever fully support that.

(I am also not sure about convertValue() and polymorphic handling -- that, too, can be problematic)

@blacelle
Copy link
Author

@cowtowncoder I switched from @JsonValue as you suggested in #5030 it would be a good alternative to some relatively simple StdSerializer. I'm not very experienced with @JsonValue so here some feedback.

If @JsonValue is known to have such limitations, I'm fine dropping such reports and get back to StdSerializer.

(i.e. I do not want to bring burden, just reporting what looks suspicious issues, with fully reproducible cases, to help increasing quality in this already great library (!).)

@JooHyukKim
Copy link
Member

(i.e. I do not want to bring burden, just reporting what looks suspicious issues, with fully reproducible cases, to help increasing quality in this already great library (!).)

Yes, try reading for more documenations, references such as stack overflows and such 👍🏼
If you are not a heavy user, most likely there already is solution out there. GitHub issues should ideally be served for bug reports and such.

@blacelle
Copy link
Author

blacelle commented Mar 23, 2025

Yes, try reading for more documenations, references such as stack overflows and such 👍🏼

Are you suggesting people should not open bug-report? The behavior looks weird (i.e. unexpected & inconsistent) enough to deserve being reported to the development team, even more as it followed some change suggested by development team.

If you are not a heavy user, most likely there already is solution out there. GitHub issues should ideally be served for bug reports and such.

What's missing here to get this ticket considered as a bug report?

(i.e. I do not want to bring burden, just reporting what looks suspicious issues, with fully reproducible cases, to help increasing quality in this already great library (!).)

The message here is: is this bug looks too complicated, or irrelevant as too edgy, just close this ; this comes from following a recommendation from development team, but given some of your feedbacks, I understand it may be pop more issues the solving ones.


@JsonValue and polymorphic subtypes / @ JsonSubTypes do have some history of unfixed issues. (e.g. #1840 #937). Though, they seem quite complex, and with a limited number of users getting into them. I'm unclear what could be done to help people knowing/remembering these 2 does not play well together.

I also understand this can be workarounded by not using @JsonValue but a simple StdSerializer (just like the original code in #5030).

@JooHyukKim
Copy link
Member

Are you suggesting people should not open bug-report? The behavior looks weird (i.e. unexpected & inconsistent) enough to deserve being reported to the development team, even more as it followed some change suggested by development team.

Oh no, not quite. Maybe I gave wrong idea @blacelle

I'm not very experienced with @JsonValue so here some feedback.

I thought you considered yourself not very experienced was asking for feedback? So I gave feedback of how to get more experience around @JsonValue, usage, etc... 🤔 Not telling anyone what to do or not do.

Always welcome for reports

@blacelle
Copy link
Author

I thought you considered yourself not very experienced was asking for feedback?

I meant, not being experienced with @JsonValue, I can not really say if my use-case is exotic or not. I understand @JsonValue+@JsonSubTypes is a bit exotic as it is often behaving in a not-satisfactory way, but not many people seem to encounter these cases (1 case every few years, generally ending with will-not-fix :D).

The suggestion to use was in #5030 (comment), but I suppose this @cowtowncoder missed @JsonSubTypes was at stake.

@cowtowncoder I let you close this if this confirmed as not relevant. I may dig into it, out of curiosity, but can not say if I would do so earlier or later.

@cowtowncoder
Copy link
Member

@blacelle Just to re-iterate: filing bugs for suspicious things is always welcome!

I think we should leave this open for now. I wish I had time to dig into it, but right now I don't (wrt Jackson 3.0 work in particular).

And yes, combination of @JsonValue and polymorphic handling (not so much @JsonSubTypes, fwtw) is unfortunately quite difficult to do. To expand a little bit: the problem really is that with @JsonTypeInfo, type information MUST be about value type itself (to get TypeSerializer and TypeDeserializer to use -- but what @JsonValue does is trick handling to get actual JsonSerializer for what is often very different type!
And since TypeSerializer (writing of Type Id) has to work with JsonSerializer (actual contents, data), there is now a mismatch.
As such, I am not sure if that is supportable: but more importantly, documenting this problem is difficult. And from User POV, @JsonValue seems like much simpler thing, complexity underneath is not obvious ("just serialize like this thing").
Jackson's modularity makes things more difficult sometimes; ability to essentially mix and match so many features, customize, replace/overwrite, is all great. Except making different aspects, permutations work together gets quite complicated.

I hope this helps explain things bit better.

@blacelle
Copy link
Author

blacelle commented Apr 3, 2025

Note for myself (@cowtowncoder Got a syntetical question for you at the bottom):

In the faulty case, com.fasterxml.jackson.core.JsonGenerator.writeTypeSuffix(WritableTypeId) gets typeIdDef.include == WRAPPER_ARRAY, which kind of explains the array output ["typeInfo", "fromValue"].

If @JsonValue is over an Object instead of a String, _valueSerializer is null in JsonValueSerializer.serializeWithType(Object, JsonGenerator, SerializerProvider, TypeSerializer), which leads to a PropertySerializerMap, which skips the

            // 09-Dec-2010, tatu: To work around natural type's refusal to add type info, we do
            //    this (note: type is for the wrapper type, not enclosed value!)
            if (_forceTypeInformation) {
                // Confusing? Type id is for POJO and NOT for value returned by JsonValue accessor...
                WritableTypeId typeIdDef = typeSer0.writeTypePrefix(gen,
                        typeSer0.typeId(bean, JsonToken.VALUE_STRING));
                ser.serialize(value, gen, ctxt);
                typeSer0.writeTypeSuffix(gen, typeIdDef);

                return;
            }

block

and gets into

        // 28-Sep-2016, tatu: As per [databind#1385], we do need to do some juggling
        //    to use different Object for type id (logical type) and actual serialization
        //    (delegate type).
        TypeSerializerRerouter rr = new TypeSerializerRerouter(typeSer0, bean);
        ser.serializeWithType(value, gen, ctxt, rr);

which seems not to write the type in the end.
.

We get into some specific forced case as _forceTypeInformation is true:

    /**
     * This is a flag that is set in rare (?) cases where this serializer
     * is used for "natural" types (boolean, int, String, double); and where
     * we actually must force type information wrapping, even though
     * one would not normally be added.
     */
    protected final boolean _forceTypeInformation;

One may think this ticket is about _forceTypeInformation being true, while @JsonValue should rather make it false (as the type information would be about the wrapped, not the @JsonValue innerValue).

                /* 09-Dec-2010, tatu: Turns out we must add special handling for
                 *   cases where "native" (aka "natural") type is being serialized,
                 *   using standard serializer
                 */
                boolean forceTypeInformation = isNaturalTypeWithStdHandling(_valueType.getRawClass(), ser);

In the Object case, _forceTypeInformation is also true, but we do not go through:

            // 09-Dec-2010, tatu: To work around natural type's refusal to add type info, we do
            //    this (note: type is for the wrapper type, not enclosed value!)
            if (_forceTypeInformation) {
                // Confusing? Type id is for POJO and NOT for value returned by JsonValue accessor...
                WritableTypeId typeIdDef = typeSer0.writeTypePrefix(gen,
                        typeSer0.typeId(bean, JsonToken.VALUE_STRING));
                ser.serialize(value, gen, ctxt);
                typeSer0.writeTypeSuffix(gen, typeIdDef);

                return;
            }

The output array format relates with AsPropertyTypeSerializer which extends AsArrayTypeSerializer as a fallback mecanism: as we can not put the type into a plain String, the String is wrapped an array, with the typeInfo as first entry.


@cowtowncoder In JsonValueSerializer.serializeWithType(Object, JsonGenerator, SerializerProvider, TypeSerializer), the code is:

JsonSerializer<Object> ser = _valueSerializer;
if (ser == null) { // no serializer yet? Need to fetch
    ser = _findDynamicSerializer(ctxt, value.getClass());
} else {
    // 09-Dec-2010, tatu: To work around natural type's refusal to add type info, we do
    //    this (note: type is for the wrapper type, not enclosed value!)
    if (_forceTypeInformation) {
        // Confusing? Type id is for POJO and NOT for value returned by JsonValue accessor...
        WritableTypeId typeIdDef = typeSer0.writeTypePrefix(gen,
                typeSer0.typeId(bean, JsonToken.VALUE_STRING));
        ser.serialize(value, gen, ctxt);
        typeSer0.writeTypeSuffix(gen, typeIdDef);

        return;
    }
}
// 28-Sep-2016, tatu: As per [databind#1385], we do need to do some juggling
//    to use different Object for type id (logical type) and actual serialization
//    (delegate type).
TypeSerializerRerouter rr = new TypeSerializerRerouter(typeSer0, bean);
ser.serializeWithType(value, gen, ctxt, rr);

Why does the lack of a valueSerializaer leads to a different way to serialize the type? I mean, naively, I would expect the code to look like:

JsonSerializer<Object> ser = _valueSerializer;
if (ser == null) { // no serializer yet? Need to fetch
    ser = _findDynamicSerializer(ctxt, value.getClass());
}

if (_forceTypeInformation) {
    // Confusing? Type id is for POJO and NOT for value returned by JsonValue accessor...
    WritableTypeId typeIdDef = typeSer0.writeTypePrefix(gen,
            typeSer0.typeId(bean, JsonToken.VALUE_STRING));
    ser.serialize(value, gen, ctxt);
    typeSer0.writeTypeSuffix(gen, typeIdDef);

    return;
} else {
  TypeSerializerRerouter rr = new TypeSerializerRerouter(typeSer0, bean);
  ser.serializeWithType(value, gen, ctxt, rr);
}

This different of behavior explains why a @JsonValue on Object behave like i would expect (no type) while a String leads to a ["typeInfo", "fromValue"]: for any reason, _valueSerializer is filled with StringSerializer in String case, while it is null in Object case. (The array coming from AsPropertyTypeSerializer uses AsArrayTypeSerializer as fallback.)

@cowtowncoder
Copy link
Member

Why does the lack of a valueSerializaer leads to a different way to serialize the type? I mean, naively, I would expect the code to look like:

I concur that does seem like it should be the way you describe. I do not remember background to this decision. If you change it, does some existing unit test fail? If not and it helps, we could definitely considering changing code.

On StringSerializer... String being "natural" type, normally I'd expect it not to get Type Id but as per some comments, sounds like there exceptions (I do not remember details of that either).
But it sounds like somewhere _forceTypeInformation has been set to true: I guesss that'd be due to @JsonValue complications on figuring out base type. Although... even that doesn't immediately make sense (when encountering plain JSON String when expecting polymorphic type, java.lang.String is deserialized instead)

The reason why _valueSerializer has to be null for declared type of java.lang.Object is simple tho: there is no serializer that can in general serialize plain old Objects -- only specific subtypes.
So serializer has to be located based on actual runtime type.

@blacelle Not sure how much this helps: you have done exemplary work digging through the code, kudos!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

3 participants