From 221ad9353e29d82d75ab8428573d3cfd5212e70d Mon Sep 17 00:00:00 2001
From: Oliver Drotbohm <odrotbohm@pivotal.io>
Date: Wed, 25 Sep 2019 13:29:30 +0200
Subject: [PATCH] DATACMNS-1581 - Tighten nullability constraints for Sort
 parameters in PageRequest factory methods.

We now explicitly rejects null values for the Sort parameters in the constructor of PageRequest to avoid the creation of invalid instances if the IDE validation of nullability constraints is not in use.
---
 .../org/springframework/data/domain/PageRequest.java   |  7 +++++--
 .../springframework/data/querydsl/QPageRequest.java    | 10 +++++++---
 .../data/domain/PageRequestUnitTests.java              |  8 ++++++++
 .../data/querydsl/QPageRequestUnitTests.java           |  7 +++++++
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/springframework/data/domain/PageRequest.java b/src/main/java/org/springframework/data/domain/PageRequest.java
index 8e78cdae0..fcaff8f4f 100644
--- a/src/main/java/org/springframework/data/domain/PageRequest.java
+++ b/src/main/java/org/springframework/data/domain/PageRequest.java
@@ -17,6 +17,7 @@ package org.springframework.data.domain;
 
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 
 /**
  * Basic Java Bean implementation of {@code Pageable}.
@@ -33,14 +34,16 @@ public class PageRequest extends AbstractPageRequest {
 	/**
 	 * Creates a new {@link PageRequest} with sort parameters applied.
 	 *
-	 * @param page zero-based page index.
-	 * @param size the size of the page to be returned.
+	 * @param page zero-based page index, must not be negative.
+	 * @param size the size of the page to be returned, must be greater than 0.
 	 * @param sort must not be {@literal null}, use {@link Sort#unsorted()} instead.
 	 */
 	protected PageRequest(int page, int size, Sort sort) {
 
 		super(page, size);
 
+		Assert.notNull(sort, "Sort must not be null!");
+
 		this.sort = sort;
 	}
 
diff --git a/src/main/java/org/springframework/data/querydsl/QPageRequest.java b/src/main/java/org/springframework/data/querydsl/QPageRequest.java
index cef375d67..19fef992d 100644
--- a/src/main/java/org/springframework/data/querydsl/QPageRequest.java
+++ b/src/main/java/org/springframework/data/querydsl/QPageRequest.java
@@ -18,6 +18,7 @@ package org.springframework.data.querydsl;
 import org.springframework.data.domain.AbstractPageRequest;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
+import org.springframework.util.Assert;
 
 import com.querydsl.core.types.OrderSpecifier;
 
@@ -57,13 +58,16 @@ public class QPageRequest extends AbstractPageRequest {
 	/**
 	 * Creates a new {@link QPageRequest} with sort parameters applied.
 	 *
-	 * @param page
-	 * @param size
-	 * @param sort
+	 * @param page must not be negative.
+	 * @param size must be greater or equal to 0.
+	 * @param sort must not be {@literal null}.
 	 */
 	public QPageRequest(int page, int size, QSort sort) {
+
 		super(page, size);
 
+		Assert.notNull(sort, "QSort must not be null!");
+
 		this.sort = sort;
 	}
 
diff --git a/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java b/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
index 03c16628e..e83595ebd 100755
--- a/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
+++ b/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
@@ -15,6 +15,7 @@
  */
 package org.springframework.data.domain;
 
+import static org.assertj.core.api.Assertions.*;
 import static org.springframework.data.domain.UnitTestUtils.*;
 
 import org.junit.Test;
@@ -61,4 +62,11 @@ public class PageRequestUnitTests extends AbstractPageRequestUnitTests {
 		// Is not equal to instance with another sort
 		assertNotEqualsAndHashcode(request, PageRequest.of(0, 10, Direction.ASC, "foo"));
 	}
+
+	@Test // DATACMNS-1581
+	public void rejectsNullSort() {
+
+		assertThatExceptionOfType(IllegalArgumentException.class) //
+				.isThrownBy(() -> PageRequest.of(0, 10, null));
+	}
 }
diff --git a/src/test/java/org/springframework/data/querydsl/QPageRequestUnitTests.java b/src/test/java/org/springframework/data/querydsl/QPageRequestUnitTests.java
index 957c77228..e22dc6502 100755
--- a/src/test/java/org/springframework/data/querydsl/QPageRequestUnitTests.java
+++ b/src/test/java/org/springframework/data/querydsl/QPageRequestUnitTests.java
@@ -51,4 +51,11 @@ public class QPageRequestUnitTests extends AbstractPageRequestUnitTests {
 
 		assertThat(pageRequest.getSort()).isEqualTo(QSort.by(user.firstname.asc()));
 	}
+
+	@Test // DATACMNS-1581
+	public void rejectsNullSort() {
+
+		assertThatExceptionOfType(IllegalArgumentException.class) //
+				.isThrownBy(() -> new QPageRequest(0, 10, (QSort) null));
+	}
 }