Skip to content

Use OptionalInt instead of Optional<Integer> #34220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void testRankEvalRequest() throws IOException {
if (id.equals("berlin") || id.equals("amsterdam5")) {
assertFalse(hit.getRating().isPresent());
} else {
assertEquals(1, hit.getRating().get().intValue());
assertEquals(1, hit.getRating().getAsInt());
}
}
EvalQueryQuality berlinQueryQuality = partialResults.get("berlin_query");
Expand All @@ -100,7 +100,7 @@ public void testRankEvalRequest() throws IOException {
for (RatedSearchHit hit : hitsAndRatings) {
String id = hit.getSearchHit().getId();
if (id.equals("berlin")) {
assertEquals(1, hit.getRating().get().intValue());
assertEquals(1, hit.getRating().getAsInt());
} else {
assertFalse(hit.getRating().isPresent());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.stream.Collectors;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
Expand Down Expand Up @@ -119,8 +119,8 @@ public Integer getUnknownDocRating() {


@Override
public Optional<Integer> forcedSearchSize() {
return Optional.of(k);
public OptionalInt forcedSearchSize() {
return OptionalInt.of(k);
}

@Override
Expand All @@ -130,9 +130,13 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits,
List<Integer> ratingsInSearchHits = new ArrayList<>(ratedHits.size());
int unratedResults = 0;
for (RatedSearchHit hit : ratedHits) {
// unknownDocRating might be null, in which case unrated docs will be ignored in the dcg calculation.
// we still need to add them as a placeholder so the rank of the subsequent ratings is correct
ratingsInSearchHits.add(hit.getRating().orElse(unknownDocRating));
if (hit.getRating().isPresent()) {
ratingsInSearchHits.add(hit.getRating().getAsInt());
} else {
// unknownDocRating might be null, in which case unrated docs will be ignored in the dcg calculation.
// we still need to add them as a placeholder so the rank of the subsequent ratings is correct
ratingsInSearchHits.add(unknownDocRating);
}
if (hit.getRating().isPresent() == false) {
unratedResults++;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -64,9 +64,9 @@ static List<RatedSearchHit> joinHitsWithRatings(SearchHit[] hits, List<RatedDocu
DocumentKey key = new DocumentKey(hit.getIndex(), hit.getId());
RatedDocument ratedDoc = ratedDocumentMap.get(key);
if (ratedDoc != null) {
ratedSearchHits.add(new RatedSearchHit(hit, Optional.of(ratedDoc.getRating())));
ratedSearchHits.add(new RatedSearchHit(hit, OptionalInt.of(ratedDoc.getRating())));
} else {
ratedSearchHits.add(new RatedSearchHit(hit, Optional.empty()));
ratedSearchHits.add(new RatedSearchHit(hit, OptionalInt.empty()));
}
}
return ratedSearchHits;
Expand All @@ -93,7 +93,7 @@ default double combine(Collection<EvalQueryQuality> partialResults) {
* this method. The default implementation returns an empty optional.
* @return the number of search hits this metrics requests
*/
default Optional<Integer> forcedSearchSize() {
return Optional.empty();
default OptionalInt forcedSearchSize() {
return OptionalInt.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
Expand Down Expand Up @@ -126,8 +126,8 @@ public Integer getUnknownDocRating() {


@Override
public Optional<Integer> forcedSearchSize() {
return Optional.of(k);
public OptionalInt forcedSearchSize() {
return OptionalInt.of(k);
}

@Override
Expand All @@ -139,9 +139,13 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits, List<RatedDocu
List<Integer> ratingsInSearchHits = new ArrayList<>(ratedHits.size());
int unratedResults = 0;
for (RatedSearchHit hit : ratedHits) {
// unknownDocRating might be null, in which case unrated will be ignored in the calculation.
// we still need to add them as a placeholder so the rank of the subsequent ratings is correct
ratingsInSearchHits.add(hit.getRating().orElse(unknownDocRating));
if (hit.getRating().isPresent()) {
ratingsInSearchHits.add(hit.getRating().getAsInt());
} else {
// unknownDocRating might be null, in which case unrated docs will be ignored in the dcg calculation.
// we still need to add them as a placeholder so the rank of the subsequent ratings is correct
ratingsInSearchHits.add(unknownDocRating);
}
if (hit.getRating().isPresent() == false) {
unratedResults++;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
Expand Down Expand Up @@ -90,8 +90,8 @@ int getK() {
}

@Override
public Optional<Integer> forcedSearchSize() {
return Optional.of(k);
public OptionalInt forcedSearchSize() {
return OptionalInt.of(k);
}

@Override
Expand All @@ -115,9 +115,9 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits, List<RatedDocu
int firstRelevant = -1;
int rank = 1;
for (RatedSearchHit hit : ratedHits) {
Optional<Integer> rating = hit.getRating();
OptionalInt rating = hit.getRating();
if (rating.isPresent()) {
if (rating.get() >= this.relevantRatingThreshhold) {
if (rating.getAsInt() >= this.relevantRatingThreshhold) {
firstRelevant = rank;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;

import javax.naming.directory.SearchResult;

Expand Down Expand Up @@ -144,8 +144,8 @@ public boolean getIgnoreUnlabeled() {
}

@Override
public Optional<Integer> forcedSearchSize() {
return Optional.of(k);
public OptionalInt forcedSearchSize() {
return OptionalInt.of(k);
}

public static PrecisionAtK fromXContent(XContentParser parser) {
Expand All @@ -164,9 +164,9 @@ public EvalQueryQuality evaluate(String taskId, SearchHit[] hits,
int falsePositives = 0;
List<RatedSearchHit> ratedSearchHits = joinHitsWithRatings(hits, ratedDocs);
for (RatedSearchHit hit : ratedSearchHits) {
Optional<Integer> rating = hit.getRating();
OptionalInt rating = hit.getRating();
if (rating.isPresent()) {
if (rating.get() >= this.relevantRatingThreshhold) {
if (rating.getAsInt() >= this.relevantRatingThreshhold) {
truePositives++;
} else {
falsePositives++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,40 +33,40 @@

import java.io.IOException;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;

/**
* Combines a {@link SearchHit} with a document rating.
*/
public class RatedSearchHit implements Writeable, ToXContentObject {

private final SearchHit searchHit;
private final Optional<Integer> rating;
private final OptionalInt rating;

public RatedSearchHit(SearchHit searchHit, Optional<Integer> rating) {
public RatedSearchHit(SearchHit searchHit, OptionalInt rating) {
this.searchHit = searchHit;
this.rating = rating;
}

RatedSearchHit(StreamInput in) throws IOException {
this(SearchHit.readSearchHit(in),
in.readBoolean() == true ? Optional.of(in.readVInt()) : Optional.empty());
in.readBoolean() == true ? OptionalInt.of(in.readVInt()) : OptionalInt.empty());
}

@Override
public void writeTo(StreamOutput out) throws IOException {
searchHit.writeTo(out);
out.writeBoolean(rating.isPresent());
if (rating.isPresent()) {
out.writeVInt(rating.get());
out.writeVInt(rating.getAsInt());
}
}

public SearchHit getSearchHit() {
return this.searchHit;
}

public Optional<Integer> getRating() {
public OptionalInt getRating() {
return this.rating;
}

Expand All @@ -75,22 +75,21 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
throws IOException {
builder.startObject();
builder.field("hit", (ToXContent) searchHit);
builder.field("rating", rating.orElse(null));
builder.field("rating", rating.isPresent() ? rating.getAsInt() : null);
builder.endObject();
return builder;
}

private static final ParseField HIT_FIELD = new ParseField("hit");
private static final ParseField RATING_FIELD = new ParseField("rating");
@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<RatedSearchHit, Void> PARSER = new ConstructingObjectParser<>("rated_hit", true,
a -> new RatedSearchHit((SearchHit) a[0], (Optional<Integer>) a[1]));
a -> new RatedSearchHit((SearchHit) a[0], (OptionalInt) a[1]));

static {
PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> SearchHit.fromXContent(p), HIT_FIELD);
PARSER.declareField(ConstructingObjectParser.constructorArg(),
(p) -> p.currentToken() == XContentParser.Token.VALUE_NULL ? Optional.empty() : Optional.of(p.intValue()), RATING_FIELD,
ValueType.INT_OR_NULL);
(p) -> p.currentToken() == XContentParser.Token.VALUE_NULL ? OptionalInt.empty() : OptionalInt.of(p.intValue()),
RATING_FIELD, ValueType.INT_OR_NULL);
}

public static RatedSearchHit parse(XContentParser parser) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ LoggingDeprecationHandler.INSTANCE, new BytesArray(resolvedRequest), XContentTyp
}

if (metric.forcedSearchSize().isPresent()) {
evaluationRequest.size(metric.forcedSearchSize().get());
evaluationRequest.size(metric.forcedSearchSize().getAsInt());
}

ratedRequestsInSearch.add(ratedRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,24 @@ public void testEqualsAndHash() throws IOException {
}

private static PrecisionAtK copy(PrecisionAtK original) {
return new PrecisionAtK(original.getRelevantRatingThreshold(), original.getIgnoreUnlabeled(), original.forcedSearchSize().get());
return new PrecisionAtK(original.getRelevantRatingThreshold(), original.getIgnoreUnlabeled(),
original.forcedSearchSize().getAsInt());
}

private static PrecisionAtK mutate(PrecisionAtK original) {
PrecisionAtK pAtK;
switch (randomIntBetween(0, 2)) {
case 0:
pAtK = new PrecisionAtK(original.getRelevantRatingThreshold(), !original.getIgnoreUnlabeled(),
original.forcedSearchSize().get());
original.forcedSearchSize().getAsInt());
break;
case 1:
pAtK = new PrecisionAtK(randomValueOtherThan(original.getRelevantRatingThreshold(), () -> randomIntBetween(0, 10)),
original.getIgnoreUnlabeled(), original.forcedSearchSize().get());
original.getIgnoreUnlabeled(), original.forcedSearchSize().getAsInt());
break;
case 2:
pAtK = new PrecisionAtK(original.getRelevantRatingThreshold(),
original.getIgnoreUnlabeled(), original.forcedSearchSize().get() + 1);
original.getIgnoreUnlabeled(), original.forcedSearchSize().getAsInt() + 1);
break;
default:
throw new IllegalStateException("The test should only allow three parameters mutated");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void testPrecisionAtRequest() {
if (id.equals("1") || id.equals("6")) {
assertFalse(hit.getRating().isPresent());
} else {
assertEquals(RELEVANT_RATING_1, hit.getRating().get().intValue());
assertEquals(RELEVANT_RATING_1, hit.getRating().getAsInt());
}
}
}
Expand All @@ -139,7 +139,7 @@ public void testPrecisionAtRequest() {
for (RatedSearchHit hit : hitsAndRatings) {
String id = hit.getSearchHit().getId();
if (id.equals("1")) {
assertEquals(RELEVANT_RATING_1, hit.getRating().get().intValue());
assertEquals(RELEVANT_RATING_1, hit.getRating().getAsInt());
} else {
assertFalse(hit.getRating().isPresent());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.function.Predicate;

import static java.util.Collections.singleton;
Expand Down Expand Up @@ -182,6 +182,6 @@ private static RatedSearchHit searchHit(String index, int docId, Integer rating)
SearchHit hit = new SearchHit(docId, docId + "", new Text(""), Collections.emptyMap());
hit.shard(new SearchShardTarget("testnode", new Index(index, "uuid"), 0, null));
hit.score(1.0f);
return new RatedSearchHit(hit, rating != null ? Optional.of(rating) : Optional.empty());
return new RatedSearchHit(hit, rating != null ? OptionalInt.of(rating) : OptionalInt.empty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,28 @@

import java.io.IOException;
import java.util.Collections;
import java.util.Optional;
import java.util.OptionalInt;

import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;

public class RatedSearchHitTests extends ESTestCase {

public static RatedSearchHit randomRatedSearchHit() {
Optional<Integer> rating = randomBoolean() ? Optional.empty()
: Optional.of(randomIntBetween(0, 5));
OptionalInt rating = randomBoolean() ? OptionalInt.empty()
: OptionalInt.of(randomIntBetween(0, 5));
SearchHit searchHit = new SearchHit(randomIntBetween(0, 10), randomAlphaOfLength(10),
new Text(randomAlphaOfLength(10)), Collections.emptyMap());
RatedSearchHit ratedSearchHit = new RatedSearchHit(searchHit, rating);
return ratedSearchHit;
}

private static RatedSearchHit mutateTestItem(RatedSearchHit original) {
Optional<Integer> rating = original.getRating();
OptionalInt rating = original.getRating();
SearchHit hit = original.getSearchHit();
switch (randomIntBetween(0, 1)) {
case 0:
rating = rating.isPresent() ? Optional.of(rating.get() + 1) : Optional.of(randomInt(5));
rating = rating.isPresent() ? OptionalInt.of(rating.getAsInt() + 1) : OptionalInt.of(randomInt(5));
break;
case 1:
hit = new SearchHit(hit.docId(), hit.getId() + randomAlphaOfLength(10),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;

/**
* This class acts as a functional wrapper around the {@code index.auto_expand_replicas} setting.
Expand Down Expand Up @@ -93,7 +93,7 @@ int getMaxReplicas(int numDataNodes) {
return Math.min(maxReplicas, numDataNodes-1);
}

Optional<Integer> getDesiredNumberOfReplicas(int numDataNodes) {
private OptionalInt getDesiredNumberOfReplicas(int numDataNodes) {
if (enabled) {
final int min = getMinReplicas();
final int max = getMaxReplicas(numDataNodes);
Expand All @@ -105,10 +105,10 @@ Optional<Integer> getDesiredNumberOfReplicas(int numDataNodes) {
}

if (numberOfReplicas >= min && numberOfReplicas <= max) {
return Optional.of(numberOfReplicas);
return OptionalInt.of(numberOfReplicas);
}
}
return Optional.empty();
return OptionalInt.empty();
}

@Override
Expand Down