Skip to content

Allow interfaces in domain model hierarchies. #2201

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

Closed
hamidbm opened this issue Mar 26, 2021 · 10 comments
Closed

Allow interfaces in domain model hierarchies. #2201

hamidbm opened this issue Mar 26, 2021 · 10 comments
Assignees
Labels
type: bug A general bug

Comments

@hamidbm
Copy link

hamidbm commented Mar 26, 2021

I have few Entity node classes (PurchaseEntity, PLIEntity, EntitlementSetEntity, EntitlementEntity, ...) and the server can only start if not more than one repository interface is defined. Currently I have one repository interface (PurchaseRepository), but when I add another repository (like EntitlementRepository for example for the entity Entitlement), the server cannot start and throws the following error:

Caused by: org.springframework.data.mapping.MappingException: The schema already contains a node description under the primary label PLI

Only the entity PLIEntity is annotated with @node("PLI"), the other entities have their own labels, but somehow SDN is thinking all entities are labeled with PLI label

Entities are like this:

@node("Purchase")
public class PurchaseEntity {...}

@node("PLI")
public class PLIEntity {...}

@node("Entitlement")
public class { ... }

etc...

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 26, 2021
@meistermeier
Copy link
Collaborator

I created an example from your description here: https://github.com/meistermeier/neo4j-issues-examples/tree/master/gh-2201
This starts up completely fine. Can you tell me the differences regarding mapping / entity definition that I have missed to reproduce your problem?

@meistermeier meistermeier added blocked: awaiting feedback and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 29, 2021
@hamidbm
Copy link
Author

hamidbm commented Mar 31, 2021

I think the issue is this: when you create a data model (a set of entities) and something is not well configured in one of the entities, SDN when trying to resolve and register the model, it makes another call to the same method again and the entity is found already registered and then it throws an exception. The exception may be valid but it is not informative at all and it does not tell you what is the real issue with the data model (could be a field type in the data model that is not supported by SDN, etc... the issue could be anything on the entity, but the error is not helping to figure out what is wrong with the data model.
I can zip my project and share it with you. How can I send you the zip file?

@hamidbm
Copy link
Author

hamidbm commented Apr 5, 2021

I added you to the repo: https://github.com/hamidbm/springboot-rest-datastore
Thanks Gerrit.

@michael-simons michael-simons self-assigned this Apr 12, 2021
@michael-simons
Copy link
Collaborator

Hi

  • implementation 'org.neo4j:neo4j-ogm-bolt-driver:3.2.5'

This is SDN 5.x and Neo4j-OGM. It is not needed and will cause issues. This is not a driver, this a transport module for OGM.

  • The custom config
public class Config  extends AbstractNeo4jConfig implements Trace {

    @Value("${spring.neo4j.uri}")
    private String url;

    @Value("${spring.neo4j.authentication.username}")
    private String username;

    @Value("${spring.neo4j.authentication.password}")
    private String password;

    @Bean
    public Driver driver() {
        String neo4jURL = System.getProperty("NEO4J_URL");
        if ( neo4jURL == null ) neo4jURL = url;
        //info("neo4jURL is: {}", neo4jURL);
        return GraphDatabase.driver(neo4jURL, AuthTokens.basic(username, password));
    }

    @Bean
    public Neo4jTemplate neo4jTemplate(Neo4jMappingContext mappingContext) {
        return new Neo4jTemplate(neo4jClient(driver()), mappingContext, databaseSelectionProvider());
    }
}

is unnecessary when using our starter. You are duplicating what is there 100%.

  • Module path: We don't test on the module path as Spring Data Commons is not likely to work without errors on the module path. I'm unsure if your issue is related, but will let you know as soon as I am sure.

@michael-simons
Copy link
Collaborator

Same with the application: Why excluding the auto config and than recreating it?

@SpringBootApplication(exclude={Neo4jDataAutoConfiguration.class})
//@EnableNeo4jRepositories("XXX.neo4j.repository")
//@ComponentScan({"XXX.sb"})
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @Bean
    public Neo4jDataProperties neo4jDataProperties() { return new Neo4jDataProperties(); }
}

@michael-simons
Copy link
Collaborator

