Skip to content
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

Wrong serialization of Type Ids for certain types of Enum values #4733

Closed
1 task done
nlisker opened this issue Oct 6, 2024 · 10 comments
Closed
1 task done

Wrong serialization of Type Ids for certain types of Enum values #4733

nlisker opened this issue Oct 6, 2024 · 10 comments
Labels
2.18 enum Related to handling of Enum values

Comments

@nlisker
Copy link

nlisker commented Oct 6, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

This class serializes and deserializes enum constants. I assume that when serializing there should be enough info to deserialize it, and if deserialization fails, then it's a bug.

@JsonTypeInfo(use = Id.___) // see values below
@JsonSubTypes({
	@JsonSubTypes.Type(value = A.class),
})
interface Inter {

	default void yes() {}

	enum A implements Inter {
	
		A1,
		A2 {
			@Override
			public void yes() {
				
			}
		},
	}
}

public class Start {

	public static void main(String[] args) throws JsonProcessingException {
		JsonMapper MAPPER = JsonMapper.builder().build();
		var string1 = MAPPER.writeValueAsString(Inter.A.A1);
		System.out.println(string1);
		var string2 = MAPPER.writeValueAsString(Inter.A.A2);
		System.out.println(string2);
		A value1 = MAPPER.readValue(string1, Inter.A.class);
		System.out.println(value1);
		A value2 = MAPPER.readValue(string2, Inter.A.class);
		System.out.println(value2);
	}
}

Results for Id values:

CLASS

["example.Inter$A","A1"]
["example.Inter$A","A2"]
A1
A2

NONE

"A1"
"A2"
A1
A2

MINIMAL_CLASS

[".Inter$A","A1"]
[".Inter$A$1","A2"]
A1
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No enum constants for class example.Inter$A$1
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 15]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1888)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:321)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174)
	at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:636)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:203)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:100)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromScalar(AsArrayTypeDeserializer.java:69)
	at com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer.deserializeWithType(StdScalarDeserializer.java:66)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4917)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at example.Start.main(Start.java:41)

NAME

["Inter$A","A1"]
["Inter$A$1","A2"]
A1
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'Inter$A$1' as a subtype of `example.Inter$A`: known type ids = [Inter$A]
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 14]
	at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
	at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:2041)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnknownTypeId(DeserializationContext.java:1590)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._handleUnknownTypeId(TypeDeserializerBase.java:300)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:165)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:100)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromScalar(AsArrayTypeDeserializer.java:69)
	at com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer.deserializeWithType(StdScalarDeserializer.java:66)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4917)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at example.Start.main(Start.java:41)

SIMPLE_NAME

["A","A1"]
["1","A2"] <--- serializing as 1 looks especially wrong since it strips out the A$
A1
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id '1' as a subtype of `example.Inter$A`: known type ids = [A]
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 6]
	at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
	at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:2041)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnknownTypeId(DeserializationContext.java:1590)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._handleUnknownTypeId(TypeDeserializerBase.java:300)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:165)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:100)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromScalar(AsArrayTypeDeserializer.java:69)
	at com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer.deserializeWithType(StdScalarDeserializer.java:66)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4917)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at example.Start.main(Start.java:41)

DEDUCTION

"A1"
"A2"
Exception in thread "main" com.fasterxml.jackson.databind.exc.MismatchedInputException: Unexpected token (VALUE_STRING), expected START_ARRAY: need Array value to contain `As.WRAPPER_ARRAY` type information for class example.Inter$A
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
	at com.fasterxml.jackson.databind.DeserializationContext.wrongTokenException(DeserializationContext.java:1914)
	at com.fasterxml.jackson.databind.DeserializationContext.reportWrongTokenException(DeserializationContext.java:1699)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._locateTypeId(AsArrayTypeDeserializer.java:150)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:99)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromScalar(AsArrayTypeDeserializer.java:69)
	at com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer.deserializeWithType(StdScalarDeserializer.java:66)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4917)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at example.Start.main(Start.java:39)

I'm not sure what is considered correct behavior. Except for DEDUCTION, I would assume they should all pass.

Version Information

2.18.0
2.17.2

@nlisker nlisker added the to-evaluate Issue that has been received but not yet evaluated label Oct 6, 2024
@JooHyukKim
Copy link
Member

Interesting analysis. I think we can come up with some improvements. But sady serialization and deserialization symmetry is and always be in most cases achieved best-effort.

@nlisker
Copy link
Author

nlisker commented Oct 7, 2024

