diff --git a/core/src/main/java/org/neo4j/ogm/metadata/FieldInfo.java b/core/src/main/java/org/neo4j/ogm/metadata/FieldInfo.java index 142aa44d4f..93d853f658 100644 --- a/core/src/main/java/org/neo4j/ogm/metadata/FieldInfo.java +++ b/core/src/main/java/org/neo4j/ogm/metadata/FieldInfo.java @@ -400,10 +400,6 @@ public Class type() { return fieldType; } - public String relationshipName() { - return this.relationship(); - } - public boolean forScalar() { return !Iterable.class.isAssignableFrom(type()) && !type().isArray(); } diff --git a/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java b/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java index de83e3a4af..b3b45538db 100644 --- a/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java +++ b/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java @@ -303,11 +303,10 @@ public static FieldInfo getRelationalWriter(ClassInfo classInfo, String relation public static FieldInfo getRelationalWriter(ClassInfo classInfo, String relationshipType, String relationshipDirection, Class objectType) { - final ClassInfo lookupClassInfo = classInfo; final DirectedRelationshipForType directedRelationship = new DirectedRelationshipForType(relationshipType, relationshipDirection, objectType); final Map typeFieldInfoMap = relationalWriterCache - .computeIfAbsent(lookupClassInfo, key -> new ConcurrentHashMap<>()); + .computeIfAbsent(classInfo, key -> new ConcurrentHashMap<>()); if (typeFieldInfoMap.containsKey(directedRelationship)) { return typeFieldInfoMap.get(directedRelationship); @@ -361,11 +360,14 @@ public static FieldInfo getRelationalWriter(ClassInfo classInfo, String relation // 4th, try to find a unique field that has the same type as the parameter List 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; + FieldInfo candidateField = fieldInfos.iterator().next(); + + if (!candidateField.relationshipDirection(Relationship.UNDIRECTED).equals(Relationship.INCOMING)) { + + if (candidateField.relationshipTypeAnnotation() == null) { + typeFieldInfoMap.put(directedRelationship, candidateField); + return candidateField; + } } } } @@ -386,11 +388,10 @@ public static FieldInfo getRelationalWriter(ClassInfo classInfo, String relation public static FieldInfo getRelationalReader(ClassInfo classInfo, String relationshipType, String relationshipDirection) { - final ClassInfo lookupClassInfo = classInfo; final DirectedRelationship directedRelationship = new DirectedRelationship(relationshipType, relationshipDirection); final Map relationshipFieldInfoMap = relationalReaderCache - .computeIfAbsent(lookupClassInfo, key -> new ConcurrentHashMap<>()); + .computeIfAbsent(classInfo, key -> new ConcurrentHashMap<>()); if (relationshipFieldInfoMap.containsKey(directedRelationship)) { return relationshipFieldInfoMap.get(directedRelationship); diff --git a/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode1.java b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode1.java new file mode 100644 index 0000000000..03f0bf6391 --- /dev/null +++ b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode1.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2002-2019 "Neo4j," + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.ogm.domain.gh666; + +import org.neo4j.ogm.annotation.GeneratedValue; +import org.neo4j.ogm.annotation.Id; +import org.neo4j.ogm.annotation.NodeEntity; +import org.neo4j.ogm.annotation.Relationship; + +/** + * @author Michael J. Simons + */ +@NodeEntity +public class MessedUpNode1 { + + @Id + @GeneratedValue + private Long id; + + @Relationship(type = "RELATION_A") + private MessedUpNode2 ref; + + public Long getId() { + return id; + } + + public MessedUpNode2 getRef() { + return ref; + } + + public void setRef(MessedUpNode2 ref) { + this.ref = ref; + } +} diff --git a/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode2.java b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode2.java new file mode 100644 index 0000000000..6b7f4ca825 --- /dev/null +++ b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode2.java @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2002-2019 "Neo4j," + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.ogm.domain.gh666; + +import java.util.ArrayList; +import java.util.List; + +import org.neo4j.ogm.annotation.GeneratedValue; +import org.neo4j.ogm.annotation.Id; +import org.neo4j.ogm.annotation.Labels; +import org.neo4j.ogm.annotation.NodeEntity; + +/** + * @author Michael J. Simons + */ +@NodeEntity +public class MessedUpNode2 { + + @Id + @GeneratedValue + private Long id; + + @Labels + private List types = new ArrayList<>(); + + public Long getId() { + return id; + } + + public List getTypes() { + return types; + } + + public void setTypes(List types) { + this.types = types; + } +} diff --git a/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode3.java b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode3.java new file mode 100644 index 0000000000..073c237b92 --- /dev/null +++ b/test/src/test/java/org/neo4j/ogm/domain/gh666/MessedUpNode3.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2002-2019 "Neo4j," + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.ogm.domain.gh666; + +import org.neo4j.ogm.annotation.GeneratedValue; +import org.neo4j.ogm.annotation.Id; +import org.neo4j.ogm.annotation.NodeEntity; +import org.neo4j.ogm.annotation.Relationship; + +/** + * @author Michael J. Simons + */ +@NodeEntity +public class MessedUpNode3 { + + @Id + @GeneratedValue + private Long id; + + private MessedUpNode2 relationA; + + public Long getId() { + return id; + } + + public MessedUpNode2 getRef() { + return relationA; + } + + public void setRef(MessedUpNode2 ref) { + this.relationA = ref; + } +} diff --git a/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterAnnotatedFieldsTest.java b/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterAnnotatedFieldsTest.java index 3da1f5f490..993a4a7214 100644 --- a/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterAnnotatedFieldsTest.java +++ b/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterAnnotatedFieldsTest.java @@ -33,6 +33,7 @@ /** * @author Vince Bickers + * @author Michael J. Simons */ public class RelationshipWriterAnnotatedFieldsTest { @@ -47,7 +48,7 @@ public void shouldFindWriterForCollection() { .getRelationalWriter(classInfo, "LIST", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("LIST"); + assertThat(objectAccess.relationship()).isEqualTo("LIST"); assertThat(objectAccess.type()).isEqualTo(List.class); } @@ -60,7 +61,7 @@ public void shouldFindWriterForScalar() { .getRelationalWriter(classInfo, "SCALAR", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("SCALAR"); + assertThat(objectAccess.relationship()).isEqualTo("SCALAR"); assertThat(objectAccess.type()).isEqualTo(T.class); } @@ -73,7 +74,7 @@ public void shouldFindWriterForArray() { .getRelationalWriter(classInfo, "ARRAY", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("ARRAY"); + assertThat(objectAccess.relationship()).isEqualTo("ARRAY"); assertThat(objectAccess.type()).isEqualTo(T[].class); } diff --git a/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterPlainFieldsTest.java b/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterPlainFieldsTest.java index 60152573dc..ce42387762 100644 --- a/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterPlainFieldsTest.java +++ b/test/src/test/java/org/neo4j/ogm/metadata/reflect/RelationshipWriterPlainFieldsTest.java @@ -30,6 +30,7 @@ /** * @author Vince Bickers + * @author Michael J. Simons */ public class RelationshipWriterPlainFieldsTest { @@ -44,7 +45,7 @@ public void shouldFindWriterForCollection() { .getRelationalWriter(classInfo, "LIST", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("LIST"); + assertThat(objectAccess.relationship()).isEqualTo("LIST"); } @Test @@ -56,7 +57,7 @@ public void shouldFindWriterForScalar() { .getRelationalWriter(classInfo, "SCALAR", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("SCALAR"); + assertThat(objectAccess.relationship()).isEqualTo("SCALAR"); } @Test @@ -68,7 +69,7 @@ public void shouldFindWriterForArray() { .getRelationalWriter(classInfo, "ARRAY", Relationship.OUTGOING, new T()); assertThat(objectAccess).as("The resultant object accessor shouldn't be null").isNotNull(); assertThat(objectAccess instanceof FieldInfo).as("The access mechanism should be via the field").isTrue(); - assertThat(objectAccess.relationshipName()).isEqualTo("ARRAY"); + assertThat(objectAccess.relationship()).isEqualTo("ARRAY"); } static class S { diff --git a/test/src/test/java/org/neo4j/ogm/persistence/model/RelationshipMappingTest.java b/test/src/test/java/org/neo4j/ogm/persistence/model/RelationshipMappingTest.java index cd95585ccd..497fcf52e1 100644 --- a/test/src/test/java/org/neo4j/ogm/persistence/model/RelationshipMappingTest.java +++ b/test/src/test/java/org/neo4j/ogm/persistence/model/RelationshipMappingTest.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Function; import org.junit.Before; import org.junit.BeforeClass; @@ -44,6 +45,7 @@ import org.neo4j.ogm.domain.policy.Policy; import org.neo4j.ogm.domain.typed_relationships.SomeEntity; import org.neo4j.ogm.domain.typed_relationships.TypedEntity; +import org.neo4j.ogm.metadata.ClassInfo; import org.neo4j.ogm.session.Session; import org.neo4j.ogm.session.SessionFactory; import org.neo4j.ogm.testutil.GraphTestUtils; @@ -67,6 +69,7 @@ public static void oneTimeSetup() { "org.neo4j.ogm.domain.gh641", "org.neo4j.ogm.domain.typed_relationships", "org.neo4j.ogm.domain.gh656", + "org.neo4j.ogm.domain.gh666", "org.neo4j.ogm.domain.gh704"); } @@ -505,4 +508,63 @@ public void deletesOfEntitiesWithTheSameButNotEqualParentShouldWork3() { assertThat(actual).hasSize(1); assertThat(actual.iterator().next()).containsEntry("relTwo", true); } + + @Test // GH-666 + public void shouldNotMessUpNodes() { + assertMapping(org.neo4j.ogm.domain.gh666.MessedUpNode1.class, node -> node.getRef(), false); + } + + @Test // GH-666 + public void mayMessUpNodes() { + assertMapping(org.neo4j.ogm.domain.gh666.MessedUpNode3.class, node -> node.getRef(), true); + } + + private void assertMapping(Class targetClass, Function refExtractor, + boolean wildCardMappingAllowed) { + + ClassInfo targetclassInfo = sessionFactory.metaData().classInfo(targetClass); + + Map ids = sessionFactory.openSession().query("" + + "MERGE (n3:MessedUpNode2:TypeY) <-[:RELATION_A] - (n1:" + targetclassInfo.neo4jName() + + ")-[:RELATION_NOT_MODELED]->(n2:MessedUpNode2:TypeX) " + + "RETURN id(n1) as id1, id(n2) as id2, id(n3) as id3", + Collections.emptyMap() + ).queryResults().iterator().next(); + + Session session = sessionFactory.openSession(); + T n1 = session.load(targetClass, (long) ids.get("id1")); + org.neo4j.ogm.domain.gh666.MessedUpNode2 n2 = session.load(org.neo4j.ogm.domain.gh666.MessedUpNode2.class, (long) ids.get("id2")); + org.neo4j.ogm.domain.gh666.MessedUpNode2 n3 = session.load(org.neo4j.ogm.domain.gh666.MessedUpNode2.class, (long) ids.get("id3")); + + assertThat(n1).isNotNull(); + assertThat(n2).isNotNull(); + assertThat(n3).isNotNull(); + + // Expected behaviour + assertThat(refExtractor.apply(n1)).isEqualTo(n3); + + // Loaded from the session + String query = "MATCH (n1:" + targetclassInfo.neo4jName() + + ")-[rel:RELATION_NOT_MODELED]->(n2:`TypeX`) WHERE id(n1) = $id1 RETURN n1, rel, n2"; + Iterable> r = session + .query(query, Collections.singletonMap("id1", (long) ids.get("id1")), true).queryResults(); + assertCorrectRelationShip(r, wildCardMappingAllowed ? n2 : n3); + + // Loaded from the database + session = sessionFactory.openSession(); + r = session.query(query, Collections.singletonMap("id1", (long) ids.get("id1")), true).queryResults(); + assertCorrectRelationShip(r, wildCardMappingAllowed ? n2 : null); + } + + private static void assertCorrectRelationShip(Iterable> result, org.neo4j.ogm.domain.gh666.MessedUpNode2 expectedNode) { + assertThat(result).hasSize(1); + assertThat(result).allSatisfy(row -> { + assertThat(row).containsKeys("n1"); + if (expectedNode != null) { + assertThat(row.get("n1")).extracting("ref").extracting("id").containsExactly(expectedNode.getId()); + } else { + assertThat(row.get("n1")).extracting("ref").hasSize(1).containsNull(); + } + }); + } }