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

Deduction of supertypes #3300

Open
wants to merge 3 commits into
base: 2.13
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ protected Map<BitSet, String> buildFingerprints(DeserializationConfig config, Co
boolean ignoreCase = config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);

int nextField = 0;
Map<BitSet, String> fingerprints = new HashMap<>();
// Sorting by fingerprints such that supertypes always come first
Map<BitSet, String> fingerprints = new TreeMap<>(new BitSetComparator().reversed());

for (NamedType subtype : subtypes) {
JavaType subtyped = config.getTypeFactory().constructType(subtype.getType());
Expand Down Expand Up @@ -126,7 +127,7 @@ public Object deserializeTypedFromObject(JsonParser p, DeserializationContext ct
final TokenBuffer tb = ctxt.bufferForInputBuffering(p);
boolean ignoreCase = ctxt.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);

for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) {
for (; t == JsonToken.FIELD_NAME && !candidates.isEmpty(); t = p.nextToken()) {
String name = p.currentName();
if (ignoreCase) name = name.toLowerCase();

Expand All @@ -141,12 +142,29 @@ public Object deserializeTypedFromObject(JsonParser p, DeserializationContext ct
}
}
}
// We have zero or multiple candidates after parsing the whole input

// We have zero or multiple candidates, deduction has failed
if (!candidates.isEmpty() && firstCandidateIsSupertypeOfOthers(candidates)) {
return _deserializeTypedForId(p, ctxt, tb, subtypeFingerprints.get(candidates.get(0)));
}

// deduction has failed
String msgToReportIfDefaultImplFailsToo = String.format("Cannot deduce unique subtype of %s (%d candidates match)", ClassUtil.getTypeDescription(_baseType), candidates.size());
return _deserializeTypedUsingDefaultImpl(p, ctxt, tb, msgToReportIfDefaultImplFailsToo);
}

private static boolean firstCandidateIsSupertypeOfOthers(List<BitSet> candidates) {
// Pre-sorted fingerprints means the _first_ item may be a supertype...
BitSet supertypeFields = candidates.get(0);

// bitset is mutable, must take a copy :( ThreadLocal?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in love with this method being part of the live processing. Have a sneaking feeling I've missed some logic reduction here

BitSet commonFields = (BitSet)supertypeFields.clone();
for (BitSet candidate : candidates) {
commonFields.and(candidate);
}
return commonFields.equals(supertypeFields);
}

