From 59aa91f99492644e99d3b4c0b96280853ad2fbe3 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Mon, 12 Jun 2023 11:15:08 +0800 Subject: [PATCH] Exclude id if provided sort properties is keyset qualified See GH-2996 --- .../query/KeysetScrollSpecification.java | 28 +++++---- .../support/JpaEntityInformation.java | 15 ++++- .../JpaMetamodelEntityInformation.java | 61 +++++++++++++++++-- .../data/jpa/domain/sample/Product.java | 4 ++ .../KeysetScrollSpecificationUnitTests.java | 18 ++++++ 5 files changed, 109 insertions(+), 17 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java index 42846ccc62..afb2b8341a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java @@ -40,6 +40,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author Yanming Zhou * @since 3.1 */ public record KeysetScrollSpecification (KeysetScrollPosition position, Sort sort, @@ -63,21 +64,26 @@ public static Sort createSort(KeysetScrollPosition position, Sort sort, JpaEntit KeysetScrollDelegate delegate = KeysetScrollDelegate.of(position.getDirection()); - Collection sortById; Sort sortToUse; - if (entity.hasCompositeId()) { - sortById = new ArrayList<>(entity.getIdAttributeNames()); - } else { - sortById = new ArrayList<>(1); - sortById.add(entity.getRequiredIdAttribute().getName()); - } - - sort.forEach(it -> sortById.remove(it.getProperty())); - if (sortById.isEmpty()) { + if (entity.isKeysetQualified(sort.stream().map(Order::getProperty).toList())) { sortToUse = sort; } else { - sortToUse = sort.and(Sort.by(sortById.toArray(new String[0]))); + Collection sortById; + if (entity.hasCompositeId()) { + sortById = new ArrayList<>(entity.getIdAttributeNames()); + } else { + sortById = new ArrayList<>(1); + sortById.add(entity.getRequiredIdAttribute().getName()); + } + + sort.forEach(it -> sortById.remove(it.getProperty())); + + if (sortById.isEmpty()) { + sortToUse = sort; + } else { + sortToUse = sort.and(Sort.by(sortById.toArray(new String[0]))); + } } return delegate.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 d59e6316f9..9f4d95698c 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 @@ -30,6 +30,7 @@ * @author Oliver Gierke * @author Thomas Darimont * @author Mark Paluch + * @author Yanming Zhou */ public interface JpaEntityInformation extends EntityInformation, JpaEntityMetadata { @@ -79,7 +80,8 @@ public interface JpaEntityInformation extends EntityInformation, J 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 @@ -87,4 +89,15 @@ public interface JpaEntityInformation extends EntityInformation, J * @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 362bf21c8a..3d5dc4965d 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; @@ -44,6 +45,7 @@ import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Implementation of {@link org.springframework.data.repository.core.EntityInformation} that uses JPA {@link Metamodel} @@ -55,6 +57,7 @@ * @author Mark Paluch * @author Jens Schauder * @author Greg Turnquist + * @author Yanming Zhou */ public class JpaMetamodelEntityInformation extends JpaEntityInformationSupport { @@ -236,12 +239,14 @@ public Map getKeyset(Iterable propertyPaths, T entity) { Map keyset = new LinkedHashMap<>(); - if (hasCompositeId()) { - for (String idAttributeName : getIdAttributeNames()) { - keyset.put(idAttributeName, getter.apply(idAttributeName)); + if(!isKeysetQualified(propertyPaths)) { + if (hasCompositeId()) { + for (String idAttributeName : getIdAttributeNames()) { + keyset.put(idAttributeName, getter.apply(idAttributeName)); + } + } else { + keyset.put(getIdAttribute().getName(), getId(entity)); } - } else { - keyset.put(getIdAttribute().getName(), getId(entity)); } for (String propertyPath : propertyPaths) { @@ -251,6 +256,52 @@ 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; + } + + @Nullable + 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(); + } + } 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(); + } + } 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..583a912c04 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,9 @@ public class Product { @Id @GeneratedValue private Long id; + @Column(unique = true) + private String code; + 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 09362c5c5d..7db8e6624f 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:infrastructure.xml" }) @@ -74,4 +79,17 @@ 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"); + } }