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

Using Jackson support for Kotlin fails in native mode. #3954

Closed
rdifrango opened this issue Sep 10, 2019 · 68 comments · Fixed by #6325
Closed

Using Jackson support for Kotlin fails in native mode. #3954

rdifrango opened this issue Sep 10, 2019 · 68 comments · Fixed by #6325

Comments

@rdifrango
Copy link

rdifrango commented Sep 10, 2019

Describe the bug
When using the Jackson Kotlin module with a Kotlin data class, it fails in native mode with the following:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `com.capitalone.polyglot.quarkus.model.Address` (no Creators, like default construct, exist): cannot deserialize from Object value (no delegate- or property-based Creator)

The example code is pretty straight forward:

@ApplicationScoped
class CustomObjectMapperConfig {
    @Singleton
    @Produces
    fun objectMapper(): ObjectMapper {
        return ObjectMapper().registerModule(KotlinModule())
    }
}

data class Address(val addressLine1 : String, val addressLine2 : String, val addressLine3 : String, val addressLine4 : String, val city : String, val state : String, val country : String)

@Path("/deposits/tooling/validate-address")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
class AddressValidationResource {

    @POST
    fun validateAddress(address : Address) = address.copy(
                addressLine1 = "${address.addressLine1} ${address.addressLine2} ${address.addressLine3} ${address.addressLine4}",
                addressLine2 = "",
                addressLine3 = "",
                addressLine4 = "")
}

Expected behavior
(Describe the expected behavior clearly and concisely.)
If I run this in JVM mode, aka mvn compile quarkus:dev, or as a runnable jar it works just fine.

Actual behavior
(Describe the actual behavior clearly and concisely.)

To Reproduce
Steps to reproduce the behavior:
1.
2.
3.

Configuration

# Add your application.properties here, if applicable.

Screenshots
(If applicable, add screenshots to help explain your problem.)

Environment (please complete the following information):

  • Output of uname -a or ver:
  • Output of java -version:
openjdk version "1.8.0_212"
OpenJDK Runtime Environment (build 1.8.0_212-20190523183630.graal2.jdk8u-src-tar-gz-b03)
OpenJDK 64-Bit GraalVM CE 19.1.0 (build 25.212-b03-jvmci-20-b04, mixed mode)
  • GraalVM version (if different from Java):
  • Quarkus version or git rev:
    <quarkus.version>0.21.2</quarkus.version>

Additional context
(Add any other context about the problem here.)
Related #3652

@rdifrango rdifrango added the kind/bug Something isn't working label Sep 10, 2019
@rdifrango
Copy link
Author

For a short-term work-around the following working:

@RegisterForReflection
data class Address(var addressLine1 : String = "", 
                   var addressLine2 : String = "", 
                   var addressLine3 : String = "", 
                   var addressLine4 : String = "", 
                   var city : String = "", 
                   var state : String = "", 
                   var country : String = "")

@gsmet
Copy link
Member

gsmet commented Sep 11, 2019

I think this is a legit issue. As a parameter of a REST method, the Address type should be registered for reflection automatically.

There's something wrong going on.

@gsmet
Copy link
Member

gsmet commented Sep 11, 2019

@geoand any chance you could have a look to this one?

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

@gsmet sure thing

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

This doesn't appear to be a Quarkus issue. The Address class does get registered for reflection correctly when I tested.
In order to confirm this, @rdifrango can you please use your workaround without the @RegisterForReflection annotation? If I am correct, it should still work.
In that case you'll need to go read up on what the proper way to use Kotlin data classes and Jackson.

@rdifrango
Copy link
Author

rdifrango commented Sep 11, 2019

@geoand you're correct, it still works if I remove @RegisterForReflection.

I also tested by swap back to val from var but with default values and that's when it fails again, so working:

data class Address(var addressLine1 : String = "", 
                   var addressLine2 : String = "", 
                   var addressLine3 : String = "", 
                   var addressLine4 : String = "", 
                   var city : String = "", 
                   var state : String = "", 
                   var country : String = "")

NOT WORKING

