diff --git a/pom.xml b/pom.xml index 88a3ce592b..34435bf009 100644 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ org.springframework.data spring-data-neo4j-parent - 5.3.0.BUILD-SNAPSHOT + 5.3.0.DATAGRAPH-1286-SNAPSHOT pom Spring Data Neo4j diff --git a/spring-data-neo4j-distribution/pom.xml b/spring-data-neo4j-distribution/pom.xml index 5d55d528f7..be5c6f7ff3 100644 --- a/spring-data-neo4j-distribution/pom.xml +++ b/spring-data-neo4j-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-neo4j-parent - 5.3.0.BUILD-SNAPSHOT + 5.3.0.DATAGRAPH-1286-SNAPSHOT ../pom.xml diff --git a/spring-data-neo4j/pom.xml b/spring-data-neo4j/pom.xml index 7c80070248..3af2debba0 100644 --- a/spring-data-neo4j/pom.xml +++ b/spring-data-neo4j/pom.xml @@ -27,7 +27,7 @@ org.springframework.data spring-data-neo4j-parent - 5.3.0.BUILD-SNAPSHOT + 5.3.0.DATAGRAPH-1286-SNAPSHOT ../pom.xml diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jPersistentProperty.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jPersistentProperty.java index b26d9315f8..74b60826a1 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jPersistentProperty.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/mapping/Neo4jPersistentProperty.java @@ -143,6 +143,13 @@ public boolean isIdProperty() { return propertyType.idProperty; } + /** + * @return True if this property describes the internal ID property. + */ + public boolean isInternalIdProperty() { + return propertyType == PropertyType.INTERNAL_ID_PROPERTY; + } + PropertyType getPropertyType() { return propertyType; } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/FilterBuildersDefinition.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/FilterBuildersDefinition.java index 231d6b502a..dd04b1f8ad 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/FilterBuildersDefinition.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/FilterBuildersDefinition.java @@ -18,9 +18,13 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.function.Predicate; import org.neo4j.ogm.cypher.BooleanOperator; import org.neo4j.ogm.cypher.Filters; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.neo4j.mapping.Neo4jMappingContext; +import org.springframework.data.neo4j.mapping.Neo4jPersistentProperty; import org.springframework.data.neo4j.repository.query.filter.FilterBuilder; import org.springframework.data.repository.query.parser.Part; @@ -39,17 +43,24 @@ class FilterBuildersDefinition { private final Part basePart; private final List filterBuilders; + private final Predicate isInternalIdProperty; - static UnstartedBuild forType(Class entityType) { - return new UnstartedBuild(entityType); + static UnstartedBuild forType(Neo4jMappingContext mappingContext, Class entityType) { + return new UnstartedBuild(mappingContext, entityType); } - private FilterBuildersDefinition(Class entityType, Part basePart) { + private FilterBuildersDefinition(Neo4jMappingContext mappingContext, Class entityType, Part basePart) { this.entityType = entityType; this.basePart = basePart; this.filterBuilders = new LinkedList<>(); - - this.filterBuilders.add(FilterBuilder.forPartAndEntity(basePart, entityType, BooleanOperator.NONE)); + this.isInternalIdProperty = part -> { + PersistentPropertyPath path = mappingContext + .getPersistentPropertyPath(part.getProperty()); + Neo4jPersistentProperty possibleIdProperty = path.getRequiredLeafProperty(); + return possibleIdProperty.isInternalIdProperty(); + }; + this.filterBuilders.add(FilterBuilder.forPartAndEntity(basePart, entityType, BooleanOperator.NONE, + isInternalIdProperty)); } TemplatedQuery buildTemplatedQuery() { @@ -66,24 +77,28 @@ Part getBasePart() { } FilterBuildersDefinition and(Part part) { - this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.AND)); + this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.AND, isInternalIdProperty)); return this; } FilterBuildersDefinition or(Part part) { - this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.OR)); + this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.OR, isInternalIdProperty)); return this; } static class UnstartedBuild { + + private final Neo4jMappingContext mappingContext; + private final Class entityType; - UnstartedBuild(Class entityType) { + UnstartedBuild(Neo4jMappingContext mappingContext, Class entityType) { + this.mappingContext = mappingContext; this.entityType = entityType; } FilterBuildersDefinition startWith(Part firstPart) { - return new FilterBuildersDefinition(entityType, firstPart); + return new FilterBuildersDefinition(mappingContext, entityType, firstPart); } } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryMethod.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryMethod.java index 7c2b0ca501..4b4c0a2a0c 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryMethod.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryMethod.java @@ -118,19 +118,6 @@ private Integer getQueryDepthParamIndex(Method method) { } } } - /* - //Java 8 only - Parameter[] parameters = method.getParameters(); - for (int i = 0; i < method.getParameterCount(); i++) { - if (parameters[i].isAnnotationPresent(Depth.class)) { - if (parameters[i].getType() == Integer.class || parameters[i].getType() == int.class) { - return i; - } - else { - throw new IllegalArgumentException("Depth parameter in " + method.getName() + " must be an integer"); - } - } - }*/ return null; } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/PartTreeNeo4jQuery.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/PartTreeNeo4jQuery.java index aa0e05921e..5b848844b4 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/PartTreeNeo4jQuery.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/PartTreeNeo4jQuery.java @@ -22,6 +22,7 @@ import org.neo4j.ogm.session.Session; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.data.neo4j.mapping.Neo4jMappingContext; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.parser.PartTree; @@ -49,11 +50,11 @@ public PartTreeNeo4jQuery(GraphQueryMethod graphQueryMethod, MetaData metaData, super(graphQueryMethod, metaData, session); Class domainType = graphQueryMethod.getEntityInformation().getJavaType(); - this.graphQueryMethod = graphQueryMethod; this.tree = new PartTree(graphQueryMethod.getName(), domainType); - this.queryTemplate = new TemplatedQueryCreator(this.tree, domainType).createQuery(); + this.queryTemplate = new TemplatedQueryCreator(this.tree, + (Neo4jMappingContext) this.graphQueryMethod.getMappingContext(), domainType).createQuery(); } @Override diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQueryCreator.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQueryCreator.java index b278d469ef..773e08cebc 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQueryCreator.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQueryCreator.java @@ -19,6 +19,7 @@ import java.util.Optional; import org.springframework.data.domain.Sort; +import org.springframework.data.neo4j.mapping.Neo4jMappingContext; import org.springframework.data.repository.query.parser.AbstractQueryCreator; import org.springframework.data.repository.query.parser.Part; import org.springframework.data.repository.query.parser.PartTree; @@ -33,17 +34,19 @@ */ class TemplatedQueryCreator extends AbstractQueryCreator { + private final Neo4jMappingContext mappingContext; private final Class entityType; - public TemplatedQueryCreator(PartTree tree, Class entityType) { + public TemplatedQueryCreator(PartTree tree, Neo4jMappingContext mappingContext, Class entityType) { super(tree); + this.mappingContext = mappingContext; this.entityType = entityType; } @Override protected FilterBuildersDefinition create(Part part, Iterator iterator) { - return FilterBuildersDefinition.forType(entityType) // + return FilterBuildersDefinition.forType(mappingContext, entityType) // .startWith(part); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/FilterBuilder.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/FilterBuilder.java index 352ae9e660..052bb2be25 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/FilterBuilder.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/FilterBuilder.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Objects; import java.util.Stack; +import java.util.function.Predicate; import org.neo4j.ogm.cypher.BooleanOperator; import org.neo4j.ogm.cypher.Filter; @@ -41,27 +42,40 @@ public abstract class FilterBuilder { protected Part part; protected BooleanOperator booleanOperator; protected Class entityType; + protected Predicate isInternalIdProperty = part -> false; - public static FilterBuilder forPartAndEntity(Part part, Class entityType, BooleanOperator booleanOperator) { + public static FilterBuilder forPartAndEntity(Part part, Class entityType, BooleanOperator booleanOperator, + Predicate isInternalIdProperty) { + FilterBuilder filterBuilder; switch (part.getType()) { case NEAR: - return new DistanceComparisonBuilder(part, booleanOperator, entityType); + filterBuilder = new DistanceComparisonBuilder(part, booleanOperator, entityType); + break; case BETWEEN: - return new BetweenComparisonBuilder(part, booleanOperator, entityType); + filterBuilder = new BetweenComparisonBuilder(part, booleanOperator, entityType); + break; case NOT_CONTAINING: case CONTAINING: - return resolveMatchingContainsFilterBuilder(part, entityType, booleanOperator); + filterBuilder = resolveMatchingContainsFilterBuilder(part, entityType, booleanOperator); + break; case IS_NULL: case IS_NOT_NULL: - return new IsNullFilterBuilder(part, booleanOperator, entityType); + filterBuilder = new IsNullFilterBuilder(part, booleanOperator, entityType); + break; case EXISTS: - return new ExistsFilterBuilder(part, booleanOperator, entityType); + filterBuilder = new ExistsFilterBuilder(part, booleanOperator, entityType); + break; case TRUE: case FALSE: - return new BooleanComparisonBuilder(part, booleanOperator, entityType); + filterBuilder = new BooleanComparisonBuilder(part, booleanOperator, entityType); + break; default: - return new PropertyComparisonBuilder(part, booleanOperator, entityType); + filterBuilder = new PropertyComparisonBuilder(part, booleanOperator, entityType); + break; } + + filterBuilder.isInternalIdProperty = isInternalIdProperty; + return filterBuilder; } FilterBuilder(Part part, BooleanOperator booleanOperator, Class entityType) { diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/NativeIdFilterFunction.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/NativeIdFilterFunction.java new file mode 100644 index 0000000000..ec5e6b027d --- /dev/null +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/NativeIdFilterFunction.java @@ -0,0 +1,87 @@ +/* + * 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.repository.query.filter; + +import static org.neo4j.ogm.cypher.ComparisonOperator.*; + +import java.util.HashMap; +import java.util.Map; + +import org.neo4j.ogm.cypher.ComparisonOperator; +import org.neo4j.ogm.cypher.Filter; +import org.neo4j.ogm.cypher.function.FilterFunction; + +/** + * This is a specialised filter function taking care of filtering native id properties. + * + * @author Michael J. Simons + * @soundtrack Freddie Mercury - Never Boring + */ +final class NativeIdFilterFunction implements FilterFunction { + + // This function belongs somewhat more into OGM than SDN. The reason having it here is simple: The filter is build + // explicitly and not via reflection and we don't want to have yet another shim managing separate possible versions + // OGM like we already have with the embedded support, entity instantiator and some other things. ^ms + + private final ComparisonOperator operator; + private final Object value; + private Filter filter; + + NativeIdFilterFunction(ComparisonOperator operator, Object value) { + this.operator = operator; + this.value = value; + } + + @Override + public Object getValue() { + return this.value; + } + + @Override + public Filter getFilter() { + return filter; + } + + @Override + public void setFilter(Filter filter) { + this.filter = filter; + } + + @Override + public String expression(String nodeIdentifier) { + + switch (operator) { + case EQUALS: + case GREATER_THAN: + case GREATER_THAN_EQUAL: + case LESS_THAN: + case LESS_THAN_EQUAL: + case IN: + return String.format("id(%s) %s $`%s` ", nodeIdentifier, operator.getValue(), filter.uniqueParameterName()); + default: + throw new IllegalArgumentException("Unsupported comparision operator for an ID attribute."); + } + } + + @Override + public Map parameters() { + Map map = new HashMap<>(); + if (operator.isOneOf(EQUALS, GREATER_THAN, GREATER_THAN_EQUAL, LESS_THAN, LESS_THAN_EQUAL, IN)) { + map.put(filter.uniqueParameterName(), filter.getTransformedPropertyValue()); + } + return map; + } +} diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/PropertyComparisonBuilder.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/PropertyComparisonBuilder.java index b7b4ffd320..18f89c9c42 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/PropertyComparisonBuilder.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/PropertyComparisonBuilder.java @@ -45,7 +45,14 @@ public List build(Stack params) { Object value = params.pop(); - Filter filter = new Filter(nestedAttributes.isEmpty() ? propertyName() : nestedAttributes.getLeafPropertySegment(), convertToComparisonOperator(part.getType()), value); + Filter filter; + String propertyName = nestedAttributes.isEmpty() ? propertyName() : nestedAttributes.getLeafPropertySegment(); + if (isInternalIdProperty.test(part)) { + filter = new Filter(new NativeIdFilterFunction(convertToComparisonOperator(part.getType()), value)); + filter.setPropertyName(propertyName); + } else { + filter = new Filter(propertyName, convertToComparisonOperator(part.getType()), value); + } filter.setOwnerEntityType(entityType); filter.setBooleanOperator(booleanOperator); filter.setNegated(isNegated()); diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/domain/sample/NodeWithUUIDAsId.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/domain/sample/NodeWithUUIDAsId.java index 84adefbfb7..9c4ae64d7d 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/domain/sample/NodeWithUUIDAsId.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/domain/sample/NodeWithUUIDAsId.java @@ -28,8 +28,6 @@ */ public class NodeWithUUIDAsId { - private Long id; - @Id @GeneratedValue(strategy = UuidStrategy.class) @Convert(UuidStringConverter.class) private UUID myNiceId; private String someProperty; @@ -38,10 +36,6 @@ public NodeWithUUIDAsId(String someProperty) { this.someProperty = someProperty; } - public Long getId() { - return id; - } - public UUID getMyNiceId() { return myNiceId; } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java index b779c2ee83..fce5e01a68 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java @@ -138,6 +138,12 @@ public interface UserRepository extends PersonRepository { Slice findByNameAndRatingsStars(String name, int stars, Pageable pageable); + Page findAllByIdIn(Iterable id, Pageable pageable); + + List findAllByIdInAndNameLike(Iterable id, String name); + + List findAllByIdAndName(Long id, String name); + @Query("invalid") void invalidQuery(); diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTests.java index 9dd3a6e0d2..375cd297f9 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTests.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTests.java @@ -16,11 +16,12 @@ package org.springframework.data.neo4j.queries; import static org.assertj.core.api.Assertions.*; -import static org.assertj.core.api.Assertions.assertThat; +import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Optional; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -623,6 +624,59 @@ public void shouldSliceDerivedFinderQueries() { assertThat(page.hasNext()).isFalse(); } + @Test // DATAGRAPH-1286 + public void findAllByIdShouldSupportPageableParameter() { + + List ids = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + User u = new User("U" + i); + userRepository.save(u); + ids.add(u.getId()); + } + + // Just make sure stuff exists and OGM correctly uses id(n) instead of n.id :( + Optional randomUser = userRepository.findById(ids.get(ids.size() - 2)); + assertThat(randomUser).isPresent().map(User::getName).hasValue("U8"); + + // Assert findAllById works _at all_ + Iterable allUsers = userRepository.findAllById(ids); + assertThat(allUsers).hasSize(ids.size()); + + Pageable pageable = PageRequest.of(0, 2); + Page page = userRepository.findAllByIdIn(ids.subList(0, 4), pageable); + assertThat(page.getSize()).isEqualTo(2); + assertThat(page.getContent()).hasSize(2); + assertThat(page.hasNext()).isTrue(); + } + + @Test // DATAGRAPH-1286 + public void findByIdInInDerivedQueryMethodShouldWork() { + + List ids = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + User u = new User("U" + i); + userRepository.save(u); + ids.add(u.getId()); + } + + List users = userRepository.findAllByIdInAndNameLike(ids.subList(0, 4), "U*"); + assertThat(users).hasSize(4); + } + + @Test // DATAGRAPH-1286 + public void findByIdEqualsInDerivedQueryMethodShouldWork() { + + List ids = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + User u = new User("U" + i); + userRepository.save(u); + ids.add(u.getId()); + } + + List users = userRepository.findAllByIdAndName(ids.get(2), "U2"); + assertThat(users).hasSize(1); + } + @Test // DATAGRAPH-1093 public void shouldFindNodeEntitiesByAttributeIgnoringCase() { executeUpdate("CREATE (:Director {name:'Patty Jenkins'})\n" + // diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/Neo4jRepositoryTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/Neo4jRepositoryTests.java index 381aeaf86e..b7f081905d 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/Neo4jRepositoryTests.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/Neo4jRepositoryTests.java @@ -80,6 +80,18 @@ public void explicitIdsWithCustomTypesShouldWork() throws Exception { assertThat(nodeWithUUIDAsIdRepository.count()).isEqualTo(0L); } + @Test // DATAGRAPH-1286 + public void findByIdEqualsInDerivedQueryMethodShouldWork() { + + NodeWithUUIDAsId entity = new NodeWithUUIDAsId("someProperty"); + nodeWithUUIDAsIdRepository.save(entity); + + Optional retrievedEntity = nodeWithUUIDAsIdRepository + .findOneByMyNiceIdAndSomeProperty(entity.getMyNiceId(), entity.getSomeProperty()); + assertThat(retrievedEntity.isPresent()).isTrue(); + assertThat(retrievedEntity.get()).isEqualTo(entity); + } + @Configuration @Neo4jIntegrationTest(domainPackages = "org.springframework.data.neo4j.domain.sample") static class Config {} @@ -87,4 +99,7 @@ static class Config {} interface SampleEntityRepository extends Neo4jRepository {} -interface NodeWithUUIDAsIdRepository extends Neo4jRepository {} +interface NodeWithUUIDAsIdRepository extends Neo4jRepository { + + Optional findOneByMyNiceIdAndSomeProperty(UUID id, String someProperty); +}