jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

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

Open blacelle opened this issue 8 months ago • 12 comments

Search before asking

  • [x] 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 https://github.com/FasterXML/jackson-databind/issues/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 avatar Mar 19 '25 17:03 blacelle

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

JooHyukKim avatar Mar 21 '25 01:03 JooHyukKim

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?

cowtowncoder avatar Mar 21 '25 05:03 cowtowncoder

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

blacelle avatar Mar 21 '25 06:03 blacelle

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)

cowtowncoder avatar Mar 21 '25 15:03 cowtowncoder

@cowtowncoder I switched from @JsonValue as you suggested in https://github.com/FasterXML/jackson-databind/issues/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 (!).)

blacelle avatar Mar 21 '25 17:03 blacelle

(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.

JooHyukKim avatar Mar 23 '25 04:03 JooHyukKim

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. https://github.com/FasterXML/jackson-databind/issues/1840 https://github.com/FasterXML/jackson-databind/issues/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).

blacelle avatar Mar 23 '25 06:03 blacelle

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

JooHyukKim avatar Mar 24 '25 12:03 JooHyukKim

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 https://github.com/FasterXML/jackson-databind/issues/5030#issuecomment-2735156496, 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.

blacelle avatar Mar 24 '25 12:03 blacelle

@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.

cowtowncoder avatar Mar 24 '25 15:03 cowtowncoder

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.)

blacelle avatar Apr 03 '25 12:04 blacelle

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!

cowtowncoder avatar Apr 04 '25 03:04 cowtowncoder

@cowtowncoder I'm having a new look into this. Here is a brief overview of my goal, the observation, and a synthetic analysis of a very similar case (Enum/EnumSerializer, while previous analysis was on Bean/JsonValueSerializer).

  1. My functional need is: I want to have a simpler JSON representation for common cases, and a more complex one for custom cases.
  2. I mostly achieve this with @JsonTypeInfo for the multiple implementation aspect, and defaultImpl + @JsonValue for the native case (I default to a custom class, which has a @JsonValue for shorter JSON representation). I expect custom cases to have their type managed by @JsonTypeInfo, for instance through an additional property.
  3. A typical example is I consider an Option interface, with native options from NativeOptions enum, and custom options. I want native options to be represented as "SomeNativeOption" (as .name() of NativeOptions enum). I expect custom options to be represented by a dynamic type through @JsonTypeInfo.
  4. This work in various vases, and does not work/work weirdly in other cases.
  5. This ticket initially focused on JsonValueSerializer as I was considering such a dynamic representation on class Object. Considering this pattern for Enum, pushed the investigation in a different angle (as in this case, Jackson relies on EnumSerializer).
  6. Here is an example test-case for the Enum case:
package com.fasterxml.jackson.databind.ser.benoit;

import java.util.Locale;

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

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

public class TestIntConstructor_Enum_WeirdSerialization {

	@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
			include = JsonTypeInfo.As.PROPERTY,
			property = "type",
			defaultImpl = NativeOption.class)
	public interface SomeOption {
	}

	public static enum NativeOption implements SomeOption {
		A, B;

		@JsonCreator
		public static NativeOption forValue(String value) {
			return NativeOption.valueOf(value.toUpperCase(Locale.US));
		}

	}

	public static enum CustomOption implements SomeOption {
		C, D;

		@JsonCreator
		public static NativeOption forValue(String value) {
			return NativeOption.valueOf(value.toUpperCase(Locale.US));
		}

	}

	@Test
	public void testNative_string() throws JsonProcessingException {
		NativeOption matcher = NativeOption.A;

		ObjectMapper objectMapper = new ObjectMapper();

		String asString = objectMapper.writeValueAsString(matcher);
		Assertions.assertThat(asString).isEqualTo("A");
	}

	@Test
	public void testCustom_string() throws JsonProcessingException {
		CustomOption matcher = CustomOption.C;

		ObjectMapper objectMapper = new ObjectMapper();

		String asString = objectMapper.writeValueAsString(matcher);
		Assertions.assertThat(asString).isEqualTo("type: CustomOption, value: A");
	}
}

