-
Notifications
You must be signed in to change notification settings - Fork 260
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
Update Neo4j extension to use Java 17 and Spring Data Neo4j #125
Conversation
spring-batch-neo4j/README.md
Outdated
|
||
## Minimal Spring Boot example | ||
|
||
With a Spring Boot application containing the additional dependencies `spring-boot-starter-neo4j` and `spring-batch-neo4j`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should go together with the following sentence.
x, y, z are needed and b must be excluded. See the following gradle for a minimal example,
Is there any ETA on when this will be merged ? |
I cannot neither make an assumption on it, nor merge it myself. |
If there is demand to get this in, I would like to get some feedback from @mdeinum or @dgray16 here (at least they seem pretty active in the past). If this is up to us alone, I am technical able to do the merge, but please let us know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not have a chance to work with Neo4j so I cannot add a proper review for the logic, but dropping you some general ideas.
Also it would be great to have integration tests with Testcotainers.
Anyway LGTM. Thanks for your efforts!
...ng-batch-neo4j/src/main/java/org/springframework/batch/extensions/neo4j/Neo4jItemWriter.java
Outdated
Show resolved
Hide resolved
...ng-batch-neo4j/src/main/java/org/springframework/batch/extensions/neo4j/Neo4jItemWriter.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/springframework/batch/extensions/neo4j/builder/Neo4jItemWriterBuilder.java
Outdated
Show resolved
Hide resolved
...tch-neo4j/src/test/java/org/springframework/batch/extensions/neo4j/Neo4jItemReaderTests.java
Outdated
Show resolved
Hide resolved
...tch-neo4j/src/test/java/org/springframework/batch/extensions/neo4j/Neo4jItemWriterTests.java
Outdated
Show resolved
Hide resolved
00982c4
to
925c8cc
Compare
Thanks for your input, @dgray16 |
This is a rather large PR because it will remove the Neo4j-OGM dependency and replace it with the Spring Data Neo4j.
Some background information to clarify the situation:
The official Spring Data Neo4j was rewritten a few years ago to not depend on Neo4j-OGM anymore (in Spring Boot version since 2.4).
This current (old) implementation is based on the fact that the Spring Boot starter still brings Neo4j-OGM as a dependency and makes use of the configured
SessionFactory
instance, that does not exist anymore.Pros: No need to specify the query parts anymore but only one query/statement based on the Neo4j Cypher-DSL (https://github.com/neo4j-contrib/cypher-dsl) for readers
Cons: The writer needs now, instead of the single
SessionFactory
,Neo4jMappingContext
to have knowledge about the entitiesNeo4jTemplate
for loading and mapping entities(Neo4j)Driver
for the simple delete operations.While changing the mapping layer component, I decided also to upgrade all other dependencies to latest.