Skip to content

Commit

Permalink
HSEARCH-3708 Rename Range.of(Object, Object) to Range.canonical(Objec…
Browse files Browse the repository at this point in the history
…t, Object)

So that it won't be used lightly with range predicates.
  • Loading branch information
yrodiere committed Sep 18, 2019
1 parent 9dccbea commit 66dae46
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 84 deletions.
Expand Up @@ -281,9 +281,9 @@ public void range() {
// end::range[]
assertThat( countsByPrice )
.containsExactly(
entry( Range.of( 0.0, 10.0 ), 1L ),
entry( Range.of( 10.0, 20.0 ), 2L ),
entry( Range.of( 20.0, null ), 1L )
entry( Range.canonical( 0.0, 10.0 ), 1L ),
entry( Range.canonical( 10.0, 20.0 ), 2L ),
entry( Range.canonical( 20.0, null ), 1L )
);
} );

Expand All @@ -294,7 +294,7 @@ public void range() {
.predicate( f -> f.matchAll() )
.aggregation( countsByPriceKey, f -> f.range()
.field( "price", Double.class )
.range( Range.of( 0.0, 10.0 ) ) // <1>
.range( Range.canonical( 0.0, 10.0 ) ) // <1>
.range( Range.of( 10.0, RangeBoundInclusion.INCLUDED,
20.0, RangeBoundInclusion.EXCLUDED ) ) // <2>
.range( Range.atLeast( 20.0 ) ) // <3>
Expand All @@ -304,9 +304,9 @@ public void range() {
// end::range-objects[]
assertThat( countsByPrice )
.containsExactly(
entry( Range.of( 0.0, 10.0 ), 1L ),
entry( Range.of( 10.0, 20.0 ), 2L ),
entry( Range.of( 20.0, null ), 1L )
entry( Range.canonical( 0.0, 10.0 ), 1L ),
entry( Range.canonical( 10.0, 20.0 ), 2L ),
entry( Range.canonical( 20.0, null ), 1L )
);
} );

Expand Down Expand Up @@ -335,15 +335,15 @@ public void range() {
assertThat( countsByPrice )
.containsExactly(
entry(
Range.of(
Range.canonical(
null,
LocalDate.of( 1970, 1, 1 )
.atStartOfDay().toInstant( ZoneOffset.UTC )
),
2L
),
entry(
Range.of(
Range.canonical(
LocalDate.of( 1970, 1, 1 )
.atStartOfDay().toInstant( ZoneOffset.UTC ),
LocalDate.of( 2000, 1, 1 )
Expand All @@ -352,7 +352,7 @@ public void range() {
1L
),
entry(
Range.of(
Range.canonical(
LocalDate.of( 2000, 1, 1 )
.atStartOfDay().toInstant( ZoneOffset.UTC ),
null
Expand Down
Expand Up @@ -26,7 +26,7 @@ public interface RangeAggregationRangeStep<F> {
* @return The next step.
*/
default RangeAggregationRangeMoreStep<F> range(F lowerBound, F upperBound) {
return range( Range.of( lowerBound, upperBound ) );
return range( Range.canonical( lowerBound, upperBound ) );
}

/**
Expand Down
Expand Up @@ -70,17 +70,17 @@ public void float_infinityIncluded() {
SearchResultAssert.assertThat(
matchAllQuery()
.aggregation( aggregationKey, f -> f.range().field( fieldPath, Float.class )
.range( Range.of( null, 0f ) )
.range( Range.of( 0f, null ) )
.range( Range.canonical( null, 0f ) )
.range( Range.canonical( 0f, null ) )
.range( Range.all() )
)
.toQuery()
)
.aggregation(
aggregationKey,
a -> assertThat( a ).containsExactly(
entry( Range.of( null, 0f ), 1L ),
entry( Range.of( 0f, null ), 2L ),
entry( Range.canonical( null, 0f ), 1L ),
entry( Range.canonical( 0f, null ), 2L ),
entry( Range.<Float>all(), 3L )
)
);
Expand Down Expand Up @@ -130,17 +130,17 @@ public void double_infinityIncluded() {
SearchResultAssert.assertThat(
matchAllQuery()
.aggregation( aggregationKey, f -> f.range().field( fieldPath, Double.class )
.range( Range.of( null, 0d ) )
.range( Range.of( 0d, null ) )
.range( Range.canonical( null, 0d ) )
.range( Range.canonical( 0d, null ) )
.range( Range.all() )
)
.toQuery()
)
.aggregation(
aggregationKey,
a -> assertThat( a ).containsExactly(
entry( Range.of( null, 0d ), 1L ),
entry( Range.of( 0d, null ), 2L ),
entry( Range.canonical( null, 0d ), 1L ),
entry( Range.canonical( 0d, null ), 2L ),
entry( Range.<Double>all(), 3L )
)
);
Expand Down
Expand Up @@ -146,7 +146,7 @@ public void rangeLessThan() {
.aggregation(
aggregationKey,
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.canonical( null, ascendingValues.get( 2 ) ), 2L );
} )
);
}
Expand Down Expand Up @@ -199,7 +199,7 @@ public void rangeGreaterThan() {

@Test
@PortedFromSearch5(original = "org.hibernate.search.test.query.facet.RangeFacetingTest.testRangeWithExcludeLimitsAtEachLevel")
public void rangesOf() {
public void rangesCanonical() {
String fieldPath = indexMapping.fieldModel.relativeFieldName;

AggregationKey<Map<Range<F>, Long>> aggregationKey = AggregationKey.of( AGGREGATION_NAME );
Expand All @@ -208,19 +208,19 @@ public void rangesOf() {
matchAllQuery()
.aggregation( aggregationKey, f -> f.range().field( fieldPath, typeDescriptor.getJavaType() )
.ranges( Arrays.asList(
Range.of( null, ascendingValues.get( 3 ) ),
Range.of( ascendingValues.get( 3 ), ascendingValues.get( 5 ) ),
Range.of( ascendingValues.get( 5 ), null )
Range.canonical( null, ascendingValues.get( 3 ) ),
Range.canonical( ascendingValues.get( 3 ), ascendingValues.get( 5 ) ),
Range.canonical( ascendingValues.get( 5 ), null )
) )
)
.toQuery()
)
.aggregation(
aggregationKey,
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 3 ) ), 3L );
c.accept( Range.of( ascendingValues.get( 3 ), ascendingValues.get( 5 ) ), 2L );
c.accept( Range.of( ascendingValues.get( 5 ), null ), 2L );
c.accept( Range.canonical( null, ascendingValues.get( 3 ) ), 3L );
c.accept( Range.canonical( ascendingValues.get( 3 ), ascendingValues.get( 5 ) ), 2L );
c.accept( Range.canonical( ascendingValues.get( 5 ), null ), 2L );
} )
);
}
Expand Down Expand Up @@ -282,19 +282,19 @@ public void rangesOverlap() {
matchAllQuery()
.aggregation( aggregationKey, f -> f.range().field( fieldPath, typeDescriptor.getJavaType() )
.ranges( Arrays.asList(
Range.of( null, ascendingValues.get( 3 ) ),
Range.of( ascendingValues.get( 1 ), ascendingValues.get( 5 ) ),
Range.of( ascendingValues.get( 2 ), null )
Range.canonical( null, ascendingValues.get( 3 ) ),
Range.canonical( ascendingValues.get( 1 ), ascendingValues.get( 5 ) ),
Range.canonical( ascendingValues.get( 2 ), null )
) )
)
.toQuery()
)
.aggregation(
aggregationKey,
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 3 ) ), 3L );
c.accept( Range.of( ascendingValues.get( 1 ), ascendingValues.get( 5 ) ), 4L );
c.accept( Range.of( ascendingValues.get( 2 ), null ), 5L );
c.accept( Range.canonical( null, ascendingValues.get( 3 ) ), 3L );
c.accept( Range.canonical( ascendingValues.get( 1 ), ascendingValues.get( 5 ) ), 4L );
c.accept( Range.canonical( ascendingValues.get( 2 ), null ), 5L );
} )
);
}
Expand Down Expand Up @@ -338,7 +338,7 @@ public void rangesContainingNull() {
indexManager.createScope().aggregation().range()
.field( fieldPath, typeDescriptor.getJavaType() )
.ranges( Arrays.asList(
Range.of( ascendingValues.get( 0 ), ascendingValues.get( 1 ) ),
Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 1 ) ),
null
) )
)
Expand Down Expand Up @@ -388,10 +388,10 @@ public void predicate() {
aggregationKey,
// Only document 1 and 5 should be taken into account by the aggregation
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 2 ) ), 1L );
c.accept( Range.canonical( null, ascendingValues.get( 2 ) ), 1L );
// Ranges with 0 matching documents should still be returned
c.accept( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 0L );
c.accept( Range.of( ascendingValues.get( 5 ), null ), 1L );
c.accept( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 0L );
c.accept( Range.canonical( ascendingValues.get( 5 ), null ), 1L );
} )
);
}
Expand All @@ -418,9 +418,9 @@ public void limitAndOffset() {
aggregationKey,
// All documents should be taken into account by the aggregation, even those excluded by the limit/offset
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.of( ascendingValues.get( 5 ), null ), 2L );
c.accept( Range.canonical( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.canonical( ascendingValues.get( 5 ), null ), 2L );
} )
);
}
Expand Down Expand Up @@ -450,13 +450,13 @@ public void rangeOverlap() {
.aggregation(
aggregationKey,
containsExactly( c -> {
c.accept( Range.of( ascendingValues.get( 0 ), null ), 7L );
c.accept( Range.of( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.of( null, null ), 7L );
c.accept( Range.of( ascendingValues.get( 0 ), ascendingValues.get( 7 ) ), 7L );
c.accept( Range.of( ascendingValues.get( 5 ), null ), 2L );
c.accept( Range.of( null, ascendingValues.get( 6 ) ), 6L );
c.accept( Range.canonical( ascendingValues.get( 0 ), null ), 7L );
c.accept( Range.canonical( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.canonical( null, null ), 7L );
c.accept( Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 7 ) ), 7L );
c.accept( Range.canonical( ascendingValues.get( 5 ), null ), 2L );
c.accept( Range.canonical( null, ascendingValues.get( 6 ) ), 6L );
} )
);
}
Expand All @@ -482,9 +482,9 @@ public void order_asDefined() {
.aggregation(
aggregationKey,
containsExactly( c -> {
c.accept( Range.of( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.of( ascendingValues.get( 5 ), null ), 2L );
c.accept( Range.canonical( null, ascendingValues.get( 2 ) ), 2L );
c.accept( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 3L );
c.accept( Range.canonical( ascendingValues.get( 5 ), null ), 2L );
} )
);
}
Expand Down
Expand Up @@ -62,16 +62,16 @@ > getSingleFieldAggregationExpectations(FieldTypeDescriptor<F> typeDescriptor) {
otherIndexDocumentFieldValues.add( ascendingValues.get( 1 ) );
otherIndexDocumentFieldValues.add( ascendingValues.get( 6 ) );

mainIndexExpected.put( Range.of( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 3L );
mainIndexExpected.put( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 5L );
mainIndexExpected.put( Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 3L );
mainIndexExpected.put( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 5L );
mainIndexExpected.put( Range.atLeast( ascendingValues.get( 5 ) ), 2L );

mainAndOtherIndexExpected.put( Range.of( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 4L );
mainAndOtherIndexExpected.put( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 5L );
mainAndOtherIndexExpected.put( Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 4L );
mainAndOtherIndexExpected.put( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 5L );
mainAndOtherIndexExpected.put( Range.atLeast( ascendingValues.get( 5 ) ), 3L );

noIndexedValueExpected.put( Range.of( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 0L );
noIndexedValueExpected.put( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 0L );
noIndexedValueExpected.put( Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 0L );
noIndexedValueExpected.put( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 0L );
noIndexedValueExpected.put( Range.atLeast( ascendingValues.get( 5 ) ), 0L );

// Dataset and expectations for the multi-valued index
Expand All @@ -88,8 +88,8 @@ > getSingleFieldAggregationExpectations(FieldTypeDescriptor<F> typeDescriptor) {
Arrays.asList( ascendingValues.get( 3 ), ascendingValues.get( 4 ) )
);

multiValuedIndexExpected.put( Range.of( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 2L );
multiValuedIndexExpected.put( Range.of( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 2L );
multiValuedIndexExpected.put( Range.canonical( ascendingValues.get( 0 ), ascendingValues.get( 2 ) ), 2L );
multiValuedIndexExpected.put( Range.canonical( ascendingValues.get( 2 ), ascendingValues.get( 5 ) ), 2L );
multiValuedIndexExpected.put( Range.atLeast( ascendingValues.get( 5 ) ), 2L );

return ExpectationsAlternative.supported( new SupportedSingleFieldAggregationExpectations<F>(
Expand Down
Expand Up @@ -394,10 +394,10 @@ public void searchFaceted() {
);
assertThat( result.getTotalHitCountByCollectionSize() )
.containsExactly(
entry( Range.of( 0, 1_000 ), 2L ),
entry( Range.of( 1_000, 5_000 ), 0L ),
entry( Range.of( 5_000, 10_000 ), 1L ),
entry( Range.of( 10_000, null ), 1L )
entry( Range.canonical( 0, 1_000 ), 2L ),
entry( Range.canonical( 1_000, 5_000 ), 0L ),
entry( Range.canonical( 5_000, 10_000 ), 1L ),
entry( Range.canonical( 10_000, null ), 1L )
);
assertThat( result.getTotalHitCountByService() )
.containsExactly(
Expand All @@ -414,10 +414,10 @@ public void searchFaceted() {
);
assertThat( result.getTotalHitCountByCollectionSize() )
.containsExactly(
entry( Range.of( 0, 1_000 ), 0L ),
entry( Range.of( 1_000, 5_000 ), 0L ),
entry( Range.of( 5_000, 10_000 ), 1L ),
entry( Range.of( 10_000, null ), 1L )
entry( Range.canonical( 0, 1_000 ), 0L ),
entry( Range.canonical( 1_000, 5_000 ), 0L ),
entry( Range.canonical( 5_000, 10_000 ), 1L ),
entry( Range.canonical( 10_000, null ), 1L )
);
assertThat( result.getTotalHitCountByService() )
.containsExactly(
Expand All @@ -435,10 +435,10 @@ public void searchFaceted() {
);
assertThat( result.getTotalHitCountByCollectionSize() )
.containsExactly(
entry( Range.of( 0, 1_000 ), 1L ),
entry( Range.of( 1_000, 5_000 ), 0L ),
entry( Range.of( 5_000, 10_000 ), 1L ),
entry( Range.of( 10_000, null ), 1L )
entry( Range.canonical( 0, 1_000 ), 1L ),
entry( Range.canonical( 1_000, 5_000 ), 0L ),
entry( Range.canonical( 5_000, 10_000 ), 1L ),
entry( Range.canonical( 10_000, null ), 1L )
);
assertThat( result.getTotalHitCountByService() )
.containsExactly(
Expand Down
Expand Up @@ -27,20 +27,6 @@
*/
public final class Range<T> {

/**
* @param lowerBoundValue The lower bound of the range.
* May be {@code null} to represent {@code -Infinity} (no lower bound),
* @param upperBoundValue The upper bound of the range.
* May be {@code null} to represent {@code +Infinity} (no upper bound).
* @param <T> The type of range bounds.
* @return The range {@code [lowerBoundValue, upperBoundValue)} (lower bound included, upper bound excluded),
* or {@code [lowerBoundValue, +Infinity]} (both bounds included) if the upper bound is {@code +Infinity}.
*/
public static <T> Range<T> of(T lowerBoundValue, T upperBoundValue) {
return new Range<>( lowerBoundValue, RangeBoundInclusion.INCLUDED,
upperBoundValue, upperBoundValue == null ? RangeBoundInclusion.INCLUDED : RangeBoundInclusion.EXCLUDED );
}

/**
* @param lowerBoundValue The value of the lower bound of the range.
* May be {@code null} to represent {@code -Infinity} (no lower bound).
Expand All @@ -56,6 +42,27 @@ public static <T> Range<T> of(T lowerBoundValue, RangeBoundInclusion lowerBoundI
return new Range<>( lowerBoundValue, lowerBoundInclusion, upperBoundValue, upperBoundInclusion );
}

/**
* Create a canonical range, i.e. a range in the form
* {@code [lowerBoundValue, upperBoundValue)} (lower bound included, upper bound excluded),
* or {@code [lowerBoundValue, +Infinity]} (both bounds included) if the upper bound is {@code +Infinity}.
* <p>
* This is mostly useful when creating multiple, contiguous ranges,
* like for example in range aggregations.
*
* @param lowerBoundValue The lower bound of the range.
* May be {@code null} to represent {@code -Infinity} (no lower bound),
* @param upperBoundValue The upper bound of the range.
* May be {@code null} to represent {@code +Infinity} (no upper bound).
* @param <T> The type of range bounds.
* @return The range {@code [lowerBoundValue, upperBoundValue)} (lower bound included, upper bound excluded),
* or {@code [lowerBoundValue, +Infinity]} (both bounds included) if the upper bound is {@code +Infinity}.
*/
public static <T> Range<T> canonical(T lowerBoundValue, T upperBoundValue) {
return new Range<>( lowerBoundValue, RangeBoundInclusion.INCLUDED,
upperBoundValue, upperBoundValue == null ? RangeBoundInclusion.INCLUDED : RangeBoundInclusion.EXCLUDED );
}

/**
* @param <T> The type of range bounds.
* @return The range {@code [-Infinity, +Infinity]} (both bounds included).
Expand Down

0 comments on commit 66dae46

Please sign in to comment.