Upgrade to Spring Data Neo4j 6 #22299

meistermeier · 2020-07-10T06:46:43Z

This PR adapts Spring Boot 2.4 to Spring Data Neo4j 6.

For some background:
Neo4j has developed two projects in the last year

A starter dedicated for the Java Driver for Neo4j (dubbed the "Bolt Driver") https://github.com/neo4j/neo4j-java-driver-spring-boot-starter
SDN/RX: https://github.com/neo4j/sdn-rx Which does imperative and reactive Spring Data Neo4j purely based on the Bolt Driver

SDN/RX has become SDN 6 and is now merged into Spring Data Neo4j.

That means Spring Boot depending on the Ockham release train will not build anymore.

This adapts Spring Boot 2.4 to the SDN 6

It brings automatic configuration for the Bolt Driver under the new namespace spring.neo4j, similar to others (like mongo and redis)
Adapts automatic configuration for Spring Data Neo4j

This removes the dependency on Neo4j-OGM as well.

SDN 6 won't have support for bringing up an embedded instance, which is in line with our strategy to recommend Testcontainers or the Neo4j Testharness to our customers and users.

We refrained from adding support for the Neo4j Testharness to the test slices to not broaden the topic more.
However, if we want to help users to bring something up running very fast, that would be an additional option.

The change https://github.com/spring-projects/spring-boot/compare/master...meistermeier:feature/neo4j-starter?expand=1#diff-76ff4fe38f0762e0fcb54986cae8e0b6R1608 is in there to enable SDN 6 for now because the dependency to Spring Data is on Ockham M1 where SDN 6 is not available.

We are looking forward to your feedback.

wilkinsona · 2020-07-10T08:22:06Z

Thanks for the PR, @meistermeier.

This looks great but there is quite a lot to take in here. To make it easier to review, I wonder if it would be possible to split it out into separate commits or, ideally, separate pull requests please? For example, the addition of metrics looks great but I think it warrants a separate pull request.

In addition to making things easier for us to review, splitting things up will also make the changes easier to understand if we're reviewing a diff in the future. Perhaps most importantly, separate pull requests will result in a separate entries in the release notes which will hopefully make it easier for users to discover the new features.

meistermeier · 2020-07-10T08:28:48Z

Thanks for your quick response, Andy.
When I saw the changes in the PR, I also thought "wow" and you are right this should get splitted in sensible parts (commits or PRs).
Can wel keep this PR open for the SDN spring-boot starter update? I will go ahead and try to extract driver autoconfig and metrics into different PRs.

wilkinsona · 2020-07-10T08:39:55Z

Can wel keep this PR open for the SDN spring-boot starter update? I will go ahead and try to extract driver autoconfig and metrics into different PRs.

Thanks. That sounds good to me. This PR modified so that it's just for compatibility with the latest Spring Data Ockham snapshots, and then others on top of that to introduce new features.

meistermeier · 2020-07-10T11:58:58Z

So I splitted this up into several PRs but unfortunately they cannot get merged independently :(
Bringing this into an order:
Neo4j Java Driver autoconfig: #22301
Spring Data Neo4j starter (this): #22299
Neo4j Actuator: #22302

Please tell me if I did something terrible wrong during the split :)

snicoll

Thank you very much for the PR.

I've made a first pass and that's indeed a lot to take in but this work is super useful to me as I am not familiar with Spring Data Neo4j 6 yet.

I've made quite a lot of comments. Most are formatting and consistency thing that we can easily fix but there are a few that might requite some additional discussion. Can you please have a look to these? Do not hesitate if something isn't clear.

spring-boot-project/spring-boot-dependencies/build.gradle

.../main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java

...ure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataProperties.java

...mework/boot/autoconfigure/data/neo4j/Neo4jRepositoriesAutoConfigurationIntegrationTests.java

snicoll · 2020-07-16T15:20:18Z

...g/springframework/boot/autoconfigure/data/neo4j/Neo4jRepositoriesAutoConfigurationTests.java

+						.hasSingleBean(ReactiveCountryRepository.class));
+	}
+
+	@Configuration


For consistency this should have proxyBeanMethods = false.

.../java/org/springframework/boot/test/autoconfigure/data/neo4j/DataNeo4jTypeExcludeFilter.java

snicoll · 2020-07-16T15:25:45Z

.../main/java/org/springframework/boot/test/autoconfigure/data/neo4j/ReactiveDataNeo4jTest.java

