diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java index 1c231fa1..9a8ec66e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java @@ -140,6 +140,7 @@ public interface JdbcAggregateOperations { * @param the type of the aggregate roots. Must not be {@code null}. * @param sort the sorting information. Must not be {@code null}. * @return Guaranteed to be not {@code null}. + * @since 2.0 */ Iterable findAll(Class domainType, Sort sort); @@ -150,6 +151,7 @@ public interface JdbcAggregateOperations { * @param the type of the aggregate roots. Must not be {@code null}. * @param pageable the pagination information. Must not be {@code null}. * @return Guaranteed to be not {@code null}. + * @since 2.0 */ Page findAll(Class domainType, Pageable pageable); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java index 0e82af0b..81e8e92b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java @@ -226,6 +226,7 @@ public interface DataAccessStrategy extends RelationResolver { * @param the type of entities to load. * @param sort the sorting information. Must not be {@code null}. * @return Guaranteed to be not {@code null}. + * @since 2.0 */ Iterable findAll(Class domainType, Sort sort); @@ -236,6 +237,7 @@ public interface DataAccessStrategy extends RelationResolver { * @param the type of entities to load. * @param pageable the pagination information. Must not be {@code null}. * @return Guaranteed to be not {@code null}. + * @since 2.0 */ Iterable findAll(Class domainType, Pageable pageable); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index f0c6ddd8..b14d5ba2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -17,16 +17,7 @@ package org.springframework.data.jdbc.core.convert; import lombok.Value; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -420,24 +411,29 @@ class SqlGenerator { } private SelectBuilder.SelectOrdered selectBuilder(Collection keyColumns, Sort sort, Pageable pageable) { - SelectBuilder.SelectWhere baseSelect = this.selectBuilder(keyColumns); - if (baseSelect instanceof SelectBuilder.SelectFromAndJoin) { - if (pageable.isPaged()) { - return ((SelectBuilder.SelectFromAndJoin) baseSelect).limitOffset(pageable.getPageSize(), pageable.getOffset()) - .orderBy(extractOrderByFields(sort)); - } - return ((SelectBuilder.SelectFromAndJoin) baseSelect).orderBy(extractOrderByFields(sort)); + SelectBuilder.SelectOrdered sortable = this.selectBuilder(keyColumns); + sortable = applyPagination(pageable, sortable); + return sortable.orderBy(extractOrderByFields(sort)); - } else if (baseSelect instanceof SelectBuilder.SelectFromAndJoinCondition) { - if (pageable.isPaged()) { - return ((SelectBuilder.SelectFromAndJoinCondition) baseSelect) - .limitOffset(pageable.getPageSize(), pageable.getOffset()).orderBy(extractOrderByFields(sort)); - } - return baseSelect.orderBy(extractOrderByFields(sort)); - } else { - throw new RuntimeException("Unexpected type found!"); + } + + private SelectBuilder.SelectOrdered applyPagination(Pageable pageable, SelectBuilder.SelectOrdered select) { + + if (!pageable.isPaged()) { + return select; } + + Assert.isTrue(select instanceof SelectBuilder.SelectLimitOffset, + () -> String.format("Can't apply limit clause to statement of type %s", select.getClass())); + + SelectBuilder.SelectLimitOffset limitable = (SelectBuilder.SelectLimitOffset) select; + SelectBuilder.SelectLimitOffset limitResult = limitable.limitOffset(pageable.getPageSize(), pageable.getOffset()); + + Assert.state(limitResult instanceof SelectBuilder.SelectOrdered, + String.format("The result of applying the limit-clause must be of type SelectOrdered in order to apply the order-by-clause but is of type %s.", select.getClass())); + + return (SelectBuilder.SelectOrdered) limitResult; } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 4f5bdebb..d3d3d50e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -347,6 +347,7 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { */ @Override public Iterable findAll(Class domainType, Sort sort) { + Map additionalContext = new HashMap<>(); additionalContext.put("sort", sort); return sqlSession().selectList(namespace(domainType) + ".findAllSorted", @@ -359,6 +360,7 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { */ @Override public Iterable findAll(Class domainType, Pageable pageable) { + Map additionalContext = new HashMap<>(); additionalContext.put("pageable", pageable); return sqlSession().selectList(namespace(domainType) + ".findAllPaged", diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/SimpleJdbcRepository.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/SimpleJdbcRepository.java index cd1d64dc..92fbd1bb 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/SimpleJdbcRepository.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/SimpleJdbcRepository.java @@ -40,7 +40,7 @@ import org.springframework.transaction.annotation.Transactional; */ @RequiredArgsConstructor @Transactional(readOnly = true) -public class SimpleJdbcRepository implements CrudRepository, PagingAndSortingRepository { +public class SimpleJdbcRepository implements PagingAndSortingRepository { private final @NonNull JdbcAggregateOperations entityOperations; private final @NonNull PersistentEntity entity; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index c666d562..6066508e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -39,7 +39,6 @@ import org.junit.Assume; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; @@ -225,41 +224,51 @@ public class JdbcAggregateTemplateIntegrationTests { Iterable reloadedLegoSets = template.findAll(LegoSet.class); - assertThat(reloadedLegoSets).hasSize(1).extracting("id", "manual.id", "manual.content") - .contains(tuple(legoSet.getId(), legoSet.getManual().getId(), legoSet.getManual().getContent())); + assertThat(reloadedLegoSets) // + .extracting("id", "manual.id", "manual.content") // + .containsExactly(tuple(legoSet.getId(), legoSet.getManual().getId(), legoSet.getManual().getContent())); } @Test // DATAJDBC-101 public void saveAndLoadManyEntitiesWithReferencedEntitySorted() { + template.save(createLegoSet("Lava")); template.save(createLegoSet("Star")); template.save(createLegoSet("Frozen")); Iterable reloadedLegoSets = template.findAll(LegoSet.class, Sort.by("name")); - assertThat(reloadedLegoSets).hasSize(3).extracting("name").isEqualTo(Arrays.asList("Frozen", "Lava", "Star")); + assertThat(reloadedLegoSets) // + .extracting("name") // + .containsExactly("Frozen", "Lava", "Star"); } @Test // DATAJDBC-101 public void saveAndLoadManyEntitiesWithReferencedEntityPaged() { + template.save(createLegoSet("Lava")); template.save(createLegoSet("Star")); template.save(createLegoSet("Frozen")); Iterable reloadedLegoSets = template.findAll(LegoSet.class, PageRequest.of(1, 1)); - assertThat(reloadedLegoSets).hasSize(1).extracting("name").isEqualTo(singletonList("Star")); + assertThat(reloadedLegoSets) // + .extracting("name") // + .containsExactly("Star"); } @Test // DATAJDBC-101 public void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() { + template.save(createLegoSet("Lava")); template.save(createLegoSet("Star")); template.save(createLegoSet("Frozen")); Iterable reloadedLegoSets = template.findAll(LegoSet.class, PageRequest.of(1, 2, Sort.by("name"))); - assertThat(reloadedLegoSets).hasSize(1).extracting("name").isEqualTo(singletonList("Star")); + assertThat(reloadedLegoSets) // + .extracting("name") // + .containsExactly("Star"); } @Test // DATAJDBC-112 @@ -749,12 +758,10 @@ public class JdbcAggregateTemplateIntegrationTests { AggregateWithImmutableVersion savedAgain = template.save(reloadedAggregate); AggregateWithImmutableVersion reloadedAgain = template.findById(id, aggregate.getClass()); - assertThat(savedAgain.version) - .describedAs("The object returned by save should have an increased version") + assertThat(savedAgain.version).describedAs("The object returned by save should have an increased version") .isEqualTo(2L); - assertThat(reloadedAgain.getVersion()) - .describedAs("version field should increment by one with each save") + assertThat(reloadedAgain.getVersion()).describedAs("version field should increment by one with each save") .isEqualTo(2L); assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 1L)))