diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java index 9a9841b425..f5e1e2ddee 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java @@ -34,6 +34,7 @@ * Delegate for keyset scrolling. * * @author Mark Paluch + * @author Yanming Zhou * @since 3.1 */ public class KeysetScrollDelegate { @@ -142,19 +143,24 @@ public Sort createSort(Sort sort, JpaEntityInformation entity) { Collection sortById; Sort sortToUse; - if (entity.hasCompositeId()) { - sortById = new ArrayList<>(entity.getIdAttributePaths()); - } else { - sortById = new ArrayList<>(1); - sortById.add(entity.getRequiredIdAttribute().getName()); + if (entity.isKeysetQualified(sort.stream().map(Order::getProperty).toList())) { + sortToUse = sort; } + else { + if (entity.hasCompositeId()) { + sortById = new ArrayList<>(entity.getIdAttributePaths()); + } else { + sortById = new ArrayList<>(1); + sortById.add(entity.getRequiredIdAttribute().getName()); + } - sort.forEach(it -> sortById.remove(it.getProperty())); + sort.forEach(it -> sortById.remove(it.getProperty())); - if (sortById.isEmpty()) { - sortToUse = sort; - } else { - sortToUse = sort.and(Sort.by(sortById.toArray(new String[0]))); + if (sortById.isEmpty()) { + sortToUse = sort; + } else { + sortToUse = sort.and(Sort.by(sortById.toArray(new String[0]))); + } } return getSortOrders(sortToUse); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformation.java index 012cf8e48b..cebbe44245 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformation.java @@ -31,6 +31,7 @@ * @author Oliver Gierke * @author Thomas Darimont * @author Mark Paluch + * @author Yanming Zhou */ public interface JpaEntityInformation extends EntityInformation, JpaEntityMetadata { @@ -90,7 +91,8 @@ default Collection getIdAttributePaths() { Object getCompositeIdAttributeValue(Object id, String idAttribute); /** - * Extract a keyset for {@code propertyPaths} and the primary key (including composite key components if applicable). + * Extract a keyset for {@code propertyPaths}, and the primary key (including composite key components if applicable) + * if {@code propertyPaths} is not qualified. * * @param propertyPaths the property paths that make up the keyset in combination with the composite key components. * @param entity the entity to extract values from @@ -98,4 +100,15 @@ default Collection getIdAttributePaths() { * @since 3.1 */ Map getKeyset(Iterable propertyPaths, T entity); + + /** + * Determine whether propertyPaths is qualified for keyset. + * + * @param propertyPaths the property paths that make up the keyset in combination with the composite key components. + * @return {@code propertyPaths} is qualified for keyset. + * @since 3.2 + */ + default boolean isKeysetQualified(Iterable propertyPaths) { + return false; + } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java index dbc66002f3..59445800dd 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java @@ -15,6 +15,7 @@ */ package org.springframework.data.jpa.repository.support; +import jakarta.persistence.Column; import jakarta.persistence.IdClass; import jakarta.persistence.PersistenceUnitUtil; import jakarta.persistence.Tuple; @@ -46,6 +47,7 @@ import org.springframework.data.jpa.util.JpaMetamodel; import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Implementation of {@link org.springframework.data.repository.core.EntityInformation} that uses JPA {@link Metamodel} @@ -57,6 +59,7 @@ * @author Mark Paluch * @author Jens Schauder * @author Greg Turnquist + * @author Yanming Zhou */ public class JpaMetamodelEntityInformation extends JpaEntityInformationSupport { @@ -270,12 +273,14 @@ public Map getKeyset(Iterable propertyPaths, T entity) { Map keyset = new LinkedHashMap<>(); - if (hasCompositeId()) { - for (String idAttributeName : getIdAttributePaths()) { - keyset.put(idAttributeName, getter.apply(idAttributeName)); + if(!isKeysetQualified(propertyPaths)) { + if (hasCompositeId()) { + for (String idAttributeName : getIdAttributePaths()) { + keyset.put(idAttributeName, getter.apply(idAttributeName)); + } + } else { + keyset.put(getIdAttribute().getName(), getId(entity)); } - } else { - keyset.put(getIdAttribute().getName(), getId(entity)); } for (String propertyPath : propertyPaths) { @@ -285,6 +290,51 @@ public Map getKeyset(Iterable propertyPaths, T entity) { return keyset; } + @Override + public boolean isKeysetQualified(Iterable propertyPaths) { + + if (propertyPaths.iterator().hasNext()) { + for (String property : propertyPaths) { + if (isUnique(property)) { + return true; + } + } + } + + return false; + } + + private boolean isUnique(String property) { + + Class clazz = getJavaType(); + + while (clazz != Object.class) { + + try { + Column column = clazz.getDeclaredField(property).getAnnotation(Column.class); + if (column != null) { + return column.unique() && !column.nullable(); + } + } catch (NoSuchFieldException ex) { + // ignore + } + + try { + String getter = "get" + StringUtils.capitalize(property); + Column column = clazz.getDeclaredMethod(getter).getAnnotation(Column.class); + if (column != null) { + return column.unique() && !column.nullable(); + } + } catch (NoSuchMethodException ex) { + // ignore + } + + clazz = clazz.getSuperclass(); + } + + return false; + } + private Function getPropertyValueFunction(Object entity) { if (entity instanceof Tuple t) { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Product.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Product.java index 304d4d52fd..19769528e0 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Product.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/Product.java @@ -1,5 +1,6 @@ package org.springframework.data.jpa.domain.sample; +import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; @@ -9,6 +10,12 @@ public class Product { @Id @GeneratedValue private Long id; + @Column(unique = true, nullable = false) + private String code; + + @Column(unique = true) + private String secondaryCode; + public Long getId() { return id; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecificationUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecificationUnitTests.java index 8e3351f7ad..cc5680b0c1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecificationUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecificationUnitTests.java @@ -25,6 +25,7 @@ import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.jpa.domain.sample.Product; import org.springframework.data.jpa.domain.sample.SampleWithIdClass; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformation; @@ -32,10 +33,14 @@ import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.transaction.annotation.Transactional; +import java.util.List; +import java.util.Map; + /** * Unit tests for {@link KeysetScrollSpecification}. * * @author Mark Paluch + * @author Yanming Zhou */ @ExtendWith(SpringExtension.class) @ContextConfiguration("classpath:hibernate-infrastructure.xml") @@ -74,4 +79,31 @@ void shouldSkipExistingIdentifiersInSort() { assertThat(sort).extracting(Order::getProperty).containsExactly("id", "firstname"); } + @Test // GH-3013 + void shouldSkipIdentifiersInSortIfUniquePropertyPresent() { + + JpaMetamodelEntityInformation info = new JpaMetamodelEntityInformation<>(Product.class, em.getMetamodel(), + em.getEntityManagerFactory().getPersistenceUnitUtil()); + Map keyset = info.getKeyset(List.of("code"), new Product()); + + assertThat(keyset).containsOnlyKeys("code"); + + Sort sort = KeysetScrollSpecification.createSort(ScrollPosition.keyset(), Sort.by("code"), info); + + assertThat(sort).extracting(Order::getProperty).containsExactly("code"); + } + + @Test // GH-3013 + void shouldNotSkipIdentifiersInSortIfUniquePropertyPresentButNullable() { + + JpaMetamodelEntityInformation info = new JpaMetamodelEntityInformation<>(Product.class, em.getMetamodel(), + em.getEntityManagerFactory().getPersistenceUnitUtil()); + Map keyset = info.getKeyset(List.of("secondaryCode"), new Product()); + + assertThat(keyset).containsOnlyKeys("secondaryCode", "id"); + + Sort sort = KeysetScrollSpecification.createSort(ScrollPosition.keyset(), Sort.by("secondaryCode"), info); + + assertThat(sort).extracting(Order::getProperty).containsExactly("secondaryCode", "id"); + } }