diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphParametersParameterAccessor.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphParametersParameterAccessor.java index 9c09e9210..83612957a 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphParametersParameterAccessor.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphParametersParameterAccessor.java @@ -20,7 +20,9 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.domain.Sort; import org.springframework.data.neo4j.annotation.Depth; +import org.springframework.data.neo4j.util.PagingAndSortingUtils; import org.springframework.data.repository.query.ParametersParameterAccessor; +import org.springframework.lang.Nullable; /** * Custom {@link ParametersParameterAccessor} to allow access to the {@link Depth} parameter. @@ -67,17 +69,7 @@ public class GraphParametersParameterAccessor extends ParametersParameterAccesso @Override public SortOrder getOgmSort() { - SortOrder sortOrder = new SortOrder(); - if (getSort() != null) { - for (Sort.Order order : getSort()) { - if (order.isAscending()) { - sortOrder.add(order.getProperty()); - } else { - sortOrder.add(SortOrder.Direction.DESC, order.getProperty()); - } - } - } - return sortOrder; + return PagingAndSortingUtils.convert(getSort()); } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryExecution.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryExecution.java index 8735a176a..c3f160b81 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryExecution.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/GraphQueryExecution.java @@ -18,14 +18,18 @@ package org.springframework.data.neo4j.repository.query; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.function.LongSupplier; +import java.util.Optional; +import org.neo4j.ogm.cypher.query.Pagination; +import org.neo4j.ogm.cypher.query.SortOrder; import org.neo4j.ogm.model.Result; import org.neo4j.ogm.session.Session; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.SliceImpl; +import org.springframework.data.domain.Sort; import org.springframework.data.neo4j.annotation.QueryResult; +import org.springframework.data.neo4j.util.PagingAndSortingUtils; import org.springframework.data.repository.support.PageableExecutionUtils; import org.springframework.util.Assert; @@ -89,7 +93,10 @@ public interface GraphQueryExecution { @Override public Object execute(Query query, Class type) { if (query.isFilterQuery()) { - return session.loadAll(type, query.getFilters(), accessor.getOgmSort(), accessor.getDepth()); + Pagination pagination = query.getOptionalPagination(null, false); + SortOrder ogmSort = PagingAndSortingUtils.convert(Optional.ofNullable(accessor.getSort()).filter(Sort::isSorted).orElseGet(query::getOptionalSort)); + return pagination == null ? session.loadAll(type, query.getFilters(), ogmSort, accessor.getDepth()) : session.loadAll(type, query.getFilters(), + ogmSort, pagination, accessor.getDepth()); } else { if (type.getAnnotation(QueryResult.class) != null || Map.class.isAssignableFrom(type)) { return session.query(query.getCypherQuery(accessor.getSort()), query.getParameters()).queryResults(); @@ -135,7 +142,7 @@ public interface GraphQueryExecution { long count; if (query.isFilterQuery()) { result = (List) session.loadAll(type, query.getFilters(), accessor.getOgmSort(), - query.getPagination(pageable, false), accessor.getDepth()); + query.getOptionalPagination(pageable, false), accessor.getDepth()); count = session.count(type, query.getFilters()); } else { if (type.getAnnotation(QueryResult.class) != null) { @@ -146,7 +153,7 @@ public interface GraphQueryExecution { count = (result.size() > 0) ? countTotalNumberOfElements(query) : 0; } - return PageableExecutionUtils.getPage(result, pageable, (LongSupplier) () -> count); + return PageableExecutionUtils.getPage(result, pageable, () -> count); } private Integer countTotalNumberOfElements(Query query) { @@ -176,7 +183,7 @@ public interface GraphQueryExecution { if (query.isFilterQuery()) { // For a slice, need one extra result to determine if there is a next page result = (List) session.loadAll(type, query.getFilters(), accessor.getOgmSort(), - query.getPagination(pageable, true), accessor.getDepth()); + query.getOptionalPagination(pageable, true), accessor.getDepth()); } else { String cypherQuery = query.getCypherQuery(pageable, true); if (type.getAnnotation(QueryResult.class) != 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 48fa5584c..8334eb75b 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 @@ -52,13 +52,12 @@ public class PartTreeNeo4jQuery extends AbstractGraphRepositoryQuery { Class domainType = graphQueryMethod.getEntityInformation().getJavaType(); this.graphQueryMethod = graphQueryMethod; this.tree = new PartTree(graphQueryMethod.getName(), domainType); - this.queryTemplate = new TemplatedQueryCreator(this.tree, (Neo4jMappingContext) this.graphQueryMethod.getMappingContext(), domainType).createQuery(); } @Override - protected Object doExecute(Query params, Object[] parameters) { + protected Object doExecute(Query query, Object[] parameters) { if (LOG.isDebugEnabled()) { LOG.debug("Executing query for method {}", graphQueryMethod.getName()); @@ -72,7 +71,7 @@ public class PartTreeNeo4jQuery extends AbstractGraphRepositoryQuery { } ResultProcessor processor = graphQueryMethod.getResultProcessor().withDynamicProjection(accessor); - Object results = getExecution(accessor).execute(params, processor.getReturnedType().getDomainType()); + Object results = getExecution(accessor).execute(query, processor.getReturnedType().getDomainType()); return processor.processResult(results); } @@ -81,7 +80,7 @@ public class PartTreeNeo4jQuery extends AbstractGraphRepositoryQuery { protected Query getQuery(Object[] parameters) { Map resolvedParameters = resolveParameters(parameters); - return this.queryTemplate.createExecutableQuery(resolvedParameters); + return this.queryTemplate.createExecutableQuery(resolvedParameters, this.tree.isLimiting() ? this.tree.getMaxResults() : null, tree.getSort()); } @Override diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/Query.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/Query.java index 60fa9a8c9..ba1569cdf 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/Query.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/Query.java @@ -37,19 +37,25 @@ import org.springframework.util.Assert; */ public class Query { - private static final String SKIP = "sdnSkip"; - private static final String LIMIT = "sdnLimit"; - private static final String SKIP_LIMIT = " SKIP $" + SKIP + " LIMIT $" + LIMIT; + private static final String SKIP_PARAM = "sdnSkip"; + private static final String LIMIT_PARAM = "sdnLimit"; + private static final String SKIP_LIMIT = " SKIP $" + SKIP_PARAM + " LIMIT $" + LIMIT_PARAM; + private static final String LIMIT = "LIMIT $" + LIMIT_PARAM; private static final String ORDER_BY_CLAUSE = " ORDER BY %s"; private Filters filters; private String cypherQuery; private Map parameters; private @Nullable String countQuery; + private @Nullable Integer optionalLimit; + private @Nullable Sort optionalSort; + + public Query(Filters filters, @Nullable Integer optionalLimit, Sort optionalSort) { - public Query(Filters filters) { Assert.notNull(filters, "Filters must not be null."); this.filters = filters; + this.optionalLimit = optionalLimit; + this.optionalSort = optionalSort; } public Query(String cypherQuery, @Nullable String countQuery, Map parameters) { @@ -107,11 +113,11 @@ public class Query { // Custom queries in the OGM do not support pageable cypherQuery = formatBaseQuery(cypherQuery); cypherQuery = cypherQuery + SKIP_LIMIT; - parameters.put(SKIP, pageable.getOffset()); + parameters.put(SKIP_PARAM, pageable.getOffset()); if (forSlicing) { - parameters.put(LIMIT, pageable.getPageSize() + 1); + parameters.put(LIMIT_PARAM, pageable.getPageSize() + 1); } else { - parameters.put(LIMIT, pageable.getPageSize()); + parameters.put(LIMIT_PARAM, pageable.getPageSize()); } return cypherQuery; } @@ -153,14 +159,25 @@ public class Query { return cypherQuery; } - public Pagination getPagination(Pageable pageable, boolean forSlicing) { - Pagination pagination = new Pagination(pageable.getPageNumber(), pageable.getPageSize() + ((forSlicing) ? 1 : 0)); - pagination.setOffset(pageable.getPageNumber() * pageable.getPageSize()); + @Nullable Pagination getOptionalPagination(@Nullable Pageable pageable, boolean forSlicing) { + + if(pageable != null) { + Pagination pagination = new Pagination(pageable.getPageNumber(),pageable.getPageSize() + ((forSlicing) ? 1 : 0)); + pagination.setOffset(pageable.getPageNumber() * pageable.getPageSize()); + return pagination; + } + + if(this.optionalLimit == null) { + return null; + } + + Pagination pagination = new Pagination(0, optionalLimit); + pagination.setOffset(0); return pagination; } - public SortOrder getSort(Pageable pageable) { - return PagingAndSortingUtils.convert(pageable.getSort()); + @Nullable Sort getOptionalSort() { + return optionalSort; } private String sanitize(String cypherQuery) { diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQuery.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQuery.java index f7cfb884e..541c9753c 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQuery.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/TemplatedQuery.java @@ -23,7 +23,9 @@ import java.util.Stack; import org.neo4j.ogm.cypher.Filter; import org.neo4j.ogm.cypher.Filters; +import org.springframework.data.domain.Sort; import org.springframework.data.neo4j.repository.query.filter.FilterBuilder; +import org.springframework.lang.Nullable; /** * A template query based on filters. {@link #createExecutableQuery(Map)} is used to create an executable query from @@ -47,7 +49,8 @@ class TemplatedQuery { this.filterBuilders = filterBuilders; } - Query createExecutableQuery(Map resolvedParameters) { + Query createExecutableQuery(Map resolvedParameters, @Nullable Integer optionalLimit, @Nullable + Sort optionalSort) { // building a stack of parameter values, so that the builders can pull them // according to their needs (zero, one or more parameters) @@ -65,6 +68,6 @@ class TemplatedQuery { filters.addAll(filterBuilder.build(parametersStack)); } - return new Query(new Filters(filters)); + return new Query(new Filters(filters), optionalLimit, optionalSort); } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/util/PagingAndSortingUtils.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/util/PagingAndSortingUtils.java index 2e5012582..58b04630d 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/util/PagingAndSortingUtils.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/util/PagingAndSortingUtils.java @@ -17,14 +17,18 @@ package org.springframework.data.neo4j.util; import org.neo4j.ogm.cypher.query.SortOrder; import org.springframework.data.domain.Sort; +import org.springframework.lang.Nullable; +/** + * @author Nicolas Mervaillie + * @author Michael J. Simons + */ public class PagingAndSortingUtils { - public static SortOrder convert(Sort sort) { - + public static SortOrder convert(@Nullable Sort sort) { SortOrder sortOrder = new SortOrder(); - if (sort != Sort.unsorted()) { + if (sort != null && sort != Sort.unsorted()) { for (Sort.Order order : sort) { if (order.isAscending()) { sortOrder.add(order.getProperty()); 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 2eabadc0c..3b4810488 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 @@ -55,6 +55,12 @@ public interface UserRepository extends PersonRepository { List findByRatingsStarsIgnoreCase(int stars); + List findFirstByOrderByNameDesc(); + + List findTop5ByOrderByNameDesc(); + + Slice findTop5ByOrderByNameDesc(Pageable pageable); + @Query("MATCH (c:User) SET c.surname = 'Helge' RETURN c") List bulkUpdateReturningNode(); 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 f21efd30e..961213cff 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 @@ -15,9 +15,13 @@ */ package org.springframework.data.neo4j.queries; -import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import java.util.ArrayList; import java.util.Collection; @@ -40,6 +44,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; +import org.springframework.data.domain.Sort; import org.springframework.data.neo4j.examples.movies.domain.Cinema; import org.springframework.data.neo4j.examples.movies.domain.Director; import org.springframework.data.neo4j.examples.movies.domain.TempMovie; @@ -748,6 +753,30 @@ public class DerivedQueryTests { assertThat(foundUser.getName()).isEqualTo(userName); } + @Test // GH-1807 + public void limitingQueriesShouldWork() { + + transactionTemplate.executeWithoutResult(tx -> { + + userRepository.deleteAll(); + for(int i=0; i<7; ++i) { + User user = new User("U" + i); + userRepository.save(user); + } + }); + + // Limiting via find + assertThat(userRepository.findFirstByOrderByNameDesc()).extracting(User::getName).containsExactly("U6"); + + // Limiting via topN + assertThat(userRepository.findTop5ByOrderByNameDesc()).extracting(User::getName) + .containsExactly("U6", "U5", "U4", "U3", "U2"); + + // Mixing limiting and page request should make the slice win + assertThat(userRepository.findTop5ByOrderByNameDesc(PageRequest.of(0, 1, Sort.by("name").ascending()))).extracting(User::getName) + .containsExactly("U0"); + } + class DerivedQueryRunner implements Runnable { private final CountDownLatch latch; diff --git a/spring-data-neo4j/src/test/resources/logback-test.xml b/spring-data-neo4j/src/test/resources/logback-test.xml index 0d269f1ed..186565208 100644 --- a/spring-data-neo4j/src/test/resources/logback-test.xml +++ b/spring-data-neo4j/src/test/resources/logback-test.xml @@ -26,6 +26,7 @@ +