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

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

Closed
CXwudi opened this issue Aug 22, 2020 · 7 comments
Labels
Milestone

Comments

@CXwudi
Copy link

CXwudi commented Aug 22, 2020

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
Copy link
Member

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
Copy link
Author

CXwudi commented Aug 23, 2020

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.

@CXwudi CXwudi changed the title [eclipse-collection] can not deserialize concrete class instance inside nested eclipse-collection [eclipse-collection] can not deserialize concrete class instance inside nested immutable eclipse-collection Aug 23, 2020
@yawkat
Copy link
Member

yawkat commented Aug 23, 2020

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

This came up in the initial PR: #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
Copy link
Member

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
Copy link
Member

yawkat commented Aug 24, 2020

@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
Copy link
Member

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 added a commit to yawkat/jackson-datatypes-collections that referenced this issue Aug 24, 2020
@yawkat
Copy link
Member

yawkat commented Aug 24, 2020

#72

@cowtowncoder cowtowncoder added this to the 2.11.3 milestone Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants