When using type id with `As.EXTERNAL_PROPERTY` together with `@JsonValue` inside type the serialiser omits external type id field from result when `@JsonValue` value is null

[aurimasniekis]

Describe the bug

When using type id with As.EXTERNAL_PROPERTY together with @jsonvalue inside type the serialiser omits external type id field from result when @jsonvalue value is null.

Version information
2.13.3

To Reproduce

package org.example;

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

public class Main {
  public static interface GenericType {
    String getValue();
    void setValue(String value);
  }

  public static abstract class AbstractGenericType implements GenericType
  {
    protected String value;
    public AbstractGenericType() {}
    public AbstractGenericType(String value) {this.value = value;}
    @Override @JsonValue public String getValue() {return value;}
    @Override @JsonValue public void setValue(String value) {this.value = value;}
  }

  public static class FooType extends AbstractGenericType {
    public FooType() {super();}
    public FooType(String value) {super(value);}
  }
  public static class Container {
    @JsonTypeInfo(use = Id.MINIMAL_CLASS, include = As.EXTERNAL_PROPERTY, property = "type", visible = true)
    @JsonSubTypes(value = {@Type(value = FooType.class)})
    protected GenericType value;

    public GenericType getValue() { return value; }
    public void setValue(GenericType value) { this.value = value; }
  }

  public static void main(String[] args) throws JsonProcessingException {
    var om = new ObjectMapper();

    var container = new Container();

    System.out.println(om.writeValueAsString(container)); // {"value":null}
    // Its fine as container::value == null so external type id field is missing

    container.setValue(new FooType());

    System.out.println(om.writeValueAsString(container)); // {"value":null}
    // Its not fine as container::value != null, external type id field should be set ".Main$FooType" value

    container.setValue(new FooType("foobar"));

    System.out.println(om.writeValueAsString(container)); // {"value":"foobar","type":".Main$FooType"}
    // Now external type id field is present
  }
}

Expected behavior

Expected that external type id field should be present with type id value when type is not null.

P.S. It would be also great if it was possible to still include external type id field with for e.g. null value when type is null.

[aurimasniekis]

I think I found a place where this situation happens.

This code helps to work with it:

// and if we got null, can also just write it directly
JsonSerializer<Object> ser = _valueSerializer;
if (value == null) {
    ctxt.defaultSerializeNull(gen);

    if (bean != null && ser != null) {
        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));
            typeSer0.writeTypeSuffix(gen, typeIdDef);
        }
    }

    return;
}

For this spot:

jackson-databind/src/main/java/com/fasterxml/jackson/databind/ser/std/JsonValueSerializer.java

Lines 271 to 276 in aebeedf

	// and if we got null, can also just write it directly
	if (value == null) {
	ctxt.defaultSerializeNull(gen);
	return;
	}
	JsonSerializer<Object> ser = _valueSerializer;

I managed to make it run locally, and it worked as expected, but I couldn't make the when type == null to also print as it doesn't enter that function at all. Sorry I am still pretty noob with Java

[cowtowncoder]

Ok. Just to save you some trouble: I suspect you can not really combine use of @JsonValue and As.EXTERNAL_PROPERTY inclusion mechanism. I don't think this will work. Use of other inclusion modes might work, although the combination of polymorphic deserialization and @JsonValue itself is problematic.
The issue is with handling of delegating needed for serialization of "delegate" value (in place of the original value); and the question of identity, in a way; does type information relate to underlying true type, or delegate?

If I remember code correctly, it assumes former ("true" / actual Java type; NOT one returned by @JsonValue annotated getter or field). And the problem them becomes of trying to stitch together type (de)serializer for that type with actual content (de)serializer for delegate type -- this combination can get tricky if and when types of true/delegate are very different (like POJO vs String).

I think it would be good if there was a way to indicate clearly that this combination cannot work -- throw an exception in case where use is attempted, describing the problematic combination -- and I'd be happy to get a patch for this.
But I am not confident that the combination itself can be supported.

Then again if you (or anyone else) manages to show I am wrong here, would be happy to be proven wrong :)

[aurimasniekis]

@cowtowncoder Thanks for the reply, I am having my own custom type id resolver with introspector and it works really well, with @JsonValue just the condition I mentioned about then @JsonValue value is null. I can provide some code as I had some issues before making other functionality.

But maybe the case is with standard annotation @JsonTypeInfo implementation.

[aurimasniekis]

Oh maybe important tip is that @JsonValue is not on generic type, but a property inside the type. In my screenshot it's TextValueType has get/set value with @JsonValue attribute.

[cowtowncoder]

Hmmh. I see. It might actually be ok for @JsonValue to be within surrogate value.

One thing to note, which may not matter a lot -- @JsonValue only ever affects serialization so there's no need to add it to getter and setter (actually it should never be needed on both; annotations are combined).
But for deserialization you typically would need matching @JsonCreator on String-taking constructor, to maintain structural equality (that is, that serialization as JSON String, here, can be deserialized).
But I don't think that's the problem here.

What is likely more problematic, however, is that Java/JSON String is considered a "natural" type. So serialization of String value never takes in Type Id -- other "natural" types are Booleans, Longs and Doubles.
Specifically, on deserialization, seeing a plain JSON String is probably taken as indication of there not being any Type Id available. For other inclusion types this makes sense (only JSON Objects can have As.PROPERTY/As.EXISTING_PROPERTY; WRAPPER_ARRAY and WRAPPER_OBJECT only work with matchng types), but for EXTERNAL_PROPERTY this is not valid logic as the Type Id would be outside ("external") of the polymorphic value itself.

So it is likely that use of plain (JSON) String as serialization specifically causes problems. Not sure whether that helps in determining whether usage could be made to work or not.

[cowtowncoder]

Oh and forgot to outline the "why isn't Type Id" written part: that is due to String being "natural" type.
Given that this does not actually make sense for specific case of EXTERNAL_PROPERTY, perhaps the rule for that inclusion type could be changed, as a special case.

I would be slightly worried about regression for existing usage (since this does change observed behavior and quite often someone, somewhere, is relying on whatever the current behavior is at given point), although as a starting point, if a change solves your problem and does NOT fail any of existing unit test, it's a credible fix candidate.

[aurimasniekis]

I see, maybe there is a way to make something like @JsonValue or replace its serialisation logic? Because I know this can cause a lot of changes for people who expected to work like before.

BTW this all String is just simple example, I am trying out

[cowtowncoder]

Handling of @JsonValue can be replaced (there is specific JsonSerializer implementation for it), but that is probably quite a bit of work. And it is still necessary to handle deserialization as well which might need changes as well.

One thing that could help here is an actual unit test -- code above has much of what is needed, but instead of print statements, it would need to assert specific results: that is, to show what should happen (successful reading and writing), which fails. At least that'd make it easy to see if handling could be made to work.

[aurimasniekis]

I will try to make a unit test, but I am having bit hectic week so probably only next week. I figured the @JsonValue Serializer and used SerializerModifier to replace with my own JsonValueSerializer.

[cowtowncoder]

@aurimasniekis np at all. I know how limited time can be. Glad that you were able to figure out a work-around at least; that sounds like a good approach to solve your immediate needs fwtw.