data class Address(val addressLine1 : String = "", 
                   val addressLine2 : String = "", 
                   val addressLine3 : String = "", 
                   val addressLine4 : String = "", 
                   val city : String = "", 
                   val state : String = "", 
                   val country : String = "")

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

Thanks for confirming and looking into it.
It certainly looks like it's an issue with how Kotlin and Jackson interact.

So I will go ahead and close this since it doesn't appear to be a Quarkus issue. If you feel that is not correct, please reopen and provide an update.

Thanks

@geoand geoand closed this as completed Sep 11, 2019
@geoand geoand added the triage/invalid This doesn't seem right label Sep 11, 2019
@rdifrango
Copy link
Author

@geoand I still think this is an issue as the NOT WORKING case, works just fine in JVM mode, it only fails in NATIVE mode.

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

@rdifrango a small reproducer then where I can see the exact error you are getting?

@geoand geoand reopened this Sep 11, 2019
@geoand geoand removed the triage/invalid This doesn't seem right label Sep 11, 2019
@rdifrango
Copy link
Author

The code above is exactly what I used, nothing more nothing less :)

To build it we issue:

mvn package -Pnative -Dnative-image.docker-build=true

To run it:

docker run --rm -p 9090:8080 address-validation-quarkus-kotlin:1.0

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

If you could upload it to github or something, so I don't make any mistakes while trying to reproduce that would be great :)

@rdifrango
Copy link
Author

@geoand , I'll see what I can do, sadly uploading to GH is non-trivial at the office :)

Here's the maven POM file:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <groupId>com.example.polyglot</groupId>
    <artifactId>address-validation-quarkus-kotlin</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <kotlin.version>1.3.21</kotlin.version>
        <maven.compiler.source>1.8</maven.compiler.source>
        <maven.compiler.target>1.8</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
        <quarkus.version>0.21.2</quarkus.version>
        <sortpom.plugin.version>2.8.0</sortpom.plugin.version>
        <surefire-plugin.version>2.22.0</surefire-plugin.version>
    </properties>

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-bom</artifactId>
                <version>${quarkus.version}</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

    <dependencies>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-resteasy</artifactId>
        </dependency>
        <dependency>
            <groupId>org.jetbrains.kotlin</groupId>
            <artifactId>kotlin-stdlib-jdk8</artifactId>
        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-resteasy-jackson</artifactId>
        </dependency>
        <dependency>
            <groupId>com.fasterxml.jackson.module</groupId>
            <artifactId>jackson-module-kotlin</artifactId>
        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-kotlin</artifactId>
        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-junit5</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>io.rest-assured</groupId>
            <artifactId>rest-assured</artifactId>
            <scope>test</scope>
        </dependency>
    </dependencies>

    <build>
        <sourceDirectory>src/main/kotlin</sourceDirectory>
        <testSourceDirectory>src/test/kotlin</testSourceDirectory>
        <plugins>
            <plugin>
                <groupId>com.github.ekryd.sortpom</groupId>
                <artifactId>sortpom-maven-plugin</artifactId>
                <version>${sortpom.plugin.version}</version>
                <executions>
                    <execution>
                        <phase>verify</phase>
                        <goals>
                            <goal>sort</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <predefinedSortOrder>default_1_0_0</predefinedSortOrder>
                    <lineSeparator>\n</lineSeparator>
                    <encoding>${project.build.sourceEncoding}</encoding>
                    <sortProperties>true</sortProperties>
                    <keepBlankLines>true</keepBlankLines>
                    <createBackupFile>false</createBackupFile>
                    <sortDependencies>scope</sortDependencies>
                    <nrOfIndentSpace>4</nrOfIndentSpace>
                </configuration>
            </plugin>
            <plugin>
                <groupId>io.quarkus</groupId>
                <artifactId>quarkus-maven-plugin</artifactId>
                <version>${quarkus.version}</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>build</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
            <plugin>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>${surefire-plugin.version}</version>
                <configuration>
                    <systemProperties>
                        <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                    </systemProperties>
                </configuration>
            </plugin>
            <plugin>
                <groupId>org.jetbrains.kotlin</groupId>
                <artifactId>kotlin-maven-plugin</artifactId>
                <version>${kotlin.version}</version>
                <executions>
                    <execution>
                        <id>compile</id>
                        <goals>
                            <goal>compile</goal>
                        </goals>
                    </execution>
                    <execution>
                        <id>test-compile</id>
                        <goals>
                            <goal>test-compile</goal>
                        </goals>
                    </execution>
                </executions>
                <dependencies>
                    <dependency>
                        <groupId>org.jetbrains.kotlin</groupId>
                        <artifactId>kotlin-maven-allopen</artifactId>
                        <version>${kotlin.version}</version>
                    </dependency>
                </dependencies>
                <configuration>
                    <compilerPlugins>
                        <plugin>all-open</plugin>
                    </compilerPlugins>
                    <pluginOptions>
                        <option>all-open:annotation=javax.ws.rs.Path</option>
                        <option>all-open:annotation=javax.enterprise.context.ApplicationScoped</option>
                    </pluginOptions>
                </configuration>
            </plugin>
        </plugins>
    </build>
    <profiles>
        <profile>
            <id>native</id>
            <activation>
                <property>
                    <name>native</name>
                </property>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.quarkus</groupId>
                        <artifactId>quarkus-maven-plugin</artifactId>
                        <version>${quarkus.version}</version>
                        <executions>
                            <execution>
                                <goals>
                                    <goal>native-image</goal>
                                </goals>
                                <configuration>
                                    <enableHttpUrlHandler>true</enableHttpUrlHandler>
                                </configuration>
                            </execution>
                        </executions>
                    </plugin>
                    <plugin>
                        <artifactId>maven-failsafe-plugin</artifactId>
                        <version>${surefire-plugin.version}</version>
                        <executions>
                            <execution>
                                <goals>
                                    <goal>integration-test</goal>
                                    <goal>verify</goal>
                                </goals>
                                <configuration>
                                    <systemProperties>
                                        <native.image.path>${project.build.directory}/${project.build.finalName}-runner</native.image.path>
                                    </systemProperties>
                                </configuration>
                            </execution>
                        </executions>
                    </plugin>
                </plugins>
            </build>
        </profile>
    </profiles>
