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
828 stars 620 forks source link

Polymorphism: relationships not populated for subtypes #2639

Closed ddoppmeier closed 1 year ago

ddoppmeier commented 1 year ago

Hi everybody!

I'm experiencing problems loading lists of related objects when polymorphism is involved.

The data model:

Company references its employees by their common Person supertype.
The implementations of Person areSales and Developer. Developer references a list of Language.
(see below for simplified classes)

Data retrieval

Data initially is stored using a CompanyRepository extends Neo4jRepository<Company, Long>. Looking into Neo4j Browser reveals nothing unusual and everything seems fine. But when querying for companies by Company findByName(String name); the returned developers have an empty List programmingLanguages.

Versions used

Example implementation

acme-corp.zip

Classes (simplified)

@Node
public class Company {

    @Id
    @GeneratedValue
    private Long id;
    private final String name;
    @Relationship(type = "EMPLOYEE")
    private final List<Person> employees;

    public Company(String name, List<Person> employees) {
        this.name = name;
        this.employees = employees;
    }
}
@Node
public abstract class Person {

    @Id
    @GeneratedValue
    private Long id;

}
@Node
public class Sales extends Person {

    private final String name;

    public Sales(String name) {
        this.name = name;
    }

}
@Node
public class Developer extends Person {

    private final List<Language> programmingLanguages;
    private final String name;

    public Developer(String name, List<Language> programmingLanguages) {
        this.name = name;
        this.programmingLanguages = programmingLanguages;
    }

}
meistermeier commented 1 year ago

Thanks for the report. A first glance shows that the problem is somewhere between query generator and mapper. SDN will try its best to gather all fields from the extending classes (in this case Developer) but only knows about the Person as the field. This leads to the situation that the naming of the returned list for the languages is wrong or the detection algorithm does not pick this up with the right prefix. I will investigate this in more detail tomorrow. What surprises me is that this is a problem, we solved a long time ago. And your inheritance chain is far from being complex.

meistermeier commented 1 year ago

I have now a branch with a not-yet clean patch (issue/gh-2639). This should create a snapshot release in ~30 minutes (from this post timestamp on 😄 ) for you to check out with your example or project. To make use of it, define Spring's snapshot (and milestone) repository and the Spring Data Neo4j version explicitly.

<dependency>
    <groupId>org.springframework.data</groupId>
    <artifactId>spring-data-neo4j</artifactId>
    <version>6.3.7-GH-2639-SNAPSHOT</version>
</dependency>
<repositories>
  <repository>
    <id>spring-milestones</id>
    <name>Spring Milestones</name>
    <url>https://repo.spring.io/milestone</url>
    <snapshots>
      <enabled>false</enabled>
    </snapshots>
  </repository>
  <repository>
    <id>spring-snapshots</id>
    <name>Spring Snapshots</name>
    <url>https://repo.spring.io/snapshot</url>
    <releases>
      <enabled>false</enabled>
    </releases>
  </repository>
</repositories>
ddoppmeier commented 1 year ago

Thanks for the quick response and the fix!

It worked out of the box on the ACME Corp example code. For my production real world example I had to change some stuff, but I'm not yet sure which of the changes (Lombok annotations used, property names, relationship names, ...) finally solved my problem.

I will try to break ACME Corp again next week ;)

meistermeier commented 1 year ago

Thanks for your fast feedback on the changes. Yes, take your time and challenge us with your domain model 😁 In more details: The problem in detail is not that Company has Persons in general. But the deeper relationship mapping will only look for something like Developers_ProgrammingLanguages in the result, because SDN knows / did not know at this point that the definition for the result in the query creation was named Person_ProgrammingLanguage. That's because we collect all fields of the defined type and all its sub-types under the name of the defined type not the concrete property/relationship defining sub-type. This originates from a former bug fix.

ddoppmeier commented 1 year ago

I tried to add some properties to the relationships. Employes now have a salary and developers can rate their knowledge of a programming language by a score.

Looking at the database everything seems to be inserted fine, but loading the score (developer -> language) fails with Caused by: org.neo4j.driver.exceptions.value.Uncoercible: Cannot coerce NULL to Relationship. Loading the salary works.

acme-corp.zip

meistermeier commented 1 year ago

Thanks! I missed one spot for the abstraction detection. A new snapshot with also this fix in place should be available in (you guessed it) ~30 minutes.

ddoppmeier commented 1 year ago

To me everything looks good now. The test sample application works and tests of our production code "magically" turned green this night ;)

What will be the target version of this fix? Will the dependency of spring-boot-starter:2.7.6 be updated (so that we resolve this fix without any manual changes)?

meistermeier commented 1 year ago

That's what we're here for: magic 🪄 On a more serious note: The patch will get merged into 6.3.7. (and of course 7.x). This should get picked up by the next spring boot release 2.7.7. There is currently some more work to be done from my side before the merge. Want to ensure also a deeper structure with generic relationships works as expected.