Map deserialization results in different numeric classes based on json ordering (BigDecimal / Double) when used in combination with @JsonSubTypes

[mreiterer]

Describe the bug
When deserializing a jsonSubTye which contains a map with numeric values (floats) the order of the json string results in different classes constructed.

I have pasted a complete Junit5 Test which can easily be used to reproduce the issue.

Version information
Happens from 2.12.0 onwards, in 2.11.x its ok.

To Reproduce
If you have a way to reproduce this with:

package test;

import java.util.HashMap;
import java.util.Map;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonSubTypes.Type;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;

@TestInstance(Lifecycle.PER_CLASS)
public class JacksonSubTypeBehaviourTest {

	private static final Logger log = LoggerFactory.getLogger(JacksonSubTypeBehaviourTest.class);

	private ObjectMapper mapper;

	@BeforeAll
	public void before() {
		mapper = new ObjectMapper();
		mapper.disable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS);
	}

	@Test 
	public void testDeserializeWithDifferentOrdering() throws Exception {
		
		String ordering1 = "{\n"
				+ "			\"type\": \"MAP\",\n"
				+ "			\"map\": {\n"
				+ "				\"doubleValue\": 0.1\n"
				+ "			}\n"
				+ "}";
				
		TestMapContainer model1 = mapper.readValue(ordering1, TestMapContainer.class);
		log.info("clazz: {}",  model1.getMap().get("doubleValue").getClass());
		Assertions.assertTrue(model1.getMap().get("doubleValue") instanceof Double);
		
		
		String ordering2 = "{\n"
				+ "	\"map\": {\n"
				+ "		\"doubleValue\": 0.1\n"
				+ "	},\n"
				+ "	\"type\": \"MAP\"\n"
				+ "	\n"
				+ "}";
		
		TestMapContainer model2 = mapper.readValue(ordering2, TestMapContainer.class);
		
		log.info("clazz: {}",  model2.getMap().get("doubleValue").getClass());
		Assertions.assertTrue(model2.getMap().get("doubleValue") instanceof Double);
		
	}
	
	@JsonTypeInfo(  
		    use = JsonTypeInfo.Id.NAME,  
		    include = JsonTypeInfo.As.PROPERTY,  
		    property = "type")

	@JsonSubTypes({  
	    @Type(value = TestMapContainer.class, name = "MAP"),
	     })  	
	private static interface TestJsonTypeInfoInterface {
		
	}
	
	private static class TestMapContainer implements TestJsonTypeInfoInterface {
		
		private Map<String, ? extends Object> map = new HashMap<>();

		public Map<String, ? extends Object> getMap() {
			return map;
		}

		public void setMap(Map<String, ? extends Object> map) {
			this.map = map;
		}
		
	}
}

Expected behavior
Numeric values (point numbers) should always be created as Double unless stated otherwise with DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS

[cowtowncoder]

Interesting. Due to ordering difference I suspect this is due to one ordering requiring buffering to handle polymorphism (that is, type id must be available before POJO values can be instantiated); and then there is the challenge of TokenBuffer retaining equivalent information both about configuration (wrt coercion) and source information (since values are not JSON text but must be converted into Java values).

I agree that behavior should be identical; above is just talking through likely root cause that changes behavior between the cases.

Thank you for including all the information to reproduce the issue!

[mreiterer]

If you can give me a hint / entry point in the codebase i can try to fix / improve the logic and send a merge request.

[cowtowncoder]

@mreiterer I wish there was an easy answer to that question, but handling of buffering is not exactly simple. Relevant code is in TokenBuffer, but it all depends on deserializer that handles buffering, method(s) called on JsonParser to access numeric value and so on. This may get bit more complicated as buffering is not necessarily done by value deserializer in this case, given polymorphic handling which then requires delegation to TypeDeserializer.
It may be possible to use debugger to try to find code paths relevant; ultimately deserializer for expected polymorphic type does get called.

[mreiterer]

I did some debugging here and this seems to be related to #2644
There is a behaviour change from 2.11.4 to 2.12.x related to getNumberValueExact.

When i look at the "_tokens" var in the segment of the TokenBuffer the value is already of type BigDecimal before the DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS is checked. The if in Vanilla.deserialize decides if getDecimalValue or getNumberValueis used, but this does not have any effect as the cast is already done. Mostly notes to myself for further investigation.

The difference about the ordering seems to be located here: AsPropertyTypeDeserializer.deserializeTypedFromObject.

If the typePropertyName is found first in the json structure:
a) then the deserializer is called directly converting the "0.1" to Double.
b) otheriwise the tokenBuffer is copied via copyCurrentStructure -> and in _copyBufferValue getNumberValueExact is called for VALUE_NUMBER_FLOAT.

[cowtowncoder]

Correct: buffering is only used if absolutely required -- or put another way, if ordering happens to be optimal, buffering may be avoided, on assumption this is relatively common.

[mreiterer]

@cowtowncoder Why is getNumberValueExact called in case of buffer copy, and not in the more common case (UntypedObjectDeserializer line 743 - jackson-databind 2.12.5)

[cowtowncoder]

The difference is meant to defer possible truncation/coercion in case of buffering (since we have no idea what the target type is); whereas when using UntypedObjectDeserializer -- which has target type of java.lang.Object -- we know what target type we should be using, based on configuration.

This matter more for binary formats where there is specific accurate representation, but is more problematic for textual formats that do not have distinction between various FP representations. And then there is the difference between streaming-level and databind-level configuration to consider too.

Ideally, I think, the whole decoding should be deferred in buffering cases: parser would simply validate that the textual representation is valid JSON (or xml, yaml, csv etc) Number, but would not try to get Java Number out of it until explicitly requested.

[mreiterer]

Are there any plans to fix this issue in 2.14.x ?