From 78359177f9b2894f31ee7f4cdb84079d76a8f330 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 4 Nov 2024 17:28:51 -0800 Subject: [PATCH] Fix #4733: handle polymorphic typing better for enums (#4780) --- release-notes/VERSION-2.x | 5 + .../jsontype/impl/ClassNameIdResolver.java | 12 +- .../impl/MinimalClassNameIdResolver.java | 13 +- .../jsontype/impl/SimpleNameIdResolver.java | 4 + .../jsontype/impl/TypeIdResolverBase.java | 29 ++++ .../jsontype/impl/TypeNameIdResolver.java | 4 + .../jsontype/jdk/EnumTyping4733Test.java | 127 ++++++++++++++++++ 7 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 4697a9a5ee..7020b72aeb 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -4,6 +4,11 @@ Project: jackson-databind === Releases === ------------------------------------------------------------------------ +2.18.2 (not yet released) + +#4733: Wrong serialization of Type Ids for certain types of Enum values + (reported by @nlisker) + 2.18.1 (28-Oct-2024) #4741: When `Include.NON_DEFAULT` setting is used on POJO, empty values diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java index bd388b74c1..51fe6527b2 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java @@ -91,17 +91,7 @@ protected JavaType _typeFromId(String id, DatabindContext ctxt) throws IOExcepti protected String _idFrom(Object value, Class cls, TypeFactory typeFactory) { - // Need to ensure that "enum subtypes" work too - if (ClassUtil.isEnumType(cls)) { - // 29-Sep-2019, tatu: `Class.isEnum()` only returns true for main declaration, - // but NOT from sub-class thereof (extending individual values). This - // is why additional resolution is needed: we want class that contains - // enumeration instances. - if (!cls.isEnum()) { - // and this parent would then have `Enum.class` as its parent: - cls = cls.getSuperclass(); - } - } + cls = _resolveToParentAsNecessary(cls); String str = cls.getName(); if (str.startsWith(JAVA_UTIL_PKG)) { // 25-Jan-2009, tatu: There are some internal classes that we cannot access as is. diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java index 135de72360..9cd8e72366 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java @@ -57,14 +57,23 @@ public static MinimalClassNameIdResolver construct(JavaType baseType, MapperConf @Override public String idFromValue(Object value) { - String n = value.getClass().getName(); + return idFromValueAndType(value, value.getClass()); + } + + @Override + public String idFromValueAndType(Object value, Class rawType) { + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + rawType = _resolveToParentAsNecessary(rawType); + String n = rawType.getName(); if (n.startsWith(_basePackagePrefix)) { // note: we will leave the leading dot in there return n.substring(_basePackagePrefix.length()-1); } return n; + } - + @Override protected JavaType _typeFromId(String id, DatabindContext ctxt) throws IOException { diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java index 8af7473344..34afe3c36b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java @@ -120,6 +120,10 @@ protected String idFromClass(Class clazz) if (clazz == null) { return null; } + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + clazz = _resolveToParentAsNecessary(clazz); + // NOTE: although we may need to let `TypeModifier` change actual type to use // for id, we can use original type as key for more efficient lookup: final String key = clazz.getName(); diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java index 6f4710a46b..9b4d7bb884 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.jsontype.TypeIdResolver; import com.fasterxml.jackson.databind.type.TypeFactory; +import com.fasterxml.jackson.databind.util.ClassUtil; /** * Partial base implementation of {@link TypeIdResolver}: all custom implementations @@ -69,4 +70,32 @@ public JavaType typeFromId(DatabindContext context, String id) throws IOExcepti public String getDescForKnownTypeIds() { return null; } + + /** + * Helper method for ensuring we properly resolve cases where we don't + * want to use given instance class due to it being a specific inner class + * but rather enclosing (or parent) class. Specific case we know of + * currently are "enum subtypes", cases + * where simple Enum constant has overrides and uses generated sub-class + * if parent Enum type. In this case we need to ensure that we use + * the main/parent Enum type, not sub-class. + * + * @param cls Class to check and possibly resolve + * @return Resolved class to use + * @since 2.18.2 + */ + protected Class _resolveToParentAsNecessary(Class cls) { + // Need to ensure that "enum subtypes" work too + if (ClassUtil.isEnumType(cls)) { + // 29-Sep-2019, tatu: `Class.isEnum()` only returns true for main declaration, + // but NOT from sub-class thereof (extending individual values). This + // is why additional resolution is needed: we want class that contains + // enumeration instances. + if (!cls.isEnum()) { + // and this parent would then have `Enum.class` as its parent: + cls = cls.getSuperclass(); + } + } + return cls; + } } diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java index 2fa4a450bf..5fd7ea382a 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java @@ -120,6 +120,10 @@ protected String idFromClass(Class clazz) if (clazz == null) { return null; } + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + clazz = _resolveToParentAsNecessary(clazz); + // NOTE: although we may need to let `TypeModifier` change actual type to use // for id, we can use original type as key for more efficient lookup: final String key = clazz.getName(); diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java new file mode 100644 index 0000000000..2ae8503457 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java @@ -0,0 +1,127 @@ +package com.fasterxml.jackson.databind.jsontype.jdk; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; + +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +public class EnumTyping4733Test extends DatabindTestUtil +{ + // Baseline case that already worked + @JsonTypeInfo(use = Id.CLASS) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_CLASS.class), + }) + interface InterClass { + default void yes() {} + } + + enum A_CLASS implements InterClass { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.MINIMAL_CLASS) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_MIN_CLASS.class), + }) + interface InterMinimalClass { + default void yes() {} + } + + enum A_MIN_CLASS implements InterMinimalClass { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.NAME) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_NAME.class), + }) + interface InterName { + default void yes() {} + } + + enum A_NAME implements InterName { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.SIMPLE_NAME) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_SIMPLE_NAME.class), + }) + interface InterSimpleName { + default void yes() {} + } + + enum A_SIMPLE_NAME implements InterSimpleName { + A1, + A2 { + @Override + public void yes() { } + }; + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + @Test + public void testIssue4733Class() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_CLASS.A1); + String json2 = MAPPER.writeValueAsString(A_CLASS.A2); + + assertEquals(A_CLASS.A1, MAPPER.readValue(json1, A_CLASS.class)); + assertEquals(A_CLASS.A2, MAPPER.readValue(json2, A_CLASS.class)); + } + + @Test + public void testIssue4733MinimalClass() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_MIN_CLASS.A1); + String json2 = MAPPER.writeValueAsString(A_MIN_CLASS.A2); + assertEquals(A_MIN_CLASS.A1, MAPPER.readValue(json1, A_MIN_CLASS.class), + "JSON: "+json1); + assertEquals(A_MIN_CLASS.A2, MAPPER.readValue(json2, A_MIN_CLASS.class), + "JSON: "+json2); + } + + @Test + public void testIssue4733Name() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_NAME.A1); + String json2 = MAPPER.writeValueAsString(A_NAME.A2); + assertEquals(A_NAME.A1, MAPPER.readValue(json1, A_NAME.class), + "JSON: "+json1); + assertEquals(A_NAME.A2, MAPPER.readValue(json2, A_NAME.class), + "JSON: "+json2); + } + + @Test + public void testIssue4733SimpleName() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_SIMPLE_NAME.A1); + String json2 = MAPPER.writeValueAsString(A_SIMPLE_NAME.A2); + assertEquals(A_SIMPLE_NAME.A1, MAPPER.readValue(json1, A_SIMPLE_NAME.class), + "JSON: "+json1); + assertEquals(A_SIMPLE_NAME.A2, MAPPER.readValue(json2, A_SIMPLE_NAME.class), + "JSON: "+json2); + } +}