and we get the awkward:

org.opentest4j.AssertionFailedError: 
expected: "A"
 but was: "["TestIntConstructor_Enum_WeirdSerialization$NativeOption","A"]"
	at com.fasterxml.jackson.databind.ser.benoit.TestIntConstructor_Enum_WeirdSerialization.testNative_string(TestIntConstructor_Enum_WeirdSerialization.java:49)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
  1. The type is written due to @JsonTypeInfo, which kicks-in an TypeWrappedSerializer
  2. The array representation is due to TypeWrappedSerializer._typeSerializer which is an AsPropertyTypeSerializer, which ends in com.fasterxml.jackson.core.JsonGenerator._writeTypePrefixUsingWrapper(WritableTypeId) which fallabck with an array in:
        // first: can not output "as property" if value not Object; if so, must do "as array"
        if ((typeIdDef.valueShape != JsonToken.START_OBJECT) && incl.requiresObjectContext()) {
            typeIdDef.include = incl = WritableTypeId.Inclusion.WRAPPER_ARRAY;
        }

Note that in this case, typeIdDef.valueShape is VALUE_STRING (given we write the name() of the Enum).

Typical stack:

WriterBasedJsonGenerator(JsonGenerator)._writeTypePrefixUsingWrapper(WritableTypeId) line: 2043	 <-- Fallback into an ArrayWrapper
WriterBasedJsonGenerator(JsonGenerator).writeTypePrefix(WritableTypeId) line: 1992	
AsPropertyTypeSerializer(TypeSerializerBase).writeTypePrefix(JsonGenerator, WritableTypeId) line: 52	
EnumSerializer(StdScalarSerializer<T>).serializeWithType(T, JsonGenerator, SerializerProvider, TypeSerializer) line: 55	
TypeWrappedSerializer.serialize(Object, JsonGenerator, SerializerProvider) line: 32	
DefaultSerializerProvider$Impl(DefaultSerializerProvider)._serialize(JsonGenerator, Object, JsonSerializer<Object>) line: 503	
DefaultSerializerProvider$Impl(DefaultSerializerProvider).serializeValue(JsonGenerator, Object) line: 342	
ObjectMapper._writeValueAndClose(JsonGenerator, Object) line: 4926	
ObjectMapper.writeValueAsString(Object) line: 4146	
TestIntConstructor_Enum_WeirdSerialization.testNative_string() line: 48	

In JsonValueSerializer, I pin-pointed a weird branch around _forceTypeInformation. But fixing it led to always having this type as Array behavior. (The fix will remove the ability to skip _forceTypeInformation check, hence workarounding the faulty type writing). This call for *is the actual issue the fact that _forceTypeInformation is true, given @JsonTypeInfo+ @JsonValue should skip writing the type (as the output should be the @JsonValue, instead of a TypeWrapper value).

In EnumSerializer, we again face the fact we try writing the type while @JsonTypeInfo+ @JsonValue should skip writing the type. Though, I see no available mecanism which may help doing so.

Hence, is something wrong rather in AsPropertyTypeSerializer, which should not (or differently) kicks-in on @JsonTypeInfo+ @JsonValue ?

blacelle avatar Oct 26 '25 22:10 blacelle

(A one-liner of my current main interrogation: Given @JsonTypeInfo combined with @JsonValue, should the type be omitted, especially if @JsonValue is over a primitive type like String? (I believe it should skip the type and deserialization should take defaultImpl as default type , but the code is unclear) It does in some cases, it does not in others. When it does not, it generally wrap the type with an ARRAY_WRAPPER).

blacelle avatar Oct 26 '25 22:10 blacelle

Um, what is @JsonValueType ?

cowtowncoder avatar Oct 28 '25 02:10 cowtowncoder

Sorry for the confusion @cowtowncoder , I meant @JsonTypeInfo instead of @JsonValueType.

blacelle avatar Oct 28 '25 06:10 blacelle