+@AutoConfigureDataNeo4j
+@ImportAutoConfiguration
+@Import(ReactiveCapableNeo4jVersionAssertion.class)
+public @interface ReactiveDataNeo4jTest {


None of the other store has a reactive specific variant so I am not keen to introduce that. Perhaps the need of a specific annotation surfaces another problem?

I will target this together with the reactive AutoConfiguration suggestion in the next iteration.

Just checked this one and the reasons why we have chosen to introduce a parallel annotation for reactive is some convenience features:
Fail on database < 4.0 (https://github.com/spring-projects/spring-boot/pull/22299/files/b7fa3d1faca912a7c384ec3c20ca93a8fb3b21a8#diff-31539ed0ee3ec3e7c3a490aaa41e5db5R50)
and setting the spring.data.neo4j.repositories.type to reactive (https://github.com/spring-projects/spring-boot/pull/22299/files/b7fa3d1faca912a7c384ec3c20ca93a8fb3b21a8#diff-31539ed0ee3ec3e7c3a490aaa41e5db5R50)
Arguably this might be too less features for introducing this parallel infrastructure.

None of the other stores have such a variant and I am not keen to introduce it as part of the upgrade. We can reconsider once the upgrade is done of course.

...spring-boot-smoke-test-data-neo4j/src/main/java/smoketest/data/neo4j/CustomerRepository.java

meistermeier · 2020-07-16T21:28:40Z

Thanks for the detailed feedback. My plan is to resolve the trivial ones first and see how we can align our changes more with the other Spring Data modules in the discussable cases.

This commit will update the Spring Data Neo4j Spring Boot starter, autoconfig and actuator. It will also bring in autoconfiguration for the Neo4j Java driver to enable users to work with a managed instance of the driver without the high level abstraction of Spring Data.

Until we decided what to do with the reactive and imperative auto-configuration, we will keep both. To remove the `@Nested` annotation but keep it readable the tests get split up into different classes.

See spring-projectsgh-22299

snicoll · 2020-07-29T17:46:33Z

@meistermeier @michael-simons, thank you both for helping us upgrade to Spring Data Neo4j 6! For consistency with other stores, I had to undo a number of things in your proposal, see snicoll@9bc71fe for more details.

There's a bit more work still. We need to update the documentation (#22652) and I'd like to experiment supporting the spring.data.neo4j properties again (i.e. the ones used to bootstrap the connection with the client) (#22653)?

jagadeesh595 · 2020-09-03T03:47:41Z

With the latest version of SDN 6.0.0-M2, I have below observation as of now. When we go with either dynamic relationship or Relationship Properties, records are saving in database quickly, but retrieval taking more time compared to Old SDN 5.3.3 version.

In new version, below are the syntaxes to create either dynamic relationship or Relationship Properties,

Syntax for relationship with properties: Below syntax represents Sponsor relationship between two member nodes. MemberSponsor entity contains all relationship properties.

@Relationship(value = "sponsor", direction = Relationship.Direction.OUTGOING)
private Map<Member, MemberSponsor> hasSponsor = new HashMap<>();

Syntax for dynamic relationship without any properties: Below syntax represents Sponsor relationship between two member nodes. In the Map, key-String represents the relationship name, that we can configure dynamically.

@Relationship(direction = Relationship.Direction.OUTGOING)
private Map<String, Member> sponsors = new HashMap<>();

When I am trying to get Member data using below JPA method, system taking more time when any of the above relationships exists in the Member entity class. It is taking 25 mins to get the single Member data using below method from latest version SDN 6.0.0. In old version SDN 5.3.3 – getting same member data within a second. This issue especially occurring only when there is a relationship exists between two nodes with same labels. Here, we are referring relationship between two member nodes of same label.

import org.springframework.data.neo4j.repository.Neo4jRepository;

public interface MemberRepository extends Neo4jRepository<Member, Long> {
    Member findByMemberId(String memberId);
}

Can you please help me on this. Seems like issue with retrieval when there is a Dynamic relationship on entity.

meistermeier · 2020-09-03T07:04:08Z

Thanks @jagadeesh595 for reporting this.
Such problems are better placed in the official issue tracker of Spring Data Neo4j: https://jira.spring.io/projects/DATAGRAPH/
I created a ticket for this problem: https://jira.spring.io/browse/DATAGRAPH-1372

jagadeesh595 · 2020-09-03T09:02:53Z

Thanks @jagadeesh595 for reporting this.
Such problems are better placed in the official issue tracker of Spring Data Neo4j: https://jira.spring.io/projects/DATAGRAPH/
I created a ticket for this problem: https://jira.spring.io/browse/DATAGRAPH-1372

Thank you for the quick update.

meistermeier force-pushed the feature/neo4j-starter branch from 6d69ff8 to 2ed8a59 Compare July 10, 2020 06:50

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 10, 2020

meistermeier mentioned this pull request Jul 10, 2020

Add Neo4j Bolt driver option. spring-io/start.spring.io#483

Closed

wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jul 10, 2020

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 10, 2020

This was referenced Jul 10, 2020

Add auto-configuration for Neo4j driver #22301

Closed

Switch Neo4j health checks to the new Neo4j driver #22302

Closed

meistermeier force-pushed the feature/neo4j-starter branch from 2ed8a59 to b7fa3d1 Compare July 10, 2020 11:55

wilkinsona added for: team-attention An issue we'd like other members of the team to review and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jul 14, 2020

wilkinsona added this to the 2.4.x milestone Jul 14, 2020

wilkinsona added the type: enhancement A general enhancement label Jul 14, 2020

snicoll requested changes Jul 16, 2020

View reviewed changes

snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jul 16, 2020

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 16, 2020

snicoll changed the title ~~Update starter for Spring Data Neo4j.~~ Upgrade to Spring Data Neo4j 6 Jul 17, 2020

snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 17, 2020

philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jul 17, 2020

meistermeier added 3 commits July 28, 2020 10:11

First round of requested changes.

7180b08

Remove exclude filter from test support.

9b89300

meistermeier added 3 commits July 28, 2020 10:11

Inverse logic from driver auto-config.

1dc1483

Formatting and split auto-config tests.

1851701

Until we decided what to do with the reactive and imperative auto-configuration, we will keep both. To remove the `@Nested` annotation but keep it readable the tests get split up into different classes.

Polishing.

eacfd19

meistermeier force-pushed the feature/neo4j-starter branch from a3b2305 to eacfd19 Compare July 28, 2020 10:04

snicoll self-assigned this Jul 29, 2020

snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Jul 29, 2020

Upgrade to Spring Data Neo4j 6

f946981

See spring-projectsgh-22299

snicoll added a commit to snicoll/spring-boot that referenced this pull request Jul 29, 2020

Polish "Upgrade to Spring Data Neo4j 6"

d92dca2

See spring-projectsgh-22299

snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Jul 29, 2020

Upgrade to Spring Data Neo4j 6

15cd343

See spring-projectsgh-22299

snicoll added a commit to snicoll/spring-boot that referenced this pull request Jul 29, 2020

Polish "Upgrade to Spring Data Neo4j 6"

9bc71fe

See spring-projectsgh-22299

snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Jul 29, 2020

snicoll modified the milestones: 2.4.x, 2.4.0-M2 Jul 29, 2020

snicoll closed this in fdf9310 Jul 29, 2020

This was referenced Jul 29, 2020

Document Spring Data Neo4j 6 support #22652

Closed

Support spring.data.neo4j.uri and authentication-related settings again #22653

Closed

meistermeier deleted the feature/neo4j-starter branch July 30, 2020 09:07

This comment has been minimized.

Sign in to view

spring-projects-issues mentioned this pull request Dec 31, 2020

Possible performance decrease when using dynamic relationships. [DATAGRAPH-1372] spring-projects/spring-data-neo4j#1935

Closed

Upgrade to Spring Data Neo4j 6 #22299

Upgrade to Spring Data Neo4j 6 #22299

Uh oh!

Conversation

meistermeier commented Jul 10, 2020

Uh oh!

wilkinsona commented Jul 10, 2020

Uh oh!

meistermeier commented Jul 10, 2020

Uh oh!

wilkinsona commented Jul 10, 2020

Uh oh!

meistermeier commented Jul 10, 2020

Uh oh!

snicoll left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snicoll Jul 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snicoll Jul 16, 2020

Choose a reason for hiding this comment

Uh oh!

meistermeier Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

meistermeier Jul 20, 2020

Choose a reason for hiding this comment

Uh oh!

snicoll Jul 27, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meistermeier commented Jul 16, 2020

Uh oh!

snicoll commented Jul 29, 2020

Uh oh!

jagadeesh595 commented Sep 3, 2020 • edited by wilkinsona Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meistermeier commented Sep 3, 2020

Uh oh!

jagadeesh595 commented Sep 3, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

snicoll left a comment •

edited

Loading

jagadeesh595 commented Sep 3, 2020 •

edited by wilkinsona

Loading