Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Support and document user provided hints #412

Closed
jamesward opened this issue Dec 15, 2020 · 7 comments
Closed

Support and document user provided hints #412

jamesward opened this issue Dec 15, 2020 · 7 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@jamesward
Copy link

I haven't been able to get JSON deserialization in WebClient working with Kotlin without manual decoding.

Jackson:
https://github.com/jamesward/springboot-kotlin-reactive-demo/tree/graalvm-kofu-jackson

Exception in thread "reactor-http-nio-6" org.springframework.core.codec.DecodingException: JSON decoding error: Cannot construct instance of `demo.Release` (although at least one Creator exists): cannot deserialize from Object value (no delegate- or property-based Creator); nested exception is com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `demo.Release` (although at least one Creator exists): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (io.netty.buffer.ByteBufInputStream); line: 1, column: 3] (through reference chain: java.util.ArrayList[0])
	at org.springframework.http.codec.json.AbstractJackson2Decoder.processException(AbstractJackson2Decoder.java:228)
Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `demo.Release` (although at least one Creator exists): cannot deserialize from Object value (no delegate- or property-based Creator)
 at [Source: (io.netty.buffer.ByteBufInputStream); line: 1, column: 3] (through reference chain: java.util.ArrayList[0])
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1455)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1081)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1332)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:331)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:164)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:290)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:249)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:26)
	at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:2079)
	at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1453)
	at org.springframework.http.codec.json.AbstractJackson2Decoder.decode(AbstractJackson2Decoder.java:181)
	at org.springframework.http.codec.json.AbstractJackson2Decoder.lambda$decodeToMono$1(AbstractJackson2Decoder.java:172)
        ...

KTX Serialization:
https://github.com/jamesward/springboot-kotlin-reactive-demo/tree/graalvm-kofu-ktx-serialization

Exception in thread "reactor-http-nio-4" org.springframework.web.reactive.function.client.WebClientResponseException: 200 OK from GET https://api.github.com/repos/jetbrains/kotlin/tags; nested exception is org.springframework.web.reactive.function.UnsupportedMediaTypeException: Content type 'application/json;charset=utf-8' not supported for bodyType=java.util.List<demo.Release>
	at org.springframework.web.reactive.function.client.DefaultClientResponse.lambda$createException$1(DefaultClientResponse.java:216)
Caused by: org.springframework.web.reactive.function.UnsupportedMediaTypeException: Content type 'application/json;charset=utf-8' not supported for bodyType=java.util.List<demo.Release>
	at org.springframework.web.reactive.function.BodyExtractors.lambda$readWithMessageReaders$12(BodyExtractors.java:201)
@sdeleuze sdeleuze added this to the 0.9.0 milestone Dec 18, 2020
@sdeleuze
Copy link
Contributor

We should at least analyse it in 0.9.0, see potentially related spring-projects/spring-framework#21546 issue.

@sdeleuze sdeleuze self-assigned this Dec 18, 2020
@sdeleuze sdeleuze added the type: compatibility Native image compatibility issue label Jan 8, 2021
@sdeleuze sdeleuze changed the title JSON WebClient Kotlin Woes JSON WebClient Woes Jan 28, 2021
@sdeleuze
Copy link
Contributor

This issue is really interesting and is not specific to Kotlin as demonstrated in this modified webclient sample. What happens is that Jackson is using reflection to instantiate the Pojo, and we are in a use case where the related Pojo is not returned in a Spring controller endpoint or similar use case that could give us a chance to configure automatically the related reflection entries.

So what solution to we have:

  • Document this and ask users to craft a META-INF/native-image/reflect-config.json as I did in my sample: it could help short term but that also seems not a satisfying solution middle/long term
  • Using the brand new kotlinx.serialization support which is using a Kotlin compiler plugin instead of Jackson + reflection: great for Kotlin but does not help for Java
  • Allow users to provide hints in their project: that would provide a more typesafe way to define the configuration
  • Add some dedicated annotation on the Pojo to serializer/deserialize: I am not keen on native specific intrusive annotations + this need will probably be covered on GraalVM side at some point
  • Use another JSON library: I don't want to be in the business of maintaining that, and there is no standard way to do that on Java side + the solution would be JSON specific.

@aclement Any thoughts on allowing users to provide hints in their projects? Maybe via build-time transformation we generate the service loader entry in order to have and make implements NativeImageConfiguration optional to provide a more Spring-ish experience? It could also make it easier for us and contributors to test their hints before contributing them.

@aclement
Copy link
Contributor

Hints are actually already discoverable in two ways. They either are declared 'over there' separate to configuration on something implementing NativeImageConfiguration and have a trigger (and a service loader entry), or they can be specified directly on configuration and that configuration automatically acts as the trigger - we just never use it like that ourselves. If they are directly declared on configuration they don't need a service loader entry. We don't do this because the configurations are in spring framework/boot, but if they were in 'our code' we could do it, so:

@NativeImageHint(...)
@Configuration
class FooConfig {

should already work. But yes, we could also generate service loader entries.

@sdeleuze sdeleuze changed the title JSON WebClient Woes Support and document user provided hints Feb 1, 2021
@sdeleuze
Copy link
Contributor

sdeleuze commented Feb 1, 2021

Ok so I think this issue should:

  • Check user provided hints still work with the new build-time infra
  • Refine related documentation
  • Update the webclient hint to desizerialize a Pojo + usage of such user-provided hints
  • Review NativeImageHint API (could be maybe renamed to NativeHint, NativeImageConfiguration to NativeConfiguration, trigger to target)
  • Potentially generate the service loader entries

@sdeleuze sdeleuze added type: enhancement A general enhancement and removed type: compatibility Native image compatibility issue labels Feb 1, 2021
@sdeleuze
Copy link
Contributor

sdeleuze commented Feb 3, 2021

@aclement I confirm it works as you described, so I am ok to use it for now and document it. I will take this opportunity to document those annotations. Are you ok with the renaming proposed in my previous comment?

@aclement
Copy link
Contributor

aclement commented Feb 3, 2021

Happy to take Image out of those names, I prefer the shorter ones anyway. trigger to target meh, if you like, to me it is a trigger that activates the hint. But I know you can argue it is a target for where the hint should really live. Generating service loader entries can follow once the bti is the default.

@sdeleuze
Copy link
Contributor

sdeleuze commented Feb 3, 2021

I am fine to keep trigger for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement A general enhancement
Development

No branches or pull requests

3 participants