-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add unit tests for terms aggregation objects. #23149
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| import java.io.IOException; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.function.Function; | ||
| import java.util.stream.Collectors; | ||
|
|
||
|
|
@@ -110,4 +111,20 @@ public B getBucketByKey(String term) { | |
| } | ||
| return bucketMap.get(term); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean doEquals(Object obj) { | ||
| InternalMappedTerms<?,?> that = (InternalMappedTerms<?,?>) obj; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is called from equals, which already does this check.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I see now. Sorry, I wasn't being careful. |
||
| return super.doEquals(obj) | ||
| && Objects.equals(buckets, that.buckets) | ||
| && Objects.equals(format, that.format) | ||
| && Objects.equals(otherDocCount, that.otherDocCount) | ||
| && Objects.equals(showTermDocCountError, that.showTermDocCountError) | ||
| && Objects.equals(shardSize, that.shardSize); | ||
| } | ||
|
|
||
| @Override | ||
| protected int doHashCode() { | ||
| return Objects.hash(super.doHashCode(), buckets, format, otherDocCount, showTermDocCountError, shardSize); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ | |
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| import static java.util.Collections.unmodifiableList; | ||
|
|
||
|
|
@@ -135,6 +136,25 @@ public B reduce(List<B> buckets, ReduceContext context) { | |
| InternalAggregations aggs = InternalAggregations.reduce(aggregationsList, context); | ||
| return newBucket(docCount, aggs, docCountError); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (obj == null || getClass() != obj.getClass()) { | ||
| return false; | ||
| } | ||
| Bucket<?> that = (Bucket<?>) obj; | ||
| // No need to take format and showDocCountError, they are attributes | ||
| // of the parent terms aggregation object that are only copied here | ||
| // for serialization purposes | ||
| return Objects.equals(docCount, that.docCount) | ||
| && Objects.equals(docCountError, that.docCountError) | ||
| && Objects.equals(aggregations, that.aggregations); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(getClass(), docCount, docCountError, aggregations); | ||
| } | ||
| } | ||
|
|
||
| protected final Terms.Order order; | ||
|
|
@@ -269,4 +289,17 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu | |
| * Create an array to hold some buckets. Used in collecting the results. | ||
| */ | ||
| protected abstract B[] createBucketsArray(int size); | ||
|
|
||
| @Override | ||
| protected boolean doEquals(Object obj) { | ||
| InternalTerms<?,?> that = (InternalTerms<?,?>) obj; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this one is also fine. |
||
| return Objects.equals(minDocCount, that.minDocCount) | ||
| && Objects.equals(order, that.order) | ||
| && Objects.equals(requiredSize, that.requiredSize); | ||
| } | ||
|
|
||
| @Override | ||
| protected int doHashCode() { | ||
| return Objects.hash(minDocCount, order, requiredSize); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Result of the {@link TermsAggregator} when the field is some kind of whole number like a integer, long, or a date. | ||
|
|
@@ -99,6 +100,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws | |
| builder.endObject(); | ||
| return builder; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| return super.equals(obj) && Objects.equals(term, ((Bucket) obj).term); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As is this one. |
||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(super.hashCode(), term); | ||
| } | ||
| } | ||
|
|
||
| public LongTerms(String name, Terms.Order order, int requiredSize, long minDocCount, List<PipelineAggregator> pipelineAggregators, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |
| import java.io.IOException; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Result of the {@link TermsAggregator} when the field is a String. | ||
|
|
@@ -95,6 +96,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws | |
| builder.endObject(); | ||
| return builder; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| return super.equals(obj) && Objects.equals(termBytes, ((Bucket) obj).termBytes); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is fine too. |
||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(super.hashCode(), termBytes); | ||
| } | ||
| } | ||
|
|
||
| public StringTerms(String name, Terms.Order order, int requiredSize, long minDocCount, List<PipelineAggregator> pipelineAggregators, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.elasticsearch.search.aggregations.bucket.terms; | ||
|
|
||
| import org.elasticsearch.common.io.stream.Writeable.Reader; | ||
| import org.elasticsearch.search.DocValueFormat; | ||
| import org.elasticsearch.search.aggregations.InternalAggregations; | ||
| import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| public class DoubleTermsTests extends InternalTermsTestCase { | ||
|
|
||
| @Override | ||
| protected InternalTerms<?, ?> createTestInstance( | ||
| String name, | ||
| List<PipelineAggregator> pipelineAggregators, | ||
| Map<String, Object> metaData) { | ||
| Terms.Order order = Terms.Order.count(false); | ||
| long minDocCount = 1; | ||
| int requiredSize = 3; | ||
| int shardSize = requiredSize + 2; | ||
| DocValueFormat format = DocValueFormat.RAW; | ||
| boolean showTermDocCountError = false; | ||
| long docCountError = -1; | ||
| long otherDocCount = 0; | ||
| List<DoubleTerms.Bucket> buckets = new ArrayList<>(); | ||
| final int numBuckets = randomInt(shardSize); | ||
| Set<Double> terms = new HashSet<>(); | ||
| for (int i = 0; i < numBuckets; ++i) { | ||
| double term = randomValueOtherThanMany(d -> terms.add(d) == false, random()::nextDouble); | ||
| int docCount = randomIntBetween(1, 100); | ||
| buckets.add(new DoubleTerms.Bucket(term, docCount, InternalAggregations.EMPTY, | ||
| showTermDocCountError, docCountError, format)); | ||
| } | ||
| return new DoubleTerms(name, order, requiredSize, minDocCount, pipelineAggregators, | ||
| metaData, format, shardSize, showTermDocCountError, otherDocCount, buckets, docCountError); | ||
| } | ||
|
|
||
| @Override | ||
| protected Reader<InternalTerms<?, ?>> instanceReader() { | ||
| return DoubleTerms::new; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.elasticsearch.search.aggregations.bucket.terms; | ||
|
|
||
| import org.elasticsearch.search.aggregations.InternalAggregationTestCase; | ||
| import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
| import java.util.Map.Entry; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public abstract class InternalTermsTestCase extends InternalAggregationTestCase<InternalTerms<?,?>> { | ||
|
|
||
| @Override | ||
| protected InternalTerms<?, ?> createUnmappedInstance( | ||
| String name, | ||
| List<PipelineAggregator> pipelineAggregators, | ||
| Map<String, Object> metaData) { | ||
| InternalTerms<?, ?> testInstance = createTestInstance(name, pipelineAggregators, metaData); | ||
| return new UnmappedTerms(name, testInstance.order, testInstance.requiredSize, testInstance.minDocCount, | ||
| pipelineAggregators, metaData); | ||
| } | ||
|
|
||
| @Override | ||
| protected void assertReduced(InternalTerms<?, ?> reduced, List<InternalTerms<?, ?>> inputs) { | ||
| final int requiredSize = inputs.get(0).requiredSize; | ||
| Map<Object, Long> reducedCounts = toCounts(reduced.getBuckets().stream()); | ||
| Map<Object, Long> totalCounts = toCounts(inputs.stream().map(Terms::getBuckets).flatMap(List::stream)); | ||
|
|
||
| assertEquals(reducedCounts.size() == requiredSize, | ||
| totalCounts.size() >= requiredSize); | ||
|
|
||
| Map<Object, Long> expectedReducedCounts = new HashMap<>(totalCounts); | ||
| expectedReducedCounts.keySet().retainAll(reducedCounts.keySet()); | ||
| assertEquals(expectedReducedCounts, reducedCounts); | ||
|
|
||
| final long minFinalcount = reduced.getBuckets().isEmpty() | ||
| ? -1 | ||
| : reduced.getBuckets().get(reduced.getBuckets().size() - 1).getDocCount(); | ||
| Map<Object, Long> evictedTerms = new HashMap<>(totalCounts); | ||
| evictedTerms.keySet().removeAll(reducedCounts.keySet()); | ||
| Optional<Entry<Object, Long>> missingTerm = evictedTerms.entrySet().stream() | ||
| .filter(e -> e.getValue() > minFinalcount).findAny(); | ||
| if (missingTerm.isPresent()) { | ||
| fail("Missed term: " + missingTerm + " from " + reducedCounts); | ||
| } | ||
|
|
||
| final long reducedTotalDocCount = reduced.getSumOfOtherDocCounts() | ||
| + reduced.getBuckets().stream().mapToLong(Terms.Bucket::getDocCount).sum(); | ||
| final long expectedTotalDocCount = inputs.stream().map(Terms::getBuckets) | ||
| .flatMap(List::stream).mapToLong(Terms.Bucket::getDocCount).sum(); | ||
| assertEquals(expectedTotalDocCount, reducedTotalDocCount); | ||
| } | ||
|
|
||
| private static Map<Object, Long> toCounts(Stream<? extends Terms.Bucket> buckets) { | ||
| return buckets.collect(Collectors.toMap( | ||
| Terms.Bucket::getKey, | ||
| Terms.Bucket::getDocCount, | ||
| Long::sum)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs the traditional class comparison to make sure the cast doesn't blow up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The super.equals call already guarantees it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, fine then!