jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Serialization of Guava optional with polymorphic inner class is not including @class as specified by @JsonTypeInfo

Open geekbeast opened this issue 2 years ago • 9 comments

Describe the bug Serialization of Guava optional with polymorphic inner class is not including @class as specified by @JsonTypeInfo

It works correctly on Jackson 2.6.5, but stopped working at some point since then. For compatibility reasons our Android app has been locked Jackson 2.6.5 and we only noticed this once server was recently upgrade to 2.13.1 and our integration tests started failing because they use a retrofit client that uses the latest version of jackson.

Version information 2.13.1

To Reproduce Here is the interface

@JsonTypeInfo(use=JsonTypeInfo.Id.CLASS, include= JsonTypeInfo.As.PROPERTY, property="@class")
public interface SourceDevice {

}

Here is the concrete class

@JsonTypeInfo(use=JsonTypeInfo.Id.CLASS, include= JsonTypeInfo.As.PROPERTY, property="@class")
public class AndroidDevice implements SourceDevice {
    private final String              device;
    private final String              model;
    private final String              codename;
    private final String              brand;
    private final String osVersion;
    private final String              sdkVersion;
    private final String              product;
    private final String              deviceId;
    private final Map<String, Object> additionalInfo;

    @JsonCreator
    public AndroidDevice(
            @JsonProperty( DEVICE ) String device,
            @JsonProperty( MODEL ) String model,
            @JsonProperty( CODENAME ) String codename,
            @JsonProperty( BRAND ) String brand,
            @JsonProperty( OS_VERSION ) String osVersion,
            @JsonProperty( SDK_VERSION ) String sdkVersion,
            @JsonProperty( PRODUCT ) String product,
            @JsonProperty( DEVICE_ID ) String deviceId,
            @JsonProperty( ADDITIONAL_INFO ) Optional<Map<String, Object>> additionalInfo ) {
        this.device = device;
        this.model = model;
        this.codename = codename;
        this.brand = brand;
        this.osVersion=osVersion;
        this.sdkVersion = sdkVersion;
        this.product = product;
        this.deviceId = deviceId;
        this.additionalInfo = additionalInfo.or( HashMap::new );
    }
}

The setup for the object mapper:

mapper.registerModule( new GuavaModule() );
mapper.registerModule( new JodaModule() );
mapper.configure( SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false );

Our actual serialization test is here: https://github.com/openlattice/chronicle-server/blob/18be3bfc845445e20b3fdc1094c5e8194d49c1ae/src/test/kotlin/com/openlattice/chronicle/serialization/SourceDeviceSerializationTest.kt

Base serialization test class is here: https://github.com/geekbeast/rhizome-client/blob/da2c8807fc4b8fa085375a58502db47baa6f36fb/src/test/java/com/geekbeast/serializer/serializer/AbstractJacksonSerializationTest.java

Expected behavior Expected behavior is the following json string:

{"@class":"com.openlattice.chronicle.sources.AndroidDevice","device":"650ad51cf4a84729","model":"Android SDK built for x86","codename":"REL","brand":"Android","osVersion":"30","sdkVersion":"30","product":"sdk_phone_x86","deviceId":"650ad51cf4a84729","additionalInfo":{}}

Instead here is what is serialized:
{"device":"650ad51cf4a84729","model":"Android SDK built for x86","codename":"REL","brand":"Android","osVersion":"30","sdkVersion":"30","product":"sdk_phone_x86","deviceId":"650ad51cf4a84729","additionalInfo":{}}

geekbeast avatar Feb 01 '22 21:02 geekbeast

Just went and looked back at a similar bug we encountered and it looks like it might be another instance of the same thing: https://github.com/FasterXML/jackson-databind/issues/2078

The weird thing is that something changed between 2.6.5 and 2.13.1 wherein it no longer works, so would be surprised if it is type erasure. Given that we are using this within a Spring controller manually forcing serialization is tough since Optional is an abstract class in java instead of an interface. Going to try and switch server to java.util.Optional and make container interface.

geekbeast avatar Feb 01 '22 21:02 geekbeast

