From 4e2a18ba1e7460d729a6a6b5306e6e49e1069ae9 Mon Sep 17 00:00:00 2001 From: Gerrit Meier Date: Fri, 28 Aug 2020 11:14:05 +0200 Subject: [PATCH] DATAGRAPH-1373 - Improve RelationshipProperties API. The relationship with properties will now define the `TargetNode` that points to the entity that should be referred to, either outgoing or incoming. The syntax with the `Map` usage was discarded because it has limitations in the convinience usage of the derived finders and did not allow the definition of mulitple relationships of the same type to the same node. --- etc/adr/adr-005.adoc | 20 +++ etc/adr/adr-006.adoc | 20 +++ ...Relationships.java => MappingSupport.java} | 32 +++- .../data/neo4j/core/Neo4jTemplate.java | 15 +- .../neo4j/core/NestedRelationshipContext.java | 39 ++++- .../neo4j/core/ReactiveNeo4jTemplate.java | 5 +- .../core/RelationshipStatementHolder.java | 7 +- .../core/mapping/DefaultNeo4jConverter.java | 96 +++++++---- .../mapping/DefaultNeo4jPersistentEntity.java | 9 ++ .../DefaultNeo4jPersistentProperty.java | 22 ++- .../DefaultRelationshipDescription.java | 10 +- .../core/mapping/Neo4jMappingContext.java | 2 +- .../core/mapping/Neo4jPersistentEntity.java | 2 + .../core/mapping/Neo4jPersistentProperty.java | 5 +- .../neo4j/core/schema/CypherGenerator.java | 14 +- .../core/schema/RelationshipDescription.java | 2 +- .../data/neo4j/core/schema/TargetNode.java | 33 ++++ .../neo4j/repository/event/IdPopulator.java | 4 + .../repository/query/CypherQueryCreator.java | 33 ++-- .../integration/imperative/RepositoryIT.java | 152 ++++++++++++++++-- .../imperative/TypeConversionIT.java | 2 +- .../reactive/ReactiveRepositoryIT.java | 66 ++++++-- .../neo4j/integration/shared/AltHobby.java | 8 +- .../shared/AltLikedByPersonRelationship.java | 31 ++++ .../neo4j/integration/shared/AltPerson.java | 19 +++ .../data/neo4j/integration/shared/Friend.java | 48 ++++++ .../shared/FriendshipRelationship.java | 47 ++++++ .../shared/LikesHobbyRelationship.java | 12 ++ .../PersonWithRelationshipWithProperties.java | 19 ++- .../shared/WorksInClubRelationship.java | 47 ++++++ 30 files changed, 698 insertions(+), 123 deletions(-) create mode 100644 etc/adr/adr-005.adoc create mode 100644 etc/adr/adr-006.adoc rename src/main/java/org/springframework/data/neo4j/core/{support/Relationships.java => MappingSupport.java} (73%) create mode 100644 src/main/java/org/springframework/data/neo4j/core/schema/TargetNode.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/shared/Friend.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/shared/FriendshipRelationship.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/shared/WorksInClubRelationship.java diff --git a/etc/adr/adr-005.adoc b/etc/adr/adr-005.adoc new file mode 100644 index 000000000..34ebe8b1e --- /dev/null +++ b/etc/adr/adr-005.adoc @@ -0,0 +1,20 @@ +== ADR 5: Change relationship with properties from association to property + +=== Status + +rejected + +=== Context + +A definition of relationship with properties pointing to the current `@RelationshipProperties` annotated property class creates a Spring Data association. +Because the annotated class is not an entity this should not be an association in the first place but an entity property. +The contextual problem came up in the discussion of the refactoring of the relationship with properties (ADR-6). + +=== Decision + +Treat relationship with properties as properties on the defining entity. + + +=== Consequences + +Requires refactoring of internal schema creation and mapping. There should not be any consequences user-facing. diff --git a/etc/adr/adr-006.adoc b/etc/adr/adr-006.adoc new file mode 100644 index 000000000..515c8f83e --- /dev/null +++ b/etc/adr/adr-006.adoc @@ -0,0 +1,20 @@ +== ADR 6: Refactor definition of relationship with properties + +=== Status + +accepted + +=== Context + +The current implementation of relationship with properties has two major flaws: + +. No possibility to create derived finders reaching into the target entity. +. Impossible to create two relationships of the same type connecting the same target node. + +=== Decision + +Don't use the format of the `Map` but instead define the target node within the `@RelationshipProperties` class. + +=== Consequences + +Users will have to refactor the existing `@RelationshipProperties` based relationships. diff --git a/src/main/java/org/springframework/data/neo4j/core/support/Relationships.java b/src/main/java/org/springframework/data/neo4j/core/MappingSupport.java similarity index 73% rename from src/main/java/org/springframework/data/neo4j/core/support/Relationships.java rename to src/main/java/org/springframework/data/neo4j/core/MappingSupport.java index b6bd67779..a413716ef 100644 --- a/src/main/java/org/springframework/data/neo4j/core/support/Relationships.java +++ b/src/main/java/org/springframework/data/neo4j/core/MappingSupport.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.neo4j.core.support; +package org.springframework.data.neo4j.core; import java.util.AbstractMap.SimpleEntry; import java.util.Collection; @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.Map; import java.util.stream.Collectors; +import org.apiguardian.api.API; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; import org.springframework.lang.Nullable; @@ -28,7 +29,7 @@ import org.springframework.lang.Nullable; * @author Michael J. Simons * @author Philipp Tölle */ -public final class Relationships { +final class MappingSupport { /** * The value for a relationship can be a scalar object (1:1), a collection (1:n), a map (1:n, but with dynamic @@ -51,7 +52,7 @@ public final class Relationships { unifiedValue = ((Map) rawValue).entrySet(); } } else if (property.isRelationshipWithProperties()) { - unifiedValue = ((Map) rawValue).entrySet(); + unifiedValue = (Collection) rawValue; } else if (property.isCollectionLike()) { unifiedValue = (Collection) rawValue; } else { @@ -60,5 +61,28 @@ public final class Relationships { return unifiedValue; } - private Relationships() {} + private MappingSupport() {} + + /** + * Class that defines a tuple of relationship with properties and the connected target entity. + */ + @API(status = API.Status.INTERNAL) + final static class RelationshipPropertiesWithEntityHolder { + private final Object relationshipProperties; + private final Object relatedEntity; + + RelationshipPropertiesWithEntityHolder(Object relationshipProperties, Object relatedEntity) { + this.relationshipProperties = relationshipProperties; + this.relatedEntity = relatedEntity; + } + + Object getRelationshipProperties() { + return relationshipProperties; + } + + Object getRelatedEntity() { + return relatedEntity; + } + } + } diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java index 47068835a..397dcfe26 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -53,7 +53,6 @@ import org.springframework.data.neo4j.core.schema.Constants; import org.springframework.data.neo4j.core.schema.CypherGenerator; import org.springframework.data.neo4j.core.schema.NodeDescription; import org.springframework.data.neo4j.core.schema.RelationshipDescription; -import org.springframework.data.neo4j.core.support.Relationships; import org.springframework.data.neo4j.repository.NoResultException; import org.springframework.data.neo4j.repository.event.BeforeBindCallback; import org.springframework.data.util.ClassTypeInformation; @@ -404,7 +403,7 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware { NestedRelationshipContext relationshipContext = NestedRelationshipContext.of(association, propertyAccessor, neo4jPersistentEntity); - Collection relatedValuesToStore = Relationships.unifyRelationshipValue(relationshipContext.getInverse(), + Collection relatedValuesToStore = MappingSupport.unifyRelationshipValue(relationshipContext.getInverse(), relationshipContext.getValue()); RelationshipDescription relationshipDescription = relationshipContext.getRelationship(); @@ -439,13 +438,13 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware { for (Object relatedValueToStore : relatedValuesToStore) { // here map entry is not always anymore a dynamic association - Object valueToBeSavedPreEvt = relationshipContext.identifyAndExtractRelationshipValue(relatedValueToStore); - valueToBeSavedPreEvt = eventSupport.maybeCallBeforeBind(valueToBeSavedPreEvt); + Object relatedNode = relationshipContext.identifyAndExtractRelationshipTargetNode(relatedValueToStore); + relatedNode = eventSupport.maybeCallBeforeBind(relatedNode); Neo4jPersistentEntity targetNodeDescription = neo4jMappingContext - .getPersistentEntity(valueToBeSavedPreEvt.getClass()); + .getPersistentEntity(relatedNode.getClass()); - Long relatedInternalId = saveRelatedNode(valueToBeSavedPreEvt, relationshipContext.getAssociationTargetType(), + Long relatedInternalId = saveRelatedNode(relatedNode, relationshipContext.getAssociationTargetType(), targetNodeDescription, inDatabase); RelationshipStatementHolder statementHolder = RelationshipStatementHolder.createStatement(neo4jMappingContext, @@ -458,11 +457,11 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware { // if an internal id is used this must get set to link this entity in the next iteration if (targetNodeDescription.isUsingInternalIds()) { PersistentPropertyAccessor targetPropertyAccessor = targetNodeDescription - .getPropertyAccessor(valueToBeSavedPreEvt); + .getPropertyAccessor(relatedNode); targetPropertyAccessor.setProperty(targetNodeDescription.getRequiredIdProperty(), relatedInternalId); } if (processState != ProcessState.PROCESSED_ALL_VALUES) { - processNestedRelations(targetNodeDescription, valueToBeSavedPreEvt, inDatabase, stateMachine); + processNestedRelations(targetNodeDescription, relatedNode, inDatabase, stateMachine); } } }); diff --git a/src/main/java/org/springframework/data/neo4j/core/NestedRelationshipContext.java b/src/main/java/org/springframework/data/neo4j/core/NestedRelationshipContext.java index b4abcfcfe..9a4676391 100644 --- a/src/main/java/org/springframework/data/neo4j/core/NestedRelationshipContext.java +++ b/src/main/java/org/springframework/data/neo4j/core/NestedRelationshipContext.java @@ -15,6 +15,9 @@ */ package org.springframework.data.neo4j.core; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.Map; import org.springframework.data.mapping.Association; @@ -22,6 +25,7 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; import org.springframework.data.neo4j.core.schema.RelationshipDescription; +import org.springframework.data.neo4j.core.schema.TargetNode; import org.springframework.lang.Nullable; /** @@ -75,17 +79,19 @@ final class NestedRelationshipContext { return this.relationship.hasRelationshipProperties(); } - Object identifyAndExtractRelationshipValue(Object relatedValue) { + Object identifyAndExtractRelationshipTargetNode(Object relatedValue) { Object valueToBeSaved = relatedValue; if (relatedValue instanceof Map.Entry) { Map.Entry relatedValueMapEntry = (Map.Entry) relatedValue; if (this.getInverse().isDynamicAssociation()) { valueToBeSaved = relatedValueMapEntry.getValue(); - } else if (this.hasRelationshipWithProperties()) { - valueToBeSaved = relatedValueMapEntry.getKey(); } } + if (this.hasRelationshipWithProperties()) { + // here comes the entity + valueToBeSaved = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValue).getRelatedEntity(); + } return valueToBeSaved; } @@ -96,15 +102,38 @@ final class NestedRelationshipContext { Neo4jPersistentProperty inverse = handler.getInverse(); boolean inverseValueIsEmpty = propertyAccessor.getProperty(inverse) == null; + // value can be a collection or scalar of related notes, point to a relationship property (scalar or collection) + // or is a dynamic relationship (map) Object value = propertyAccessor.getProperty(inverse); RelationshipDescription relationship = neo4jPersistentEntity.getRelationships().stream() .filter(r -> r.getFieldName().equals(inverse.getName())).findFirst().get(); // if we have a relationship with properties, the targetNodeType is the map key - Class associationTargetType = relationship.hasRelationshipProperties() ? inverse.getComponentType() - : inverse.getAssociationTargetType(); + Class associationTargetType = inverse.getAssociationTargetType(); + + if (relationship.hasRelationshipProperties() && value != null) { + Neo4jPersistentEntity relationshipPropertiesEntity = (Neo4jPersistentEntity) relationship.getRelationshipPropertiesEntity(); + + List relationshipProperties = new ArrayList<>(); + + for (Object relationshipProperty : ((Collection) value)) { + + MappingSupport.RelationshipPropertiesWithEntityHolder oneOfThem = + new MappingSupport.RelationshipPropertiesWithEntityHolder(relationshipProperty, + getTargetNode(relationshipPropertiesEntity, relationshipProperty)); + relationshipProperties.add(oneOfThem); + } + value = relationshipProperties; + } return new NestedRelationshipContext(inverse, value, relationship, associationTargetType, inverseValueIsEmpty); } + + private static Object getTargetNode(Neo4jPersistentEntity relationshipPropertiesEntity, Object object) { + + PersistentPropertyAccessor propertyAccessor = relationshipPropertiesEntity.getPropertyAccessor(object); + return propertyAccessor.getProperty(relationshipPropertiesEntity.getPersistentProperty(TargetNode.class)); + + } } diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java index ad843a089..9dac6e0ca 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -57,7 +57,6 @@ import org.springframework.data.neo4j.core.schema.Constants; import org.springframework.data.neo4j.core.schema.CypherGenerator; import org.springframework.data.neo4j.core.schema.NodeDescription; import org.springframework.data.neo4j.core.schema.RelationshipDescription; -import org.springframework.data.neo4j.core.support.Relationships; import org.springframework.data.neo4j.repository.event.ReactiveBeforeBindCallback; import org.springframework.data.util.ClassTypeInformation; import org.springframework.lang.Nullable; @@ -402,7 +401,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Bea NestedRelationshipContext relationshipContext = NestedRelationshipContext.of(association, propertyAccessor, neo4jPersistentEntity); - Collection relatedValuesToStore = Relationships.unifyRelationshipValue(relationshipContext.getInverse(), + Collection relatedValuesToStore = MappingSupport.unifyRelationshipValue(relationshipContext.getInverse(), relationshipContext.getValue()); RelationshipDescription relationshipDescription = relationshipContext.getRelationship(); @@ -436,7 +435,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Bea for (Object relatedValueToStore : relatedValuesToStore) { - Object valueToBeSavedPreEvt = relationshipContext.identifyAndExtractRelationshipValue(relatedValueToStore); + Object valueToBeSavedPreEvt = relationshipContext.identifyAndExtractRelationshipTargetNode(relatedValueToStore); Mono createRelationship = eventSupport.maybeCallBeforeBind(valueToBeSavedPreEvt) .flatMap(valueToBeSaved -> { diff --git a/src/main/java/org/springframework/data/neo4j/core/RelationshipStatementHolder.java b/src/main/java/org/springframework/data/neo4j/core/RelationshipStatementHolder.java index 2db5554ee..9b0fd97a7 100644 --- a/src/main/java/org/springframework/data/neo4j/core/RelationshipStatementHolder.java +++ b/src/main/java/org/springframework/data/neo4j/core/RelationshipStatementHolder.java @@ -64,7 +64,7 @@ final class RelationshipStatementHolder { if (relationshipContext.hasRelationshipWithProperties()) { return createStatementForRelationShipWithProperties(neo4jMappingContext, neo4jPersistentEntity, - relationshipContext, relatedInternalId, (Map.Entry) relatedValue); + relationshipContext, relatedInternalId, (MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValue); } else { return createStatementForRelationshipWithoutProperties(neo4jMappingContext, neo4jPersistentEntity, relationshipContext, relatedInternalId, relatedValue); @@ -73,12 +73,13 @@ final class RelationshipStatementHolder { private static RelationshipStatementHolder createStatementForRelationShipWithProperties( Neo4jMappingContext neo4jMappingContext, Neo4jPersistentEntity neo4jPersistentEntity, - NestedRelationshipContext relationshipContext, Long relatedInternalId, Map.Entry relatedValue) { + NestedRelationshipContext relationshipContext, Long relatedInternalId, MappingSupport.RelationshipPropertiesWithEntityHolder relatedValue) { Statement relationshipCreationQuery = CypherGenerator.INSTANCE.createRelationshipWithPropertiesCreationQuery( neo4jPersistentEntity, relationshipContext.getRelationship(), relatedInternalId); Map propMap = new HashMap<>(); - neo4jMappingContext.getConverter().write(relatedValue.getValue(), propMap); + // write relationship properties + neo4jMappingContext.getConverter().write(relatedValue.getRelationshipProperties(), propMap); return new RelationshipStatementHolder(relationshipCreationQuery, propMap); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConverter.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConverter.java index 2dbf52d4c..b165b57b9 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConverter.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConverter.java @@ -57,6 +57,7 @@ import org.springframework.data.neo4j.core.convert.Neo4jConversions; import org.springframework.data.neo4j.core.convert.Neo4jConverter; import org.springframework.data.neo4j.core.schema.Constants; import org.springframework.data.neo4j.core.schema.RelationshipDescription; +import org.springframework.data.neo4j.core.schema.TargetNode; import org.springframework.data.util.TypeInformation; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; @@ -179,7 +180,7 @@ final class DefaultNeo4jConverter implements Neo4jConverter { nodeDescription.doWithProperties((Neo4jPersistentProperty p) -> { // Skip the internal properties, we don't want them to end up stored as properties - if (p.isInternalIdProperty() || p.isDynamicLabels()) { + if (p.isInternalIdProperty() || p.isDynamicLabels() || p.isEntity()) { return; } @@ -255,6 +256,19 @@ final class DefaultNeo4jConverter implements Neo4jConverter { * @return */ private ET map(MapAccessor queryResult, Neo4jPersistentEntity nodeDescription, KnownObjects knownObjects) { + return map(queryResult, nodeDescription, knownObjects, null); + } + + /** + * @param queryResult The original query result + * @param nodeDescription The node description of the current entity to be mapped from the result + * @param knownObjects The current list of known objects + * @param lastMappedEntity Previous created entity for relationships, can be null. + * @param As in entity type + * @return + */ + private ET map(MapAccessor queryResult, Neo4jPersistentEntity nodeDescription, KnownObjects knownObjects, + @Nullable Object lastMappedEntity) { List allLabels = getLabels(queryResult); NodeDescriptionAndLabels nodeDescriptionAndLabels = nodeDescriptionStore @@ -274,8 +288,8 @@ final class DefaultNeo4jConverter implements Neo4jConverter { // Fill simple properties Predicate isConstructorParameter = concreteNodeDescription .getPersistenceConstructor()::isConstructorParameter; - PropertyHandler handler = populateFrom(queryResult, propertyAccessor, - isConstructorParameter, nodeDescriptionAndLabels.getDynamicLabels()); + PropertyHandler handler = populateFrom(queryResult, knownObjects, propertyAccessor, + isConstructorParameter, nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity); concreteNodeDescription.doWithProperties(handler); // Fill associations @@ -325,9 +339,9 @@ final class DefaultNeo4jConverter implements Neo4jConverter { return INSTANTIATORS.getInstantiatorFor(nodeDescription).createInstance(nodeDescription, parameterValueProvider); } - private PropertyHandler populateFrom(MapAccessor queryResult, + private PropertyHandler populateFrom(MapAccessor queryResult, KnownObjects knownObjects, PersistentPropertyAccessor propertyAccessor, Predicate isConstructorParameter, - Collection surplusLabels) { + Collection surplusLabels, Object targetNode) { return property -> { if (isConstructorParameter.test(property)) { return; @@ -336,6 +350,10 @@ final class DefaultNeo4jConverter implements Neo4jConverter { if (property.isDynamicLabels()) { propertyAccessor.setProperty(property, createDynamicLabelsProperty(property.getTypeInformation(), surplusLabels)); + } else if (property.isAnnotationPresent(TargetNode.class)) { + if (queryResult instanceof Relationship) { + propertyAccessor.setProperty(property, targetNode); + } } else { propertyAccessor.setProperty(property, readValueForProperty(extractValueOf(property, queryResult), property.getTypeInformation())); @@ -402,7 +420,7 @@ final class DefaultNeo4jConverter implements Neo4jConverter { Value list = values.get(relationshipDescription.generateRelatedNodesCollectionName()); - Map relationshipsAndProperties = new HashMap<>(); + List relationshipsAndProperties = new ArrayList<>(); // if the list is null the mapping is based on a custom query if (Values.NULL.equals(list)) { @@ -436,42 +454,37 @@ final class DefaultNeo4jConverter implements Neo4jConverter { for (Relationship possibleRelationship : allMatchingTypeRelationshipsInResult) { if (targetIdSelector.apply(possibleRelationship) == nodeId) { - Object mappedObject = map(possibleValueNode, concreteTargetNodeDescription, knownObjects); + Object mappedObject = knownObjects.computeIfAbsent(nodeId, () -> map(possibleValueNode, concreteTargetNodeDescription, knownObjects)); if (relationshipDescription.hasRelationshipProperties()) { - Class propertiesClass = relationshipDescription.getRelationshipPropertiesClass(); - Object relationshipProperties = map(possibleRelationship, - (Neo4jPersistentEntity) nodeDescriptionStore.getNodeDescription(propertiesClass), knownObjects); - relationshipsAndProperties.put(mappedObject, relationshipProperties); + (Neo4jPersistentEntity) relationshipDescription.getRelationshipPropertiesEntity(), + knownObjects, mappedObject); + relationshipsAndProperties.add(relationshipProperties); } else { mappedObjectHandler.accept(possibleRelationship.type(), mappedObject); } + allMatchingTypeRelationshipsInResult.remove(possibleRelationship); break; } } } } else { for (Value relatedEntity : list.asList(Function.identity())) { - Neo4jPersistentProperty idProperty = concreteTargetNodeDescription.getRequiredIdProperty(); - // internal (generated) id or external set - String relatedEntityIdKey = idProperty.isInternalIdProperty() ? Constants.NAME_OF_INTERNAL_ID - : concreteTargetNodeDescription.getIdDescription().getOptionalGraphPropertyName() - .orElse(idProperty.getName()); - Object idValue = relatedEntity.get(relatedEntityIdKey); + Long internalIdValue = relatedEntity.get(Constants.NAME_OF_INTERNAL_ID).asLong(); - Object valueEntry = knownObjects.computeIfAbsent(idValue, + Object valueEntry = knownObjects.computeIfAbsent(internalIdValue, () -> map(relatedEntity, concreteTargetNodeDescription, knownObjects)); if (relationshipDescription.hasRelationshipProperties()) { Relationship relatedEntityRelationship = relatedEntity.get(RelationshipDescription.NAME_OF_RELATIONSHIP) .asRelationship(); - Class propertiesClass = relationshipDescription.getRelationshipPropertiesClass(); Object relationshipProperties = map(relatedEntityRelationship, - (Neo4jPersistentEntity) nodeDescriptionStore.getNodeDescription(propertiesClass), knownObjects); - relationshipsAndProperties.put(valueEntry, relationshipProperties); + (Neo4jPersistentEntity) relationshipDescription.getRelationshipPropertiesEntity(), + knownObjects, valueEntry); + relationshipsAndProperties.add(relationshipProperties); } else { mappedObjectHandler.accept(relatedEntity.get(RelationshipDescription.NAME_OF_RELATIONSHIP_TYPE).asString(), valueEntry); @@ -480,7 +493,9 @@ final class DefaultNeo4jConverter implements Neo4jConverter { } if (persistentProperty.getTypeInformation().isCollectionLike()) { - if (persistentProperty.getType().equals(Set.class)) { + if (relationshipDescription.hasRelationshipProperties()) { + return Optional.of(relationshipsAndProperties); + } else if (persistentProperty.getType().equals(Set.class)) { return Optional.of(new HashSet(value)); } else { return Optional.of(value); @@ -489,7 +504,7 @@ final class DefaultNeo4jConverter implements Neo4jConverter { if (relationshipDescription.isDynamic()) { return Optional.ofNullable(dynamicValue.isEmpty() ? null : dynamicValue); } else if (relationshipDescription.hasRelationshipProperties()) { - return Optional.of(relationshipsAndProperties); + return Optional.ofNullable(relationshipsAndProperties.isEmpty() ? null : relationshipsAndProperties.get(0)); } else { return Optional.ofNullable(value.isEmpty() ? null : value.get(0)); } @@ -513,14 +528,33 @@ final class DefaultNeo4jConverter implements Neo4jConverter { private final Lock read = lock.readLock(); private final Lock write = lock.writeLock(); - private Map store = new HashMap<>(); + private final Map internalIdStore = new HashMap<>(); - Object computeIfAbsent(Object key, Supplier entitySupplier) { + Object computeIfAbsent(Long internalId, Supplier entitySupplier) { + Object knownEntity = getObject(internalId); + + // if it is not in the store, it has to get re-computed also for the internalIdStore + if (knownEntity != null) { + return knownEntity; + } + + try { + write.lock(); + Object computedEntity = entitySupplier.get(); + internalIdStore.put(internalId, computedEntity); + return computedEntity; + } finally { + write.unlock(); + } + } + + @Nullable + private Object getObject(Long internalId) { try { read.lock(); - Object knownEntity = store.get(key); + Object knownEntity = internalIdStore.get(internalId); if (knownEntity != null) { return knownEntity; @@ -529,15 +563,7 @@ final class DefaultNeo4jConverter implements Neo4jConverter { } finally { read.unlock(); } - - try { - write.lock(); - Object computedEntity = entitySupplier.get(); - store.put(key, computedEntity); - return computedEntity; - } finally { - write.unlock(); - } + return null; } } } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java index 411e5bee9..51c393e1f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java @@ -42,6 +42,7 @@ import org.springframework.data.neo4j.core.schema.NodeDescription; import org.springframework.data.neo4j.core.schema.Property; import org.springframework.data.neo4j.core.schema.Relationship; import org.springframework.data.neo4j.core.schema.RelationshipDescription; +import org.springframework.data.neo4j.core.schema.RelationshipProperties; import org.springframework.data.support.IsNewStrategy; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; @@ -87,6 +88,8 @@ class DefaultNeo4jPersistentEntity extends BasicPersistentEntity dynamicLabelsProperty; + private final Lazy isRelationshipPropertiesEntity; + DefaultNeo4jPersistentEntity(TypeInformation information) { super(information); @@ -96,6 +99,7 @@ class DefaultNeo4jPersistentEntity extends BasicPersistentEntity getGraphProperties().stream().map(Neo4jPersistentProperty.class::cast) .filter(Neo4jPersistentProperty::isDynamicLabels).findFirst().orElse(null)); + this.isRelationshipPropertiesEntity = Lazy.of(() -> isAnnotationPresent(RelationshipProperties.class)); } /* @@ -154,6 +158,11 @@ class DefaultNeo4jPersistentEntity extends BasicPersistentEntity graphPropertyName; private final Lazy isAssociation; + private final boolean isEntityInRelationshipWithProperties; private final Neo4jMappingContext mappingContext; @@ -52,15 +54,16 @@ class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProperty owner, - Neo4jMappingContext mappingContext, SimpleTypeHolder simpleTypeHolder) { + Neo4jMappingContext mappingContext, SimpleTypeHolder simpleTypeHolder, boolean isEntityInRelationshipWithProperties) { super(property, owner, simpleTypeHolder); + this.isEntityInRelationshipWithProperties = isEntityInRelationshipWithProperties; this.graphPropertyName = Lazy.of(this::computeGraphPropertyName); this.isAssociation = Lazy.of(() -> { Class targetType = getActualType(); - return !(simpleTypeHolder.isSimpleType(targetType) || mappingContext.hasCustomWriteTarget(targetType)); + return !(simpleTypeHolder.isSimpleType(targetType) || mappingContext.hasCustomWriteTarget(targetType) || isAnnotationPresent(TargetNode.class)); }); this.mappingContext = mappingContext; } @@ -72,7 +75,8 @@ class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProperty type = this.mappingContext.getPersistentEntity(getActualType()).getPersistentProperty(TargetNode.class).getType(); + obverseOwner = this.mappingContext.getPersistentEntity(type); } else { obverseOwner = this.mappingContext.getPersistentEntity(this.getAssociationTargetType()); } @@ -95,7 +99,10 @@ class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProperty relationshipPropertiesClass = dynamicAssociation ? null : getMapValueType(); + Neo4jPersistentEntity relationshipPropertiesClass = + this.hasActualTypeAnnotation(RelationshipProperties.class) + ? this.mappingContext.getPersistentEntity(getActualType()) + : null; // Try to determine if there is a relationship definition that expresses logically the same relationship // on the other end. @@ -135,7 +142,12 @@ class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProperty relationshipPropertiesClass; + private final NodeDescription relationshipPropertiesClass; private RelationshipDescription relationshipObverse; DefaultRelationshipDescription(Neo4jPersistentProperty inverse, @Nullable RelationshipDescription relationshipObverse, String type, boolean dynamic, NodeDescription source, String fieldName, NodeDescription target, - Relationship.Direction direction, @Nullable Class relationshipPropertiesClass) { + Relationship.Direction direction, @Nullable NodeDescription relationshipProperties) { // the immutable obverse association-wise is always null because we cannot determine them on both sides // if we consider to support bidirectional relationships. @@ -61,7 +61,7 @@ class DefaultRelationshipDescription extends Association getRelationshipPropertiesClass() { + public NodeDescription getRelationshipPropertiesEntity() { return relationshipPropertiesClass; } @Override public boolean hasRelationshipProperties() { - return getRelationshipPropertiesClass() != null; + return getRelationshipPropertiesEntity() != null; } @Override diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java index dcef2eb34..2fa546343 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java @@ -182,7 +182,7 @@ public final class Neo4jMappingContext extends AbstractMappingContext owner, SimpleTypeHolder simpleTypeHolder) { - return new DefaultNeo4jPersistentProperty(property, owner, this, simpleTypeHolder); + return new DefaultNeo4jPersistentProperty(property, owner, this, simpleTypeHolder, owner.isRelationshipPropertiesEntity()); } @Override diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java index 3b3ee08de..aaba7cc12 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java @@ -42,4 +42,6 @@ public interface Neo4jPersistentEntity * "runtime managed" labels. */ Optional getDynamicLabelsProperty(); + + boolean isRelationshipPropertiesEntity(); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java index 9094970bd..33ed4563f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java @@ -68,7 +68,8 @@ public interface Neo4jPersistentProperty extends PersistentProperty additionalLabels = nodeDescription.getAdditionalLabels(); Node rootNode = node(primaryLabel, additionalLabels).named(Constants.NAME_OF_ROOT_NODE); - IdDescription idDescription = nodeDescription.getIdDescription(); List expressions = new ArrayList<>(); expressions.add(Constants.NAME_OF_ROOT_NODE); - if (idDescription.isInternallyGeneratedId()) { - expressions.add(Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID)); - } + expressions.add(Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID)); + return match(rootNode).where(conditionOrNoCondition(condition)).with(expressions.toArray(new Expression[] {})); } @@ -372,17 +370,15 @@ public enum CypherGenerator { continue; } - if (property.isInternalIdProperty()) { - nodePropertiesProjection.add(Constants.NAME_OF_INTERNAL_ID); - nodePropertiesProjection.add(Functions.id(node)); - } else if (!((Neo4jPersistentProperty) property).isDynamicLabels()) { + if (!((Neo4jPersistentProperty) property).isDynamicLabels()) { nodePropertiesProjection.add(property.getPropertyName()); } } nodePropertiesProjection.add(Constants.NAME_OF_LABELS); nodePropertiesProjection.add(Functions.labels(node)); - + nodePropertiesProjection.add(Constants.NAME_OF_INTERNAL_ID); + nodePropertiesProjection.add(Functions.id(node)); return nodePropertiesProjection; } diff --git a/src/main/java/org/springframework/data/neo4j/core/schema/RelationshipDescription.java b/src/main/java/org/springframework/data/neo4j/core/schema/RelationshipDescription.java index 19d169902..38abbbb7f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/schema/RelationshipDescription.java +++ b/src/main/java/org/springframework/data/neo4j/core/schema/RelationshipDescription.java @@ -85,7 +85,7 @@ public interface RelationshipDescription { * @return The type of the relationship property class for relationship with properties, otherwise {@literal null} */ @Nullable - Class getRelationshipPropertiesClass(); + NodeDescription getRelationshipPropertiesEntity(); /** * Tells if this relationship is a relationship with additional properties. In such cases diff --git a/src/main/java/org/springframework/data/neo4j/core/schema/TargetNode.java b/src/main/java/org/springframework/data/neo4j/core/schema/TargetNode.java new file mode 100644 index 000000000..2a7377593 --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/schema/TargetNode.java @@ -0,0 +1,33 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * 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 + * + * https://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.springframework.data.neo4j.core.schema; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks an entity in a {@link RelationshipProperties} as the target node. + * + * @author Gerrit Meier + * @soundtrack Goldfinger - Here in your bedroom + * @since 6.0 + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.FIELD) +public @interface TargetNode { +} diff --git a/src/main/java/org/springframework/data/neo4j/repository/event/IdPopulator.java b/src/main/java/org/springframework/data/neo4j/repository/event/IdPopulator.java index d0f23277c..4416c6a33 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/event/IdPopulator.java +++ b/src/main/java/org/springframework/data/neo4j/repository/event/IdPopulator.java @@ -48,6 +48,10 @@ final class IdPopulator { Neo4jPersistentEntity nodeDescription = neo4jMappingContext.getRequiredPersistentEntity(entity.getClass()); IdDescription idDescription = nodeDescription.getIdDescription(); + if (idDescription == null && nodeDescription.isRelationshipPropertiesEntity()) { + return entity; + } + // Filter in two steps to avoid unnecessary object creation. if (!idDescription.isExternallyGeneratedId()) { return entity; diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java index dbd5ed585..65384f89f 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java @@ -54,16 +54,16 @@ import org.springframework.data.geo.Box; import org.springframework.data.geo.Circle; import org.springframework.data.geo.Distance; import org.springframework.data.geo.Polygon; -import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext; +import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; import org.springframework.data.neo4j.core.schema.Constants; import org.springframework.data.neo4j.core.schema.CypherGenerator; import org.springframework.data.neo4j.core.schema.NodeDescription; import org.springframework.data.neo4j.core.schema.RelationshipDescription; -import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.parser.AbstractQueryCreator; import org.springframework.data.repository.query.parser.Part; @@ -186,29 +186,32 @@ final class CypherQueryCreator extends AbstractQueryCreator relationshipPropertiesEntity = relationshipDescription.getRelationshipPropertiesEntity(); + boolean hasTargetNode = hasTargetNode(relationshipPropertiesEntity); + NodeDescription targetEntity = relationshipDescription.getTarget(); Node relatedNode = Cypher.node(targetEntity.getPrimaryLabel(), targetEntity.getAdditionalLabels()); boolean lastNode = isLastNode(persistentProperty); - if (lastNode) { + if (lastNode || hasTargetNode) { relatedNode = relatedNode.named(getNodeName()); } switch (relationshipDescription.getDirection()) { case OUTGOING: - cypherRelationship = (RelationshipPattern) cypherRelationship.relationshipTo(relatedNode, - relationshipDescription.getType()); + cypherRelationship = cypherRelationship + .relationshipTo(relatedNode, relationshipDescription.getType()); break; case INCOMING: - cypherRelationship = (RelationshipPattern) cypherRelationship.relationshipFrom(relatedNode, - relationshipDescription.getType()); + cypherRelationship = cypherRelationship + .relationshipFrom(relatedNode, relationshipDescription.getType()); break; default: - cypherRelationship = (RelationshipPattern) cypherRelationship.relationshipBetween(relatedNode, - relationshipDescription.getType()); + cypherRelationship = cypherRelationship + .relationshipBetween(relatedNode, relationshipDescription.getType()); } - if (lastNode) { + if (lastNode || hasTargetNode) { cypherRelationship = ((RelationshipPattern) cypherRelationship).named(getRelationshipName()); } } @@ -216,6 +219,12 @@ final class CypherQueryCreator extends AbstractQueryCreator relationshipPropertiesEntity) { + return relationshipPropertiesEntity != null + && ((Neo4jPersistentEntity) relationshipPropertiesEntity) + .getPersistentProperty(TargetNode.class) != null; + } + // if there is no direct property access, the list size is greater than 1 and as a consequence has to contain // relationships. private boolean hasRelationships() { @@ -523,7 +532,7 @@ final class CypherQueryCreator extends AbstractQueryCreator owner = persistentProperty.getOwner(); + Neo4jPersistentEntity owner = (Neo4jPersistentEntity) persistentProperty.getOwner(); Expression expression; if (owner.equals(this.nodeDescription)) { @@ -534,7 +543,7 @@ final class CypherQueryCreator extends AbstractQueryCreator hobbies = person.getHobbies(); + assertThat(hobbies).containsExactlyInAnyOrder(rel1, rel2); + assertThat(hobbies.get(hobbies.indexOf(rel1)).getHobby()).isEqualTo(hobby1); + assertThat(hobbies.get(hobbies.indexOf(rel2)).getHobby()).isEqualTo(hobby2); + } + + @Test + void findEntityWithRelationshipWithPropertiesScalar( + @Autowired PersonWithRelationshipWithPropertiesRepository repository) { + + long personId; + + try (Session session = createSession()) { + Record record = session.run("CREATE (n:PersonWithRelationshipWithProperties{name:'Freddie'})," + + " (n)-[:WORKS_IN{since: 1995}]->(:Club{name:'Blubb'})" + + "RETURN n").single(); + + Node personNode = record.get("n").asNode(); + personId = personNode.id(); + } + + PersonWithRelationshipWithProperties person = repository.findById(personId).get(); + + WorksInClubRelationship loadedRelationship = person.getClub(); + assertThat(loadedRelationship.getSince()).isEqualTo(1995); + assertThat(loadedRelationship.getClub().getName()).isEqualTo("Blubb"); + } + + @Test + void findEntityWithRelationshipWithPropertiesSameLabel( + @Autowired FriendRepository repository) { + + long friendId; + + try (Session session = createSession()) { + Record record = session.run("CREATE (n:Friend{name:'Freddie'})," + + " (n)-[:KNOWS{since: 1995}]->(:Friend{name:'Frank'})" + + "RETURN n").single(); + + Node friendNode = record.get("n").asNode(); + friendId = friendNode.id(); + } + + Friend person = repository.findById(friendId).get(); + + List loadedRelationship = person.getFriends(); + assertThat(loadedRelationship).allSatisfy(relationship -> { + assertThat(relationship.getSince()).isEqualTo(1995); + assertThat(relationship.getFriend().getName()).isEqualTo("Frank"); + }); } @Test @@ -982,6 +1036,7 @@ class RepositoryIT { rel1.setLocalDate(rel1LocalDate); rel1.setMyEnum(rel1MyEnum); rel1.setPoint(rel1Point); + rel1.setHobby(h1); Hobby h2 = new Hobby(); h2.setName("Something else"); @@ -996,18 +1051,19 @@ class RepositoryIT { rel2.setLocalDate(rel2LocalDate); rel2.setMyEnum(rel2MyEnum); rel2.setPoint(rel2Point); + rel2.setHobby(h2); - Map hobbies = new HashMap<>(); - hobbies.put(h1, rel1); - hobbies.put(h2, rel2); - PersonWithRelationshipWithProperties clonePerson = new PersonWithRelationshipWithProperties("Freddie clone"); - clonePerson.setHobbies(hobbies); + List hobbies = new ArrayList<>(); + PersonWithRelationshipWithProperties person = new PersonWithRelationshipWithProperties("Freddie clone"); + hobbies.add(rel1); + hobbies.add(rel2); + person.setHobbies(hobbies); // when - PersonWithRelationshipWithProperties shouldBeDifferentPerson = repository.save(clonePerson); + PersonWithRelationshipWithProperties shouldBeDifferentPerson = repository.save(person); // then - assertThat(shouldBeDifferentPerson).isNotNull().isEqualToComparingOnlyGivenFields(clonePerson, "hobbies"); + assertThat(shouldBeDifferentPerson).isNotNull().isEqualToComparingOnlyGivenFields(person, "hobbies"); assertThat(shouldBeDifferentPerson.getName()).isEqualToIgnoringCase("Freddie clone"); @@ -1072,6 +1128,7 @@ class RepositoryIT { rel1.setLocalDate(LocalDate.of(1995, 2, 26)); rel1.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING); rel1.setPoint(new CartesianPoint2d(0d, 1d)); + rel1.setHobby(hobby1); Hobby hobby2 = new Hobby(); hobby2.setName("Something else"); @@ -1081,8 +1138,62 @@ class RepositoryIT { rel2.setLocalDate(LocalDate.of(2000, 6, 28)); rel2.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING_DIFFERENT); rel2.setPoint(new CartesianPoint2d(2d, 3d)); + rel2.setHobby(hobby2); - assertThat(person.getHobbies()).contains(MapEntry.entry(hobby1, rel1), MapEntry.entry(hobby2, rel2)); + List hobbies = person.getHobbies(); + assertThat(hobbies).containsExactlyInAnyOrder(rel1, rel2); + assertThat(hobbies.get(hobbies.indexOf(rel1)).getHobby()).isEqualTo(hobby1); + assertThat(hobbies.get(hobbies.indexOf(rel2)).getHobby()).isEqualTo(hobby2); + } + + @Test // DATAGRAPH-1350 + void loadEntityWithRelationshipWithPropertiesFromCustomQueryIncoming( + @Autowired HobbyWithRelationshipWithPropertiesRepository repository) { + + long personId; + + try (Session session = createSession()) { + Record record = session.run("CREATE (n:AltPerson{name:'Freddie'}), (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'}) RETURN n, h1").single(); + personId = record.get("n").asNode().id(); + } + + AltHobby hobby = repository.loadFromCustomQuery(personId); + assertThat(hobby.getName()).isEqualTo("Music"); + assertThat(hobby.getLikedBy()).hasSize(1); + assertThat(hobby.getLikedBy()).first().satisfies(entry -> { + assertThat(entry.getAltPerson().getId()).isEqualTo(personId); + assertThat(entry.getRating()).isEqualTo(5); + }); + } + + @Test + void loadSameNodeWithDoubleRelationship(@Autowired HobbyWithRelationshipWithPropertiesRepository repository) { + long personId; + + try (Session session = createSession()) { + Record record = session.run("CREATE (n:AltPerson{name:'Freddie'})," + + " (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'})," + + " (n)-[l2:LIKES {rating: 1}]->(h1)" + + " RETURN n, h1").single(); + personId = record.get("n").asNode().id(); + } + + AltHobby hobby = repository.loadFromCustomQuery(personId); + assertThat(hobby.getName()).isEqualTo("Music"); + List likedBy = hobby.getLikedBy(); + assertThat(likedBy).hasSize(2); + + AltPerson altPerson = new AltPerson("Freddie"); + altPerson.setId(personId); + AltLikedByPersonRelationship rel1 = new AltLikedByPersonRelationship(); + rel1.setRating(5); + rel1.setAltPerson(altPerson); + + AltLikedByPersonRelationship rel2 = new AltLikedByPersonRelationship(); + rel2.setRating(1); + rel2.setAltPerson(altPerson); + + assertThat(likedBy).containsExactlyInAnyOrder(rel1, rel2); } } @@ -2694,6 +2805,17 @@ class RepositoryIT { assertThat(repository.findByHobbiesSinceAndHobbiesActive(2019, true)).isNull(); assertThat(repository.findByHobbiesSinceAndHobbiesActive(2020, false)).isNull(); } + + @Test + void findByPropertyOnRelationshipWithPropertiesRelatedEntity( + @Autowired PersonWithRelationshipWithPropertiesRepository repository) { + try (Session session = createSession()) { + session.run( + "CREATE (:PersonWithRelationshipWithProperties{name:'Freddie'})-[:LIKES{since: 2020, active: true}]->(:Hobby{name: 'Bowling'})"); + } + + assertThat(repository.findByHobbiesHobbyName("Bowling").getName()).isEqualTo("Freddie"); + } } @Nested @@ -2763,6 +2885,8 @@ class RepositoryIT { PersonWithRelationshipWithProperties findByHobbiesSinceOrHobbiesActive(int since1, boolean active); PersonWithRelationshipWithProperties findByHobbiesSinceAndHobbiesActive(int since1, boolean active); + + PersonWithRelationshipWithProperties findByHobbiesHobbyName(String hobbyName); } interface PetRepository extends Neo4jRepository {} @@ -2828,6 +2952,14 @@ class RepositoryIT { @Param("customType") ThingWithCustomTypes.CustomType customType); } + interface HobbyWithRelationshipWithPropertiesRepository extends Neo4jRepository { + + @Query("MATCH (p:AltPerson)-[l:LIKES]->(h:AltHobby) WHERE id(p) = $personId RETURN h, collect(l), collect(p)") + AltHobby loadFromCustomQuery(@Param("personId") Long personId); + } + + interface FriendRepository extends Neo4jRepository {} + @SpringJUnitConfig(Config.class) static abstract class IntegrationTestBase { diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/TypeConversionIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/TypeConversionIT.java index 55df25562..5470a24fa 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/TypeConversionIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/TypeConversionIT.java @@ -96,7 +96,7 @@ class TypeConversionIT extends Neo4jConversionsITBase { assertThatExceptionOfType(MappingException.class) .isThrownBy(() -> repository.findById(ID_OF_NON_EXISTING_PRIMITIVES_NODE)) .withMessageMatching( - "Error mapping Record<\\{n: \\{__internalNeo4jId__: \\d+, someBoolean: NULL, __nodeLabels__: \\[\"NonExistingPrimitives\"\\]\\}\\}>") + "Error mapping Record<\\{n: \\{__internalNeo4jId__: \\d+, id: NULL, someBoolean: NULL, __nodeLabels__: \\[\"NonExistingPrimitives\"\\]\\}\\}>") .withStackTraceContaining( "org.springframework.dao.TypeMismatchDataAccessException: Could not convert NULL into boolean; nested exception is org.springframework.core.convert.ConversionFailedException: Failed to convert from type [null] to type [boolean] for value 'null'; nested exception is java.lang.IllegalArgumentException: A null value cannot be assigned to a primitive type") .withRootCauseInstanceOf(IllegalArgumentException.class); diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java index e4454aab9..efaa690a1 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.tuple; +import org.springframework.data.neo4j.integration.shared.AltLikedByPersonRelationship; +import org.springframework.data.neo4j.integration.shared.AltPerson; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -37,7 +39,6 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; -import org.assertj.core.data.MapEntry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Tag; @@ -876,6 +877,7 @@ class ReactiveRepositoryIT { rel1.setLocalDate(LocalDate.of(1995, 2, 26)); rel1.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING); rel1.setPoint(new CartesianPoint2d(0d, 1d)); + rel1.setHobby(hobby1); Hobby hobby2 = new Hobby(); hobby2.setName("Something else"); @@ -885,8 +887,12 @@ class ReactiveRepositoryIT { rel2.setLocalDate(LocalDate.of(2000, 6, 28)); rel2.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING_DIFFERENT); rel2.setPoint(new CartesianPoint2d(2d, 3d)); + rel2.setHobby(hobby2); - assertThat(person.getHobbies()).contains(MapEntry.entry(hobby1, rel1), MapEntry.entry(hobby2, rel2)); + List hobbies = person.getHobbies(); + assertThat(hobbies).containsExactlyInAnyOrder(rel1, rel2); + assertThat(hobbies.get(hobbies.indexOf(rel1)).getHobby()).isEqualTo(hobby1); + assertThat(hobbies.get(hobbies.indexOf(rel2)).getHobby()).isEqualTo(hobby2); }).verifyComplete(); } @@ -909,6 +915,7 @@ class ReactiveRepositoryIT { rel1.setLocalDate(rel1LocalDate); rel1.setMyEnum(rel1MyEnum); rel1.setPoint(rel1Point); + rel1.setHobby(h1); Hobby h2 = new Hobby(); h2.setName("Something else"); @@ -923,10 +930,11 @@ class ReactiveRepositoryIT { rel2.setLocalDate(rel2LocalDate); rel2.setMyEnum(rel2MyEnum); rel2.setPoint(rel2Point); + rel2.setHobby(h2); - Map hobbies = new HashMap<>(); - hobbies.put(h1, rel1); - hobbies.put(h2, rel2); + List hobbies = new ArrayList<>(); + hobbies.add(rel1); + hobbies.add(rel2); PersonWithRelationshipWithProperties clonePerson = new PersonWithRelationshipWithProperties("Freddie clone"); clonePerson.setHobbies(hobbies); @@ -1008,6 +1016,7 @@ class ReactiveRepositoryIT { rel1.setLocalDate(LocalDate.of(1995, 2, 26)); rel1.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING); rel1.setPoint(new CartesianPoint2d(0d, 1d)); + rel1.setHobby(hobby1); Hobby hobby2 = new Hobby(); hobby2.setName("Something else"); @@ -1017,15 +1026,19 @@ class ReactiveRepositoryIT { rel2.setLocalDate(LocalDate.of(2000, 6, 28)); rel2.setMyEnum(LikesHobbyRelationship.MyEnum.SOMETHING_DIFFERENT); rel2.setPoint(new CartesianPoint2d(2d, 3d)); + rel2.setHobby(hobby2); - assertThat(person.getHobbies()).contains(MapEntry.entry(hobby1, rel1), MapEntry.entry(hobby2, rel2)); + List hobbies = person.getHobbies(); + assertThat(hobbies).containsExactlyInAnyOrder(rel1, rel2); + assertThat(hobbies.get(hobbies.indexOf(rel1)).getHobby()).isEqualTo(hobby1); + assertThat(hobbies.get(hobbies.indexOf(rel2)).getHobby()).isEqualTo(hobby2); }).verifyComplete(); } @Test // DATAGRAPH-1350 void loadEntityWithRelationshipWithPropertiesFromCustomQueryIncoming( - @Autowired ReactiveHobbyithRelationshipWithPropertiesRepository repository) { + @Autowired ReactiveHobbyWithRelationshipWithPropertiesRepository repository) { long personId; @@ -1037,12 +1050,43 @@ class ReactiveRepositoryIT { StepVerifier.create(repository.loadFromCustomQuery(personId)).assertNext(hobby -> { assertThat(hobby.getName()).isEqualTo("Music"); assertThat(hobby.getLikedBy()).hasSize(1); - assertThat(hobby.getLikedBy().entrySet()).first().satisfies(entry -> { - assertThat(entry.getKey().getId()).isEqualTo(personId); - assertThat(entry.getValue().getRating()).isEqualTo(5); + assertThat(hobby.getLikedBy()).first().satisfies(entry -> { + assertThat(entry.getAltPerson().getId()).isEqualTo(personId); + assertThat(entry.getRating()).isEqualTo(5); }); }).verifyComplete(); } + + @Test + void loadSameNodeWithDoubleRelationship(@Autowired ReactiveHobbyWithRelationshipWithPropertiesRepository repository) { + long personId; + + try (Session session = createSession()) { + Record record = session.run("CREATE (n:AltPerson{name:'Freddie'})," + + " (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'})," + + " (n)-[l2:LIKES {rating: 1}]->(h1)" + + " RETURN n, h1").single(); + personId = record.get("n").asNode().id(); + } + + StepVerifier.create(repository.loadFromCustomQuery(personId)).assertNext(hobby -> { + assertThat(hobby.getName()).isEqualTo("Music"); + List likedBy = hobby.getLikedBy(); + assertThat(likedBy).hasSize(2); + + AltPerson altPerson = new AltPerson("Freddie"); + altPerson.setId(personId); + AltLikedByPersonRelationship rel1 = new AltLikedByPersonRelationship(); + rel1.setRating(5); + rel1.setAltPerson(altPerson); + + AltLikedByPersonRelationship rel2 = new AltLikedByPersonRelationship(); + rel2.setRating(1); + rel2.setAltPerson(altPerson); + + assertThat(likedBy).containsExactlyInAnyOrder(rel1, rel2); + }); + } } @Nested @@ -2108,7 +2152,7 @@ class ReactiveRepositoryIT { Mono findByHobbiesSinceAndHobbiesActive(int since1, boolean active); } - interface ReactiveHobbyithRelationshipWithPropertiesRepository + interface ReactiveHobbyWithRelationshipWithPropertiesRepository extends ReactiveNeo4jRepository { @Query("MATCH (p:AltPerson)-[l:LIKES]->(h:AltHobby) WHERE id(p) = $personId RETURN h, collect(l), collect(p)") diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/AltHobby.java b/src/test/java/org/springframework/data/neo4j/integration/shared/AltHobby.java index 626a3c6fc..9aa9c44fa 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/AltHobby.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/AltHobby.java @@ -15,8 +15,8 @@ */ package org.springframework.data.neo4j.integration.shared; -import java.util.HashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.Id; @@ -33,7 +33,7 @@ public class AltHobby { private String name; @Relationship(type = "LIKES", direction = Relationship.Direction.INCOMING) - private Map likedBy = new HashMap<>(); + private List likedBy = new ArrayList<>(); public Long getId() { return id; @@ -51,7 +51,7 @@ public class AltHobby { this.name = name; } - public Map getLikedBy() { + public List getLikedBy() { return likedBy; } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/AltLikedByPersonRelationship.java b/src/test/java/org/springframework/data/neo4j/integration/shared/AltLikedByPersonRelationship.java index 860f59724..fa9097452 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/AltLikedByPersonRelationship.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/AltLikedByPersonRelationship.java @@ -16,6 +16,9 @@ package org.springframework.data.neo4j.integration.shared; import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; + +import java.util.Objects; /** * @@author Michael J. Simons @@ -25,6 +28,9 @@ public class AltLikedByPersonRelationship { private Integer rating; + @TargetNode + private AltPerson altPerson; + public Integer getRating() { return rating; } @@ -32,4 +38,29 @@ public class AltLikedByPersonRelationship { public void setRating(Integer rating) { this.rating = rating; } + + public AltPerson getAltPerson() { + return altPerson; + } + + public void setAltPerson(AltPerson altPerson) { + this.altPerson = altPerson; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + AltLikedByPersonRelationship that = (AltLikedByPersonRelationship) o; + return rating.equals(that.rating) && altPerson.equals(that.altPerson); + } + + @Override + public int hashCode() { + return Objects.hash(rating, altPerson); + } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/AltPerson.java b/src/test/java/org/springframework/data/neo4j/integration/shared/AltPerson.java index f88315388..169770bbf 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/AltPerson.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/AltPerson.java @@ -19,6 +19,8 @@ import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.Id; import org.springframework.data.neo4j.core.schema.Node; +import java.util.Objects; + /** * @@author Michael J. Simons */ @@ -44,4 +46,21 @@ public class AltPerson { public String getName() { return name; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + AltPerson altPerson = (AltPerson) o; + return id.equals(altPerson.id) && name.equals(altPerson.name); + } + + @Override + public int hashCode() { + return Objects.hash(id, name); + } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/Friend.java b/src/test/java/org/springframework/data/neo4j/integration/shared/Friend.java new file mode 100644 index 000000000..a3bed7702 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/Friend.java @@ -0,0 +1,48 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * 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 + * + * https://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.springframework.data.neo4j.integration.shared; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +import java.util.List; + +/** + * @author Gerrit Meier + */ +@Node +public class Friend { + + @Id @GeneratedValue private Long id; + + private final String name; + + @Relationship("KNOWS") private List friends; + + public Friend(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public List getFriends() { + return friends; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/FriendshipRelationship.java b/src/test/java/org/springframework/data/neo4j/integration/shared/FriendshipRelationship.java new file mode 100644 index 000000000..ba0044e29 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/FriendshipRelationship.java @@ -0,0 +1,47 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * 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 + * + * https://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.springframework.data.neo4j.integration.shared; + +import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; + +/** + * @author Gerrit Meier + */ +@RelationshipProperties +public class FriendshipRelationship { + + private final Integer since; + + @TargetNode + private Friend friend; + + public FriendshipRelationship(Integer since) { + this.since = since; + } + + public Integer getSince() { + return since; + } + + public Friend getFriend() { + return friend; + } + + public void setFriend(Friend friend) { + this.friend = friend; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/LikesHobbyRelationship.java b/src/test/java/org/springframework/data/neo4j/integration/shared/LikesHobbyRelationship.java index 662ef195a..1247ee8fa 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/LikesHobbyRelationship.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/LikesHobbyRelationship.java @@ -19,6 +19,7 @@ import java.time.LocalDate; import java.util.Objects; import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; import org.springframework.data.neo4j.types.CartesianPoint2d; /** @@ -41,6 +42,9 @@ public class LikesHobbyRelationship { // spatial type private CartesianPoint2d point; + @TargetNode + private Hobby hobby; + public LikesHobbyRelationship(Integer since) { this.since = since; } @@ -79,6 +83,14 @@ public class LikesHobbyRelationship { return Objects.hash(since, active, localDate, myEnum, point); } + public Hobby getHobby() { + return hobby; + } + + public void setHobby(Hobby hobby) { + this.hobby = hobby; + } + /** * The missing javadoc */ diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties.java b/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties.java index b97a00656..23d68a4b9 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties.java @@ -15,7 +15,7 @@ */ package org.springframework.data.neo4j.integration.shared; -import java.util.Map; +import java.util.List; import java.util.Set; import org.springframework.data.neo4j.core.schema.GeneratedValue; @@ -34,7 +34,9 @@ public class PersonWithRelationshipWithProperties { private final String name; - @Relationship("LIKES") private Map hobbies; + @Relationship("LIKES") private List hobbies; + + @Relationship("WORKS_IN") private WorksInClubRelationship club; @Relationship("OWNS") private Set pets; @@ -46,11 +48,20 @@ public class PersonWithRelationshipWithProperties { return name; } - public Map getHobbies() { + public List getHobbies() { return hobbies; } - public void setHobbies(Map hobbies) { + public void setHobbies(List hobbies) { this.hobbies = hobbies; } + + public void setClub(WorksInClubRelationship club) { + this.club = club; + } + + public WorksInClubRelationship getClub() { + return club; + } + } diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/WorksInClubRelationship.java b/src/test/java/org/springframework/data/neo4j/integration/shared/WorksInClubRelationship.java new file mode 100644 index 000000000..7af1d198c --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/WorksInClubRelationship.java @@ -0,0 +1,47 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * 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 + * + * https://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.springframework.data.neo4j.integration.shared; + +import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; + +/** + * @author Gerrit Meier + */ +@RelationshipProperties +public class WorksInClubRelationship { + + private final Integer since; + + @TargetNode + private Club club; + + public WorksInClubRelationship(Integer since) { + this.since = since; + } + + public Integer getSince() { + return since; + } + + public Club getClub() { + return club; + } + + public void setClub(Club club) { + this.club = club; + } +}