Skip to content

Commit

Permalink
HSEARCH-3922 Do not use "get"/"set" prefixes for API methods in the u…
Browse files Browse the repository at this point in the history
…til-common module
  • Loading branch information
yrodiere committed May 20, 2020
1 parent 28d2412 commit 5bdcdb5
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 46 deletions.
Expand Up @@ -93,16 +93,16 @@ public Builder(ElasticsearchSearchContext searchContext, String absoluteFieldPat
@Override
public void range(Range<? extends K> range) {
JsonObject rangeJson = new JsonObject();
Optional<? extends K> lowerBoundValue = range.getLowerBoundValue();
Optional<? extends K> lowerBoundValue = range.lowerBoundValue();
if ( lowerBoundValue.isPresent() ) {
if ( !RangeBoundInclusion.INCLUDED.equals( range.getLowerBoundInclusion() ) ) {
if ( !RangeBoundInclusion.INCLUDED.equals( range.lowerBoundInclusion() ) ) {
throw log.elasticsearchRangeAggregationRequiresCanonicalFormForRanges( range );
}
rangeJson.add( "from", convertToFieldValue( lowerBoundValue.get() ) );
}
Optional<? extends K> upperBoundValue = range.getUpperBoundValue();
Optional<? extends K> upperBoundValue = range.upperBoundValue();
if ( upperBoundValue.isPresent() ) {
if ( !RangeBoundInclusion.EXCLUDED.equals( range.getUpperBoundInclusion() ) ) {
if ( !RangeBoundInclusion.EXCLUDED.equals( range.upperBoundInclusion() ) ) {
throw log.elasticsearchRangeAggregationRequiresCanonicalFormForRanges( range );
}
rangeJson.add( "to", convertToFieldValue( upperBoundValue.get() ) );
Expand Down
Expand Up @@ -65,25 +65,25 @@ public ElasticsearchRangePredicateBuilder(ElasticsearchSearchContext searchConte
@Override
public void range(Range<?> range, ValueConvert convertLowerBound, ValueConvert convertUpperBound) {
this.range = Range.between(
convertToFieldValue( range.getLowerBoundValue(), convertLowerBound ),
range.getLowerBoundInclusion(),
convertToFieldValue( range.getUpperBoundValue(), convertUpperBound ),
range.getUpperBoundInclusion()
convertToFieldValue( range.lowerBoundValue(), convertLowerBound ),
range.lowerBoundInclusion(),
convertToFieldValue( range.upperBoundValue(), convertUpperBound ),
range.upperBoundInclusion()
);
}

@Override
protected JsonObject doBuild(ElasticsearchSearchPredicateContext context,
JsonObject outerObject, JsonObject innerObject) {
JsonAccessor<JsonElement> accessor;
Optional<JsonElement> lowerBoundValue = range.getLowerBoundValue();
Optional<JsonElement> lowerBoundValue = range.lowerBoundValue();
if ( lowerBoundValue.isPresent() ) {
accessor = RangeBoundInclusion.EXCLUDED.equals( range.getLowerBoundInclusion() ) ? GT_ACCESSOR : GTE_ACCESSOR;
accessor = RangeBoundInclusion.EXCLUDED.equals( range.lowerBoundInclusion() ) ? GT_ACCESSOR : GTE_ACCESSOR;
accessor.set( innerObject, lowerBoundValue.get() );
}
Optional<JsonElement> upperBoundValue = range.getUpperBoundValue();
Optional<JsonElement> upperBoundValue = range.upperBoundValue();
if ( upperBoundValue.isPresent() ) {
accessor = RangeBoundInclusion.EXCLUDED.equals( range.getUpperBoundInclusion() ) ? LT_ACCESSOR : LTE_ACCESSOR;
accessor = RangeBoundInclusion.EXCLUDED.equals( range.upperBoundInclusion() ) ? LT_ACCESSOR : LTE_ACCESSOR;
accessor.set( innerObject, upperBoundValue.get() );
}

Expand Down
Expand Up @@ -34,18 +34,18 @@ private static <T> LongRange[] createLongRanges(Collection<? extends Range<? ext
LongRange[] longRanges = new LongRange[ranges.size()];
int i = 0;
for ( Range<? extends T> range : ranges ) {
T lowerBoundValue = range.getLowerBoundValue().orElse( null );
T upperBoundValue = range.getUpperBoundValue().orElse( null );
T lowerBoundValue = range.lowerBoundValue().orElse( null );
T upperBoundValue = range.upperBoundValue().orElse( null );
longRanges[i] = new LongRange(
String.valueOf( i ),
encoder.applyAsLong( lowerBoundValue == null ? lowestPossibleValue : lowerBoundValue ),
// The lower bound is included if it is explicitly included
RangeBoundInclusion.INCLUDED.equals( range.getLowerBoundInclusion() )
RangeBoundInclusion.INCLUDED.equals( range.lowerBoundInclusion() )
// ... or if it is infinity but infinity cannot be represented
|| !extremaAreInfinity && lowerBoundValue == null,
encoder.applyAsLong( upperBoundValue == null ? highestPossibleValue : upperBoundValue ),
// The upper bound is included if it is explicitly included
RangeBoundInclusion.INCLUDED.equals( range.getUpperBoundInclusion() )
RangeBoundInclusion.INCLUDED.equals( range.upperBoundInclusion() )
// ... or if it is infinity but infinity cannot be represented
|| !extremaAreInfinity && upperBoundValue == null
);
Expand Down
Expand Up @@ -59,10 +59,10 @@ protected AbstractLuceneStandardRangePredicateBuilder(
@Override
public void range(Range<?> range, ValueConvert convertLowerBound, ValueConvert convertUpperBound) {
this.range = Range.between(
convertAndEncode( range.getLowerBoundValue(), convertLowerBound ),
range.getLowerBoundInclusion(),
convertAndEncode( range.getUpperBoundValue(), convertUpperBound ),
range.getUpperBoundInclusion()
convertAndEncode( range.lowerBoundValue(), convertLowerBound ),
range.lowerBoundInclusion(),
convertAndEncode( range.upperBoundValue(), convertUpperBound ),
range.upperBoundInclusion()
);
}

Expand Down
Expand Up @@ -36,8 +36,8 @@ protected Query doBuild(LuceneSearchPredicateContext context) {
LuceneNumericDomain<E> domain = codec.getDomain();
return domain.createRangeQuery(
absoluteFieldPath,
getLowerValue( domain, range.getLowerBoundValue(), range.getLowerBoundInclusion() ),
getUpperValue( domain, range.getUpperBoundValue(), range.getUpperBoundInclusion() )
getLowerValue( domain, range.lowerBoundValue(), range.lowerBoundInclusion() ),
getUpperValue( domain, range.upperBoundValue(), range.upperBoundInclusion() )
);
}

Expand Down
Expand Up @@ -38,13 +38,13 @@ protected Query doBuild(LuceneSearchPredicateContext context) {

return new TermRangeQuery(
absoluteFieldPath,
codec.normalize( absoluteFieldPath, range.getLowerBoundValue().orElse( null ) ),
codec.normalize( absoluteFieldPath, range.getUpperBoundValue().orElse( null ) ),
codec.normalize( absoluteFieldPath, range.lowerBoundValue().orElse( null ) ),
codec.normalize( absoluteFieldPath, range.upperBoundValue().orElse( null ) ),
// we force the true value if the bound is null because of some Lucene checks down the hill
RangeBoundInclusion.INCLUDED.equals( range.getLowerBoundInclusion() )
|| !range.getLowerBoundValue().isPresent(),
RangeBoundInclusion.INCLUDED.equals( range.getUpperBoundInclusion() )
|| !range.getUpperBoundValue().isPresent()
RangeBoundInclusion.INCLUDED.equals( range.lowerBoundInclusion() )
|| !range.lowerBoundValue().isPresent(),
RangeBoundInclusion.INCLUDED.equals( range.upperBoundInclusion() )
|| !range.upperBoundValue().isPresent()
);
}
}
Expand Up @@ -98,7 +98,7 @@ protected NonRootFailureCollector(NonRootFailureCollector parent) {

@Override
public synchronized ContextualFailureCollectorImpl withContext(EventContext context) {
List<EventContextElement> elements = context.getElements();
List<EventContextElement> elements = context.elements();
// This should not happen, but we want to be extra-cautious to avoid failures while handling failures
if ( elements.isEmpty() ) {
// Just log the problem and degrade gracefully.
Expand Down Expand Up @@ -181,12 +181,12 @@ public void add(Throwable t) {
if ( t instanceof SearchException ) {
SearchException e = (SearchException) t;
ContextualFailureCollectorImpl failureCollector = this;
EventContext eventContext = e.getContext();
EventContext eventContext = e.context();
if ( eventContext != null ) {
failureCollector = failureCollector.withContext( e.getContext() );
failureCollector = failureCollector.withContext( e.context() );
}
// Do not include the context in the failure message, since we will render it as part of the failure report
failureCollector.doAdd( e, e.getMessageWithoutContext() );
failureCollector.doAdd( e, e.messageWithoutContext() );
}
else {
doAdd( t, t.getMessage() );
Expand Down
Expand Up @@ -87,7 +87,7 @@ static class CommonState<B>

CommonState<B> range(Range<?> range, ValueConvert lowerBoundConvert, ValueConvert upperBoundConvert) {
Contracts.assertNotNull( range, "range" );
if ( !range.getLowerBoundValue().isPresent() && !range.getUpperBoundValue().isPresent() ) {
if ( !range.lowerBoundValue().isPresent() && !range.upperBoundValue().isPresent() ) {
throw log.rangePredicateCannotMatchNullValue( getEventContext() );
}
for ( RangePredicateFieldMoreStepImpl<B> fieldSetState : getFieldSetStates() ) {
Expand Down
Expand Up @@ -40,11 +40,35 @@ public SearchException(Throwable cause, EventContext context) {
this( null, cause, context );
}

public String getMessageWithoutContext() {
/**
* @return The exception message, without the description of the context.
*/
public String messageWithoutContext() {
return messageWithoutContext;
}

public EventContext getContext() {
/**
* @deprecated Use {@link #messageWithoutContext} instead.
* @return The exception message, without the description of the context.
*/
@Deprecated
public String getMessageWithoutContext() {
return messageWithoutContext();
}

/**
* @return The context in which this exception occurred.
*/
public EventContext context() {
return context;
}

/**
* @deprecated Use {@link #context} instead.
* @return The context in which this exception occurred.
*/
@Deprecated
public EventContext getContext() {
return context();
}
}
Expand Up @@ -20,8 +20,8 @@
* As a result, only minimal consistency checks are performed: null-checks, mostly.
* In particular, <strong>this class does not check that the lower bound is actually lower than the upper bound</strong>,
* because it has no idea what ordering to use.
* Checking the relative order of bounds is the responsibility of callers of the {@link #getLowerBoundValue()}
* and {@link #getUpperBoundValue()} methods.
* Checking the relative order of bounds is the responsibility of callers of the {@link #lowerBoundValue()}
* and {@link #upperBoundValue()} methods.
*
* @param <T> The type of values in this range.
*/
Expand Down Expand Up @@ -196,33 +196,71 @@ public int hashCode() {
/**
* @return The value of the lower bound, or an empty optional to represent {-Infinity} (no lower bound).
*/
public Optional<T> getLowerBoundValue() {
public Optional<T> lowerBoundValue() {
return lowerBoundValue;
}

/**
* @return The value of the lower bound, or an empty optional to represent {-Infinity} (no lower bound).
* @deprecated Use {@link #lowerBoundValue()} instead.
*/
@Deprecated
public Optional<T> getLowerBoundValue() {
return lowerBoundValue();
}

/**
* @return Whether the lower bound is included in the range or excluded.
* Always {@link RangeBoundInclusion#EXCLUDED} if there is no lower bound.
*/
public RangeBoundInclusion getLowerBoundInclusion() {
public RangeBoundInclusion lowerBoundInclusion() {
return lowerBoundInclusion;
}

/**
* @return Whether the lower bound is included in the range or excluded.
* Always {@link RangeBoundInclusion#EXCLUDED} if there is no lower bound.
* @deprecated Use {@link #lowerBoundInclusion()} instead.
*/
@Deprecated
public RangeBoundInclusion getLowerBoundInclusion() {
return lowerBoundInclusion();
}

/**
* @return The value of the lower bound, or an empty optional to represent {+Infinity} (no upper bound).
*/
public Optional<T> getUpperBoundValue() {
public Optional<T> upperBoundValue() {
return upperBoundValue;
}

/**
* @return The value of the lower bound, or an empty optional to represent {+Infinity} (no upper bound).
* @deprecated Use {@link #upperBoundValue()} instead.
*/
@Deprecated
public Optional<T> getUpperBoundValue() {
return upperBoundValue();
}

/**
* @return Whether the upper bound is included in the range or excluded.
* Always {@link RangeBoundInclusion#EXCLUDED} if there is no upper bound.
*/
public RangeBoundInclusion getUpperBoundInclusion() {
public RangeBoundInclusion upperBoundInclusion() {
return upperBoundInclusion;
}

/**
* @return Whether the upper bound is included in the range or excluded.
* Always {@link RangeBoundInclusion#EXCLUDED} if there is no upper bound.
* @deprecated Use {@link #upperBoundInclusion()} instead.
*/
@Deprecated
public RangeBoundInclusion getUpperBoundInclusion() {
return upperBoundInclusion();
}

public <R> Range<R> map(Function<? super T, ? extends R> function) {
return Range.between(
lowerBoundValue.map( function ).orElse( null ),
Expand Down
Expand Up @@ -66,18 +66,30 @@ public int hashCode() {
return Objects.hash( parent, element );
}

public List<EventContextElement> getElements() {
/**
* @return The elements of this context. Never empty, does not contain {@code null} values.
*/
public List<EventContextElement> elements() {
List<EventContextElement> result = new ArrayList<>();
addTo( result );
return result;
}

/**
* @return The elements of this context. Never empty, does not contain {@code null} values.
* @deprecated Use {@link #elements()} instead.
*/
@Deprecated
public List<EventContextElement> getElements() {
return elements();
}

/**
* @return A human-readable representation of this context.
*/
public String render() {
StringJoiner contextJoiner = new StringJoiner( MESSAGES.contextSeparator() );
for ( EventContextElement element : getElements() ) {
for ( EventContextElement element : elements() ) {
contextJoiner.add( element.render() );
}
return contextJoiner.toString();
Expand Down
Expand Up @@ -30,7 +30,7 @@ private FailureReportUtils() {
*/
public static <T extends Throwable> Consumer<T> hasContext(EventContext first, EventContext... others) {
return hasContext(
EventContext.concat( first, others ).getElements().toArray( new EventContextElement[] { } )
EventContext.concat( first, others ).elements().toArray( new EventContextElement[] { } )
);
}

Expand All @@ -43,9 +43,9 @@ public static <T extends Throwable> Consumer<T> hasContext(EventContextElement..
return throwable -> {
Assertions.assertThat( throwable )
.isInstanceOf( SearchException.class );
EventContext actualContext = ( (SearchException) throwable ).getContext();
EventContext actualContext = ( (SearchException) throwable ).context();
Assertions.assertThat( actualContext ).as( "throwable.getContext()" ).isNotNull();
Assertions.assertThat( actualContext.getElements() )
Assertions.assertThat( actualContext.elements() )
.containsExactly( contextElements );
String renderedContextElements = Arrays.stream( contextElements ).map( EventContextElement::render )
.collect( Collectors.joining( ", " ) );
Expand Down

0 comments on commit 5bdcdb5

Please sign in to comment.