Never mind Optional is not a functional interface.

geekbeast avatar Feb 01 '22 21:02 geekbeast

If this is about Guava Optional, issue should be moved to jackson-datatypes-collections. However, if it could be reproduced with AtomicReference, part of JDK, it could remain here. The reason reproduction might work is that (de)serializer implementations share a common base implementation (starting with... 2.10 maybe).

One to keep in mind is that if Optional value is serialized as "root value" (one directly being serialized, and not reached via property), there are typically many possible issues, due to Java Type Erasure. Behavior in 2.6 may have been incorrect, as well.

What would help here would be to include (inline) simplest possible serialization + deserialization case that is failing; in pure Java. And like I said, ideally using AtomicReference (assuming that does reproduce the issue). I can have a look at that; but not a test that uses Kotlin; and with Guava or JDK8 Optional I'd have to test against different modules which is doable but bit more work.

cowtowncoder avatar Feb 02 '22 16:02 cowtowncoder

Here's a self contained test case using vanilla mapper that will reproduce the same thing with AtomicReference. It passed on 2.6.5 for me, but fails on 2.10 and 2.13.1

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.junit.Test;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;

public class SourceDeviceSerializationTest {
    private static final ObjectMapper mapper = new ObjectMapper();

    public static final String DEVICE = "device";
    public static final String BRAND = "brand";
    public static final String MODEL = "model";
    public static final String PRODUCT = "product";
    public static final String CODENAME = "codename";
    public static final String DEVICE_ID = "deviceId";
    public static final String OS_VERSION = "osVersion";
    public static final String SDK_VERSION = "sdkVersion";
    public static final String ADDITIONAL_INFO = "additionalInfo";

    @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "@class")
    public static interface SourceDevice {

    }

    @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "@class")
    public static class AndroidDeviceTest implements SourceDevice {
        private final String device;
        private final String model;
        private final String codename;
        private final String brand;
        private final String osVersion;
        private final String sdkVersion;
        private final String product;
        private final String deviceId;
        private final Map<String, Object> additionalInfo;

        @JsonCreator
        public AndroidDeviceTest(
                @JsonProperty(DEVICE) String device,
                @JsonProperty(MODEL) String model,
                @JsonProperty(CODENAME) String codename,
                @JsonProperty(BRAND) String brand,
                @JsonProperty(OS_VERSION) String osVersion,
                @JsonProperty(SDK_VERSION) String sdkVersion,
                @JsonProperty(PRODUCT) String product,
                @JsonProperty(DEVICE_ID) String deviceId,
                @JsonProperty(ADDITIONAL_INFO) Map<String, Object> additionalInfo) {
            this.device = device;
            this.model = model;
            this.codename = codename;
            this.brand = brand;
            this.osVersion = osVersion;
            this.sdkVersion = sdkVersion;
            this.product = product;
            this.deviceId = deviceId;
            this.additionalInfo = new HashMap<>();
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) {
                return true;
            }
            if (!(o instanceof AndroidDeviceTest)) {
                return false;
            }
            AndroidDeviceTest that = (AndroidDeviceTest) o;
            return Objects.equals(device, that.device) &&
                    Objects.equals(model, that.model) &&
                    Objects.equals(codename, that.codename) &&
                    Objects.equals(brand, that.brand) &&
                    Objects.equals(osVersion, that.osVersion) &&
                    Objects.equals(sdkVersion, that.sdkVersion) &&
                    Objects.equals(product, that.product) &&
                    Objects.equals(deviceId, that.deviceId) &&
                    Objects.equals(additionalInfo, that.additionalInfo);
        }

        @Override
        public int hashCode() {
            return Objects.hash(device, model, codename, brand, osVersion, sdkVersion, product, deviceId, additionalInfo);
        }

        @Override
        public String toString() {
            return "AndroidDevice{" +
                    "device='" + device + '\'' +
                    ", model='" + model + '\'' +
                    ", codename='" + codename + '\'' +
                    ", brand='" + brand + '\'' +
                    ", osVersion='" + osVersion + '\'' +
                    ", sdkVersion='" + sdkVersion + '\'' +
                    ", product='" + product + '\'' +
                    ", deviceId='" + deviceId + '\'' +
                    ", additionalInfo=" + additionalInfo +
                    '}';
        }

        @JsonProperty(DEVICE)
        public String getDevice() {
            return device;
        }

        @JsonProperty(MODEL)
        public String getModel() {
            return model;
        }

        @JsonProperty(CODENAME)
        public String getCodename() {
            return codename;
        }

        @JsonProperty(BRAND)
        public String getBrand() {
            return brand;
        }

        @JsonProperty(SDK_VERSION)
        public String getSdkVersion() {
            return sdkVersion;
        }

        @JsonProperty(OS_VERSION)
        public String getOsVersion() {
            return sdkVersion;
        }

        @JsonProperty(PRODUCT)
        public String getProduct() {
            return product;
        }

        @JsonProperty(DEVICE_ID)
        public String getDeviceId() {
            return deviceId;
        }

        @JsonProperty(ADDITIONAL_INFO)
        public Map<String, Object> getAdditionalInfo() {
            return additionalInfo;
        }
    }


    @Test
    public void testSerdes() throws IOException {
        final AtomicReference<AndroidDeviceTest> adt = new AtomicReference<AndroidDeviceTest>(new AndroidDeviceTest(
                "Samsung",
                "P",
                "Chocholate Chip",
                "Samsung",
                "21",
                "21",
                "",
                "",
                new HashMap<>()
        ));

        final String json = mapper.writeValueAsString(adt);
        LoggerFactory.getLogger(SourceDeviceSerializationTest.class).info("JSON: {}", json);
        final AtomicReference<AndroidDeviceTest> attempt = mapper.readValue(json, new TypeReference<AtomicReference<AndroidDeviceTest>>() {
        });
    }

}

geekbeast avatar Feb 02 '22 22:02 geekbeast

Thank you @geekbeast

cowtowncoder avatar Feb 03 '22 21:02 cowtowncoder

Ok the very first comment. before even trying out the test is this: I STRONGLY recommend NEVER to use generic type as the root value. Ever. But always have a wrapper POJO around it. This because Java Type Erasure does mess things over and over and over again.

In this sense this is likely the very same issue as with using List<MyType> or Map<String, MyType> as root value -- you actually MUST specify type during serialization since otherwise type here is only seen by JDK as AtomicReference<?>. The reason this "worked" in 2.6.x is likely an actual bug in handling (but one that happens to work out well in some cases).

But I will try to reproduce this just to make sure.

As to work-arounds: to remove generic type from root value, one quick way is to create single-use sub-class like:

public class DeviceTestReference extends AtomicReference<AndroidDeviceTest> { }

and using this type reads and writes work because now no type information is lost by Java Type Erasure.

Alternatively it is also possible to force write type with:

ObjectWriter w = mapper.writerFor(new TypeReference<AtomicReference<AndroidDeviceTest>>() { });
String json = w.writeValueAsString(value);

and this SHOULD actually pass usable type for serialization, so that things work.

cowtowncoder avatar Feb 04 '22 03:02 cowtowncoder

While I won't yet close this issue, I think that the eventual evaluation will be that of later Jackson versions working as expected; while 2.6 did work the way that was good in your case it is unfortunately incompatible with the way polymorphic handling overall works.

2.7+ handling can be made to work with polymorphic contents of Optionals as suggested by above changes, however.

cowtowncoder avatar Feb 28 '22 00:02 cowtowncoder

That makes sense we are moving away from Optional and marking the API signature with Optional as deprecated. Legacy versions of mobile app will use "working" version and the next release will call new API.

geekbeast avatar Feb 28 '22 01:02 geekbeast

Ok good @geekbeast thank you for the update. It is good that you can work around this problem. It is unfortunate there is this discontinuity, but supporting Optional has been challenging as there are (at least) 2 ways to go about it. And initial implementation did not fully consider polymorphic handling of contents either.

cowtowncoder avatar Feb 28 '22 01:02 cowtowncoder