Avoid count query in JpaEntityRepositoryBase if limit is not specifie…

…d or reached #718

crnk-data/crnk-data-jpa/src/main/java/io/crnk/data/jpa/JpaEntityRepositoryBase.java

@@ -93,20 +93,13 @@ public ResourceList<T> findAll(QuerySpec querySpec) {
 
 		JpaRepositoryUtils.prepareQuery(query, optimizedQuerySpec, computedAttrs);
 		query = JpaRepositoryUtils.filterQuery(repositoryConfig, this, optimizedQuerySpec, query);
-		JpaQueryExecutor<?> executor = query.buildExecutor();
-
-		if (optimizeForInclusion(querySpec)) {
-			IncludeRelationSpec includedRelationSpec = querySpec.getIncludedRelations().get(0);
-			executor.fetch(includedRelationSpec.getAttributePath());
-		}
-
-		boolean fetchNext = repositoryConfig.isNextFetched(optimizedQuerySpec);
-		boolean fetchTotal = repositoryConfig.isTotalFetched(optimizedQuerySpec);
-
-		JpaRepositoryUtils.prepareExecutor(executor, optimizedQuerySpec, fetchRelations());
+		JpaQueryExecutor<?> executor = createExecutor(query, optimizedQuerySpec);
 
+		boolean fetchNext = isNextFetched(optimizedQuerySpec);
+		boolean fetchTotal = isTotalFetched(optimizedQuerySpec);
+		int limit = executor.getLimit();
 		if (fetchNext) {
-			executor.setLimit(executor.getLimit() + 1);
+			executor.setLimit(limit + 1);
 		}
 
 		executor = JpaRepositoryUtils.filterExecutor(repositoryConfig, this, optimizedQuerySpec, executor);
@@ -128,8 +121,7 @@ public ResourceList<T> findAll(QuerySpec querySpec) {
 		JpaRepositoryUtils.fillResourceList(repositoryConfig, tuples, resources);
 		resources = JpaRepositoryUtils.filterResults(repositoryConfig, this, optimizedQuerySpec, resources);
 		if (fetchTotal) {
-			long totalRowCount = executor.getTotalRowCount();
-			((PagedMetaInformation) metaInfo).setTotalResourceCount(totalRowCount);
+			((PagedMetaInformation) metaInfo).setTotalResourceCount(computeTotalCount(executor, tuples));
 		}
 		if (fetchNext) {
 			((HasMoreResourcesMetaInformation) metaInfo).setHasMoreResources(hasNext);
@@ -138,6 +130,39 @@ public ResourceList<T> findAll(QuerySpec querySpec) {
 		return resources;
 	}
 
+	protected Long computeTotalCount(JpaQueryExecutor<?> executor, List<Tuple> tuples) {
+		int limit = executor.getLimit();
+		int offset = executor.getOffset();
+		if (limit == -1 || tuples.size() < limit) {
+			// we don't need to count again if we did not limit or we did not reach the limit
+			// avoids an additional query
+			return (long) offset + tuples.size();
+		} else {
+			return executor.getTotalRowCount();
+		}
+	}
+
+	protected boolean isNextFetched(QuerySpec querySpec) {
+		return repositoryConfig.isNextFetched(querySpec);
+	}
+
+	protected boolean isTotalFetched(QuerySpec querySpec) {
+		return repositoryConfig.isTotalFetched(querySpec);
+	}
+
+	protected JpaQueryExecutor<?> createExecutor(JpaQuery<?> query, QuerySpec querySpec) {
+		JpaQueryExecutor<?> executor = query.buildExecutor();
+
+		if (optimizeForInclusion(querySpec)) {
+			IncludeRelationSpec includedRelationSpec = querySpec.getIncludedRelations().get(0);
+			executor.fetch(includedRelationSpec.getAttributePath());
+		}
+
+		JpaRepositoryUtils.prepareExecutor(executor, querySpec, fetchRelations());
+		return executor;
+	}
+
+
 	/**
 	 * override to customize query.
 	 */
@@ -271,8 +296,7 @@ public void delete(I id) {
 		ResourceField idField = getIdField();
 		if (idField.getUnderlyingName().equals(primaryKeyAttribute.getName())) {
 			pk = id;
-		}
-		else {
+		} else {
 			T resource = findOne(id, new QuerySpec(getResourceClass()));
 			pk = PropertyUtils.getProperty(resource, primaryKeyAttribute.getName());
 			if (pk == null) {

crnk-data/crnk-data-jpa/src/main/java/io/crnk/data/jpa/JpaRepositoryConfig.java

@@ -214,7 +214,7 @@ && getListMetaClass() != null
     }
 
     public boolean isTotalFetched(QuerySpec querySpec) {
-        return querySpec.getLimit() != null && isTotalAvailable()
+        return (querySpec.getOffset() != 0 || querySpec.getLimit() != null) && isTotalAvailable()
                 && getListMetaClass() != null
                 && PagedMetaInformation.class.isAssignableFrom(getListMetaClass());
     }

crnk-data/crnk-data-jpa/src/main/java/io/crnk/data/jpa/internal/query/AbstractQueryExecutorImpl.java

@@ -37,7 +37,7 @@ public abstract class AbstractQueryExecutorImpl<T> implements JpaQueryExecutor<T
 	protected Map<String, Integer> selectionBindings;
 
 	public AbstractQueryExecutorImpl(EntityManager em, MetaDataObject meta, int numAutoSelections,
-			Map<String, Integer> selectionBindings) {
+									 Map<String, Integer> selectionBindings) {
 		this.em = em;
 		this.meta = meta;
 		this.numAutoSelections = numAutoSelections;
@@ -90,6 +90,11 @@ public int getLimit() {
 		return limit;
 	}
 
+	@Override
+	public int getOffset() {
+		return offset;
+	}
+
 	@Override
 	public void setPaging(PagingSpec pagingSpec) {
 		OffsetLimitPagingSpec offsetLimit = pagingSpec.convert(OffsetLimitPagingSpec.class);
@@ -126,8 +131,7 @@ public List<T> getResultList() {
 				entityList.add((T) values[0]);
 			}
 			resultList = entityList;
-		}
-		else {
+		} else {
 			resultList = (List<T>) list;
 		}
 		return resultList;
@@ -143,11 +147,9 @@ public T getUniqueResult(boolean nullable) {
 		}
 		if (!list.isEmpty()) {
 			return list.get(0);
-		}
-		else if (nullable) {
+		} else if (nullable) {
 			return null;
-		}
-		else {
+		} else {
 			throw new IllegalStateException("no result found");
 		}
 	}

crnk-data/crnk-data-jpa/src/main/java/io/crnk/data/jpa/query/JpaQueryExecutor.java

@@ -31,5 +31,7 @@ public interface JpaQueryExecutor<T> {
 
 	int getLimit();
 
+	int getOffset();
+
 	void setPaging(PagingSpec paging);
 }

crnk-data/crnk-data-jpa/src/test/java/io/crnk/data/jpa/JpaEntityRepositoryTestBase.java

@@ -112,8 +112,7 @@ public void testFindAllOrder(boolean asc) {
 		for (int i = 0; i < numTestEntities; i++) {
 			if (asc) {
 				Assert.assertEquals(i, list.get(i).getLongValue());
-			}
-			else {
+			} else {
 				Assert.assertEquals(numTestEntities - 1 - i, list.get(i).getLongValue());
 			}
 		}
@@ -331,6 +330,34 @@ public void testPagingLast() {
 		Assert.assertEquals(5, metaInformation.getTotalResourceCount().longValue());
 	}
 
+
+	@Test
+	public void testPagingLargeLimit() {
+		QuerySpec querySpec = new QuerySpec(TestEntity.class);
+		querySpec.setOffset(1L);
+		querySpec.setLimit(10L);
+
+		ResourceList<TestEntity> list = repo.findAll(querySpec);
+		Assert.assertEquals(4, list.size());
+
+		PagedMetaInformation metaInformation = list.getMeta(PagedMetaInformation.class);
+		Assert.assertEquals(5, metaInformation.getTotalResourceCount().longValue());
+	}
+
+
+	@Test
+	public void testOffserAndNoLimit() {
+		QuerySpec querySpec = new QuerySpec(TestEntity.class);
+		querySpec.setOffset(1L);
+		querySpec.setLimit(null);
+
+		ResourceList<TestEntity> list = repo.findAll(querySpec);
+		Assert.assertEquals(4, list.size());
+
+		PagedMetaInformation metaInformation = list.getMeta(PagedMetaInformation.class);
+		Assert.assertEquals(5, metaInformation.getTotalResourceCount().longValue());
+	}
+
 	@Test
 	public void testIncludeNoRelations() {
 		em.clear();