</project>

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

Thanks @rdifrango, I'll try and reproduce it. One last thing, do you have a curl command for POSTing to the endpoint?

@rdifrango
Copy link
Author

curl -X POST -H "content-type:application/json" -H "accept:application/json" http://localhost:9090/deposits/tooling/validate-address -d '{"addressLine1":"asdfasdf line 45", "addressLine2":"line2","addressLine3":"line3","addressLine4":"line4","city":"phl","state":"pa","country":"usa"}'

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

Thanks, I'll try it and let you know

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

I was able to reproduce the problem.
It doesn't look like a regular Quarkus problem (since the class is registered for reflection), but more some kind of weird interop issue.
I am guessing it will take a lot more digging than I can do at the moment, so let's leave it open and I'll take a look when I have som extra time.

@rdifrango
Copy link
Author

thank you @geoand!

@geoand
Copy link
Contributor

geoand commented Sep 11, 2019

Actually what we are seeing seems consistent with the Kotlin module not being applied in native mode.

So if anyone wants to explore more, that would be great. I will of course eventually get to this but frankly since we have workable workarounds and the root cause it probably hidden deep, I wouldn't consider it very high priority :)

@stale
Copy link

stale bot commented Nov 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you!
We are doing this automatically to ensure out-of-date issues does not stay around indefinitely.
If you believe this issue is still relevant please put a comment on it on why and if it truly needs to stay request or add 'pinned' label.

@stale stale bot added the stale label Nov 13, 2019
@maxandersen maxandersen removed the stale label Nov 13, 2019
@zerocode
Copy link

Would be nice to have this fixed.

@Mason-Harding
Copy link

I'm sure we are getting off topic, and a Quarkus dev will yell at us soon, but defaults won't work as they only work when no value is specified, and via that constructor. Jackson will set them to null.

@geoand
Copy link
Contributor

geoand commented Jan 29, 2020

Nobody is going to yell at anyone, we are as super friendly community :)

The problem is that the behavior is different between JVM and native mode. With the no-arg plugin, in JVM the defaults are used which unfortunately is not the case in native mode.

@Mason-Harding
Copy link

I am running into this same issue, except I get the error
@NotNull method kotlin/reflect/jvm/internal/impl/builtins/KotlinBuiltIns.getBuiltInClassByFqName must not return null
I tried adding kotlin.Metadata to the reflection-config file, and now am also getting
java.lang.AssertionError: Built-in class kotlin.Any is not found

I tried debugging by doing Class.forName(Any::class.java.name), which I was able to do when I added java.lang.Object to the reflection-config file, but I still get Built-in class kotlin.Any is not found when jackson tries to read a value.

@geoand
Copy link
Contributor

geoand commented Feb 6, 2020

Yeah this seems to require a chat with the Kotlin folks to see how to proceed. Unfortunately I haven't had the time to do that :(

@tellisnz-shift
Copy link

hi @geoand - have you been able to get in touch with the kotlin/graalvm folks?

@geoand
Copy link
Contributor

geoand commented Mar 15, 2020

@tellisnz-shift unfortunately no. With all the stuff that needed to be done for 1.3, I didn't follow up after not being able to reach them the first time.
I'll try again this week I hope.

@Rupert-RR
Copy link

Just as an indicator (I haven't isolated this into a barebones example), in case it helps, I am running into this problem for a data class which does not have any default values (but all properties are val, and obviously I have registered the Kotlin Jackson module on the mapper). However I do have several enum parameters for which there are default enum values (@JsonEnumDefaultValue) and I have activated the READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE on the mapper.

@miron4dev
Copy link

miron4dev commented Apr 12, 2020

Any news?

I was able to get around this problem by explicitly adding Jackson annotations.
So, this code works in JVM mode but doesn't in native mode

@RegisterForReflection
data class CreateTaskRequest(val name: String, val description: String?)

but this code works in both JVM and native modes and keeps the class immutable.

@RegisterForReflection
data class CreateTaskRequest @JsonCreator constructor(
        @JsonProperty("name")
        val name: String,

        @JsonProperty("description")
        val description: String?
)

However, it pollutes my class and I expect to write code without any Jackson annotations as in the first example.

Kotlin: 1.3.71

@geoand
Copy link
Contributor

geoand commented Apr 12, 2020

@miron4dev is that class part of a jax-rs method (I assume it's the payload of a POST request)?

@miron4dev
Copy link

@geoand yes, it's the payload of a POST request.

@geoand
Copy link
Contributor

geoand commented Apr 20, 2020

I want to give something a try soon. I'll come back when I have something to report.

@geoand
Copy link
Contributor

geoand commented Apr 20, 2020

I went through various scenaria to try and reproduce the problems with Kotlin Data classes and Jackson deserialization in native but I failed to uncover any...

Here is where I recorded what I did: https://github.com/geoand/kotlin-jackson-demo
If folks have concrete reproducible examples of issues I would love to know about them.

@codemwnci
Copy link
Contributor

I need to do a little more testing.
I wondered if it was because you had extracted the data classes out into their own files, rather than inside of the GreetingResource.kt file itself, so I tested with my previous 1.1.1.Final code. This still failed.

I need to upgrade Graal to test with 1.3.2, so this will take me a little longer. Will report back when I can.

@geoand
Copy link
Contributor

geoand commented Apr 21, 2020

@codemwnci Thanks. Feel free to ping me when you have results :)

@codemwnci
Copy link
Contributor

I have tried a number of different configurations (slow testing cycle due to the 15minute native compile!). Upgrading to Quarkus 1.3.2, and GraalVM 20.x, did not initially make any difference. I then changed the POM to reflect your POM and it worked.
I then individually started removing the POM configurations back to my original and the key (single) element that has made this work is in the Kotlin maven plugin configuration.

<javaParameters>true</javaParameters>

https://github.com/geoand/kotlin-jackson-demo/blob/cabe4f484a20c1b64332a4565060b49a3f1699c0/pom.xml#L120

So, as long as the latest POM creation through the maven archetypes generate the above setting (which appears to be the case), then this bug can be closed.

For anyone who is not creating a new project, they simply need to add the parameter to the "configuration" of the kotlin-maven-plugin, as linked to above.

@geoand
Copy link
Contributor

geoand commented Apr 22, 2020

Thanks a lot for the thorough check @codemwnci!

Indeed the project generation now includes <javaParameters>true</javaParameters> : https://github.com/quarkusio/quarkus/blob/1.4.0.CR1/devtools/platform-descriptor-json/src/main/resources/templates/basic-rest/kotlin/pom.xml-template.ftl#L109 (seems like I added it in #6310)

I'll close this issue as per both our findings

@geoand geoand closed this as completed Apr 22, 2020
@soufianesakhi
Copy link

If anyone is having troubles with Kotlin in native mode, check the configuration resources linked here: oracle/graal#2306

@oztimpower
Copy link
Contributor

oztimpower commented Feb 22, 2021

I have some experiences to add on this one. I use an IntelliJ plug-in to convert JSON to Kotlin data classes for me (large structure!)

Everything worked fine in JVM mode, Jackson-Kotlin-module clearly enabled for the mapper.readValue(jsonBody). However, a straight native-image version failed, with similar errors above. I experimented a lot with the issue, looked at the optional fields, default values and so on. The JSON structure was quite complex, with multiple levels and even an array.

What was interested was when I moved to using mapper.readTree for the json and picked out the specific parts I wanted in the hierarchy and then did a mapper.readValue(myNode), it worked fine. The Jackson-Kotlin-module in native-image clearly prefers a simpler denormalised-like structure, than many levels (why?). I still had a few levels in the JSON, i.e. moving from about 5 nodes down to 2. When I started using mapper.readTree I also started getting different errors, the Jackson parser was giving me more specific object it was struggling to convert, rather than the blanket error (as above) that it can't construct the root data class). Thus the error you get from the Jackson conversion can be a little misleading and in a hierarchy of objects it could be a leaf node that it is struggling to convert, yet reports an issue at the root node (exception probably bubbles up in the code).

So, my recommendation for anyone having these issues, is to use mapper.readTree when using native-image, pull out the bits you want from the JSON-nodes and use mapper.readValue on that, rather than relying on a large tree to object conversion. More like using a SAX parser for XML than DOM, was the logic.

I'm looking forward to using Java-records in the future in this scenario, and I believe Kotlin will map the internals of data-classes to records so it will look the same to the JVM(?). No not keen on using Lombok here as a replacement for my Kotlin data classes.

JSON source was from Yahoo Finance: https://query1.finance.yahoo.com/v8/finance/chart/AMZN
(what I could not convert with native-image using the entire tree, yet works when you pick more specific nodes of interest)

@oztimpower
Copy link
Contributor

oztimpower commented Feb 22, 2021

An addendum, I've been able to get it working with the entire object tree for native-image (fiddling with JSON to Kotlin data-class IntelliJ plugin options), generating val with nullable type and default of null, along with the Jackson field name, .i.e.:

data class Response(
	@field:JsonProperty("chart")
	val chart: ChartData? = null
)

@lbilger
Copy link

lbilger commented Mar 22, 2021

@geoand, you asked for reproducible examples of issues. I ran into this issue last week and had a hard time figuring out why your example worked and my code didn't. In the end, I found out that the issue seemed to be data classes that have non-primitive members, e.g. a data class containing a List or another data class. I'm not sure if I should open a new issue as this one has been closed for a while.
I have extended your example project and will shortly post two pull requests - one for the additional examples that break for me, and one to update to the latest Quarkus version. I hope this helps to track down the issue.

Thanks!

@geoand
Copy link
Contributor

geoand commented Mar 22, 2021

Thanks @lbilger. I'll try to take a look later on this week,

@oztimpower
Copy link
Contributor

@geoand This is the same issue I described above (complex structures) and why I added troubleshooting notes to the Kotlin documentation. Using the field annotation does overcome it, but may be considered a workaround.

@geoand
Copy link
Contributor

geoand commented Mar 22, 2021

Ah, thanks @oztimpower, I had forgot!

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

Successfully merging a pull request may close this issue.