neo4j / neo4j-ogm

Java Object-Graph Mapping Library for Neo4j
https://neo4j.com/docs/ogm-manual/
Apache License 2.0
337 stars 165 forks source link

sdn-caching-issue/feature request #490

Closed flikkes closed 6 years ago

flikkes commented 6 years ago

Since derived finders only support nesting of depth 1 (which by the way isn't clearly documented) any finder methods with deeper nesting need to use custom queries. The problem is that those don't support @Depth parameters.

NodeEntities cannot be loaded with depth > 1 from both repository queries and direct session queries unless session.loadAll(<EntityClass>, <depth>) is called before actual method call. As this workaround enables one to practically add a custom depth to repository methods annotated with @Query it would be nice to make this an official feature for such methods.

Expected Behavior

If

@Depth(5)
@Query("MATCH (p:PlasticContainer)-[:POSITION]-(gp:GarbagePosition)-[:ADDRESS]-(ad:AddressDetail)-[:HOUSEHOLD]-(h:HouseholdDetail) WHERE h.name = {name} RETURN p") 
Collection<PlasticContainer> findByAddressHouseholdName(@Param("name") String name);

is our repository method findByAddressHouseholdName("Schulze"); should return a Collection of PlasticContainer objects, each containing all related children up to depth 5 (which in this case would be every child and sub child and so on this object can have).

Current Behavior

A Collection of PlasticContainer objects, each containing only it's simple properties without any child objects, is returned.

Possible Solution

Expected behavior can be achieved by calling

session.loadAll(PlasticContainer.class, 5);
session.loadAll(GarbagePosition.class, 5);
session.loadAll(GarbageType.class, 5);
session.loadAll(AddressDetail.class, 5);
session.loadAll(HouseholdDetail.class, 5);
session.loadAll(StreetDetail.class, 5);
session.loadAll(GarbageCoordinates.class, 5);
session.loadAll(NavigationInformation.class, 5);

findByAddressHouseholdName("Schulze");

or

Collection<PlasticContainer> c =  findByAddressHouseholdName("Schulze");
session.loadAll(c, 5);

which obviously is a crappy solution as we are making use of a caching bug. It would be nice to somehow take the code responsible for this and make it usable for custom queries.

Steps to Reproduce (for bugs)

  1. git clone https://github.com/flikkes/sdn-caching-issue.git
  2. install neo4j 3.3.5
  3. create a new graph
  4. adjust neo4j credentials in sdn-caching-issue/sdn-issue/src/main/resources/application.properties accordingly
  5. run graph
  6. do mvn spring-boot:run inside sdn-caching-issue/sdn-issue/
  7. see available methods for creating and finding PlasticContainers in sdn-caching-issue/sdn-issue/src/main/java/de/flikkes/sdnissue/GarbageRestController.java
  8. create new PlasticContainers via REST and search for them using the different endpoints -> you'll see the different results (expected behavior is achieved with /workaround and /workaround2 REST call)

Context

At my company we're trying to use the benefits in speed and performance provided by neo4j to search for big lists of Entities, providing a String value of a simple property of a deeply nested child object of the respective Entity as search criteria. The only way we can currently do this in a reasonably fast and reliable manner is by using the workaround explained. We would really appreciate being able to make a cleaner approach.

Your Environment

meistermeier commented 6 years ago

Thanks for reporting this but your first assumption is wrong: Derived query methods support custom depth (https://docs.spring.io/spring-data/neo4j/docs/current/reference/html/#reference_derived-queries). So basically a:

Collection<PlasticContainer> findByPositionDisplayName(String name, @Depth int depth);

or

@Depth(1)
Collection<PlasticContainer> findByPositionDisplayName(String name);

will work. I tried this in your sample application. But a @Depth on a custom @Query is not supported because the query will be sent to the database as it is (simplified explanation). OGM can only convert and bind the properties and objects it gets from the return statement in this case. You could define your query more detailed like

@Query("MATCH (p:PlasticContainer)-[pos:POSITION]-(gp:GarbagePosition)-[a:ADDRESS]-(ad:AddressDetail)-[hh:HOUSEHOLD]-(h:HouseholdDetail) WHERE h.name = {name} RETURN *")

I named all the involving parts and return them all. This works perfectly in the sample you provided.

flikkes commented 6 years ago

Thanks for responding!

I think you misunderstood me.

@Depth(1) Collection findByPositionDisplayName(String name); will work. I tried this in your sample application.

Of couse this will work. But that is not the question... I want to search by HouseholdDetail.name, not the GarbagePosition.displayName which makes a huge difference.

Also you got me wrong on the Part with the derived query methods.

Since derived finders only support nesting of depth 1

I never said anything about a custom depth concerning OGM. What I'm saying is that I can't create a derived method which goes deep down to for example the HouseholdDetail.name. Collection<PlasticContainer> findByPositionAddressHouseholdName(String name); won't work but this is what I was looking for.

@Query("MATCH (p:PlasticContainer)-[pos:POSITION]-(gp:GarbagePosition)-[a:ADDRESS]-(ad:AddressDetail)-[hh:HOUSEHOLD]-(h:HouseholdDetail) WHERE h.name = {name} RETURN *") I named all the involving parts and return them all. This works perfectly in the sample you provided.

Although this particular cypher query won't produce the desired result as there will be several properties left as NULL, you are generally right. But imagine some more classes nested a little deeper than this... it would be a nightmare to create the queries accordingly....

Also I revise pointing

Collection c = findByAddressHouseholdName("Schulze"); session.loadAll(c, 5);

out as a working solution. As depth and the number of classes increase this will fail :(

meistermeier commented 6 years ago

Ok, I think I got your point. The deeper nested derived query was/is a lack of functionality. I created a ticket https://jira.spring.io/browse/DATAGRAPH-1091 and this should fix the dive into nested properties. Of course not the ticket but the fix behind it ;)

But just to be clear with the depth: You additionally need to define a depth also in this case. Just the query path extends the pattern to find the domain object. The return path is still only effected by the depth param.

flikkes commented 6 years ago

Thanks! This will help a lot.

But just to be clear with the depth: You additionally need to define a depth also in this case. Just the query path extends the pattern to find the domain object. The return path is still only effected by the depth param.

Of course! I made that experience a while ago ;D

meistermeier commented 6 years ago

Closing this issue because the related Spring Data Neo4j ticket is already fixed and released.