spring-projects / spring-data-neo4j

Provide support to increase developer productivity in Java when using Neo4j. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
http://spring.io/projects/spring-data-neo4j
Apache License 2.0
832 stars 618 forks source link

Objects beyond second level of hierarchy are not retrieved via ReactiveNeo4jRepository [DATAGRAPH-1431] #1993

Closed spring-projects-issues closed 3 years ago

spring-projects-issues commented 3 years ago

kibantony opened DATAGRAPH-1431 and commented

As recently as Spring Boot 2.4.0-M4 (not sure which SDN version that is? 6.0.0?), using a ReactiveNeo4jRepository it was possible to save and retrieve (via simple calls such as findById, using no custom queries) a three level nested class/node hierarchy, but in the release version of Spring Boot 2.4.0 (SDN 6.0.1?) this no longer works. The third level of the hierarchy is not retrieved.

Please refer to the example project here: TestNeo4j

If you run the test WidgetRepositoryTest in this project you'll notice it fails. If you change the Spring Boot version in the Gradle file to 2.4.0-M4, the test will succeed.

Given entity classes such as these (edited down for brevity):

@Node
public class Widget {
    @Id
    @GeneratedValue
    private UUID uuid;

    @Relationship(type = "events", direction = Relationship.Direction.OUTGOING)
    private List<WidgetEvent> events;
}

@Node
public class WidgetEvent {

    @Id
    @GeneratedValue
    private UUID uuid;

    @Relationship(type = "values", direction = Relationship.Direction.OUTGOING)
    private List<EventValue> values;
}

@Node
public class EventValue {

    @Id
    @GeneratedValue
    private UUID uuid;

    private String value;
}

The following test fails because findById returns the Widget and its list of WidgetEvents, but the list of EventValues in each WidgetEvent is null.

Again this works fine in SB-M4 and appears to be broken in the release. I can find no mention in the documentation or release notes of the expected behavior here.

Widget widget = new Widget()
        .setEvents(Arrays.asList(
                new WidgetEvent().setValues(Arrays.asList(
                        new EventValue().setValue("foo"),
                        new EventValue().setValue("bar")
                )),
                new WidgetEvent().setValues(Arrays.asList(
                        new EventValue().setValue("goo"),
                        new EventValue().setValue("car")
                ))
        ));

StepVerifier.create(repository.save(widget))
        .assertNext(saved -> {
            assertThat(saved.getEvents())
                    .hasSize(widget.getEvents().size());
            assertThat(saved.getEvents().get(0).getValues())
                    .hasSize(widget.getEvents().get(0).getValues().size());
            assertThat(saved.getEvents().get(1).getValues())
                    .hasSize(widget.getEvents().get(1).getValues().size());
        })
        .verifyComplete();

StepVerifier.create(repository.findById(widget.getUuid()))
        .assertNext(actual -> {
            assertThat(actual.getEvents())
                    .hasSize(widget.getEvents().size());
            assertThat(actual.getEvents().get(0).getValues())
                    .hasSize(widget.getEvents().get(0).getValues().size());
            assertThat(actual.getEvents().get(1).getValues())
                    .hasSize(widget.getEvents().get(1).getValues().size());
        })
        .verifyComplete();

Here's a section of the logs with relevant queries:

2020-11-14 17:32:07.198 DEBUG 58047 — [ Test worker] .d.n.c.t.ReactiveNeo4jTransactionManager : Creating new transaction with name [org.springframework.data.neo4j.repository.support.SimpleReactiveNeo4jRepository.save]: PROPAGATION_REQUIRED,ISOLATION_DEFAULT
 2020-11-14 17:32:07.797 DEBUG 58047 — [o4jDriverIO-2-3] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`Widget` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:10.902 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`Widget`)-[rel:`events`]->(:`WidgetEvent`) WHERE startNode.uuid = $fromId DELETE rel
 2020-11-14 17:32:11.146 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`WidgetEvent` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.205 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`Widget`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 1 MERGE (startNode)-[:`events`]->(endNode)
 2020-11-14 17:32:11.426 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`)-[rel:`values`]->(:`EventValue`) WHERE startNode.uuid = $fromId DELETE rel
 2020-11-14 17:32:11.481 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`EventValue` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.543 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 2 MERGE (startNode)-[:`values`]->(endNode)
 2020-11-14 17:32:11.612 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`EventValue` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.620 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 3 MERGE (startNode)-[:`values`]->(endNode)
 2020-11-14 17:32:11.675 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`WidgetEvent` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.680 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`Widget`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 4 MERGE (startNode)-[:`events`]->(endNode)
 2020-11-14 17:32:11.827 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`)-[rel:`values`]->(:`EventValue`) WHERE startNode.uuid = $fromId DELETE rel
 2020-11-14 17:32:11.835 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`EventValue` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.849 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 5 MERGE (startNode)-[:`values`]->(endNode)
 2020-11-14 17:32:11.925 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MERGE (n:`EventValue` \{uuid: $__id__}) SET n = $__properties__ RETURN id(n)
 2020-11-14 17:32:11.931 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (startNode:`WidgetEvent`) WHERE startNode.uuid = $fromId MATCH (endNode) WHERE id(endNode) = 6 MERGE (startNode)-[:`values`]->(endNode)
 2020-11-14 17:32:11.996 DEBUG 58047 — [o4jDriverIO-2-2] .d.n.c.t.ReactiveNeo4jTransactionManager : Initiating transaction commit
 2020-11-14 17:32:12.128 DEBUG 58047 — [ Test worker] .d.n.c.t.ReactiveNeo4jTransactionManager : Creating new transaction with name [org.springframework.data.neo4j.repository.support.SimpleReactiveNeo4jRepository.findById]: PROPAGATION_REQUIRED,ISOLATION_DEFAULT,readOnly
 2020-11-14 17:32:12.152 DEBUG 58047 — [o4jDriverIO-2-2] org.springframework.data.neo4j.cypher : Executing:
 MATCH (n:`Widget`) WHERE n.uuid = $__id__ WITH n, id(n) AS __internalNeo4jId__ RETURN n{.description, .tag, .uuid, __nodeLabels__: labels(n), __internalNeo4jId__: id(n), Widget_events_WidgetEvent: [(n)-[:`events`]->(n_events:`WidgetEvent`) | n_events{.tag, .uuid, __nodeLabels__: labels(n_events), __internalNeo4jId__: id(n_events), WidgetEvent_values_EventValue: [(n_events)-[:`values`]->(n_events_values:`EventValue`) | n_events_values\{.uuid, .value, __nodeLabels__: labels(n_events_values), __internalNeo4jId__: id(n_events_values)}]}]}
 2020-11-14 17:32:12.443 DEBUG 58047 — [o4jDriverIO-2-2] .d.n.c.t.ReactiveNeo4jTransactionManager : Initiating transaction commit