You can omit all the the BigDecimal converters I think…

See: https://docs.spring.io/spring-data/neo4j/docs/6.0.7/reference/html/#conversions

@michael-simons
Copy link
Collaborator

I can reproduce the behavior. The underlying issue is that you have

public interface PLI {
}

and

@Node("PLI")
public class PLIEntity implements PLI {
}

Both objects triggering the creation of a persistent entity under the label PLI. Both entities are necessary to make SDN work, but only one should go into our index lookup.
As we do think that your approach is a valid approach, we are gonna fix the behavior.

Your statement about repositories was kind of a red herring: It does happen only when you add EntitlementRepository as the entitlement implementation refers to a Entitlement value and thus triggering the bug.

Anyway, apart from the flaws I mentioned above, I do like your approach and would learn more about you having success in a modularized world.

FWIW we are preparing the CypherDSL to be fully modularized soon.

@michael-simons michael-simons added this to the 6.0.8 (2020.0.8) milestone Apr 12, 2021
michael-simons added a commit that referenced this issue Apr 13, 2021
While technically possible, Spring Data Neo4j prevented a proper use of
interfaces in domain hierachies due to the fact that it checks whether a
primary label has been used already in a domain model. That check can be
triggered at different point in times: Either when initializing the
persistence context with a predefined set of entities or when an
interface based entity is refered from an implementation of itself.

This commit changes the following approaches:
* Interfaces won't contribute to the index of entities by primary label
* The explicit or implicit names of interfaces implemented by an entity
  will contribute to the list of additional labels. However, we don't
  traverse the graph of interfaces to the top
* When hydrating instances we must check whether the target is an
  interface. If so, we go through the list of all known persistent
  entities and look for one that can be assigned to the interface and
  which spots all the mapped labels

In addition we fixed some bugs when selecting the target entity type
when saving relationships. Also, `@Relationship` without a given
relationship type name will work now without messing up the type name.

This allows for the following scenarios:

1. Creating an API module being free of Spring or Neo4j based
   annotations. That API module can than be implemented by different
   Spring Data stores that support interfaces in their hierachies like
   we do now then.
2. Putting an explicit `@Node` annotation onto an interface so that the
   implementation can be arbitrary named (Which is probably a rarer use
   case).
3. Using polymorpmic relationships: If an interface has multiple
   implementations in the same project, the interface and it's
   implementations can both be annotated. The annotation on the
   interface is going to be the primary label and the labels on the
   annotated implementations are the selectors for picking out the class
   to hydrate during loading.

This closes #2201.
@michael-simons
Copy link
Collaborator

Thank you for input, it was super valuable for us. Fix is in place and so are docs. Please also have a look at the commit message of 43a3047.

meistermeier added a commit that referenced this issue Apr 13, 2021
Add another test to verify multi-interface support.
Mention in the documentation not to use interfaces with repositories.
meistermeier added a commit that referenced this issue Apr 13, 2021
Add another test to verify multi-interface support.
Mention in the documentation not to use interfaces with repositories.
@michael-simons michael-simons changed the title Not able to create more than one repository interface Allow interfaces in domain model hierachies. Apr 14, 2021
@michael-simons michael-simons changed the title Allow interfaces in domain model hierachies. Allow interfaces in domain model hierarchies. Apr 14, 2021
@hamidbm
Copy link
Author

hamidbm commented Apr 15, 2021

Thanks Michael and Gerrit for looking into this. And please feel free to update the sample project on GitHub if you want. The point is to be able to create a good modularized template that we can share with the world (and explain in the README file how to create a well modularized app for Neo4j which can have swappable persistence layers...).
The entities need to implement an interface and only that interface is visible in business logic and web layer. The Entities classes should be only visible in persistence layer which can be swapped to any other persistence layer module (that may use Cassandra, MongoDB or any other database). I think you got the point of why I am modularizing this and using interfaces...

@michael-simons
Copy link
Collaborator

Thanks @hamidbm again for your feedback.

I have a working project on the module path:
https://github.com/michael-simons/neo4j-from-the-jvm-ecosystem/tree/master/spring-data-imperative-module-path

See especially my module-info.Java and the Dockerfile

I will watch your repo. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants