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

Support Neo4j-OGM in the Neo4j extension #8530

Closed
gastaldi opened this issue Apr 10, 2020 · 31 comments
Closed

Support Neo4j-OGM in the Neo4j extension #8530

gastaldi opened this issue Apr 10, 2020 · 31 comments
Labels
area/neo4j kind/enhancement New feature or request

Comments

@gastaldi
Copy link
Contributor

Description
The Neo4j Extension guide states that you need to use the Neo4j Cypher language to manipulate your Neo4j database. It would be nice if one could to simplify development with the Neo4j graph database and like JPA, using annotations on simple POJO domain objects.

That's the intention of the Neo4j-OGM project

Bonus points if a Panache extension is provided too.

Implementation ideas
Make the Neo4j extension depend on the neo4j-ogm libraries and allow injecting the corresponding SessionFactory/Session in CDI beans

@gastaldi gastaldi added kind/enhancement New feature or request area/neo4j labels Apr 10, 2020
@gastaldi
Copy link
Contributor Author

/cc @michael-simons

@michael-simons
Copy link
Contributor

While there actually people using OGM in CDI scenarios, it would definitely not work in native mode. Thinks that would be needed: A class index (probably Yandex), replacement for the reflection based entity instantiation and a couple of other things.

I’ll raise this in our team at least for discussion.

@fiorenzino
Copy link

Hi,
some days ago, i started to play with neo4j-ogm. I have problems with classloader in dev mode (mvn compile quarkus:dev - neo4j-ogm don't see the java classes with @NodeEntity annotation). In test and in java mode (running java -jar target/xxx.jar) quarkus and neo4j-ogm work correctly.
I would like to investigate if I can identify the classes to be highlighted for reflection to have quarkus in native mode.
My test project: https://github.com/fiorenzino/neonq

@gastaldi
Copy link
Contributor Author

gastaldi commented Apr 12, 2020 via email

@fiorenzino
Copy link

fiorenzino commented Apr 12, 2020

HI @gastaldi
In a specific extension, at deployment stage, i understand (using Jandex, we could create a SessionFactory using jandex index, to find @NodeEntity java classes annotated, and also replace the native neo4j-OGM way to create a SessionFactory using a package parameter to read all entities). But in the simple java code, with neo4j-ogm jar dependency, why the class loader is different?
Why the SessionFactory don't watch the classes in the project?
What the difference between:

  • mvn quarkus:dev (here this not work)
    and
  • java -jar target/file.jar (here quarkus and neo4j-ogm work fine)

@sdaschner
Copy link
Contributor

Huge +1 from me on this issue, having OGM would be nice.

Managed to get Quarkus working with OGM (manually, with only @Inject Driver driver being provided) and I run into the same problem as @fiorenzino, doesn't find my classes (Neo DomainInfo#create to be precise), it works in production JVM mode.

@hantsy
Copy link
Contributor

hantsy commented Apr 24, 2020

Why not use Hibernate OGM instead?

If you are familiar with Hibernate ORM and JPA, it is more friendly for developers.

@michael-simons
Copy link
Contributor

@hantsy We try to be as friendly as possible :)

Not finding things in dev mode has luckily nothing todo with Graal Native:

@sdaschner @fiorenzino Would you please raise this as an issue with @lukehutch from https://github.com/classgraph/classgraph/? It seems we're playing through all the class loaders out there.

You can basically copy my reproducer from OpenLiberty from that issue: classgraph/classgraph#414 The guts of it being:

import io.github.classgraph.ClassGraph;
import io.github.classgraph.ScanResult;

int numberOfClassesFound = 0;
        // The servlet itself should be found... But isnt.
        try (ScanResult r = new ClassGraph().verbose(true).enableRealtimeLogging().enableAllInfo().whitelistPackages("io.openliberty.guides.hello").scan()) {
            numberOfClassesFound = r.getAllClasses().size();
}

Just add a package with one empty class and see if it returns something

@sdaschner
Copy link
Contributor

@fiorenzino @michael-simons Updating Neo4J OGM to 3.2.11 solved the dev mode issue for me, you might want to try that. However, you have to add the Neo4J driver 4.0.1 dependency (Quarkus includes 4.0.0):

<!-- required for Neo4J 3.2.11, Quarkus imports 4.0.0 -->
<dependency>
    <groupId>org.neo4j.driver</groupId>
    <artifactId>neo4j-java-driver</artifactId>
    <version>4.0.1</version>
</dependency>

@hantsy Seems that there is currently no plan to support Hibernate OGM: #6514

@michael-simons
Copy link
Contributor

There's an open issue #6265 I can bump the version number there…

