Pagination now supports sorting by property.

Closes https://github.com/spring-projects/spring-data-envers/issues/379
Original Pull Request https://github.com/spring-projects/spring-data-envers/pull/381

spring-data-envers/src/main/java/org/springframework/data/envers/repository/support/EnversRevisionRepositoryImpl.java

@@ -15,14 +15,7 @@
  */
 package org.springframework.data.envers.repository.support;
 
-import static org.springframework.data.history.RevisionMetadata.RevisionType.*;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
-
 import jakarta.persistence.EntityManager;
-
 import org.hibernate.Hibernate;
 import org.hibernate.envers.AuditReader;
 import org.hibernate.envers.AuditReaderFactory;
@@ -32,10 +25,12 @@ import org.hibernate.envers.RevisionTimestamp;
 import org.hibernate.envers.RevisionType;
 import org.hibernate.envers.query.AuditEntity;
 import org.hibernate.envers.query.AuditQuery;
+import org.hibernate.envers.query.criteria.AuditProperty;
 import org.hibernate.envers.query.order.AuditOrder;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.history.AnnotationRevisionMetadata;
 import org.springframework.data.history.Revision;
 import org.springframework.data.history.RevisionMetadata;
@@ -48,6 +43,13 @@ import org.springframework.data.repository.history.support.RevisionEntityInforma
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.Assert;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+import static org.springframework.data.history.RevisionMetadata.RevisionType.*;
+
 /**
  * Repository implementation using Hibernate Envers to implement revision specific query methods.
  *
@@ -58,6 +60,7 @@ import org.springframework.util.Assert;
  * @author Julien Millau
  * @author Mark Paluch
  * @author Sander Bylemans
+ * @author Niklas Loechte
  */
 @Transactional(readOnly = true)
 public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N>>
@@ -70,14 +73,14 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 	 * Creates a new {@link EnversRevisionRepositoryImpl} using the given {@link JpaEntityInformation},
 	 * {@link RevisionEntityInformation} and {@link EntityManager}.
 	 *
-	 * @param entityInformation must not be {@literal null}.
+	 * @param entityInformation         must not be {@literal null}.
 	 * @param revisionEntityInformation must not be {@literal null}.
