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 bb5142d6d..cbc49c876 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 class Neo4jPersistentProperty extends AnnotationBasedPersistentProperty 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 @@ class FilterBuildersDefinition { } 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 7d1833828..566276747 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 @@ -112,19 +112,6 @@ public class GraphQueryMethod extends QueryMethod { } } } - /* - //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 e09bb852b..48fa5584c 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.metadata.MetaData; 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 class PartTreeNeo4jQuery extends AbstractGraphRepositoryQuery { 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 ae23bea93..46526678e 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 @@ -18,6 +18,7 @@ package org.springframework.data.neo4j.repository.query; import java.util.Iterator; 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; @@ -31,17 +32,19 @@ import org.springframework.data.repository.query.parser.PartTree; */ 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 ae187fe06..db0e30860 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.Collections; 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 000000000..d0752b86c --- /dev/null +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/filter/NativeIdFilterFunction.java @@ -0,0 +1,87 @@ +/* + * 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.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 830d4fb42..ebd823179 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 @@ class PropertyComparisonBuilder extends FilterBuilder { 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 cc80497f3..9093589bf 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 @@ import org.neo4j.ogm.typeconversion.UuidStringConverter; */ 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 class NodeWithUUIDAsId { 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 ad0251479..5791e3968 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 @@ -145,6 +145,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 a322e523e..2a56e5424 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 @@ -19,9 +19,11 @@ import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.*; +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; @@ -704,6 +706,59 @@ public class DerivedQueryTests extends MultiDriverTestClass { assertFalse(page.hasNext()); } + @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 35eeef3cf..fb683ae08 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.neo4j.repository; +import static org.assertj.core.api.Assertions.*; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; @@ -91,6 +92,18 @@ public class Neo4jRepositoryTests extends MultiDriverTestClass { assertThat(nodeWithUUIDAsIdRepository.count(), is(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 @EnableNeo4jRepositories @EnableTransactionManagement @@ -111,4 +124,7 @@ public class Neo4jRepositoryTests extends MultiDriverTestClass { interface SampleEntityRepository extends Neo4jRepository {} -interface NodeWithUUIDAsIdRepository extends Neo4jRepository {} +interface NodeWithUUIDAsIdRepository extends Neo4jRepository { + + Optional findOneByMyNiceIdAndSomeProperty(UUID id, String someProperty); +}