@gsmet
Copy link
Member

gsmet commented Apr 24, 2020

The version is already upgraded.

@lukehutch
Copy link

While there actually people using OGM in CDI scenarios, it would definitely not work in native mode. Thinks that would be needed: A class index (probably Yandex), replacement for the reflection based entity instantiation and a couple of other things.

Correct, the Quarkus classloader is supported by ClassGraph, however ClassGraph cannot work with Quarkus in native mode, because there is no traditional classfile to read. However, there is some advice on the ClassGraph wiki on how to implement build-time scanning. Presumably the Neo4J-OGM plugin could do something like this, perhaps even in an automated way, to index classes, if the compiler is building a native project. (Is there any way to force GraalVM's partial evaluation to automatically turn run-time scanning directly into build-time scanning, by running the runtime scanning logic at build-time, and caching the result in the compiled binary? Maybe this is wishing for too much.)

@sdaschner @fiorenzino Would you please raise this as an issue with @lukehutch from https://github.com/classgraph/classgraph/? It seems we're playing through all the class loaders out there.

Sorry, can you please describe the issue?

@fiorenzino
Copy link

OK: i can confirm, using latest version of neo4j libraries in quarkus 1.3.2.Final,
./mvnw quarkus:dev
works fine.

<quarkus.platform.version>1.3.2.Final</quarkus.platform.version> <neo4j-ogm-core.version>3.2.11</neo4j-ogm-core.version> <neo4j-ogm-bolt-driver.version>3.2.11</neo4j-ogm-bolt-driver.version> <neo4j-java-driver.version>4.0.1</neo4j-java-driver.version>
i will create a simple reproducer for quarkus problems in native mode.

@sdaschner
Copy link
Contributor

@fiorenzino @michael-simons There seems to be a new issue with OGM (in JVM) mode with fast-jar Quarkus packaging, which is the default since a few versions.

Issue

When I'm building a Quarkus app with Neo4J-OGM (3.2.25), in quarkus:dev everything works fine, but in live mode I get the same:

Unable to find database label for entity [...] : no results will be returned.
Make sure the class is registered, and not abstract without @NodeEntity annotation

error again, when I'm using a recent Quarkus 1.13.7.Final or 2.1.1.Final version.

Changing to quarkus.package.type=legacy-jar solves it for both Quarkus versions, however, I was wondering if there is a fix in the classloading also for that packaging. This might be connected to ClassGraph again...

(Sorry to revive this old issue, made more sense to me than opening a new one)

@lukehutch
Copy link

@sdaschner If you believe there is a ClassGraph issue, please capture the verbose logs by calling .verbose() before .scan(). Preferably please capture the logs on a working system and a broken system that are as near as possible in versions and configuration, so that the logs can be compared. A minimal reproducer project would be even better!

(Also, if you're running on JDK 16, there have been many issues already, for many libraries like ClassGraph, with the JDK team switching on strong encapsulation...)

@michael-simons
Copy link
Contributor

OGM just use Classgraph for discovering classes…
Probably it's another Classloader issues with Quarkus fast-jar mode.

I probably can't fix that.
You can probably workaround it by adding an index: https://github.com/michael-simons/neo4j-from-the-jvm-ecosystem/blob/master/quarkus-ogm/src/main/resources/META-INF/resources/org/neo4j/examples/jvm/quarkus/ogm/movies/neo4j-ogm.index

Btw related in general to this ticket is #18764
I haven't forgotten about that one, but this is in general a bigger effort.
I would be open for a call about that sometime.

@sdaschner
Copy link
Contributor

Thanks Michael, indeed, adding an index similar to yours solved this for the fast-jar mode.

However, in that case I'd rather go with the legacy-jar mode, especially for projects in which the entities are distributed in multiple packages...

Maybe someone else can comment on what might cause the issue with the Quarkus packaging modes?

@AndreasBoehme
Copy link

AndreasBoehme commented Sep 7, 2021

Just wanted to confirm @sdaschner's post - seems classloading is not working properly when using quarkus.package.type=fast-jar. (tested with various versions of Quarkus between 1.10.3.Final and 1.13.7.Final within a Docker container, Neo4j-OGM 3.2.24, OpenJDK1.8.0).
Didn't find a solution so far, except using quarkus.package.type=legacy-jar. (didn't try the index thing)
(I found this post via a google search, and it was the only one related, so I thought I could comment on this one)

@michael-simons
Copy link
Contributor

Hi @AndreasBoehme @sdaschner it would be nice if you can try out Neo4j-OGM 3.2.26, it uses latest Classgraph and should work on JVM mode with both legacy and fast jar. Thanks.

@sdaschner
Copy link
Contributor

@michael-simons Finally could give it a try :) Can confirm, works well, thanks!

