Skip to content
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

Relation that is not modeled via annotations is filled into NodeEntity #666

Closed
steffen-harbich-cognitum opened this issue Oct 7, 2019 · 3 comments

Comments

@steffen-harbich-cognitum

Please see the following demo project.
demo.zip

Expected Behavior

Excerpt from test:

        // current DB state:
        //   node1 = 1 node of label MyNode
        //   node2 = 1 node of label MyNode2 + TypeX
        //   node3 = 1 node of label MyNode2 + TypeY
        //   relation (node1)-[:RELATION_A]->(node3)
        //   relation (node1)-[:RELATION_NOT_MODELED]->(node2)

        Result r = session.query("MATCH (n1:MyNode)-[rel:RELATION_NOT_MODELED]->(n2:`TypeX`) WHERE id(n1) = {id1} " +
                        "RETURN n1, rel, n2",
                Map.of("id1", node1.getId()), true);

        // result "n1" is MyNode with "ref" filled by the RELATION_NOT_MODELED related node2
        // -> not modeled in node entities -> should not be!

A relation that is not modeled in NodeEntities should not be filled into a NodeEntity when querying that relation/related node.

Current Behavior

Not-modeled related object is filled into the returned object.

Possible Solution

Feels like OGM doesn't check the @relationship "type" in that case but considers only types or something.

When actually modeling the un-modeled relation then bahavior of OGM is correct. But this is not viable in our case.

Steps to Reproduce (for bugs)

  1. download demo project
  2. open with IDE (e.g. IntelliJ)
  3. execute gradle task "test" in debug mode (required for assertions)
  4. test produces AssertionError in NodeService line 84

Context

Our graph data is not completely modeled in Java objects.

Your Environment

  • OGM Version used: 3.1.12
  • Java Version used: 11
  • Neo4J Version used: 3.5
  • Bolt Driver Version used (if applicable):
  • Operating System and Version: Win 10
  • Link to your project: see start of issue
@meistermeier
Copy link
Collaborator

meistermeier commented Oct 23, 2019

That is not a bug but desired (and undocumented) fallback behaviour.
You can see this in the code when it comes to the mapping of the relationships.

// 4th, try to find a unique field that has the same type as the parameter
List<FieldInfo> fieldInfos = classInfo.findFields(objectType);
if (fieldInfos.size() == 1) {
FieldInfo candidateFieldInfo = fieldInfos.iterator().next();
if (!candidateFieldInfo.relationshipDirection(Relationship.UNDIRECTED)
.equals(Relationship.INCOMING)) {
typeFieldInfoMap.put(directedRelationship, candidateFieldInfo);
return candidateFieldInfo;

I leave this issue open to discuss this internally if we want to keep this or, for example, introduce something like a strict mapping mode where this will fail.

Thanks for your time to create the example.

@michael-simons
Copy link
Collaborator

I think the strictQuerying introduced with #651 would be a great fit to change that behaviour.

FWIW: I also do think that this - while intended as might have been - is very close to a bug. Some sort of surprises you don't want to have.

@michael-simons michael-simons self-assigned this Dec 6, 2019
michael-simons added a commit that referenced this issue Dec 9, 2019
…related nodes.

The entity access manager tries several ways to find a field for mapping relationships. Usually, more than 3 tries are necessary for custom queries. The last approach just checks if there is any field of the same type as a currently mapped relationship and just dumps the relationships content into it.

This is bad in case when the field type matches but there’s actually an annotation contradicting it.

This commit makes the entity access manager check for contradicting annotations and if there is one, doesn’t fill in the object.
This fixes #666.
@michael-simons
Copy link
Collaborator

It will be my pleasure to fix issue number 666.

We don't fill in the relationship anymore as soon as there's a contradicting annotation. That just doesn't make any sense. Good thing is: There are no tests for exactly that behaviour and I won't have to argue about that.

If you remove the annotation @Relationship(type = "RELATION_A") in your mapping code and map via name (aka private MessedUpNode2 relationA), than it still will be filled but I think that's actually ok. I have added tests for both scenarios.

Waiting for all tests to go green before this will make it's way over all the branches we currently support.

michael-simons added a commit that referenced this issue Dec 10, 2019
…related nodes.

The entity access manager tries several ways to find a field for mapping relationships. Usually, more than 3 tries are necessary for custom queries. The last approach just checks if there is any field of the same type as a currently mapped relationship and just dumps the relationships content into it.

This is bad in case when the field type matches but there’s actually an annotation contradicting it.

This commit makes the entity access manager check for contradicting annotations and if there is one, doesn’t fill in the object and thus fixes #666.
michael-simons added a commit that referenced this issue Dec 10, 2019
…related nodes.

The entity access manager tries several ways to find a field for mapping relationships. Usually, more than 3 tries are necessary for custom queries. The last approach just checks if there is any field of the same type as a currently mapped relationship and just dumps the relationships content into it.

This is bad in case when the field type matches but there’s actually an annotation contradicting it.

This commit makes the entity access manager check for contradicting annotations and if there is one, doesn’t fill in the object.

This is a backport of 0a9a921.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants