FasterXML / jackson-datatypes-collections

Jackson project that contains various collection-oriented datatype libraries: Eclipse Collections, Guava, HPPC, PCollections
Apache License 2.0
77 stars 52 forks source link

[eclipse-collection] can not deserialize concrete class instance inside nested immutable eclipse-collection #71

Closed CXwudi closed 4 years ago

CXwudi commented 4 years ago

Hi, the issue is like the following:
I have a field like

@JsonProperty
  private ImmutableMap<String, ImmutableList<AbstractDownloaderConfigeration>> keyToItemsList;

where AbstractDownloaderConfigeration is declared as:

@Getter
@AllArgsConstructor
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "_type")
@JsonSubTypes({
    @JsonSubTypes.Type(value = MetaYoutubeDlConfiguration .class, name = "type 1")
})
public abstract class AbstractDownloaderConfigeration{
   //some fields
}

and the MetaYoutubeDlConfiguration class is:

@Getter @ToString
@NoArgsConstructor(force = true, access = AccessLevel.PROTECTED)
public class MetaYoutubeDlConfiguration extends AbstractDownloaderConfigeration {
  //more fields
}

As you can see I have already properly set up the @JsonTypeInfo and @JsonSubTypes on the abstract class,
but I still got:

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `mikufan.cx.vocadb_pv_downloader.config.downloader.AbstractDownloaderConfigeration` (no Creators, like default constructor, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: (File); line: 9, column: 7] (through reference chain: mikufan.cx.vocadb_pv_downloader.config.entity.UserConfig["downloaderConfigs"])
    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
    at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1615)
    at com.fasterxml.jackson.databind.DatabindContext.reportBadDefinition(DatabindContext.java:400)
    at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1077)
    at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserialize(AbstractDeserializer.java:265)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.BaseCollectionDeserializer$Ref.add(BaseCollectionDeserializer.java:318)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.BaseCollectionDeserializer$Ref.add(BaseCollectionDeserializer.java:268)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.BaseCollectionDeserializer._deserializeContents(BaseCollectionDeserializer.java:80)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.BaseCollectionDeserializer.deserialize(BaseCollectionDeserializer.java:66)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.map.RefValueHandler.value(RefValueHandler.java:60)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.map.TypeHandlerPair$1.add(TypeHandlerPair.java:188)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.map.TypeHandlerPair$1.add(TypeHandlerPair.java:166)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.map.EclipseMapDeserializers$Entry$DeserializerImpl.deserializeEntry(EclipseMapDeserializers.java:311)
    at com.fasterxml.jackson.datatype.eclipsecollections.deser.map.EclipseMapDeserializer.deserialize(EclipseMapDeserializer.java:83)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:138)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:293)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:156)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4482)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3299)
    at mikufan.cx.project_vd_common_util.io.JacksonPojoTransformer.read(JacksonPojoTransformer.java:50)
    at mikufan.cx.vocadb_pv_downloader.config.parser.ArgParser.lambda$getUserConfigOrThrow$4(ArgParser.java:125)
    at mikufan.cx.project_vd_common_util.exception.ThrowableFunction.lambda$toFunction$0(ThrowableFunction.java:23)
    at mikufan.cx.project_vd_common_util.cli.parser.ParserUtil.getOrElse(ParserUtil.java:47)
    at mikufan.cx.project_vd_common_util.cli.parser.ParserUtil.getValueOrElse(ParserUtil.java:25)
    at mikufan.cx.vocadb_pv_downloader.config.parser.ArgParser.getUserConfigOrThrow(ArgParser.java:137)
    at mikufan.cx.vocadb_pv_downloader.config.ConfigFactory.getConfig(ConfigFactory.java:35)
    at mikufan.cx.vocadb_pv_downloader.Main.main(Main.java:15)

However, if I change the type of keyToItemsList from ImmutableMap<String, ImmutableList<AbstractDownloaderConfigeration>> to ImmutableMap<String, List<AbstractDownloaderConfigeration>> (using java.util.List inside the eclipse collection map), everything works fine, even though I have used Lombok in my code

I think this is a bug in this Jackson module. So I reported here

cowtowncoder commented 4 years ago

Looks like example uses Lombok: this is fine in itself, but I think we'd need a post-processed version for testing to see actual methods and annotations in used. Plus tests are not allowed to include Lombok as dependency for practical reasons (no byte- or source-processing extensions allowed)

CXwudi commented 4 years ago

Looks like example uses Lombok: this is fine in itself, but I think we'd need a post-processed version for testing to see actual methods and annotations in used. Plus tests are not allowed to include Lombok as dependency for practical reasons (no byte- or source-processing extensions allowed)

I just tried without Lombok and manually coded all getters and constructors by myself. The issue still happens. I can provide a sample source code here
Similarly, I have a class that contains

@JsonProperty
ImmutableMap<String, ImmutableList<AbstractItem>> keyToItemsList;

then the AbstractItem is like:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "_type")
@JsonSubTypes({
    @JsonSubTypes.Type(value = ConcreteItem.class, name = "type 1")
})
public abstract class AbstractItem{

  @JsonProperty
  private String name;

  public AbstractItem(String name) {
    this.name= name;
  }

  protected AbstractItem() {
    //for jackson
  }

  public String getName() {
    return this.name;
  }
}

and the ConcreteItem is simply just a POJO:

public class ConcreteItem extends AbstractItem{

  @JsonProperty
  private ImmutableMap<String, String> options;

  @JsonProperty
  private String path;

  public ConcreteItem (String name, ImmutableMap<String, String> options, String path) {
    super(name);
    this.options = options;
    this.path = path;
  }

  protected ConcreteItem () {
    super();
    //for jackson
  }
}

Also, I figured that this issue might happen to all immutable collections. So if I use ImmutableSet instead of ImmutableList inside the ImmutableMap (as an example, ImmutableMap<String, ImmutableSet<AbstractItem>> keyToItemsList;), the issue also happens. However, if I use any mutable collections like MutableList or MutableSet, the issue is gone.
I guess this issue might relate to the fact that mutable collections implement java.util collections, (like MutableList implements java.util.List) while immutable collections are not.

yawkat commented 4 years ago

The issue here is ImmutableList/Map. It works fine with MutableList, because that implements java.util.List.

This came up in the initial PR: https://github.com/FasterXML/jackson-datatypes-collections/pull/30

Somewhat inconsistent wrt to annotations on mutable collections (which implement Collection) and immutable ones (which don't) but I suppose supporting this for some collections is better than supporting it for nothing.

The issue is that with ImmutableList, we get a call to findBeanDeserializer instead of findCollectionDeserializer or findCollectionLikeDeserializer. In the original PR, this is the core issue that wasn't solved. It's been a while but IIRC the choice of which method is called is done in jackson-databind, so I'm not sure there's a great solution here.

cowtowncoder commented 4 years ago

If the problem is that of how to detect ImmutableList/-Map as CollectionLikeType/MapLikeType, I can help -- what is needed is TypeModifier to register via Module to "refine" type. I could help with that.

But as there are no default deserializers for collection-/map-like types (they are just convenience types that allow better introspection of various things to help simplify (de)serializer implementations), there is probably need for serializer/deserializer implementations too? I see ImmutableListDeserializer.java and ImmutableSetDeserializer.java existing however... maybe they are just missing handling of polymorphic types?

I am probably forgetting earlier discussions with @yawkat so apologies for lacking necessary context here.

yawkat commented 4 years ago

@cowtowncoder A type modifier is enough to fix this apparently! Is that the "proper" solution? The EclipseCollectionsDeserializers already returns the right deserializers if they're passed as collection-like.

Also, what branch is the one to PR to, 2.12?

cowtowncoder commented 4 years ago

Yes, TypeModifier was designed to allow modules to indicate 3rd party types to behave more like "standard" Map, Collection and Reference (Option[al], AtomicReference) types, and help use some of existing support. It does not have to be used (SimpleType can be used) but often simplifies implementations -- for Collection-/Map-like it helps handle annotations and polymorphic type handling.

As to branch you can choose 2.10, 2.11 or 2.12 -- I can merge forward from any of those. I would probably suggest 2.11.

yawkat commented 4 years ago

72