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

Blackbird fails to deserialize varargs array #141

Closed
naveg opened this issue Jun 24, 2021 · 12 comments
Closed

Blackbird fails to deserialize varargs array #141

naveg opened this issue Jun 24, 2021 · 12 comments
Labels
blackbird Issue related to Blackbird module

Comments

@naveg
Copy link

naveg commented Jun 24, 2021

The following code snippet fails. However, the same snippet works if either Blackbird is removed, or the constructor argument is changed to use double[] notation instead of double...

public class Scratch {
  public static void main(String[] args) throws JsonProcessingException {
    ObjectMapper mapper = new ObjectMapper();
    mapper.registerModule(new BlackbirdModule());

    Foo foo = new Foo(new double[] { 1d, 2d, 3d });
    String serialized = mapper.writeValueAsString(foo);
    Foo foo2 = mapper.readValue(serialized, Foo.class);
  }

  static class Foo {

    @JsonProperty("bar")
    double[] bar;

    @JsonCreator
    public Foo(@JsonProperty("bar") double... bar) {
      this.bar = bar;
    }
  }
}
Exception in thread "main" com.fasterxml.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `com.alloymetrics.restapi.Scratch$Foo`, problem: class [D cannot be cast to class java.lang.Number ([D and java.lang.Number are in module java.base of loader 'bootstrap')
 at [Source: (String)"{"bar":[1.0,2.0,3.0]}"; line: 1, column: 21]
	at com.fasterxml.jackson.databind.exc.ValueInstantiationException.from(ValueInstantiationException.java:47)
	at com.fasterxml.jackson.databind.DeserializationContext.instantiationException(DeserializationContext.java:1907)
	at com.fasterxml.jackson.databind.DeserializationContext.handleInstantiationProblem(DeserializationContext.java:1260)
	at com.fasterxml.jackson.module.blackbird.deser.OptimizedValueInstantiator.createFromObjectWith(OptimizedValueInstantiator.java:51)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:288)
	at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:454)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
	at com.fasterxml.jackson.module.blackbird.deser.SuperSonicBeanDeserializer.deserializeFromObject(SuperSonicBeanDeserializer.java:247)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
	at com.fasterxml.jackson.module.blackbird.deser.SuperSonicBeanDeserializer.deserialize(SuperSonicBeanDeserializer.java:116)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4593)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3548)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3516)
@cowtowncoder cowtowncoder added 2.12 blackbird Issue related to Blackbird module labels Jun 25, 2021
@cowtowncoder
Copy link
Member

Ok sounds like a bug. I assume it'd likely fail with Afterburner as well.

@cowtowncoder
Copy link
Member

Does not look like this fails with Afterburner, for whatever reason.

@cowtowncoder
Copy link
Member

@stevenschlansker You might have a better idea of where type check or casting goes wrong here: I added test DoubleArrayDeser141Test in com.fasterxml.jackson.module.blackbird.failing package (branch 2.13 -- although if fix is simple, can backport in 2.12).

@cowtowncoder cowtowncoder added 2.13 and removed 2.12 labels Jun 26, 2021
@stevenschlansker
Copy link
Contributor

Strange, usually varargs and arrays behave the same, but there are a ton of edge cases.
I am on vacation this week but can look next week.

@cowtowncoder
Copy link
Member

Yeah I thought they should be (i.e. just syntactic sugar). But I guess there are specific edge cases.

@carterkozak
Copy link
Contributor

I've just encountered this as well. Would you be willing to backport a fix to 2.12.x if I can put something together? I imagine the blackbird module will become increasingly popular as jdk17 LTS is released in a couple months and folks replace afterburner.

carterkozak added a commit to carterkozak/jackson-modules-base that referenced this issue Jul 7, 2021
carterkozak added a commit to carterkozak/jackson-modules-base that referenced this issue Jul 7, 2021
@carterkozak
Copy link
Contributor

I've created a simple PR which avoids attempting to optimize varargs methods: #145

I'm not sure what dragons lurk in the deep if we attempt to optimize them, however I suspect it's not worthwhile to optimize varargs methods because they're generally expensive due to implicit array allocation, and tend not to be used in performance critical code.

@stevenschlansker
Copy link
Contributor

Thank you for the PR! I still want to take at least one attempt to optimize it, in case it turns out to be easy - but I will definitely borrow the test cases and this is a good fallback option if it turns out to be hard to optimize.

@cowtowncoder
Copy link
Member

On backporting: yes, I think this would make sense to backport in some form. Unfortunate timing-wise is that I just pushed 2.12.4 so the fix would likely only get released as a micro-patch (which I can do if there is need).
2.13.0-rc1 should be done soon so we may need to decide on whether to do short-term patch to block optimization as per PR, or if @stevenschlansker can find a way to make varargs work. I don't have strong opinion.

stevenschlansker added a commit to stevenschlansker/jackson-modules-base that referenced this issue Jul 7, 2021
stevenschlansker added a commit to stevenschlansker/jackson-modules-base that referenced this issue Jul 7, 2021
stevenschlansker added a commit to stevenschlansker/jackson-modules-base that referenced this issue Jul 7, 2021
@stevenschlansker
Copy link
Contributor

Please consider and test #146 which I believe solves this issue.
As usual, it takes hours of close reading of javadocs and web resources to figure out the one line change :)

cowtowncoder added a commit that referenced this issue Jul 8, 2021
@stevenschlansker
Copy link
Contributor

The merged version should patch cleanly against 2.12 if that's a priority.

cowtowncoder added a commit that referenced this issue Jul 8, 2021
@cowtowncoder
Copy link
Member

Backported just the fix -- not sure if or when there might be release but seems reasonable to make that possible at least.
Will now focus on 2.13.0 RC phase.

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

No branches or pull requests

4 participants