-	 * @param entityManager must not be {@literal null}.
+	 * @param entityManager             must not be {@literal null}.
 	 */
 	public EnversRevisionRepositoryImpl(JpaEntityInformation<T, ?> entityInformation,
-			RevisionEntityInformation revisionEntityInformation, EntityManager entityManager) {
+										RevisionEntityInformation revisionEntityInformation, EntityManager entityManager) {
 
-		Assert.notNull(revisionEntityInformation, "RevisionEntityInformation must not be null");
+		Assert.notNull(revisionEntityInformation, "RevisionEntityInformation must not be null!");
 
 		this.entityInformation = entityInformation;
 		this.entityManager = entityManager;
@@ -91,7 +94,7 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 				.setMaxResults(1) //
 				.getResultList();
 
-		Assert.state(singleResult.size() <= 1, "We expect at most one result");
+		Assert.state(singleResult.size() <= 1, "We expect at most one result.");
 
 		if (singleResult.isEmpty()) {
 			return Optional.empty();
@@ -104,14 +107,14 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 	@SuppressWarnings("unchecked")
 	public Optional<Revision<N, T>> findRevision(ID id, N revisionNumber) {
 
-		Assert.notNull(id, "Identifier must not be null");
-		Assert.notNull(revisionNumber, "Revision number must not be null");
+		Assert.notNull(id, "Identifier must not be null!");
+		Assert.notNull(revisionNumber, "Revision number must not be null!");
 
 		List<Object[]> singleResult = (List<Object[]>) createBaseQuery(id) //
 				.add(AuditEntity.revisionNumber().eq(revisionNumber)) //
 				.getResultList();
 
-		Assert.state(singleResult.size() <= 1, "We expect at most one result");
+		Assert.state(singleResult.size() <= 1, "We expect at most one result.");
 
 		if (singleResult.isEmpty()) {
 			return Optional.empty();
@@ -133,15 +136,46 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 		return Revisions.of(revisionList);
 	}
 
-	@SuppressWarnings("unchecked")
-	public Page<Revision<N, T>> findRevisions(ID id, Pageable pageable) {
 
-		AuditOrder sorting = RevisionSort.getRevisionDirection(pageable.getSort()).isDescending() //
+	private AuditOrder mapRevisionSort(RevisionSort revisionSort) {
+
+		return RevisionSort.getRevisionDirection(revisionSort).isDescending() //
 				? AuditEntity.revisionNumber().desc() //
 				: AuditEntity.revisionNumber().asc();
+	}
+
+	private List<AuditOrder> mapPropertySort(Sort sort) {
+
+		if (sort.isEmpty()) {
+			return Collections.singletonList(AuditEntity.revisionNumber().asc());
+		}
+
+		List<AuditOrder> result = new ArrayList<>();
+		for (Sort.Order order : sort) {
+
+			AuditProperty<Object> property = AuditEntity.property(order.getProperty());
+			AuditOrder auditOrder = order.getDirection().isAscending() ?
+					property.asc() :
+					property.desc();
+
+			result.add(auditOrder);
+		}
+
+		return result;
+	}
+
+	@SuppressWarnings("unchecked")
+	public Page<Revision<N, T>> findRevisions(ID id, Pageable pageable) {
+
+		AuditQuery baseQuery = createBaseQuery(id);
+
+		List<AuditOrder> orderMapped = (pageable.getSort() instanceof RevisionSort) ?
+				Collections.singletonList(mapRevisionSort((RevisionSort) pageable.getSort())) :
+				mapPropertySort(pageable.getSort());
+
+		orderMapped.forEach(baseQuery::addOrder);
 
-		List<Object[]> resultList = createBaseQuery(id) //
-				.addOrder(sorting) //
+		List<Object[]> resultList = baseQuery //
 				.setFirstResult((int) pageable.getOffset()) //
 				.setMaxResults(pageable.getPageSize()) //
 				.getResultList();
@@ -185,7 +219,7 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 			Assert.notNull(data, "Data must not be null");
 			Assert.isTrue( //
 					data.length == 3, //
-					() -> String.format("Data must have length three, but has length %d", data.length));
+					() -> String.format("Data must have length three, but has length %d.", data.length));
 			Assert.isTrue( //
 					data[2] instanceof RevisionType, //
 					() -> String.format("The third array element must be of type Revision type, but is of type %s",
@@ -201,7 +235,7 @@ public class EnversRevisionRepositoryImpl<T, ID, N extends Number & Comparable<N
 			return metadata instanceof DefaultRevisionEntity //
 					? new DefaultRevisionMetadata((DefaultRevisionEntity) metadata, revisionType) //
 					: new AnnotationRevisionMetadata<>(Hibernate.unproxy(metadata), RevisionNumber.class, RevisionTimestamp.class,
-							revisionType);
+					revisionType);
 		}
 
 		private static RevisionMetadata.RevisionType convertRevisionType(RevisionType datum) {

spring-data-envers/src/test/java/org/springframework/data/envers/repository/support/RepositoryIntegrationTests.java

@@ -15,23 +15,13 @@
  */
 package org.springframework.data.envers.repository.support;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.springframework.data.history.RevisionMetadata.RevisionType.DELETE;
-import static org.springframework.data.history.RevisionMetadata.RevisionType.INSERT;
-import static org.springframework.data.history.RevisionMetadata.RevisionType.UPDATE;
-
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Optional;
-
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.envers.Config;
 import org.springframework.data.envers.sample.Country;
 import org.springframework.data.envers.sample.CountryRepository;
@@ -43,19 +33,30 @@ import org.springframework.data.history.Revisions;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.springframework.data.history.RevisionMetadata.RevisionType.*;
+
 /**
  * Integration tests for repositories.
  *
  * @author Oliver Gierke
  * @author Jens Schauder
- * @author Krzysztof Krason
+ * @author Niklas Loechte
  */
 @ExtendWith(SpringExtension.class)
 @ContextConfiguration(classes = Config.class)
 class RepositoryIntegrationTests {
 
-	@Autowired LicenseRepository licenseRepository;
-	@Autowired CountryRepository countryRepository;
+	@Autowired
+	LicenseRepository licenseRepository;
+	@Autowired
+	CountryRepository countryRepository;
 
 	@BeforeEach
 	void setUp() {
@@ -64,13 +65,6 @@ class RepositoryIntegrationTests {
 		countryRepository.deleteAll();
 	}
 
-	@AfterEach
-	void tearDown() {
-
-		licenseRepository.deleteAll();
-		countryRepository.deleteAll();
-	}
-
 	@Test
 	void testLifeCycle() {
 
@@ -241,7 +235,27 @@ class RepositoryIntegrationTests {
 	@Test // #47
 	void paginationWithEmptyResult() {
 
-		check(23L, 0, 0, 0);
+		check(-23L, 0, 0, 0);
+	}
+
+
+	@Test // Envers #379
+	void testSort_pageableByProperty() {
+
+		Country de = new Country();
+		de.code = "de";
+		de.name = "Deutschland";
+		de.timestamp = Instant.parse("2000-01-01T00:00:00Z");
+		countryRepository.save(de);
+
+		de.timestamp = Instant.parse("2000-01-04T00:01:00Z");
+		countryRepository.save(de);
+
+		de.timestamp = Instant.parse("2000-01-04T00:00:00Z");
+		countryRepository.save(de);
+
+		assertThat(countryRepository.findRevisions(de.id, PageRequest.of(0, 3, Sort.by("timestamp"))).map(Revision::getEntity).map(country -> country.timestamp).getContent())
+				.isSortedAccordingTo(Instant::compareTo);
 	}
 
 	void check(Long id, int page, int expectedSize, int expectedTotalSize) {

spring-data-envers/src/test/java/org/springframework/data/envers/sample/Country.java

@@ -20,11 +20,14 @@ import jakarta.persistence.Entity;
 import lombok.ToString;
 import org.hibernate.envers.Audited;
 
+import java.time.Instant;
+
 /**
  * Sample domain class.
  *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Niklas Loechte
  */
 @Audited
 @Entity
@@ -32,5 +35,8 @@ import org.hibernate.envers.Audited;
 public class Country extends AbstractEntity {
 
 	public String code;
+
+	public Instant timestamp;
+
 	public String name;
 }