Affects: 6.0.1 (2020.0.1)

Backported to: 6.0.2 (2020.0.2)

spring-projects-issues commented 3 years ago

kibantony commented

Edited the description to reflect M4 passes the test, RC-1 fails

spring-projects-issues commented 3 years ago

Gerrit Meier commented

Thanks for reporting this. This was indeed a bug in the mapping internals that got introduced by another change. We are currently working on the fix and it will be solved with the next patch

spring-projects-issues commented 3 years ago

kibantony commented

Thanks for the quick fix guys. I just verified that with SDN 6.0.2 this now works in the test project and in my real project. (y)

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

I'm still encountering some error while mapping to more than one level with version 6.0.2. It's not always the case, but sometimes SDN cannot retrieve my entities in the tree of relationships.

It happens, for example when I have the following pattern:  A->B->C->D. Saving A and retrieving A is working, but not always. And retrieving all Bs is never working. The error states that C could not be constructed because D was null. 

D has a Composite propertiy with a custom converter (don't know if this can help or not)

spring-projects-issues commented 3 years ago

Gerrit Meier commented

Could you update your example to give us more insights?

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

Hello,

Here's a minimal project with my issue https://we.tl/t-n0Wsbif8S6  The sepcific test is called 

Update of a graph view will not remove the removed entities,

Thanks a lot for your reactivity !

spring-projects-issues commented 3 years ago

Gerrit Meier commented

I can see that there is a problem but this is not due to the loading and retrieving but the null value for the provderInstance in ProviderData that Kotlin does not like.

@Relationship(type = ProviderDataOutgoingRelationships.USES_BY_DEFAULT, direction = Relationship.Direction.OUTGOING)
val providerInstance: ProviderInstance?

Helps here and it is working

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

Yes but in this case, the provider instance is set to null, and not to its actual value even after the repository call ends, which does not match the graph. (The provider instance exists and is related to the ProviderData if you look at the saved Graph View)

I don't understand why this would be necessary for this field only, and it looks to me as a loading and retrieving bug, no?

spring-projects-issues commented 3 years ago

Gerrit Meier commented

I thought that there is nothing set, but you are right, there is a providerInstance but we are not querying for it

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

This is what I thought when I tried to debug around the test in the spring data neo4j code, it looks like it's stopping the path traversal at some level, but could not figure out why

spring-projects-issues commented 3 years ago

Gerrit Meier commented

Could you try it with version 6.0.2-DATAGRAPH-1433-SNAPSHOT ?  It works at least on my machine ;) 

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

-I cannot retrieve this version maven says :/- 

It's ok now, and working in this version, thanks ! Do you know when the 6.0.2 will be released please?

spring-projects-issues commented 3 years ago

Gerrit Meier commented

Tomorrow ;) https://calendar.spring.io/

spring-projects-issues commented 3 years ago

BreadAndRoses95 commented

Hello,

 

I still get errors when mapping with DSN 6.0.2. (I have several nodes that are connected to the same entity with @CompositProperties),  Have a look at the test called 

Update of a graph view will not remove the removed entities

in this repo: https://wetransfer.com/downloads/bba26e111e8f9eade016948a9258121320201215164448/996997b443115ccc80e81aa1b34a5c7d20201215164449/ad2556

The generated path is extremely long, and ends up by throwing an error 

Caused by: java.lang.NullPointerException: Parameter specified as non-null is null: method com.devel.test.testspringdataneo4j.domain.model.system.SystemImplementation.<init>, parameter system

Do you know how to handle this?