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

DeserializationFeature.FAIL_ON_INVALID_SUBTYPE does not work #511

Closed
sbelikov opened this issue Jul 31, 2014 · 5 comments
Closed

DeserializationFeature.FAIL_ON_INVALID_SUBTYPE does not work #511

sbelikov opened this issue Jul 31, 2014 · 5 comments
Milestone

Comments

@sbelikov
Copy link

When the DeserializationFeature.FAIL_ON_INVALID_SUBTYPE is disabled the com.fasterxml.jackson.databind.JsonMappingException is thrown instead of just a null added to collection.

Consider the following code:

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Before;
import org.junit.Test;

import java.io.IOException;
import java.util.List;

import static org.junit.Assert.assertNotNull;

public class JacksonBugTest {

    ObjectMapper mapper ;


    @Before
    public void configureMapper() {
        mapper = new ObjectMapper()
                .disable(
                        DeserializationFeature.FAIL_ON_INVALID_SUBTYPE,
                        DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,
                        DeserializationFeature.FAIL_ON_IGNORED_PROPERTIES
                ) ;
    }

    @Test
    public void test() throws IOException {
        String json = "{\"many\":[{\"sub1\":{\"a\":\"foo\"}},{\"sub2\":{\"b\":\"bar\"}}]}" ;
        Good goodResult = mapper.readValue(json, Good.class) ;
        assertNotNull("failed conversion", goodResult) ;
        Bad badResult = mapper.readValue(json, Bad.class) ;
    }


    public static class Bad {
        @JsonProperty("many")
        List<BadItem> many ;
    }

    @JsonTypeInfo(
            use = JsonTypeInfo.Id.NAME,
            include = JsonTypeInfo.As.WRAPPER_OBJECT
    )
    @JsonSubTypes(
            @JsonSubTypes.Type(name="sub1", value = BadSub1.class)
    )
    public static class BadItem {}


    public static class BadSub1 extends BadItem {
        @JsonProperty("a")
        String a ;

    }

    public static class Good {
        @JsonProperty("many")
        List<GoodItem> many ;
    }

    @JsonTypeInfo(
            use = JsonTypeInfo.Id.NAME,
            include = JsonTypeInfo.As.WRAPPER_OBJECT
    )
    @JsonSubTypes({
            @JsonSubTypes.Type(name="sub1", value = GoodSub1.class),
            @JsonSubTypes.Type(name="sub2", value = GoodSub2.class)
    })
    public static class GoodItem {}

    public static class GoodSub1 extends GoodItem {
        @JsonProperty("a")
        String a ;

    }
    public static class GoodSub2 extends GoodItem {
        @JsonProperty("b")
        String b ;

    }
}
@cowtowncoder
Copy link
Member

Sounds like a bug, yes. Thank you for reproducible test case.

cowtowncoder added a commit that referenced this issue Sep 23, 2014
cowtowncoder added a commit that referenced this issue Sep 23, 2014
@cowtowncoder cowtowncoder modified the milestones: 2.5.0, 2.4.3 Sep 26, 2014
@mikelfinch
Copy link

Is anyone still having trouble with this issue? From Maven I am using core-2.4.4, databind 2.4.4 and annotations-2.4.0. I am try to deserialize an Object value from an external app, I get a "class not found" error. We are building a JAR that will be shared between apps... App A will set Event.value to an instance of a local class. When App B tries to deserialize this object it will get a "class not found" error. This is not the typical pattern, Event.value classes are usually in shared JARs.

Look at my ObjectMapper code below and let me know what I am missing, thanks.

                    ObjectMapper mapper = new ObjectMapper()
                    .disable(
                            DeserializationFeature.FAIL_ON_INVALID_SUBTYPE,
                            DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,
                            DeserializationFeature.FAIL_ON_IGNORED_PROPERTIES
                    );
                    callRecord = mapper.readValue(data, CallRecord.class);

Sample classes..

public class CallRecord
{
    public float version;
    public String application;
    public String environment;
    public History history;

    public CallRecord() {
        history = new History();
    }
}

public class History {       
    public ArrayList<Item> items = new ArrayList<Item>();    
}

@JsonTypeInfo(use=JsonTypeInfo.Id.MINIMAL_CLASS, include=JsonTypeInfo.As.PROPERTY, property="@class")
public class Item {          
    public Calendar timestamp;
}

public final class Entry extends Item {
    public String state;

    public Entry() {        
    }

    public Entry(String state) {
        this.state = state;
    }
}

public final class Event extends Item {    
    public String location;

    public Object type;

    @JsonTypeInfo(use=JsonTypeInfo.Id.MINIMAL_CLASS, include=JsonTypeInfo.As.PROPERTY, property="@class")
    public Object value;

    public Event() {
    }

    public Event(String location, Object type, Object value) {
        this.location = location;
        this.type = type;
        this.value = value;
    }
}

@cowtowncoder
Copy link
Member

I think this is because all work so far has focused on using type names as id, not class names.
Class names are quite different since there is no way to know if something is invalid without trying to resolve it (whereas set of names is bounded at given point in time).

Could you file a separate issue for resolving remaining issue with invalid class names?
While it is obviously conceptually related, solution is somewhat different, and release notes are easier to maintain (wrt fixed-in-version) that way.

@mikelfinch
Copy link

Cowtowncoder,

I can definitely file a separate issue, that is fine. Above you mention something about "using type names as id, not class names.". So, is there another way to accomplish what I am trying to do above using "type names as ids"? I am looking for any solution that will either 1.) deserialize Event.value to a specify class instance or 2.) set Event.value to null if the class is not found.

Cheers!

@cowtowncoder
Copy link
Member

I just mean that in your we have:

@JsonTypeInfo(use=JsonTypeInfo.Id.MINIMAL_CLASS,

instead of

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,

so that type identifier is specific class name, and no translation is done before trying to load the class definition. With type names the problem would be detected earlier, when loading code, if Class referenced was not found.

But I think the proper fix is to catch exception and handle it as unknown I think.

It may be possible to solve this also by using a custom TypeIdResolver, perhaps a sub-class of default one for used for Id.MINIMAL_CLASS, in which you could catch exception. I have not done that myself, but I can't see a reason why it could not be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants