From a2f47a5b49b1e0ea0df4d4ed6675cbfa56e29c5e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 27 Jun 2024 14:39:41 +0200 Subject: [PATCH] Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip Query.setFirstResult(…) if the pagable offset is not zero. See #3242 Original pull request: #3454 --- .../jpa/repository/query/ParameterBinder.java | 16 +++++++---- .../query/ParameterBinderUnitTests.java | 28 +++++++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java index dfd0dabce..78fc9531e 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java @@ -17,6 +17,7 @@ package org.springframework.data.jpa.repository.query; import jakarta.persistence.Query; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling; import org.springframework.data.jpa.support.PageableUtils; import org.springframework.util.Assert; @@ -98,16 +99,19 @@ public class ParameterBinder { bind(query, metadata, accessor); - if (!useJpaForPaging || !parameters.hasLimitingParameters() || accessor.getPageable().isUnpaged()) { + Pageable pageable = accessor.getPageable(); + + if (!useJpaForPaging || !parameters.hasLimitingParameters() || pageable.isUnpaged()) { return query; } - // see #3242 - if (!parameters.hasLimitParameter()) { - // offset is meaningless if Limit parameter present - query.setFirstResult(PageableUtils.getOffsetAsInteger(accessor.getPageable())); + // Apply offset only if it is not 0 (the default). + int offset = PageableUtils.getOffsetAsInteger(pageable); + if (offset != 0) { + query.setFirstResult(offset); } - query.setMaxResults(accessor.getPageable().getPageSize()); + + query.setMaxResults(pageable.getPageSize()); return query; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java index 3cd30f291..4f90c40c7 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterBinderUnitTests.java @@ -126,37 +126,41 @@ class ParameterBinderUnitTests { verify(query).setParameter(eq(1), eq("foo")); } - @Test + @Test // GH-3242 void bindAndPrepareWorksWithPageable() throws Exception { Method validWithPageable = SampleRepository.class.getMethod("validWithPageable", String.class, Pageable.class); - Object[] values = { "foo", Pageable.ofSize(10).withPage(3) }; + bindAndPrepare(validWithPageable, values); - verify(query).setParameter(eq(1), eq("foo")); - verify(query).setFirstResult(eq(30)); - verify(query).setMaxResults(eq(10)); + + verify(query).setParameter(1, "foo"); + verify(query).setFirstResult(30); + verify(query).setMaxResults(10); } - @Test + @Test // GH-3242 void bindWorksWithNullForLimit() throws Exception { Method validWithLimit = SampleRepository.class.getMethod("validWithLimit", String.class, Limit.class); - Object[] values = { "foo", null }; + bind(validWithLimit, values); - verify(query).setParameter(eq(1), eq("foo")); + + verify(query).setParameter(1, "foo"); + verify(query, never()).setFirstResult(anyInt()); } - @Test + @Test // GH-3242 void bindAndPrepareWorksWithLimit() throws Exception { Method validWithLimit = SampleRepository.class.getMethod("validWithLimit", String.class, Limit.class); - Object[] values = { "foo", Limit.of(10) }; + bindAndPrepare(validWithLimit, values); - verify(query).setParameter(eq(1), eq("foo")); - verify(query).setMaxResults(eq(10)); + + verify(query).setParameter(1, "foo"); + verify(query).setMaxResults(10); verify(query, never()).setFirstResult(anyInt()); }