diff --git a/release-notes/VERSION b/release-notes/VERSION index ee2a8a7291..580265073f 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -52,6 +52,9 @@ Versions: 3.x (for earlier see VERSION-2.x) creator property" when deserializing JSON with dup property to single-property Record (reported by @sseelmann) (fix contributed by @JacksonJang) +#4708: `JsonTypeInfo.Id.DEDUCTION` should block signatures for non-instantiable + types (abstract classes, interfaces) + (implemented by @cowtowncoder, w/ Claude code) #5115: `@JsonUnwrapped` Record deserialization can't handle name collision (reported by Viktor S) (fix contributed by @JacksonJang) diff --git a/src/main/java/tools/jackson/databind/DeserializationFeature.java b/src/main/java/tools/jackson/databind/DeserializationFeature.java index 018340c182..99b5d3a4ef 100644 --- a/src/main/java/tools/jackson/databind/DeserializationFeature.java +++ b/src/main/java/tools/jackson/databind/DeserializationFeature.java @@ -171,6 +171,24 @@ public enum DeserializationFeature implements ConfigFeature */ FAIL_ON_INVALID_SUBTYPE(true), + /** + * Feature that determines whether abstract types (abstract classes, interfaces) + * should be ignored when building the type fingerprints for polymorphic type + * deduction using {@link com.fasterxml.jackson.annotation.JsonTypeInfo.Id#DEDUCTION}. + * When enabled, non-concrete types are excluded from deduction since they cannot + * be instantiated; when disabled, they participate in deduction which may cause + * signature conflicts with their concrete subclasses. + *

+ * This feature only affects deduction-based polymorphic deserialization; other + * type resolution mechanisms (NAME, CLASS, etc.) are not affected. + *

