Skip to content

Commit

Permalink
GH-951 - Direct relationships can overrule relationship entities.
Browse files Browse the repository at this point in the history
If the source model has a relationship of a specific type defined that
is also covered by a relationship entity, Neo4j-OGM would eagerly
try to map everything to this entity before it notices that the source
is not using the entity and does not put in the related node because
the types are mismatching.
This commit will fix this behaviour while keeping already existing functionality in place as it is.
  • Loading branch information
meistermeier committed Jul 14, 2023
1 parent 62909e9 commit bce80bd
Show file tree
Hide file tree
Showing 4 changed files with 397 additions and 9 deletions.
34 changes: 27 additions & 7 deletions core/src/main/java/org/neo4j/ogm/context/GraphEntityMapper.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002-2022 "Neo4j,"
* Copyright (c) 2002-2023 "Neo4j,"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
Expand Down Expand Up @@ -320,7 +320,7 @@ private Set<Long> mapRelationships(GraphModel graphModel) {
mappedRelationshipIds.add(edge.getId());

// check whether this edge should in fact be handled as a relationship entity
ClassInfo relationshipEntityClassInfo = getRelationshipEntity(edge);
ClassInfo relationshipEntityClassInfo = getRelationshipEntity(edge, true);

if (relationshipEntityClassInfo != null) {
mapRelationshipEntity(oneToMany, edge, source, target, relationshipEntityClassInfo);
Expand Down Expand Up @@ -383,7 +383,7 @@ private void mapRelationshipEntity(List<Edge> oneToMany, Edge edge, Object sourc

private Object createRelationshipEntity(Edge edge, Object startEntity, Object endEntity) {

ClassInfo relationClassInfo = getRelationshipEntity(edge);
ClassInfo relationClassInfo = getRelationshipEntity(edge, false);
if (relationClassInfo == null) {
throw new MappingException("Could not find a class to map for relation " + edge);
}
Expand Down Expand Up @@ -564,10 +564,18 @@ private void mapOneToMany(Object instance, Class<?> valueType, Object values, St
logger.debug("Unable to map iterable of type: {} onto property of {}", valueType, classInfo.name());
}

// Find the correct RE associated with the edge. The edge type may be polymorphic, so we need to do a bit of work
// to identify the correct RE to bind to. We must not cache the value when found, because the correct determination
// depends on the runtime values of the edge in the mapping context, which may vary for the same edge pattern.
private ClassInfo getRelationshipEntity(Edge edge) {
/**
* Find the correct RE associated with the edge. The edge type may be polymorphic, so we need to do a bit of work
* to identify the correct RE to bind to. We must not cache the value when found, because the correct determination
* depends on the runtime values of the edge in the mapping context, which may vary for the same edge pattern.
*
* @param edge edge object
* @param strict defines that if there is a relationship on the source side that does not represent
* the relationship entity but a direct relationship, the registered relationship entity
* for this type, source and end node types should not be returned (it will be {@code null}).
* @return matching relationship entity {@link ClassInfo}.
*/
private ClassInfo getRelationshipEntity(Edge edge, boolean strict) {

Object source = mappingContext.getNodeEntity(edge.getStartNode());
Object target = mappingContext.getNodeEntity(edge.getEndNode());
Expand Down Expand Up @@ -596,6 +604,18 @@ private ClassInfo getRelationshipEntity(Edge edge) {
return classInfo;
}
}
if (strict) {
ClassInfo sourceClassInfo = metadata.classInfo(source);
Set<FieldInfo> sourceFieldInfos = sourceClassInfo.candidateRelationshipFields(edge.getType(), OUTGOING, true);
if (sourceFieldInfos.size() == 1) {
return null;
}
ClassInfo targetClassInfo = metadata.classInfo(target);
Set<FieldInfo> targetFieldInfos = targetClassInfo.candidateRelationshipFields(edge.getType(), INCOMING, true);
if (targetFieldInfos.size() == 1) {
return null;
}
}
// we've made our best efforts to find the correct RE. If we've failed it means
// that either a matching RE doesn't exist, or its start and end nodes don't declare a reference
// to the RE. If we can find a single properly-formed matching RE for this edge we'll assume it is
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,316 @@
package org.neo4j.ogm.persistence.relationships.direct.withrelentity;

import java.io.IOException;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;

import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.neo4j.ogm.annotation.EndNode;
import org.neo4j.ogm.annotation.GeneratedValue;
import org.neo4j.ogm.annotation.Id;
import org.neo4j.ogm.annotation.NodeEntity;
import org.neo4j.ogm.annotation.Relationship;
import org.neo4j.ogm.annotation.RelationshipEntity;
import org.neo4j.ogm.annotation.StartNode;
import org.neo4j.ogm.session.Session;
import org.neo4j.ogm.session.SessionFactory;
import org.neo4j.ogm.testutil.TestContainersTestBase;
import org.omg.PortableInterceptor.INACTIVE;

import static org.assertj.core.api.Assertions.*;

/**
* @author Gerrit Meier
*/
public class IgnoreRelationshipEntityTest extends TestContainersTestBase {

private static SessionFactory sessionFactory;
private Session session;

@BeforeClass
public static void oneTimeSetup() {
sessionFactory = new SessionFactory(getDriver(), "org.neo4j.ogm.persistence.relationships.direct.withrelentity");
}

@Before
public void init() throws IOException {
session = sessionFactory.openSession();
session.purgeDatabase();
}

@After
public void cleanup() throws IOException {
session.purgeDatabase();
}

@Test
public void ignoreRelationshipDefinitionIfDirectRelationshipIsPresentOutgoing() {
C c1 = new C("c1");
C c2 = new C("c2");
Set<C> cs = new HashSet<>();
cs.add(c1);
cs.add(c2);
B b = new B("b", cs);
Set<B> bs = new HashSet<>();
bs.add(b);
A a = new A("a", bs);

session.save(a);
session = sessionFactory.openSession();

A loaded = session.load(A.class, "a", -1);

assertThat(loaded).isNotNull();
Set<B> loadedBs = loaded.getB();
assertThat(loadedBs).hasSize(1);
B loadedB = loadedBs.iterator().next();
assertThat(loadedB.id).isEqualTo("b");
Set<C> loadedCs = loadedB.c;
assertThat(loadedCs).hasSize(2);
assertThat(loadedCs).containsExactlyInAnyOrder(c1, c2);

}

@Test
public void ignoreRelationshipDefinitionIfDirectRelationshipIsPresentIncoming() {
A a = new A();
a.id = "target";
D d = new D();
d.a = a;
d.id = "source";

session.save(d);
session = sessionFactory.openSession();

D loaded = session.load(D.class, "source", -1);

assertThat(loaded).isNotNull();
A loadedA = loaded.a;
assertThat(loadedA).isNotNull();
assertThat(loadedA.id).isEqualTo("target");

}

// Scenario for OUTGOING relationship
@NodeEntity("AA")
public static class A {
@Id String id;
@Relationship(type = "HAS_B")
Set<B> b;

public A() {
}

public A(String id, Set<B> b) {
this.id = id;
this.b = b;
}

public String getId() {
return id;
}

public void setId(String id) {
this.id = id;
}

public Set<B> getB() {
return b;
}

public void setB(Set<B> b) {
this.b = b;
}

@Override public String toString() {
return "A{" +
"id='" + id + '\'' +
", b=" + b +
'}';
}
}

@RelationshipEntity(type = "HAS_B")
public static class HasB {

@Id @GeneratedValue Long id;
@StartNode A startNode;
@EndNode B endNode;

public HasB() {
}

public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}

public A getStartNode() {
return startNode;
}

public void setStartNode(A startNode) {
this.startNode = startNode;
}

public B getEndNode() {
return endNode;
}

public void setEndNode(B endNode) {
this.endNode = endNode;
}

@Override public String toString() {
return "HasB{" +
"id=" + id +
", startNode=" + startNode +
", endNode=" + endNode +
'}';
}
}

@NodeEntity("BB")
public static class B {
@Id String id;
@Relationship(type = "HAS_C")
Set<C> c;

public B() {
}

public B(String id, Set<C> c) {
this.id = id;
this.c = c;
}

public String getId() {
return id;
}

public void setId(String id) {
this.id = id;
}

public Set<C> getC() {
return c;
}

public void setC(Set<C> c) {
this.c = c;
}

@Override public String toString() {
return "B{" +
"id='" + id + '\'' +
", c=" + c +
'}';
}
}

@NodeEntity("CC")
public static class C {
@Id String id;

public C() {
}

public C(String id) {
this.id = id;
}

public String getId() {
return id;
}

public void setId(String id) {
this.id = id;
}

@Override public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
C c = (C) o;
return Objects.equals(id, c.id);
}

@Override
public int hashCode() {
return Objects.hash(id);
}

@Override public String toString() {
return "C{" +
"id='" + id + '\'' +
'}';
}
}

// classes for INCOMING
@RelationshipEntity("HAS_D")
public static class HasD {
@Id @GeneratedValue Long id;
@StartNode A startNode;
@EndNode D endNode;

public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}

public A getStartNode() {
return startNode;
}

public void setStartNode(A startNode) {
this.startNode = startNode;
}

public D getEndNode() {
return endNode;
}

public void setEndNode(D endNode) {
this.endNode = endNode;
}
}

@NodeEntity("DD")
public static class D {
@Id String id;

@Relationship(type = "HAS_D", direction = Relationship.INCOMING)
public A a;

public String getId() {
return id;
}

public void setId(String id) {
this.id = id;
}

public A getA() {
return a;
}

public void setA(A a) {
this.a = a;
}
}

}
Loading

0 comments on commit bce80bd

Please sign in to comment.