From 22dc91b68f71129ddf9aa20cf3bcfef352157444 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 25 Apr 2017 13:52:05 +0200 Subject: [PATCH] DATACMNS-1050 - Encapsulate boundaries in value objects for Range. We now encapsulate a boundary in Range within a Bound value object. Bound consists of a value and whether the value is inclusive or exclusive. Boundaries without a value are unbounded. We introduced factory methods for Range and Boundary creation using primitives and a builder to build a Range. Range range = Range.unbounded(); Range range = Range.from(Bound.inclusive(10)).to(Bound.inclusive(20)); Range range = Range.of(Bound.inclusive(10), Bound.inclusive(20)); Original pull request: #121. --- .../springframework/data/domain/Range.java | 356 ++++++++++++++++-- .../data/domain/RangeUnitTests.java | 109 +++++- .../data/geo/DistanceUnitTests.java | 10 +- 3 files changed, 447 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/springframework/data/domain/Range.java b/src/main/java/org/springframework/data/domain/Range.java index 76093a362..ce109629d 100644 --- a/src/main/java/org/springframework/data/domain/Range.java +++ b/src/main/java/org/springframework/data/domain/Range.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,42 +17,37 @@ package org.springframework.data.domain; import lombok.Value; +import java.util.Optional; + import org.springframework.util.Assert; /** - * Simple value object to work with ranges. + * Simple value object to work with ranges and boundaries. * * @author Oliver Gierke + * @author Mark Paluch * @since 1.10 */ @Value public class Range> { + private final static Range UNBOUNDED = Range.of(Bound.unbounded(), Bound.UNBOUNDED); + /** * The lower bound of the range. */ - private final T lowerBound; + private final Bound lowerBound; /** * The upper bound of the range. */ - private final T upperBound; - - /** - * Whether the lower bound is considered inclusive. - */ - private final boolean lowerInclusive; - - /** - * Whether the lower bound is considered inclusive. - */ - private final boolean upperInclusive; + private final Bound upperBound; /** * Creates a new {@link Range} with the given lower and upper bound. Treats the given values as inclusive bounds. Use * {@link #Range(Comparable, Comparable, boolean, boolean)} to configure different bound behavior. * - * @see #Range(Comparable, Comparable, boolean, boolean) + * @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}. */ @@ -68,13 +63,127 @@ public class Range> { * @param upperBound can be {@literal null}. * @param lowerInclusive * @param upperInclusive + * @deprecated since 2.0. Use {@link Range#of(Bound, Bound)} and {@link Bound} factory methods: + * {@link Bound#inclusive(Comparable)}, {@link Bound#exclusive(Comparable)}/{@link Bound#unbounded()}. */ + @Deprecated public Range(T lowerBound, T upperBound, boolean lowerInclusive, boolean upperInclusive) { + this.lowerBound = lowerBound == null ? Bound.unbounded() + : lowerInclusive ? Bound.inclusive(lowerBound) : Bound.exclusive(lowerBound); + + this.upperBound = upperBound == null ? Bound.unbounded() + : 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; - this.lowerInclusive = lowerInclusive; - this.upperInclusive = upperInclusive; + } + + /** + * Returns an unbounded {@link Range}. + * + * @return + * @since 2.0 + */ + @SuppressWarnings("unchecked") + public static > Range unbounded() { + return (Range) UNBOUNDED; + } + + /** + * Create a {@link RangeBuilder} given the lower {@link Bound}. + * + * @param lower must not be {@literal null}. + * @return + * @since 2.0 + */ + public static > RangeBuilder from(Bound lower) { + + Assert.notNull(lower, "Lower bound must not be null!"); + return new RangeBuilder<>(lower); + } + + /** + * Creates a new {@link Range} with the given lower and upper bound. + * + * @param lowerBound must not be {@literal null}. + * @param upperBound must not be {@literal null}. + * @since 2.0 + */ + public static > Range of(Bound lowerBound, Bound upperBound) { + 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()}. + */ + @Deprecated + public boolean isLowerInclusive() { + return lowerBound.isInclusive(); + } + + /** + * @return + * @deprecated since 2.0, use {@link #getUpperBound()} and {@link Bound#isInclusive()}. + */ + @Deprecated + public boolean isUpperInclusive() { + return upperBound.isInclusive(); } /** @@ -87,11 +196,216 @@ public class Range> { Assert.notNull(value, "Reference value must not be null!"); - boolean greaterThanLowerBound = lowerBound == null ? true - : lowerInclusive ? lowerBound.compareTo(value) <= 0 : lowerBound.compareTo(value) < 0; - boolean lessThanUpperBound = upperBound == null ? true - : upperInclusive ? upperBound.compareTo(value) >= 0 : upperBound.compareTo(value) > 0; + boolean greaterThanLowerBound = lowerBound.getValue() // + .map(it -> lowerBound.isInclusive() ? it.compareTo(value) <= 0 : it.compareTo(value) < 0) // + .orElse(true); + + boolean lessThanUpperBound = upperBound.getValue() // + .map(it -> upperBound.isInclusive() ? it.compareTo(value) >= 0 : it.compareTo(value) > 0) // + .orElse(true); return greaterThanLowerBound && lessThanUpperBound; } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return String.format("%s-%s", lowerBound.toPrefixString(), upperBound.toSuffixString()); + } + + /** + * Value object representing a boundary. A boundary can either be {@link #unbounded() unbounded}, + * {@link #inclusive(Comparable) including its value} or {@link #exclusive(Comparable) its value}. + * + * @author Mark Paluch + * @since 2.0 + * @soundtrack Mohamed Ragab - Excelsior Sessions (March 2017) + */ + @Value + public static class Bound> { + + @SuppressWarnings({ "rawtypes", "unchecked" }) // + private static final Bound UNBOUNDED = new Bound(Optional.empty(), true); + + private final Optional value; + private final boolean inclusive; + + /** + * Creates an unbounded {@link Bound}. + */ + @SuppressWarnings("unchecked") + public static > Bound unbounded() { + return (Bound) UNBOUNDED; + } + + /** + * Returns whether this boundary is bounded. + * + * @return + */ + public boolean isBounded() { + return value.isPresent(); + } + + /** + * Creates a boundary including {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static > Bound inclusive(T value) { + + Assert.notNull(value, "Value must not be null!"); + return new Bound<>(Optional.of(value), true); + } + + /** + * Creates a boundary including {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound inclusive(int value) { + return inclusive((Integer) value); + } + + /** + * Creates a boundary including {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound inclusive(long value) { + return inclusive((Long) value); + } + + /** + * Creates a boundary including {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound inclusive(float value) { + return inclusive((Float) value); + } + + /** + * Creates a boundary including {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound inclusive(double value) { + return inclusive((Double) value); + } + + /** + * Creates a boundary excluding {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static > Bound exclusive(T value) { + + Assert.notNull(value, "Value must not be null!"); + return new Bound<>(Optional.of(value), false); + } + + /** + * Creates a boundary excluding {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound exclusive(int value) { + return exclusive((Integer) value); + } + + /** + * Creates a boundary excluding {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound exclusive(long value) { + return exclusive((Long) value); + } + + /** + * Creates a boundary excluding {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound exclusive(float value) { + return exclusive((Float) value); + } + + /** + * Creates a boundary excluding {@code value}. + * + * @param value must not be {@literal null}. + * @return + */ + public static Bound exclusive(double value) { + return exclusive((Double) value); + } + + String toPrefixString() { + + return getValue() // + .map(Object::toString) // + .map(it -> isInclusive() ? "[".concat(it) : "(".concat(it)) // + .orElse("unbounded"); + } + + String toSuffixString() { + + return getValue() // + .map(Object::toString) // + .map(it -> isInclusive() ? it.concat("]") : it.concat(")")) // + .orElse("unbounded"); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return value.map(Object::toString).orElse("unbounded"); + } + + } + + /** + * Builder for {@link Range} allowing to specify the upper boundary. + * + * @author Mark Paluch + * @since 2.0 + * @soundtrack Aly and Fila - Future Sound Of Egypt 493 + */ + public static class RangeBuilder> { + + private final Bound lower; + + RangeBuilder(Bound lower) { + this.lower = lower; + } + + /** + * Create a {@link Range} given the upper {@link Bound}. + * + * @param upper must not be {@literal null}. + * @return + */ + public Range to(Bound upper) { + + Assert.notNull(upper, "Upper bound must not be null!"); + return new Range<>(lower, upper); + } + } } diff --git a/src/test/java/org/springframework/data/domain/RangeUnitTests.java b/src/test/java/org/springframework/data/domain/RangeUnitTests.java index 42a836978..15c271f43 100755 --- a/src/test/java/org/springframework/data/domain/RangeUnitTests.java +++ b/src/test/java/org/springframework/data/domain/RangeUnitTests.java @@ -18,11 +18,13 @@ package org.springframework.data.domain; import static org.assertj.core.api.Assertions.*; import org.junit.Test; +import org.springframework.data.domain.Range.Bound; /** * Unit tests for {@link Range}. * * @author Oliver Gierke + * @author Mark Paluch * @since 1.10 */ public class RangeUnitTests { @@ -35,7 +37,7 @@ public class RangeUnitTests { @Test // DATACMNS-651 public void usesBoundsInclusivelyByDefault() { - Range range = new Range<>(10L, 20L); + Range range = Range.from(10L, 20L); assertThat(range.contains(10L)).isTrue(); assertThat(range.contains(20L)).isTrue(); @@ -68,7 +70,7 @@ public class RangeUnitTests { assertThat(range.contains(25L)).isFalse(); } - @Test // DATACMNS-651 + @Test // DATACMNS-651, DATACMNS-1050 public void handlesOpenUpperBoundCorrectly() { Range range = new Range<>(10L, null); @@ -78,9 +80,12 @@ public class RangeUnitTests { assertThat(range.contains(15L)).isTrue(); assertThat(range.contains(5L)).isFalse(); assertThat(range.contains(25L)).isTrue(); + assertThat(range.getLowerBound().isBounded()).isTrue(); + assertThat(range.getUpperBound().isBounded()).isFalse(); + assertThat(range.toString()).isEqualTo("[10-unbounded"); } - @Test // DATACMNS-651 + @Test // DATACMNS-651, DATACMNS-1050 public void handlesOpenLowerBoundCorrectly() { Range range = new Range<>(null, 20L); @@ -90,5 +95,103 @@ public class RangeUnitTests { assertThat(range.contains(15L)).isTrue(); assertThat(range.contains(5L)).isTrue(); assertThat(range.contains(25L)).isFalse(); + assertThat(range.getLowerBound().isBounded()).isFalse(); + assertThat(range.getUpperBound().isBounded()).isTrue(); + } + + @Test // DATACMNS-1050 + public void createsInclusiveBoundaryCorrectly() { + + Bound bound = Bound.inclusive(10); + + assertThat(bound.isInclusive()).isTrue(); + assertThat(bound.getValue()).contains(10); + } + + @Test // DATACMNS-1050 + public void createsExclusiveBoundaryCorrectly() { + + Bound bound = Bound.exclusive(10d); + + assertThat(bound.isInclusive()).isFalse(); + assertThat(bound.getValue()).contains(10d); + } + + @Test // DATACMNS-1050 + public void createsRangeFromBoundariesCorrectly() { + + Bound lower = Bound.inclusive(10L); + Bound upper = Bound.inclusive(20L); + + Range range = Range.of(lower, upper); + + assertThat(range.contains(9L)).isFalse(); + assertThat(range.contains(10L)).isTrue(); + assertThat(range.contains(20L)).isTrue(); + assertThat(range.contains(21L)).isFalse(); + } + + @Test // DATACMNS-1050 + public void shouldExclusiveBuildRangeLowerFirst() { + + Range range = Range.from(Bound.exclusive(10L)).to(Bound.exclusive(20L)); + + assertThat(range.contains(9L)).isFalse(); + assertThat(range.contains(10L)).isFalse(); + assertThat(range.contains(11L)).isTrue(); + assertThat(range.contains(19L)).isTrue(); + assertThat(range.contains(20L)).isFalse(); + assertThat(range.contains(21L)).isFalse(); + assertThat(range.toString()).isEqualTo("(10-20)"); + } + + @Test // DATACMNS-1050 + public void shouldBuildRange() { + + Range range = Range.from(Bound.inclusive(10L)).to(Bound.inclusive(20L)); + + assertThat(range.contains(9L)).isFalse(); + assertThat(range.contains(10L)).isTrue(); + assertThat(range.contains(11L)).isTrue(); + assertThat(range.contains(19L)).isTrue(); + assertThat(range.contains(20L)).isTrue(); + assertThat(range.contains(21L)).isFalse(); + assertThat(range.toString()).isEqualTo("[10-20]"); + } + + @Test // DATACMNS-1050 + public void createsUnboundedRange() { + + Range range = Range.unbounded(); + + assertThat(range.contains(10L)).isTrue(); + 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(); } } diff --git a/src/test/java/org/springframework/data/geo/DistanceUnitTests.java b/src/test/java/org/springframework/data/geo/DistanceUnitTests.java index f9c7fff60..f380491a0 100755 --- a/src/test/java/org/springframework/data/geo/DistanceUnitTests.java +++ b/src/test/java/org/springframework/data/geo/DistanceUnitTests.java @@ -21,6 +21,7 @@ import static org.springframework.data.geo.Metrics.*; import org.assertj.core.data.Offset; import org.junit.Test; import org.springframework.data.domain.Range; +import org.springframework.data.domain.Range.Bound; import org.springframework.util.SerializationUtils; /** @@ -28,6 +29,7 @@ import org.springframework.util.SerializationUtils; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch */ public class DistanceUnitTests { @@ -129,8 +131,8 @@ public class DistanceUnitTests { Range range = Distance.between(twoKilometers, tenKilometers); assertThat(range).isNotNull(); - assertThat(range.getLowerBound()).isEqualTo(twoKilometers); - assertThat(range.getUpperBound()).isEqualTo(tenKilometers); + assertThat(range.getLowerBound()).isEqualTo(Bound.inclusive(twoKilometers)); + assertThat(range.getUpperBound()).isEqualTo(Bound.inclusive(tenKilometers)); } @Test // DATACMNS-651 @@ -142,8 +144,8 @@ public class DistanceUnitTests { Range range = Distance.between(2, KILOMETERS, 10, KILOMETERS); assertThat(range).isNotNull(); - assertThat(range.getLowerBound()).isEqualTo(twoKilometers); - assertThat(range.getUpperBound()).isEqualTo(tenKilometers); + assertThat(range.getLowerBound()).isEqualTo(Bound.inclusive(twoKilometers)); + assertThat(range.getUpperBound()).isEqualTo(Bound.inclusive(tenKilometers)); } @Test // DATACMNS-651