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; + } +}