@AndreasBoehme
Copy link

AndreasBoehme commented Nov 30, 2021

@michael-simons finally had some time to test it - I can, too, confirm it works with fast-jar! Thank you!

However, heads up others: key is the use of io.github.classgraph:classgraph:4.8.116, which is a transitive dependency of the mentioned org.neo4j:neo4j-ogm-core:3.2.26.

I'm mentioning this, because I fixed the classgraph version in my parent to an older version, thus raising the same errors.
Can't remember the details, but seems this was necessary in combination with eu.michael-simons.neo4j:neo4j-migrations:0.2.1 (v 1.1.0 is already released - will try to upgrade soon).

@michael-simons
Copy link
Contributor

michael-simons commented Nov 30, 2021

Thank you so much @AndreasBoehme for your feedback!
Neo4j Migrations is on 4.8.134. I will bump OGM, too!
OGM 3.2.28 has just been released, though, but: It has some nice performance improvements as well.

Anyway, yes, you need classgraph >= 4.8.116 for things to work.

Also, Classgraph is moving quite fast, I think I have to bump both.

Also (2): Neo4j-Migrations should be stable right now. I try to be strict with semver from 1.0.x onwards.

@lukehutch
Copy link

Yes, it would be worth it to bump the required ClassGraph version, if you're depending upon 4.8.116.

ClassGraph is converging on a bug rate of zero -- I have pushed out a lot of releases in the last few months, but almost all of them have been to fix tiny corner cases. I don't plan any other major features (other than that once the library totally stabilizes, and there have been no bug reports for many months, I'll push out a version 5.0 that will remove deprecated API and will bump the minimum supported JDK version from 7 to 8).

@michael-simons
Copy link
Contributor

Hey @lukehutch nice to see you. My remark was not meant as a critic. All updates the last months worked API wise (for us) as simple drop ins. 👍

@lukehutch
Copy link

@michael-simons I know -- no problem at all! Just wanted to provide some perspective on the stability of the project at this point. I don't think there will be many changes in the future other than small tweaks, as long as you're not using JDK 7 (do you have JDK 7 compatibility in your builds?), and as long as you're not using deprecated API.

@michael-simons
Copy link
Contributor

@AndreasBoehme Can you drop me line if you have time how you use https://github.com/michael-simons/neo4j-migrations in your Quarkus setup? I have been ruminating about adding a Quarkus plugin much like the Boot one in the project, but I do struggle to get it right for native so that users would not have to add much themselves apart from the necessary resources.

My email is in the pom.

@michael-simons
Copy link
Contributor

Hello everyone.

This is soon to be released: https://github.com/michael-simons/neo4j-ogm-quarkus

It depends on the official Quarkiverse Neo4j extension.
If you want / like / need to use OGM, use the one above!
It will configure a OGM SessionFactory in a way that it works with Quarkus in both native and JVM mode, without any manual index work.

@gastaldi
Copy link
Contributor Author

gastaldi commented Feb 23, 2022

@michael-simons that's awesome! Are there any plans to migrate this work to the Quarkiverse Neo4j repository or under another Quarkiverse repository?

PS: Moving to the Quarkiverse Neo4j repo makes more sense to me 😉

@michael-simons
Copy link
Contributor

Thank you.

We will push first a release out and check with a bunch of people if that’s enough for them to have a better experience.

(If you have time, check out the project and mvn quarkus:dev and have a look at the dev ui, I love that).

I do see this project more in the Quarkiverse than in a Neo4j repo. However, I would like to keep it separate from the low level Neo4j client (or driver as we call it).

There are a couple of interesting things in the project. Among them the integration test for the dev ui and the fact that one needs a native built item for generated resources. I was a bit surprised, thought that Quarkus would assume that.

Anyway, stellar work on your side, found everything I needed in the Quarkus infrastructure. 👍

@AndreasBoehme
Copy link

Hello everyone.

This is soon to be released: https://github.com/michael-simons/neo4j-ogm-quarkus

It depends on the official Quarkiverse Neo4j extension. If you want / like / need to use OGM, use the one above! It will configure a OGM SessionFactory in a way that it works with Quarkus in both native and JVM mode, without any manual index work.

I've tried v1.0.0 today - works great!
Dev-Console integration is nice, too!
Now I can remove some of my homebrew code for configuring the SessionFactory in my Quarkus App.

@gastaldi
Copy link
Contributor Author

Closing as this is already done in https://github.com/michael-simons/neo4j-ogm-quarkus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/neo4j kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants