Rothamsted / knetbuilder

KnetBuilder data integration platform for building knowledge graphs. Previously known as ondex.
https://knetminer.com
MIT License
12 stars 11 forks source link

Migration to Java 17 #64

Open marco-brandizi opened 2 years ago

marco-brandizi commented 2 years ago

We've almost completed this, adding this issue keep track of the introduced changes, and keeping the issue opened until the migration is finalised.

Essentially, we can't merge this into master right now, since Neo4j isn't compatible with Java 17. For now, I've disabled a number of Neo4j-related Ondex components, just to work with the rest and ensure it can be built and run under J17.

Notes on this on comments below.

marco-brandizi commented 2 years ago

I should have fixed the JAXB problems with the damn Java modules. See my branch here. That one now builds and the binaries it produces seem to be running right.

How I did it, TLDR

I've replaced the Glassfish-based implementation of the XML binding libraries (used by JAXB) with a new implementation provided by the Jakarta project. The old one was a dead project and it was producing the issues with the Java modules.

How I did it, details

OXL files define node attribute types (called attribute names in Ondex) in a dynamic way, ie, by setting properties like name and data type. The latter points to a Java class (java.lang.String, Java.lang.Integer, etc) and it's then passed to the JAXB libraries, to map XML to/from Java objects.

Now, a few of text .oxl files have attributes mapped to java.awt.geom.Point2D as data type (no idea why, probably they used to save coordinates from the desktop GUI). When the old JAXB-related lib named above tried to map from XML to Point2D objects, the problem with modules arose.

Essentially, that was happening because such library is buggy and doesn't define its own module correctly (as far as I could understand). Because it's also unmaintained, I did the upgrades above. Actually, I wanted to do them a when I migrated from 8 to 11, but at the time I didn't cause the Jakarta alternative has changed the javax.xml.bind namespace (they use jakarta.* now, due to a dispute with Oracle). Now I've changed all the import statements in the .java files.

In detail. the done replacement can be seen in the root POM:

<!-- Since JDK 11, we need this as a separate lib -->
<dependency>
    <groupId>jakarta.xml.bind</groupId>
    <artifactId>jakarta.xml.bind-api</artifactId>
    <version>4.0.0</version>
</dependency>
<!--This is the implementation of jakarta.xml.bind-api -->
<dependency>
    <groupId>com.sun.xml.bind</groupId>
    <artifactId>jaxb-impl</artifactId>
    <version>4.0.0</version>
</dependency>                       
<!-- TODO:remove we switched to Jakarta in JDK 17
<dependency>
    <groupId>javax.xml.bind</groupId>
    <artifactId>jaxb-api</artifactId>
    <version>2.3.1</version>
</dependency>
<dependency>
    <groupId>org.glassfish.jaxb</groupId>
    <artifactId>jaxb-runtime</artifactId>
    <version>2.3.2</version>
</dependency> -->

Other notes

marco-brandizi commented 2 years ago

@jojicunnunni, please verify that it builds and runs on your PC. After the build, run the GUI (available at ondex-knet-builder/installer/target/installer-6.0-SNAPSHOT-packaged-distro.zip) and the workflow tool (at ondex-knet-builder/ondex-mini/target/ondex-mini-6.0-SNAPSHOT-packaged-distro.zip.

marco-brandizi commented 2 years ago

More updates: since the Neo4j upgrade to J17 isn't expected before Oct 2022, it's sensible to do this sooner than later:

The above changes are compatible with J11 (except for minor details like the compiler level in the POM), embedding them now into master will avoid that we diverge too much from the J17 branch by the time we'll be ready for completing this migration.