+ * Feature is enabled by default (since 3.1) to exclude abstract types from + * deduction and avoid signature conflicts. + * + * @since 3.1 + */ + IGNORE_ABSTRACT_TYPES_FOR_DEDUCTION(true), + /** * Feature that determines what happens when reading JSON content into tree * ({@link JsonNode} and a duplicate key diff --git a/src/main/java/tools/jackson/databind/jsontype/impl/AsDeductionTypeDeserializer.java b/src/main/java/tools/jackson/databind/jsontype/impl/AsDeductionTypeDeserializer.java index 5534233115..ff0f2ee26a 100644 --- a/src/main/java/tools/jackson/databind/jsontype/impl/AsDeductionTypeDeserializer.java +++ b/src/main/java/tools/jackson/databind/jsontype/impl/AsDeductionTypeDeserializer.java @@ -64,8 +64,14 @@ protected Map buildFingerprints(DeserializationContext ctxt, int nextProperty = 0; Map fingerprints = new HashMap<>(); + final boolean removeAbstract = ctxt.isEnabled(DeserializationFeature.IGNORE_ABSTRACT_TYPES_FOR_DEDUCTION); for (NamedType subtype : subtypes) { JavaType subtyped = ctxt.constructType(subtype.getType()); + // [databind#4708]: Skip non-concrete types (abstract classes, interfaces) + // since they cannot be instantiated and should not participate in deduction + if (removeAbstract && !ClassUtil.isConcrete(subtyped.getRawClass())) { + continue; + } List properties = ctxt.introspectBeanDescription(subtyped).findProperties(); BitSet fingerprint = new BitSet(nextProperty + properties.size()); diff --git a/src/test/java/tools/jackson/databind/jsontype/DeductionWithAbstractSubtype4708Test.java b/src/test/java/tools/jackson/databind/jsontype/DeductionWithAbstractSubtype4708Test.java new file mode 100644 index 0000000000..9d2981a4d2 --- /dev/null +++ b/src/test/java/tools/jackson/databind/jsontype/DeductionWithAbstractSubtype4708Test.java @@ -0,0 +1,214 @@ +package tools.jackson.databind.jsontype; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; + +import tools.jackson.databind.*; +import tools.jackson.databind.exc.InvalidDefinitionException; +import tools.jackson.databind.testutil.DatabindTestUtil; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for [databind#4708]: DEDUCTION mode should ignore abstract classes + * and interfaces since they cannot be instantiated. + */ +public class DeductionWithAbstractSubtype4708Test extends DatabindTestUtil +{ + // Simulating Kotlin sealed class hierarchy with abstract intermediate class + @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION) + @JsonSubTypes({ + @JsonSubTypes.Type(Ingredient.AbstractItemById.class), // Abstract class registered! + @JsonSubTypes.Type(Ingredient.ItemById.class), + @JsonSubTypes.Type(Ingredient.ItemByTag.class) + }) + sealed interface Ingredient permits Ingredient.Item { + + @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION) + @JsonSubTypes({ + @JsonSubTypes.Type(Ingredient.AbstractItemById.class), // Abstract class registered! + @JsonSubTypes.Type(Ingredient.ItemById.class), + @JsonSubTypes.Type(Ingredient.ItemByTag.class) + }) + sealed interface Item extends Ingredient + permits Ingredient.AbstractItemById, Ingredient.ItemByTag { + } + + // Abstract class with properties - should be IGNORED during deduction + // Previously this would cause signature conflicts + non-sealed abstract class AbstractItemById implements Item { + @JsonProperty("item") + public String id; + public int count = 1; + + public AbstractItemById() {} + public AbstractItemById(String id, int count) { + this.id = id; + this.count = count; + } + } + + // Concrete implementation of the abstract class + final class ItemById extends AbstractItemById { + public ItemById() {} + public ItemById(String id, int count) { + super(id, count); + } + } + + // Another concrete class with different signature + final class ItemByTag implements Item { + @JsonProperty("tag") + public String tag; + public int count = 1; + + public ItemByTag() {} + public ItemByTag(String tag, int count) { + this.tag = tag; + this.count = count; + } + } + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + @Test + public void testDeductionWithAbstractIntermediateClass() throws Exception + { + // Should deduce to ItemById, ignoring the abstract AbstractItemById + String json1 = a2q("{'item':'minecraft:stone','count':64}"); + Ingredient result1 = MAPPER.readValue(json1, Ingredient.class); + + assertNotNull(result1); + assertTrue(result1 instanceof Ingredient.ItemById, + "Should deserialize to concrete ItemById, not abstract class"); + Ingredient.ItemById item1 = (Ingredient.ItemById) result1; + assertEquals("minecraft:stone", item1.id); + assertEquals(64, item1.count); + + // Should deduce to ItemByTag + String json2 = a2q("{'tag':'minecraft:logs','count':32}"); + Ingredient result2 = MAPPER.readValue(json2, Ingredient.class); + + assertNotNull(result2); + assertTrue(result2 instanceof Ingredient.ItemByTag); + Ingredient.ItemByTag item2 = (Ingredient.ItemByTag) result2; + assertEquals("minecraft:logs", item2.tag); + assertEquals(32, item2.count); + } + + @Test + public void testDeductionWithItemInterface() throws Exception + { + // When deserializing as Item interface, should also work + String json = a2q("{'item':'test','count':1}"); + + JavaType itemType = MAPPER.constructType(Ingredient.Item.class); + Ingredient.Item result = MAPPER.readValue(json, itemType); + + assertNotNull(result); + assertTrue(result instanceof Ingredient.ItemById); + assertEquals("test", ((Ingredient.ItemById) result).id); + } + + // Simpler test case with just abstract class and concrete subclass + @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION) + @JsonSubTypes({ + @JsonSubTypes.Type(Animal.class), // Abstract class registered! + @JsonSubTypes.Type(ConcreteAnimal.class) + }) + abstract static class Animal { + public String name; + } + + static class ConcreteAnimal extends Animal { + public int age; + } + + @Test + public void testSimpleAbstractClassIgnored() throws Exception + { + // Abstract Animal should be ignored, only ConcreteAnimal should be considered + String json = a2q("{'name':'Fido','age':5}"); + Animal result = MAPPER.readValue(json, Animal.class); + + assertNotNull(result); + assertTrue(result instanceof ConcreteAnimal); + assertEquals("Fido", result.name); + assertEquals(5, ((ConcreteAnimal) result).age); + } + + // Test with interface in the mix + @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION) + @JsonSubTypes({ + @JsonSubTypes.Type(Dog.class), + @JsonSubTypes.Type(Cat.class) + }) + interface Pet { + // Interface should be ignored + } + + static class Dog implements Pet { + public String breed; + } + + static class Cat implements Pet { + public boolean indoor; + } + + @Test + public void testInterfaceIgnored() throws Exception + { + // Interface Pet should be ignored during fingerprinting + String json1 = a2q("{'breed':'Labrador'}"); + Pet result1 = MAPPER.readValue(json1, Pet.class); + + assertNotNull(result1); + assertTrue(result1 instanceof Dog); + assertEquals("Labrador", ((Dog) result1).breed); + + String json2 = a2q("{'indoor':true}"); + Pet result2 = MAPPER.readValue(json2, Pet.class); + + assertNotNull(result2); + assertTrue(result2 instanceof Cat); + assertTrue(((Cat) result2).indoor); + } + + // Test that the feature can be disabled to get old behavior + @Test + public void testFeatureCanBeDisabled() throws Exception + { + // When feature is disabled, abstract types participate in deduction + // which causes signature conflicts (old buggy behavior) + ObjectMapper mapper = jsonMapperBuilder() + .disable(DeserializationFeature.IGNORE_ABSTRACT_TYPES_FOR_DEDUCTION) + .build(); + + String json = a2q("{'item':'minecraft:stone','count':64}"); + + try { + mapper.readValue(json, Ingredient.class); + fail("Should have failed with signature conflict"); + } catch (InvalidDefinitionException e) { + verifyException(e, "Subtypes"); + verifyException(e, "have the same signature"); + verifyException(e, "cannot be uniquely deduced"); + // Verify it mentions both the abstract class and concrete class + verifyException(e, "AbstractItemById"); + verifyException(e, "ItemById"); + } + } + + // Test that feature is enabled by default + @Test + public void testFeatureEnabledByDefault() throws Exception + { + ObjectMapper mapper = newJsonMapper(); + assertTrue(mapper.isEnabled(DeserializationFeature.IGNORE_ABSTRACT_TYPES_FOR_DEDUCTION), + "IGNORE_ABSTRACT_TYPES_FOR_DEDUCTION should be enabled by default"); + } +}