// Keep only fingerprints containing this field
private static void prune(List<BitSet> candidates, int bit) {
for (Iterator<BitSet> iter = candidates.iterator(); iter.hasNext(); ) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.fasterxml.jackson.databind.jsontype.impl;

import java.util.BitSet;
import java.util.Comparator;

/**
* Terse but inefficient (for large arrays) ordering of {@link BitSet} as though it
* were an unsigned integer.
*/
public class BitSetComparator implements Comparator<BitSet> {

// This could be done much more efficiently with access to the long[]
@Override
public int compare(BitSet left, BitSet right) {
if (left.equals(right)) return 0;
BitSet diff = (BitSet)left.clone();
diff.xor(right); // diff = left ^ right
int firstDifference = diff.length() - 1;

if (firstDifference == -1) return 0;
// due to xor, this high-bit is 1 in either left OR right
return right.get(firstDifference) ? 1 : -1;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.databind.type.TypeFactory;

Expand All @@ -23,7 +25,7 @@
public class TestPolymorphicDeduction extends BaseMapTest {

@JsonTypeInfo(use = DEDUCTION)
@JsonSubTypes( {@Type(LiveCat.class), @Type(DeadCat.class), @Type(Fleabag.class)})
@JsonSubTypes( {@Type(LiveCat.class), @Type(DeadCat.class), @Type(Fleabag.class)}) // NOT including Cat
// A general supertype with no properties - used for tests involving {}
interface Feline {}

Expand Down Expand Up @@ -60,10 +62,11 @@ static class Box {
/**********************************************************
*/

private static final String catJson = a2q("{'name':'Felix'}");
private static final String deadCatJson = a2q("{'name':'Felix','causeOfDeath':'entropy'}");
private static final String liveCatJson = a2q("{'name':'Felix','angry':true}");
private static final String luckyCatJson = a2q("{'name':'Felix','angry':true,'lives':8}");
private static final String ambiguousCatJson = a2q("{'name':'Felix','age':2}");
private static final String unknownCatJson = a2q("{'name':'Felix','age':2}");
private static final String fleabagJson = a2q("{}");
private static final String box1Json = a2q("{'feline':" + liveCatJson + "}");
private static final String box2Json = a2q("{'feline':" + deadCatJson + "}");
Expand Down Expand Up @@ -183,70 +186,117 @@ public void testArrayInference() throws Exception {
}

public void testIgnoreProperties() throws Exception {
Cat cat = MAPPER.reader()
.without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.readValue(luckyCatJson, Cat.class);
assertTrue(cat instanceof LiveCat);
// Given:
ObjectReader reader = MAPPER.reader()
.without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
// When:
Cat cat = reader.readValue(luckyCatJson, Cat.class);
// Then:
assertTrue("Should ignore 'lives' property", cat instanceof LiveCat);
assertSame(cat.getClass(), LiveCat.class);
assertEquals("Felix", cat.name);
assertTrue(((LiveCat)cat).angry);
}

public void testAmbiguousPropertiesMatchSupertype() throws Exception {
// Given:
JsonMapper mapper = JsonMapper.builder() // Don't use shared mapper!
.build();
// When:
Feline feline = mapper.readValue(catJson, Cat.class);
// Then:
assertTrue("Cat is included in subtypes and is an exact match", feline instanceof Cat);
assertSame(feline.getClass(), Cat.class);
assertEquals("Felix", ((Cat)feline).name);
}

public void testIgnorePropertiesCanMatchSupertype() throws Exception {
// Given:
ObjectReader reader = MAPPER.reader()
.without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
// When:
Cat cat = reader.readValue(unknownCatJson, Cat.class);
// Then:
assertTrue("Should ignore 'age' property, leaving 'name' to match Cat", cat instanceof Cat);
assertSame(cat.getClass(), Cat.class);
assertEquals("Felix", cat.name);
}

static class AnotherLiveCat extends Cat {
public boolean angry;
}

public void testAmbiguousClasses() throws Exception {
try {
// Given:
ObjectMapper mapper = JsonMapper.builder() // Don't use shared mapper!
.registerSubtypes(AnotherLiveCat.class)
.build();
/*Cat cat =*/ mapper.readValue(liveCatJson, Cat.class);
fail("Should not get here");
.registerSubtypes(AnotherLiveCat.class)
.build();
// When:
mapper.readValue(liveCatJson, Cat.class);

fail("AnotherLiveCat and LiveCat have same signature and should fail to register");
} catch (InvalidDefinitionException e) {
verifyException(e, "Subtypes ");
verifyException(e, "have the same signature");
verifyException(e, "cannot be uniquely deduced");
// Then:
verifyException(e, "Subtypes ");
verifyException(e, "have the same signature");
verifyException(e, "cannot be uniquely deduced");
}
}

public void testAmbiguousProperties() throws Exception {
try {
/*Cat cat =*/ MAPPER.readValue(ambiguousCatJson, Cat.class);
fail("Should not get here");
// When:
MAPPER.readValue(catJson, Feline.class);

fail("Feline doesn't include the Cat subtype");
} catch (InvalidTypeIdException e) {
verifyException(e, "Cannot deduce unique subtype");
// Then:
verifyException(e, "Cannot deduce unique subtype");
}
}

public void testFailOnInvalidSubtype() throws Exception {
// Given:
JsonMapper mapper = JsonMapper.builder() // Don't use shared mapper!
.disable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE)
.build();
// When:
Cat cat = mapper.readValue(ambiguousCatJson, Cat.class);
ObjectReader reader = MAPPER.reader()
.without(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE);
// Given:
Feline cat = reader.readValue(unknownCatJson, Feline.class);
// Then:
assertNull(cat);
assertNull("Should return null not an Exception", cat);
}

@JsonTypeInfo(use = DEDUCTION, defaultImpl = Cat.class)
abstract static class CatMixin {
@JsonTypeInfo(use = DEDUCTION, defaultImpl = AnotherLiveCat.class)
abstract static class FelineMixin {
}

public void testDefaultImpl() throws Exception {
// Given:
JsonMapper mapper = JsonMapper.builder() // Don't use shared mapper!
.addMixIn(Cat.class, CatMixin.class)
.addMixIn(Feline.class, FelineMixin.class)
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.build();
// When:
Cat cat = mapper.readValue(ambiguousCatJson, Cat.class);
Feline cat = mapper.readValue(unknownCatJson, Feline.class);
// Then:
// Even though "age":2 implies this was a failed subtype, we are instructed to fallback to Cat regardless.
assertTrue(cat instanceof Cat);
assertSame(Cat.class, cat.getClass());
assertEquals("Felix", cat.name);
assertTrue("Should fallback when unknown property blocks deduction", cat instanceof AnotherLiveCat);
assertSame(AnotherLiveCat.class, cat.getClass());
}

public void testDefaultImplSupercededByUnknownProperty() throws Exception {
try {
// Given:
JsonMapper mapper = JsonMapper.builder() // Don't use shared mapper!
.addMixIn(Cat.class, FelineMixin.class)
//.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.build();
// When:
mapper.readValue(unknownCatJson, Cat.class);

fail("Unknown property supercedes defaultImpl");
} catch (UnrecognizedPropertyException e) {
// Then:
}
}

public void testSimpleSerialization() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package com.fasterxml.jackson.databind.jsontype.impl;

import java.util.ArrayList;
import java.util.BitSet;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.TreeSet;
import java.util.stream.Collectors;

import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.assertEquals;

public class BitSetComparatorTest {

private final TreeSet<BitSet> tree = new TreeSet<>(new BitSetComparator());

private Map<BitSet, String> dictionary = new HashMap<>();

@Before
public void setUp() {
dictionary.put(bitset("11111111_11111111"), "a");
dictionary.put(bitset("00111111_11111111"), "b");
dictionary.put(bitset("00111111_00111111"), "c");
dictionary.put(bitset("00000001_00111111"), "d");
dictionary.put(bitset("00000000_11111111"), "e");
dictionary.put(bitset("11111110"), "f");
dictionary.put(bitset("00011110"), "g");
dictionary.put(bitset("1"), "h");
dictionary.put(bitset(""), "i");
}

@Test
public void sortsInputs() {
// Given:
List<BitSet> inputs = new ArrayList<>(dictionary.keySet());
Collections.shuffle(inputs, new Random(1966));
// When:
tree.addAll(inputs);
// Then:
String output = tree.stream().map(dictionary::get).collect(Collectors.joining());
assertEquals("abcdefghi", output);
}

private static BitSet bitset(String content) {
BitSet bitset = new BitSet(content.length());
int bit = 0;
char[] chars = content.toCharArray();

for (int i = chars.length-1; i >= 0; i--) {
if (chars[i] >= '0') {
bitset.set(bit++, chars[i] != '0');
}
}
return bitset;
}

}