From 82d135071055f48aeee8a232bbf35cefa10e88ed Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Tue, 30 Nov 2021 10:04:40 +0100 Subject: [PATCH 1/4] Prepare branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 619aed47a0..8ffb51f3eb 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ org.springframework.data spring-data-neo4j - 6.3.0-SNAPSHOT + 6.3.0-GH2420-SNAPSHOT Spring Data Neo4j Next generation Object-Graph-Mapping for Spring Data. From 78360ce684675ffcd54bd3e8bffffdce8e670d78 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Tue, 30 Nov 2021 16:42:35 +0100 Subject: [PATCH 2/4] GH-2420 - Provide saveAs overloads with a predicate for selecting projected attributes. Closes #2420. --- .../data/neo4j/core/Neo4jOperations.java | 38 ++++++++ .../data/neo4j/core/Neo4jTemplate.java | 35 +++++-- .../neo4j/core/ReactiveNeo4jOperations.java | 38 ++++++++ .../neo4j/core/ReactiveNeo4jTemplate.java | 35 +++++-- .../data/neo4j/core/TemplateSupport.java | 24 +++++ .../neo4j/core/mapping/PropertyTraverser.java | 91 +++++++++++++++++++ .../core/mapping/PropertyTraverserTest.java | 88 ++++++++++++++++++ .../imperative/Neo4jTemplateIT.java | 72 +++++++++++++++ .../reactive/ReactiveNeo4jTemplateIT.java | 73 +++++++++++++++ 9 files changed, 482 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java create mode 100644 src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java index f568826705..ff41b52dc2 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java @@ -18,10 +18,12 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.BiPredicate; import org.apiguardian.api.API; import org.neo4j.cypherdsl.core.Statement; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; import org.springframework.data.neo4j.repository.NoResultException; import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters; @@ -191,6 +193,24 @@ public interface Neo4jOperations { */ T save(T instance); + /** + * Saves an instance of an entity, using the provided predicate to shape the stored graph. One can think of the predicate + * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include + * the association property as well (meaning the predicate must return {@literal true} for that property, too). + *

+ * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. + * + * @param instance the entity to be saved. Must not be {@code null}. + * @param includeProperty A predicate to determine the properties to save. + * @param the type of the entity. + * @return the saved instance. + * @since 6.3 + */ + default T saveAs(T instance, BiPredicate includeProperty) { + throw new UnsupportedOperationException(); + } + /** * Saves an instance of an entity, including the properties and relationship defined by the projected {@code resultType}. * @@ -213,6 +233,24 @@ default R saveAs(T instance, Class resultType) { */ List saveAll(Iterable instances); + /** + * Saves several instances of an entity, using the provided predicate to shape the stored graph. One can think of the predicate + * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include + * the association property as well (meaning the predicate must return {@literal true} for that property, too). + *

+ * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. + * + * @param instances the instances to be saved. Must not be {@code null}. + * @param includeProperty A predicate to determine the properties to save. + * @param the type of the entity. + * @return the saved instances. + * @since 6.3 + */ + default List saveAllAs(Iterable instances, BiPredicate includeProperty) { + throw new UnsupportedOperationException(); + } + /** * Saves an instance of an entity, including the properties and relationship defined by the project {@code resultType}. * 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 c11e42eba5..57f3357925 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -32,6 +32,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.BiPredicate; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -343,6 +344,16 @@ public T save(T instance) { return saveImpl(instance, Collections.emptyMap(), null); } + @Override + public T saveAs(T instance, BiPredicate includeProperty) { + + if (instance == null) { + return null; + } + + return saveImpl(instance, TemplateSupport.computeIncludedPropertiesFromPredicate(this.neo4jMappingContext, instance.getClass(), includeProperty), null); + } + @Override public R saveAs(T instance, Class resultType) { @@ -451,10 +462,10 @@ private DynamicLabels determineDynamicLabels(T entityToBeSaved, Neo4jPersist @Override public List saveAll(Iterable instances) { - return saveAllImpl(instances, Collections.emptyMap()); + return saveAllImpl(instances, Collections.emptyMap(), null); } - private List saveAllImpl(Iterable instances, Map includedProperties) { + private List saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { Set> types = new HashSet<>(); List entities = new ArrayList<>(); @@ -469,13 +480,19 @@ private List saveAllImpl(Iterable instances, Map 1; Class domainClass = types.iterator().next(); + + Map pps = includeProperty == null ? + includedProperties : + TemplateSupport.computeIncludedPropertiesFromPredicate(this.neo4jMappingContext, domainClass, + includeProperty); + Neo4jPersistentEntity entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass); if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty() || entityMetaData.getDynamicLabelsProperty().isPresent()) { log.debug("Saving entities using single statements."); NestedRelationshipProcessingStateMachine stateMachine = new NestedRelationshipProcessingStateMachine(neo4jMappingContext); - return entities.stream().map(e -> saveImpl(e, includedProperties, stateMachine)).collect(Collectors.toList()); + return entities.stream().map(e -> saveImpl(e, pps, stateMachine)).collect(Collectors.toList()); } class Tuple3 { @@ -497,7 +514,7 @@ class Tuple3 { // Save roots @SuppressWarnings("unchecked") // We can safely assume here that we have a humongous collection with only one single type being either T or extending it Function> binderFunction = neo4jMappingContext.getRequiredBinderFunctionFor((Class) domainClass); - binderFunction = TemplateSupport.createAndApplyPropertyFilter(includedProperties, entityMetaData, binderFunction); + binderFunction = TemplateSupport.createAndApplyPropertyFilter(pps, entityMetaData, binderFunction); List> entityList = entitiesToBeSaved.stream().map(h -> h.modifiedInstance).map(binderFunction) .collect(Collectors.toList()); Map idToInternalIdMapping = neo4jClient @@ -515,10 +532,16 @@ class Tuple3 { Neo4jPersistentProperty idProperty = entityMetaData.getRequiredIdProperty(); Object id = convertIdValues(idProperty, propertyAccessor.getProperty(idProperty)); Long internalId = idToInternalIdMapping.get(id); - return this.processRelations(entityMetaData, propertyAccessor, t.wasNew, new NestedRelationshipProcessingStateMachine(neo4jMappingContext, t.originalInstance, internalId), TemplateSupport.computeIncludePropertyPredicate(includedProperties, entityMetaData)); + return this.processRelations(entityMetaData, propertyAccessor, t.wasNew, new NestedRelationshipProcessingStateMachine(neo4jMappingContext, t.originalInstance, internalId), TemplateSupport.computeIncludePropertyPredicate(pps, entityMetaData)); }).collect(Collectors.toList()); } + @Override + public List saveAllAs(Iterable instances, BiPredicate includeProperty) { + + return saveAllImpl(instances, null, includeProperty); + } + @Override public List saveAllAs(Iterable instances, Class resultType) { @@ -537,7 +560,7 @@ public List saveAllAs(Iterable instances, Class resultType) { Map pps = PropertyFilterSupport.addPropertiesFrom(commonElementType, resultType, projectionFactory, neo4jMappingContext); - List savedInstances = saveAllImpl(instances, pps); + List savedInstances = saveAllImpl(instances, pps, null); if (projectionInformation.isClosed()) { return savedInstances.stream().map(instance -> projectionFactory.createProjection(resultType, instance)) diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java index 5f5139d25c..a12431b062 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java @@ -15,12 +15,14 @@ */ package org.springframework.data.neo4j.core; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.Map; +import java.util.function.BiPredicate; import org.apiguardian.api.API; import org.neo4j.cypherdsl.core.Statement; @@ -190,6 +192,24 @@ public interface ReactiveNeo4jOperations { */ Mono save(T instance); + /** + * Saves an instance of an entity, using the provided predicate to shape the stored graph. One can think of the predicate + * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include + * the association property as well (meaning the predicate must return {@literal true} for that property, too). + *

+ * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. + * + * @param instance the entity to be saved. Must not be {@code null}. + * @param includeProperty A predicate to determine the properties to save. + * @param the type of the entity. + * @return the saved instance. + * @since 6.3 + */ + default Mono saveAs(T instance, BiPredicate includeProperty) { + throw new UnsupportedOperationException(); + } + /** * Saves an instance of an entity, including the properties and relationship defined by the projected {@code resultType}. * @@ -212,6 +232,24 @@ default Mono saveAs(T instance, Class resultType) { */ Flux saveAll(Iterable instances); + /** + * Saves several instances of an entity, using the provided predicate to shape the stored graph. One can think of the predicate + * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include + * the association property as well (meaning the predicate must return {@literal true} for that property, too). + *

+ * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. + * + * @param instances the instances to be saved. Must not be {@code null}. + * @param includeProperty A predicate to determine the properties to save. + * @param the type of the entity. + * @return the saved instances. + * @since 6.3 + */ + default Flux saveAllAs(Iterable instances, BiPredicate includeProperty) { + throw new UnsupportedOperationException(); + } + /** * Saves several instances of an entity, including the properties and relationship defined by the project {@code resultType}. * 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 d869dc49db..4b17fedb3e 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -40,6 +40,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; +import java.util.function.BiPredicate; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -327,6 +328,16 @@ public Mono save(T instance) { return saveImpl(instance, Collections.emptyMap(), null); } + @Override + public Mono saveAs(T instance, BiPredicate includeProperty) { + + if (instance == null) { + return null; + } + + return saveImpl(instance, TemplateSupport.computeIncludedPropertiesFromPredicate(this.neo4jMappingContext, instance.getClass(), includeProperty), null); + } + @Override public Mono saveAs(T instance, Class resultType) { @@ -463,7 +474,13 @@ private Mono> determineDynamicLabels(T entityToBeSa @Override public Flux saveAll(Iterable instances) { - return saveAllImpl(instances, Collections.emptyMap()); + return saveAllImpl(instances, Collections.emptyMap(), null); + } + + @Override + public Flux saveAllAs(Iterable instances, BiPredicate includeProperty) { + + return saveAllImpl(instances, null, includeProperty); } @Override @@ -481,7 +498,7 @@ public Flux saveAllAs(Iterable instances, Class resultType) { Map pps = PropertyFilterSupport.addPropertiesFrom(commonElementType, resultType, projectionFactory, neo4jMappingContext); - Flux savedInstances = saveAllImpl(instances, pps); + Flux savedInstances = saveAllImpl(instances, pps, null); if (projectionInformation.isClosed()) { return savedInstances.map(instance -> projectionFactory.createProjection(resultType, instance)); } @@ -495,7 +512,7 @@ public Flux saveAllAs(Iterable instances, Class resultType) { }).map(instance -> projectionFactory.createProjection(resultType, instance)); } - private Flux saveAllImpl(Iterable instances, @Nullable Map includedProperties) { + private Flux saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { Set> types = new HashSet<>(); List entities = new ArrayList<>(); @@ -510,18 +527,24 @@ private Flux saveAllImpl(Iterable instances, @Nullable Map 1; Class domainClass = types.iterator().next(); + + Map pps = includeProperty == null ? + includedProperties : + TemplateSupport.computeIncludedPropertiesFromPredicate(this.neo4jMappingContext, domainClass, + includeProperty); + Neo4jPersistentEntity entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass); if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty() || entityMetaData.getDynamicLabelsProperty().isPresent()) { log.debug("Saving entities using single statements."); NestedRelationshipProcessingStateMachine stateMachine = new NestedRelationshipProcessingStateMachine(neo4jMappingContext); - return Flux.fromIterable(entities).concatMap(e -> this.saveImpl(e, includedProperties, stateMachine)); + return Flux.fromIterable(entities).concatMap(e -> this.saveImpl(e, pps, stateMachine)); } @SuppressWarnings("unchecked") // We can safely assume here that we have a humongous collection with only one single type being either T or extending it Function> binderFunction = TemplateSupport.createAndApplyPropertyFilter( - includedProperties, entityMetaData, + pps, entityMetaData, neo4jMappingContext.getRequiredBinderFunctionFor((Class) domainClass)); return Flux.fromIterable(entities) // Map all entities into a tuple @@ -550,7 +573,7 @@ private Flux saveAllImpl(Iterable instances, @Nullable Map FilteredBinderFunction createAndApplyPropertyFilter( })); } + /** + * Helper function that computes the map of included properties for a dynamic projection as expected in 6.2, but + * for fully dynamic projection + * + * @param mappingContext The context to work on + * @param domainType The projected domain type + * @param predicate The predicate to compute the included columns + * @param Type of the domain type + * @return A map as expected by the property filter. + */ + static Map computeIncludedPropertiesFromPredicate(Neo4jMappingContext mappingContext, + Class domainType, @Nullable BiPredicate predicate) { + if (predicate == null) { + return Collections.emptyMap(); + } + Map pps = new HashMap<>(); + PropertyTraverser traverser = new PropertyTraverser(mappingContext); + traverser.traverse(domainType, predicate, (path, property) -> pps.put(path, false)); + return Collections.unmodifiableMap(pps); + } + /** * A wrapper around a {@link Function} from entity to {@link Map} which is filtered the {@link PropertyFilter} included as well. * diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java b/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java new file mode 100644 index 0000000000..abcb7c2cf9 --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java @@ -0,0 +1,91 @@ +/* + * Copyright 2011-2021 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.mapping; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.BiPredicate; + +import org.apiguardian.api.API; +import org.springframework.data.mapping.Association; +import org.springframework.data.mapping.PropertyPath; +import org.springframework.lang.Nullable; + +/** + * A strategy for traversing all properties (including association) once, without going in circles with cyclic mappings. + * Uses the same idea of relationship isomorphism like Cypher does (Relationship isomorphism means that one relationship + * or association cannot be returned more than once for each entity). + * + * @author Michael J. Simons + * @since 6.3 + */ +@API(status = API.Status.INTERNAL) +public final class PropertyTraverser { + + private final Neo4jMappingContext ctx; + private final Set> pathsTraversed = new HashSet<>(); + + public PropertyTraverser(Neo4jMappingContext ctx) { + this.ctx = ctx; + } + + public void traverse( + Class root, + BiConsumer sink + ) { + traverse(root, (path, toProperty) -> true, sink); + } + + public synchronized void traverse( + Class root, + BiPredicate predicate, + BiConsumer sink + ) { + this.pathsTraversed.clear(); + traverseImpl(ctx.getRequiredPersistentEntity(root), null, predicate, sink, false); + } + + private void traverseImpl( + Neo4jPersistentEntity root, + @Nullable PropertyPath base, + BiPredicate predicate, + BiConsumer sink, + boolean pathAlreadyVisited + ) { + root.doWithAll(p -> { + PropertyPath path = + base == null ? PropertyPath.from(p.getName(), p.getOwner().getType()) : base.nested(p.getName()); + + if (!predicate.test(path, p)) { + return; + } + + sink.accept(path, p); + if (p.isAssociation() && !pathAlreadyVisited) { + Class associationTargetType = p.getAssociationTargetType(); + if (associationTargetType == null) { + return; + } + + Neo4jPersistentEntity targetEntity = ctx.getRequiredPersistentEntity(associationTargetType); + boolean recalledForProperties = pathsTraversed.contains(p.getAssociation()); + pathsTraversed.add(p.getAssociation()); + traverseImpl(targetEntity, path, predicate, sink, recalledForProperties); + } + }); + } +} diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java new file mode 100644 index 0000000000..5869587c28 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java @@ -0,0 +1,88 @@ +/* + * Copyright 2011-2021 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.mapping; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import org.junit.jupiter.api.Test; +import org.springframework.data.neo4j.integration.movies.shared.Movie; + +/** + * @author Michael J. Simons + */ +class PropertyTraverserTest { + + private final Neo4jMappingContext ctx; + + PropertyTraverserTest() { + this.ctx = new Neo4jMappingContext(); + Set> entities = new HashSet<>(); + entities.add(Movie.class); + this.ctx.setInitialEntitySet(entities); + this.ctx.afterPropertiesSet(); + } + + @Test + void shouldTraverseAll() { + + PropertyTraverser traverser = new PropertyTraverser(this.ctx); + Map includedProperties = new TreeMap<>(); + traverser.traverse(Movie.class, (path, property) -> includedProperties.put(path.toString(), property.isAssociation())); + + // The traversal order might depend on the order in which classes have been loaded + assertThat(includedProperties).hasSize(74); + } + + @Test + void onlyMovieDirectFields() { + + PropertyTraverser traverser = new PropertyTraverser(this.ctx); + Map includedProperties = new TreeMap<>(); + traverser.traverse(Movie.class, + (path, property) -> !property.isAssociation(), + (path, property) -> includedProperties.put(path.toString(), property.isAssociation())); + + Map expected = new LinkedHashMap<>(); + expected.put("Movie.description", false); + expected.put("Movie.released", false); + expected.put("Movie.title", false); + + assertThat(includedProperties).containsExactlyEntriesOf(expected); + } + + @Test + void onlyDirectors() { + + PropertyTraverser traverser = new PropertyTraverser(this.ctx); + Map includedProperties = new TreeMap<>(); + traverser.traverse(Movie.class, + (path, property) -> property.getName().equals("directors") || (path.toDotPath().startsWith("directors.") + && property.getName().equals("name")), + (path, property) -> includedProperties.put(path.toString(), property.isAssociation())); + + Map expected = new LinkedHashMap<>(); + expected.put("Movie.directors", true); + expected.put("Movie.directors.name", false); + + assertThat(includedProperties).containsExactlyEntriesOf(expected); + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jTemplateIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jTemplateIT.java index 21ae8a0d8b..08c38481d9 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jTemplateIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jTemplateIT.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.BiPredicate; import java.util.function.Function; import org.junit.jupiter.api.BeforeEach; @@ -46,12 +47,14 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.neo4j.config.AbstractNeo4jConfig; import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.Neo4jTemplate; import org.springframework.data.neo4j.core.mapping.Constants; 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.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; import org.springframework.data.neo4j.integration.issues.gh2415.BaseNodeEntity; @@ -352,6 +355,16 @@ interface CountryProjection { } } + private static BiPredicate create2LevelProjectingPredicate() { + BiPredicate predicate = (path, property) -> false; + predicate = predicate.or((path, property) -> property.getName().equals("lastName")); + predicate = predicate.or((path, property) -> property.getName().equals("address") + || path.toDotPath().startsWith("address.") && property.getName().equals("street")); + predicate = predicate.or((path, property) -> property.getName().equals("country") + || path.toDotPath().contains("address.country.") && property.getName().equals("name")); + return predicate; + } + @Data static class DtoPersonProjection { @@ -545,6 +558,31 @@ void saveAsWithClosedProjectionOnSecondLevelShouldWork() { assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); } + @Test // GH-2420 + void saveAsWithDynamicProjectionOnSecondLevelShouldWork() { + + Person p = neo4jTemplate.findOne("MATCH (p:Person {lastName: $lastName})-[r:LIVES_AT]-(a:Address) RETURN p, collect(r), collect(a)", + Collections.singletonMap("lastName", "Siemons"), Person.class).get(); + + p.getAddress().setCity("Braunschweig"); + p.getAddress().setStreet("Single Trail"); + Person.Address.Country country = new Person.Address.Country(); + country.setName("Foo"); + country.setCountryCode("DE"); + p.getAddress().setCountry(country); + + BiPredicate predicate = create2LevelProjectingPredicate(); + + Person projection = neo4jTemplate.saveAs(p, predicate); + + assertThat(projection.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(projection.getAddress().getCountry().getName()).isEqualTo("Foo"); + p = neo4jTemplate.findById(p.getId(), Person.class).get(); + assertThat(p.getAddress().getCity()).isEqualTo("Aachen"); + assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(p.getAddress().getCountry().getName()).isEqualTo("Foo"); + } + @Test // GH-2407 void saveAllAsWithClosedProjectionOnSecondLevelShouldWork() { @@ -568,6 +606,40 @@ void saveAllAsWithClosedProjectionOnSecondLevelShouldWork() { assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); } + @Test // GH-2420 + void saveAllAsWithDynamicProjectionOnSecondLevelShouldWork() { + + Person p = neo4jTemplate.findOne("MATCH (p:Person {lastName: $lastName})-[r:LIVES_AT]-(a:Address) RETURN p, collect(r), collect(a)", + Collections.singletonMap("lastName", "Siemons"), Person.class).get(); + + p.setFirstName("Klaus"); + p.setLastName("Simons"); + p.getAddress().setCity("Braunschweig"); + p.getAddress().setStreet("Single Trail"); + Person.Address.Country country = new Person.Address.Country(); + country.setName("Foo"); + country.setCountryCode("DE"); + p.getAddress().setCountry(country); + + BiPredicate predicate = create2LevelProjectingPredicate(); + + List projections = neo4jTemplate.saveAllAs(Collections.singletonList(p), predicate); + + assertThat(projections) + .hasSize(1).first() + .satisfies(projection -> { + assertThat(projection.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(projection.getAddress().getCountry().getName()).isEqualTo("Foo"); + }); + + p = neo4jTemplate.findById(p.getId(), Person.class).get(); + assertThat(p.getFirstName()).isEqualTo("Michael"); + assertThat(p.getLastName()).isEqualTo("Simons"); + assertThat(p.getAddress().getCity()).isEqualTo("Aachen"); + assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(p.getAddress().getCountry().getName()).isEqualTo("Foo"); + } + @Test // GH-2407 void shouldSaveNewProjectedThing() { diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jTemplateIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jTemplateIT.java index 81ed167d81..e4f370fa22 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jTemplateIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jTemplateIT.java @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.BiPredicate; import java.util.function.Function; import org.junit.jupiter.api.BeforeEach; @@ -48,12 +49,14 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig; import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider; import org.springframework.data.neo4j.core.ReactiveNeo4jTemplate; import org.springframework.data.neo4j.core.mapping.Constants; 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.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager; import org.springframework.data.neo4j.integration.issues.gh2415.BaseNodeEntity; @@ -357,6 +360,16 @@ interface CountryProjection { } } + private static BiPredicate create2LevelProjectingPredicate() { + BiPredicate predicate = (path, property) -> false; + predicate = predicate.or((path, property) -> property.getName().equals("lastName")); + predicate = predicate.or((path, property) -> property.getName().equals("address") + || path.toDotPath().startsWith("address.") && property.getName().equals("street")); + predicate = predicate.or((path, property) -> property.getName().equals("country") + || path.toDotPath().contains("address.country.") && property.getName().equals("name")); + return predicate; + } + @Data static class DtoPersonProjection { @@ -589,6 +602,66 @@ void saveAsWithClosedProjectionOnSecondLevelShouldWork(@Autowired ReactiveNeo4jT .verifyComplete(); } + @Test // GH-2420 + void saveAsWithDynamicProjectionOnSecondLevelShouldWork(@Autowired ReactiveNeo4jTemplate template) { + + template.findOne("MATCH (p:Person {lastName: $lastName})-[r:LIVES_AT]-(a:Address) RETURN p, collect(r), collect(a)", + Collections.singletonMap("lastName", "Siemons"), Person.class) + .flatMapMany(p -> { + + p.getAddress().setCity("Braunschweig"); + p.getAddress().setStreet("Single Trail"); + Person.Address.Country country = new Person.Address.Country(); + country.setName("Foo"); + country.setCountryCode("DE"); + p.getAddress().setCountry(country); + return neo4jTemplate.saveAs(p, create2LevelProjectingPredicate()); + }) + .as(StepVerifier::create) + .assertNext(projection -> { + assertThat(projection.getAddress().getStreet()).isEqualTo("Single Trail"); + }) + .verifyComplete(); + template.findById(simonsId, Person.class) + .as(StepVerifier::create) + .assertNext(p -> { + assertThat(p.getAddress().getCity()).isEqualTo("Aachen"); + assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(p.getAddress().getCountry().getName()).isEqualTo("Foo"); + }) + .verifyComplete(); + } + + @Test // GH-2420 + void saveAllAsWithDynamicProjectionOnSecondLevelShouldWork(@Autowired ReactiveNeo4jTemplate template) { + + template.findOne("MATCH (p:Person {lastName: $lastName})-[r:LIVES_AT]-(a:Address) RETURN p, collect(r), collect(a)", + Collections.singletonMap("lastName", "Siemons"), Person.class) + .flatMapMany(p -> { + + p.getAddress().setCity("Braunschweig"); + p.getAddress().setStreet("Single Trail"); + Person.Address.Country country = new Person.Address.Country(); + country.setName("Foo"); + country.setCountryCode("DE"); + p.getAddress().setCountry(country); + return neo4jTemplate.saveAllAs(Collections.singletonList(p), create2LevelProjectingPredicate()); + }) + .as(StepVerifier::create) + .assertNext(projection -> { + assertThat(projection.getAddress().getStreet()).isEqualTo("Single Trail"); + }) + .verifyComplete(); + template.findById(simonsId, Person.class) + .as(StepVerifier::create) + .assertNext(p -> { + assertThat(p.getAddress().getCity()).isEqualTo("Aachen"); + assertThat(p.getAddress().getStreet()).isEqualTo("Single Trail"); + assertThat(p.getAddress().getCountry().getName()).isEqualTo("Foo"); + }) + .verifyComplete(); + } + @Test void saveAsWithClosedProjectionOnThreeLevelShouldWork(@Autowired ReactiveNeo4jTemplate template) { From 599d54b9b22dc2eaf2893d7638773a588860e83e Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Wed, 1 Dec 2021 09:33:13 +0100 Subject: [PATCH 3/4] Make sure properties are sorted at least stable on each level. --- .../neo4j/core/mapping/PropertyTraverser.java | 6 +- .../core/mapping/PropertyTraverserTest.java | 79 ++++++++++++++++++- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java b/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java index abcb7c2cf9..e6ec92ad03 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/PropertyTraverser.java @@ -15,8 +15,10 @@ */ package org.springframework.data.neo4j.core.mapping; +import java.util.Comparator; import java.util.HashSet; import java.util.Set; +import java.util.TreeSet; import java.util.function.BiConsumer; import java.util.function.BiPredicate; @@ -66,7 +68,9 @@ private void traverseImpl( BiConsumer sink, boolean pathAlreadyVisited ) { - root.doWithAll(p -> { + Set sortedProperties = new TreeSet<>(Comparator.comparing(Neo4jPersistentProperty::getName)); + root.doWithAll(sortedProperties::add); + sortedProperties.forEach(p -> { PropertyPath path = base == null ? PropertyPath.from(p.getName(), p.getOwner().getType()) : base.nested(p.getName()); diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java index 5869587c28..ed61e2b689 100644 --- a/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/PropertyTraverserTest.java @@ -48,8 +48,83 @@ void shouldTraverseAll() { Map includedProperties = new TreeMap<>(); traverser.traverse(Movie.class, (path, property) -> includedProperties.put(path.toString(), property.isAssociation())); - // The traversal order might depend on the order in which classes have been loaded - assertThat(includedProperties).hasSize(74); + Map expected = new LinkedHashMap<>(); + expected.put("Movie.actors", true); + expected.put("Movie.actors.id", false); + expected.put("Movie.actors.person", false); + expected.put("Movie.actors.roles", false); + expected.put("Movie.description", false); + expected.put("Movie.directors", true); + expected.put("Movie.directors.actedIn", true); + expected.put("Movie.directors.actedIn.actors", true); + expected.put("Movie.directors.actedIn.actors.id", false); + expected.put("Movie.directors.actedIn.actors.person", false); + expected.put("Movie.directors.actedIn.actors.roles", false); + expected.put("Movie.directors.actedIn.description", false); + expected.put("Movie.directors.actedIn.directors", true); + expected.put("Movie.directors.actedIn.directors.actedIn", true); + expected.put("Movie.directors.actedIn.directors.born", false); + expected.put("Movie.directors.actedIn.directors.id", false); + expected.put("Movie.directors.actedIn.directors.name", false); + expected.put("Movie.directors.actedIn.directors.reviewed", true); + expected.put("Movie.directors.actedIn.released", false); + expected.put("Movie.directors.actedIn.sequel", true); + expected.put("Movie.directors.actedIn.sequel.actors", true); + expected.put("Movie.directors.actedIn.sequel.actors.id", false); + expected.put("Movie.directors.actedIn.sequel.actors.person", false); + expected.put("Movie.directors.actedIn.sequel.actors.roles", false); + expected.put("Movie.directors.actedIn.sequel.description", false); + expected.put("Movie.directors.actedIn.sequel.directors", true); + expected.put("Movie.directors.actedIn.sequel.directors.actedIn", true); + expected.put("Movie.directors.actedIn.sequel.directors.born", false); + expected.put("Movie.directors.actedIn.sequel.directors.id", false); + expected.put("Movie.directors.actedIn.sequel.directors.name", false); + expected.put("Movie.directors.actedIn.sequel.directors.reviewed", true); + expected.put("Movie.directors.actedIn.sequel.released", false); + expected.put("Movie.directors.actedIn.sequel.sequel", true); + expected.put("Movie.directors.actedIn.sequel.sequel.actors", true); + expected.put("Movie.directors.actedIn.sequel.sequel.description", false); + expected.put("Movie.directors.actedIn.sequel.sequel.directors", true); + expected.put("Movie.directors.actedIn.sequel.sequel.released", false); + expected.put("Movie.directors.actedIn.sequel.sequel.sequel", true); + expected.put("Movie.directors.actedIn.sequel.sequel.title", false); + expected.put("Movie.directors.actedIn.sequel.title", false); + expected.put("Movie.directors.actedIn.title", false); + expected.put("Movie.directors.born", false); + expected.put("Movie.directors.id", false); + expected.put("Movie.directors.name", false); + expected.put("Movie.directors.reviewed", true); + expected.put("Movie.directors.reviewed.actors", true); + expected.put("Movie.directors.reviewed.actors.id", false); + expected.put("Movie.directors.reviewed.actors.person", false); + expected.put("Movie.directors.reviewed.actors.roles", false); + expected.put("Movie.directors.reviewed.description", false); + expected.put("Movie.directors.reviewed.directors", true); + expected.put("Movie.directors.reviewed.directors.actedIn", true); + expected.put("Movie.directors.reviewed.directors.born", false); + expected.put("Movie.directors.reviewed.directors.id", false); + expected.put("Movie.directors.reviewed.directors.name", false); + expected.put("Movie.directors.reviewed.directors.reviewed", true); + expected.put("Movie.directors.reviewed.released", false); + expected.put("Movie.directors.reviewed.sequel", true); + expected.put("Movie.directors.reviewed.sequel.actors", true); + expected.put("Movie.directors.reviewed.sequel.description", false); + expected.put("Movie.directors.reviewed.sequel.directors", true); + expected.put("Movie.directors.reviewed.sequel.released", false); + expected.put("Movie.directors.reviewed.sequel.sequel", true); + expected.put("Movie.directors.reviewed.sequel.title", false); + expected.put("Movie.directors.reviewed.title", false); + expected.put("Movie.released", false); + expected.put("Movie.sequel", true); + expected.put("Movie.sequel.actors", true); + expected.put("Movie.sequel.description", false); + expected.put("Movie.sequel.directors", true); + expected.put("Movie.sequel.released", false); + expected.put("Movie.sequel.sequel", true); + expected.put("Movie.sequel.title", false); + expected.put("Movie.title", false); + + assertThat(includedProperties).containsExactlyEntriesOf(expected); } @Test From 28f3c6c7370773429d924b1deee3999acc7635fd Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Wed, 1 Dec 2021 13:43:56 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Gerrit Meier --- .../org/springframework/data/neo4j/core/Neo4jOperations.java | 2 +- .../java/org/springframework/data/neo4j/core/Neo4jTemplate.java | 2 +- .../data/neo4j/core/ReactiveNeo4jOperations.java | 2 +- .../springframework/data/neo4j/core/ReactiveNeo4jTemplate.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java index ff41b52dc2..b0c4b53852 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jOperations.java @@ -238,7 +238,7 @@ default R saveAs(T instance, Class resultType) { * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include * the association property as well (meaning the predicate must return {@literal true} for that property, too). *

- * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * Be careful when reusing the returned instances for further persistence operations, as they will most likely not be * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. * * @param instances the instances to be saved. Must not be {@code null}. 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 57f3357925..045f649d57 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -465,7 +465,7 @@ public List saveAll(Iterable instances) { return saveAllImpl(instances, Collections.emptyMap(), null); } - private List saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { + private List saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { Set> types = new HashSet<>(); List entities = new ArrayList<>(); diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java index a12431b062..5038ca05bf 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jOperations.java @@ -237,7 +237,7 @@ default Mono saveAs(T instance, Class resultType) { * as a dynamic projection. If you want to save or update properties of associations (aka related nodes), you must include * the association property as well (meaning the predicate must return {@literal true} for that property, too). *

- * Be careful when reusing the returned instance for further persistence operations, as it will most likely not be + * Be careful when reusing the returned instances for further persistence operations, as they will most likely not be * fully hydrated and without using a static or dynamic projection, you will most likely cause data loss. * * @param instances the instances to be saved. Must not be {@code null}. 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 4b17fedb3e..b37d9e84da 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -512,7 +512,7 @@ public Flux saveAllAs(Iterable instances, Class resultType) { }).map(instance -> projectionFactory.createProjection(resultType, instance)); } - private Flux saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { + private Flux saveAllImpl(Iterable instances, @Nullable Map includedProperties, @Nullable BiPredicate includeProperty) { Set> types = new HashSet<>(); List entities = new ArrayList<>();