From 78374f3cb7447305af1025efc1ceb95bebbf8be3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 26 Apr 2017 16:22:44 +0200 Subject: [PATCH] DATACMNS-1050 - Polishing. Used @RequiredArgsConstructor in a couple of places in favour of manually declared constructors. Removed obsolete factory methods on Range. Adapted test cases accordingly. Original pull request: #212. --- .../springframework/data/domain/Range.java | 68 +++---------------- .../data/domain/RangeUnitTests.java | 48 ++----------- 2 files changed, 14 insertions(+), 102 deletions(-) diff --git a/src/main/java/org/springframework/data/domain/Range.java b/src/main/java/org/springframework/data/domain/Range.java index ce109629d..40081ea0d 100644 --- a/src/main/java/org/springframework/data/domain/Range.java +++ b/src/main/java/org/springframework/data/domain/Range.java @@ -15,6 +15,9 @@ */ package org.springframework.data.domain; +import lombok.AccessLevel; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; import lombok.Value; import java.util.Optional; @@ -29,6 +32,7 @@ import org.springframework.util.Assert; * @since 1.10 */ @Value +@RequiredArgsConstructor(access = AccessLevel.PRIVATE) public class Range> { private final static Range UNBOUNDED = Range.of(Bound.unbounded(), Bound.UNBOUNDED); @@ -36,12 +40,12 @@ public class Range> { /** * The lower bound of the range. */ - private final Bound lowerBound; + private final @NonNull Bound lowerBound; /** * The upper bound of the range. */ - private final Bound upperBound; + private final @NonNull Bound upperBound; /** * Creates a new {@link Range} with the given lower and upper bound. Treats the given values as inclusive bounds. Use @@ -50,7 +54,9 @@ public class Range> { * @see Range#of(Bound, Bound) * @param lowerBound can be {@literal null} in case upperBound is not {@literal null}. * @param upperBound can be {@literal null} in case lowerBound is not {@literal null}. + * @deprecated since 2.0 in favor of {@link Range#of(Bound, Bound)}. */ + @Deprecated public Range(T lowerBound, T upperBound) { this(lowerBound, upperBound, true, true); } @@ -76,15 +82,6 @@ public class Range> { : upperInclusive ? Bound.inclusive(upperBound) : Bound.exclusive(upperBound); } - private Range(Bound lowerBound, Bound upperBound) { - - Assert.notNull(lowerBound, "Lower boundary must not be null!"); - Assert.notNull(upperBound, "Upper boundary must not be null!"); - - this.lowerBound = lowerBound; - this.upperBound = upperBound; - } - /** * Returns an unbounded {@link Range}. * @@ -120,54 +117,6 @@ public class Range> { return new Range<>(lowerBound, upperBound); } - /** - * Creates a new {@link Integer} {@link Range} with the given lower and upper bound. Treats the given values as - * including bounds. Use {@link #of(Bound, Bound)} to configure different bound behavior. - * - * @param lowerBound - * @param upperBound - * @since 2.0 - */ - public static Range from(int lowerBoundInclusive, int upperBoundInclusive) { - return of(Bound.inclusive(lowerBoundInclusive), Bound.inclusive(upperBoundInclusive)); - } - - /** - * Creates a new {@link Long} {@link Range} with the given lower and upper bound. Treats the given values as including - * bounds. Use {@link #of(Bound, Bound)} to configure different bound behavior. - * - * @param lowerBound - * @param upperBound - * @since 2.0 - */ - public static Range from(long lowerBoundInclusive, long upperBoundInclusive) { - return of(Bound.inclusive(lowerBoundInclusive), Bound.inclusive(upperBoundInclusive)); - } - - /** - * Creates a new {@link Float} {@link Range} with the given lower and upper bound. Treats the given values as - * including bounds. Use {@link #of(Bound, Bound)} to configure different bound behavior. - * - * @param lowerBound - * @param upperBound - * @since 2.0 - */ - public static Range from(float lowerBoundInclusive, float upperBoundInclusive) { - return of(Bound.inclusive(lowerBoundInclusive), Bound.inclusive(upperBoundInclusive)); - } - - /** - * Creates a new {@link Double} {@link Range} with the given lower and upper bound. Treats the given values as - * including bounds. Use {@link #of(Bound, Bound)} to configure different bound behavior. - * - * @param lowerBound - * @param upperBound - * @since 2.0 - */ - public static Range from(double lowerBoundInclusive, double upperBoundInclusive) { - return of(Bound.inclusive(lowerBoundInclusive), Bound.inclusive(upperBoundInclusive)); - } - /** * @return * @deprecated since 2.0, use {@link #getLowerBound()} and {@link Bound#isInclusive()}. @@ -225,6 +174,7 @@ public class Range> { * @soundtrack Mohamed Ragab - Excelsior Sessions (March 2017) */ @Value + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) public static class Bound> { @SuppressWarnings({ "rawtypes", "unchecked" }) // diff --git a/src/test/java/org/springframework/data/domain/RangeUnitTests.java b/src/test/java/org/springframework/data/domain/RangeUnitTests.java index 15c271f43..fbceabfcf 100755 --- a/src/test/java/org/springframework/data/domain/RangeUnitTests.java +++ b/src/test/java/org/springframework/data/domain/RangeUnitTests.java @@ -31,25 +31,13 @@ public class RangeUnitTests { @Test(expected = IllegalArgumentException.class) // DATACMNS-651 public void rejectsNullReferenceValuesForContains() { - new Range<>(10L, 20L).contains(null); - } - - @Test // DATACMNS-651 - public void usesBoundsInclusivelyByDefault() { - - Range range = Range.from(10L, 20L); - - assertThat(range.contains(10L)).isTrue(); - assertThat(range.contains(20L)).isTrue(); - assertThat(range.contains(15L)).isTrue(); - assertThat(range.contains(5L)).isFalse(); - assertThat(range.contains(25L)).isFalse(); + Range.from(Bound.inclusive(10L)).to(Bound.inclusive(20L)).contains(null); } @Test // DATACMNS-651 public void excludesLowerBoundIfConfigured() { - Range range = new Range<>(10L, 20L, false, true); + Range range = Range.from(Bound.exclusive(10L)).to(Bound.inclusive(20L)); assertThat(range.contains(10L)).isFalse(); assertThat(range.contains(20L)).isTrue(); @@ -61,7 +49,7 @@ public class RangeUnitTests { @Test // DATACMNS-651 public void excludesUpperBoundIfConfigured() { - Range range = new Range<>(10L, 20L, true, false); + Range range = Range.of(Bound.inclusive(10L), Bound.exclusive(20L)); assertThat(range.contains(10L)).isTrue(); assertThat(range.contains(20L)).isFalse(); @@ -73,7 +61,7 @@ public class RangeUnitTests { @Test // DATACMNS-651, DATACMNS-1050 public void handlesOpenUpperBoundCorrectly() { - Range range = new Range<>(10L, null); + Range range = Range.of(Bound.inclusive(10L), Bound.unbounded()); assertThat(range.contains(10L)).isTrue(); assertThat(range.contains(20L)).isTrue(); @@ -88,7 +76,7 @@ public class RangeUnitTests { @Test // DATACMNS-651, DATACMNS-1050 public void handlesOpenLowerBoundCorrectly() { - Range range = new Range<>(null, 20L); + Range range = Range.of(Bound.unbounded(), Bound.inclusive(20L)); assertThat(range.contains(10L)).isTrue(); assertThat(range.contains(20L)).isTrue(); @@ -168,30 +156,4 @@ public class RangeUnitTests { assertThat(range.getLowerBound().getValue()).isEmpty(); assertThat(range.getUpperBound().getValue()).isEmpty(); } - - @Test // DATACMNS-1050 - public void createsPrimitiveIntInclusiveRange() { - - Range range = Range.from(10, 20); - - assertThat(range.contains(9)).isFalse(); - assertThat(range.contains(10)).isTrue(); - assertThat(range.contains(11)).isTrue(); - assertThat(range.contains(19)).isTrue(); - assertThat(range.contains(20)).isTrue(); - assertThat(range.contains(21)).isFalse(); - } - - @Test // DATACMNS-1050 - public void createsPrimitiveDoubleInclusiveRange() { - - Range range = Range.from(10d, 20f); - - assertThat(range.contains(9d)).isFalse(); - assertThat(range.contains(10d)).isTrue(); - assertThat(range.contains(11d)).isTrue(); - assertThat(range.contains(19d)).isTrue(); - assertThat(range.contains(20d)).isTrue(); - assertThat(range.contains(21d)).isFalse(); - } }