+ * An initial {@link OffsetScrollPosition} does not point to a specific element and is different to a the Po
*
* @author Mark Paluch
* @author Oliver Drotbohm
+ * @author Christoph Strobl
* @since 3.1
*/
public final class OffsetScrollPosition implements ScrollPosition {
- private static final OffsetScrollPosition INITIAL = new OffsetScrollPosition(0);
+ private static final OffsetScrollPosition INITIAL = new OffsetScrollPosition(null);
- private final long offset;
+ @Nullable private final Long offset;
/**
* Creates a new {@link OffsetScrollPosition} for the given non-negative offset.
*
* @param offset must be greater or equal to zero.
*/
- private OffsetScrollPosition(long offset) {
+ private OffsetScrollPosition(@Nullable Long offset) {
- Assert.isTrue(offset >= 0, "Offset must not be negative");
+ Assert.isTrue(offset == null || offset >= 0, "Offset must not be negative");
this.offset = offset;
}
@@ -62,7 +65,7 @@ static OffsetScrollPosition initial() {
* @return will never be {@literal null}.
*/
static OffsetScrollPosition of(long offset) {
- return offset == 0 ? initial() : new OffsetScrollPosition(offset);
+ return new OffsetScrollPosition(offset);
}
/**
@@ -80,10 +83,15 @@ public static IntFunction
+ * An {@link #isInitial() initial} position does not define an offset and will raise an error.
*
* @return the offset.
+ * @throws IllegalStateException if {@link #isInitial()}.
*/
public long getOffset() {
+
+ Assert.state(offset != null, "Initial state does not have an offset. Make sure to check #isInitial()");
return offset;
}
@@ -96,14 +104,14 @@ public long getOffset() {
*/
public OffsetScrollPosition advanceBy(long delta) {
- var value = offset + delta;
+ var value = isInitial() ? delta : offset + delta;
return new OffsetScrollPosition(value < 0 ? 0 : value);
}
@Override
public boolean isInitial() {
- return offset == 0;
+ return offset == null;
}
@Override
@@ -117,7 +125,7 @@ public boolean equals(@Nullable Object o) {
return false;
}
- return offset == that.offset;
+ return Objects.equals(offset, that.offset);
}
@Override
@@ -141,7 +149,7 @@ public OffsetScrollPosition apply(int offset) {
throw new IndexOutOfBoundsException(offset);
}
- return of(startOffset + offset + 1);
+ return of(startOffset + offset);
}
}
}
diff --git a/src/main/java/org/springframework/data/domain/Pageable.java b/src/main/java/org/springframework/data/domain/Pageable.java
index 36916c161c..f84b7822aa 100644
--- a/src/main/java/org/springframework/data/domain/Pageable.java
+++ b/src/main/java/org/springframework/data/domain/Pageable.java
@@ -191,8 +191,12 @@ default Limit toLimit() {
/**
* Returns an {@link OffsetScrollPosition} from this pageable if the page request {@link #isPaged() is paged}.
+ *
+ * Given the exclusive nature of scrolling the {@link ScrollPosition} for {@code Page(0, 10)} translates an
+ * {@link ScrollPosition#isInitial() initial} position, where as {@code Page(1, 10)} will point to the last element of
+ * {@code Page(0,10)} resulting in {@link ScrollPosition#offset(long) ScrollPosition(9)}.
*
- * @return
+ * @return new instance of {@link OffsetScrollPosition}.
* @throws IllegalStateException if the request is {@link #isUnpaged()}
* @since 3.1
*/
@@ -202,6 +206,7 @@ default OffsetScrollPosition toScrollPosition() {
throw new IllegalStateException("Cannot create OffsetScrollPosition from an unpaged instance");
}
- return ScrollPosition.offset(getOffset());
+ return getOffset() > 0 ? ScrollPosition.offset(getOffset() - 1 /* scrolling is exclusive */)
+ : ScrollPosition.offset();
}
}
diff --git a/src/test/java/org/springframework/data/domain/AbstractPageRequestUnitTests.java b/src/test/java/org/springframework/data/domain/AbstractPageRequestUnitTests.java
index bf745673aa..6b61561362 100755
--- a/src/test/java/org/springframework/data/domain/AbstractPageRequestUnitTests.java
+++ b/src/test/java/org/springframework/data/domain/AbstractPageRequestUnitTests.java
@@ -84,4 +84,11 @@ void getOffsetShouldNotCauseOverflow() {
assertThat(request.getOffset()).isGreaterThan(Integer.MAX_VALUE);
}
+
+ @Test // GH-2151, GH-3070
+ void createsOffsetScrollPosition() {
+
+ assertThat(newPageRequest(0, 10).toScrollPosition()).returns(true, ScrollPosition::isInitial);
+ assertThat(newPageRequest(1, 10).toScrollPosition()).returns(9L, OffsetScrollPosition::getOffset);
+ }
}
diff --git a/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java b/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
index bf7101214c..181d51f37e 100755
--- a/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
+++ b/src/test/java/org/springframework/data/domain/PageRequestUnitTests.java
@@ -71,12 +71,4 @@ void rejectsNullSort() {
assertThatIllegalArgumentException() //
.isThrownBy(() -> PageRequest.of(0, 10, null));
}
-
- @Test // GH-2151
- void createsOffsetScrollPosition() {
-
- PageRequest request = PageRequest.of(1, 10);
-
- assertThat(request.toScrollPosition()).isEqualTo(ScrollPosition.offset(10));
- }
}
diff --git a/src/test/java/org/springframework/data/domain/ScrollPositionUnitTests.java b/src/test/java/org/springframework/data/domain/ScrollPositionUnitTests.java
index f3137abae7..baf81f9b21 100644
--- a/src/test/java/org/springframework/data/domain/ScrollPositionUnitTests.java
+++ b/src/test/java/org/springframework/data/domain/ScrollPositionUnitTests.java
@@ -29,6 +29,7 @@
*
* @author Mark Paluch
* @author Oliver Drotbohm
+ * @author Christoph Strobl
*/
class ScrollPositionUnitTests {
@@ -56,14 +57,14 @@ void equalsAndHashCodeForOffsets() {
assertThat(foo1).isNotEqualTo(bar).doesNotHaveSameHashCodeAs(bar);
}
- @Test // GH-2151
+ @Test // GH-2151, GH-3070
void shouldCreateCorrectIndexPosition() {
- assertThat(positionFunction(0).apply(0)).isEqualTo(ScrollPosition.offset(1));
- assertThat(positionFunction(0).apply(1)).isEqualTo(ScrollPosition.offset(2));
+ assertThat(positionFunction(0).apply(0)).isEqualTo(ScrollPosition.offset(0));
+ assertThat(positionFunction(0).apply(1)).isEqualTo(ScrollPosition.offset(1));
- assertThat(positionFunction(100).apply(0)).isEqualTo(ScrollPosition.offset(101));
- assertThat(positionFunction(100).apply(1)).isEqualTo(ScrollPosition.offset(102));
+ assertThat(positionFunction(100).apply(0)).isEqualTo(ScrollPosition.offset(100));
+ assertThat(positionFunction(100).apply(1)).isEqualTo(ScrollPosition.offset(101));
}
@Test // GH-2151
@@ -80,6 +81,23 @@ void advanceOffsetBelowZeroCapsAtZero() {
assertThat(offset.advanceBy(-10)).isEqualTo(ScrollPosition.offset(0));
}
+ @Test // GH-3070
+ void advanceOffsetForward() {
+
+ OffsetScrollPosition offset = ScrollPosition.offset(5);
+
+ assertThat(offset.getOffset()).isEqualTo(5);
+ assertThat(offset.advanceBy(5)).isEqualTo(ScrollPosition.offset(10));
+ }
+
+ @Test // GH-3070
+ void advanceInitialOffsetForward() {
+
+ OffsetScrollPosition offset = ScrollPosition.offset();
+
+ assertThat(offset.advanceBy(5)).isEqualTo(ScrollPosition.offset(5));
+ }
+
@Test // GH-2824
void setsUpForwardScrolling() {
@@ -120,13 +138,22 @@ void setsUpBackwardScrolling() {
assertThat(position.reverse()).isEqualTo(forward);
}
- @Test // GH-2824
+ @Test // GH-2824, GH-3070
void initialOffsetPosition() {
OffsetScrollPosition position = ScrollPosition.offset();
assertThat(position.isInitial()).isTrue();
- assertThat(position.getOffset()).isEqualTo(0);
+ assertThatExceptionOfType(IllegalStateException.class).isThrownBy(position::getOffset);
+ }
+
+ @Test // GH-3070
+ void initialOffsetPositionIsNotEqualToPositionOfFirstElement() {
+
+ OffsetScrollPosition first = ScrollPosition.offset(0);
+
+ assertThat(first.isInitial()).isFalse();
+ assertThat(first).isNotEqualTo(ScrollPosition.offset());
}
@Test // GH-2824, GH-2840
diff --git a/src/test/java/org/springframework/data/domain/WindowUnitTests.java b/src/test/java/org/springframework/data/domain/WindowUnitTests.java
index c5bfbdd438..bda23c3a3c 100644
--- a/src/test/java/org/springframework/data/domain/WindowUnitTests.java
+++ b/src/test/java/org/springframework/data/domain/WindowUnitTests.java
@@ -53,18 +53,18 @@ void allowsIteration() {
}
}
- @Test // GH-2151
+ @Test // GH-2151, GH-3070
void shouldCreateCorrectPositions() {
Window