Yes, there is no guarantee that serialization will contain enough information for deserialization in the general case. Here, however, an enum constant is defined by a single identifier. Since the type is known on serialization (the second argument of readValue or the object's structure that contains an enum field), any of the Ids should work.

DEDUCTION is the only case where both deserializations fail, and that's OK because it's best effort (although "A1" and "A2" are rather unambiguous). The others that fail manage to deserialize A1, but not A2, even though from a data point of view there is no difference. Jackson shouldn't care if an enum constant overrides a method or not because it's the same object graph. "A2" is the singleton constant with or without overriding. This is why there should be sufficient data for deserialization.

Except in MINIMAL_NAME, but there I would say the error is in the serialization process because it omits part of the constant name, and that should never happed because it's a single information unit. If a class's name is "ViewMeter" you can never serialize it as "View" or "Meter" (unless the user wrote their own serializer, then it's not your problem).

@cowtowncoder
Copy link
Member

It seems to me code itself is wrong, you should not read with

A value2 = MAPPER.readValue(string2, Inter.A.class);

since the base type is Inter.class -- why would you give concrete impl class in such case?

Put another way: deserialization target type generally MUST be the level at which @JsonTypeInfo is specified; not at higher level (subtype).

So I am not sure this is valid bug.

@nlisker
Copy link
Author

nlisker commented Oct 19, 2024

You have a point, my example was too direct. Here is a more real scenario:

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.json.JsonMapper;

import example.Inter.A;

@JsonTypeInfo(use = Id.MINIMAL_CLASS)
@JsonSubTypes({
	@JsonSubTypes.Type(value = A.class),
})
interface Inter {

	default void yes() {}

	enum A implements Inter {
	
		A1,
		A2 {
			@Override
			public void yes() {
				
			}
		},
	}
}

class Ser {
	@JsonProperty
	Inter inter1;
	@JsonProperty
	Inter inter2;
	// toString
}

public class Start {

	public static void main(String[] args) throws JsonProcessingException {
		JsonMapper MAPPER = JsonMapper.builder().build();
		Ser ser = new Ser();
		ser.inter1 = A.A1;
		ser.inter2 = A.A2;
		
		var string1 = MAPPER.writeValueAsString(ser);
		System.out.println(string1);
		Ser value1 = MAPPER.readValue(string1, Ser.class);
		System.out.println(value1);
	}
}

My Ser class is supposed to be fit for serialization and deserialization regardless of the values of the 2 Inter fields.
When using CLASS, the process works as it should and prints:

{"inter1":["example.Inter$A","A1"],"inter2":["example.Inter$A","A2"]}
Ser(inter1=A1, inter2=A2)

With MINIMAL_CLASS, it prints:

{"inter1":[".Inter$A","A1"],"inter2":[".Inter$A$1","A2"]}
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No enum constants for class example.Inter$A$1
...

but there is enough information to deserialize correctly. Similarly, NAME prints:

{"inter1":["Inter$A","A1"],"inter2":["Inter$A$1","A2"]}
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'Inter$A$1' as a subtype of `example.Inter`: known type ids = [Inter$A] (for POJO property 'inter2')
...

And SIMPLE_NAME is probably already wrong in the serialization phase (the class name is 1, which is invalid):

{"inter1":["A","A1"],"inter2":["1","A2"]}
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id '1' as a subtype of `example.Inter`: known type ids = [A] (for POJO property 'inter2')
...

@cowtowncoder
Copy link
Member

Ok, so the biggest hurdle here comes from Enum implementation (by JDK) -- Value Inter.A.A2 is (I think) an instance of a sub-class of Inter.A as opposed to Inter.A.A1 which is of type Inter.A.
And this subtype linkage is not resolve by Jackson's type traverser. But it probably could be resolved, I think. Change for that would go in StdSubtypeResolver, method collectAndResolveSubtypesByClass(), I think.

As to name to use for inner classes, that's probably another related problem, needing to consider case of inner classes. Although for NAME and SIMPLE_NAME the issue is probably more with not seeing subtype for A2 than actual name (although names are colliding too in some cases).

@nlisker
Copy link
Author

nlisker commented Oct 29, 2024

And this subtype linkage is not resolve by Jackson's type traverser. But it probably could be resolved, I think.

Yes, I don't see why not, all the info is there. For example, with MINIMAL_CLASS:

{"inter1":[".Inter$A","A1"],"inter2":[".Inter$A$1","A2"]}

Inter$A is the enum, and both A1 and A2 are equally valid enum constants. The fact that one is specialized doesn't matter. It's "basically" (it's always harder than it looks) looking up the enum constant by name, with or without overridden methods. If it works for A1 it should work for A2.

@cowtowncoder
Copy link
Member

Actually, having said all of that; maybe the problem is reverse: sub-class of Enum A for value A2 (an inner class Inter$A$1 should NOT be used, but rather it should be serialized same as A (that is, Inter$A -- this because actual value deserializer will be able to deserialize A2 for Enum class A. But not necessarily weird inner class.

I think there are couple of special type for which class used for getting Type Id is changed as part of processing. Would just need to remember where this was done.

@nlisker
Copy link
Author

nlisker commented Oct 29, 2024

If it solves the problem I don't see why it wouldn't be valid.

@cowtowncoder
Copy link
Member

@nlisker Right, just writing out aloud possible ways to tackle the issue (and what the issue actually is) :)

@cowtowncoder cowtowncoder changed the title Possible wrong serialization of enum instances with some @JsonTypeInfo "use" values Wrong serialization of Type Ids for certain types of Enum values Nov 5, 2024
@cowtowncoder cowtowncoder added enum Related to handling of Enum values 2.18 and removed to-evaluate Issue that has been received but not yet evaluated labels Nov 5, 2024
@cowtowncoder
Copy link
Member

@nlisker Turns out ClassNamedIdResolver had the fix and just needed to be added to all 4 other resolvers. Fix going in via #4780, for 2.18 (next release 2.18.2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 enum Related to handling of Enum values
Projects
None yet
Development

No branches or pull requests

3 participants