Skip to content

Commit 38ac66a

Browse files
csoulioscsoulios
committed
Refactor aggregation base classes to remove doEquals() and doHashCode() (elastic#43214)
A number of the aggregation base classes have an abstract doEquals() and doHashCode() (e.g. InternalAggregation.java, AbstractPipelineAggregationBuilder.java). Theoretically this is so the sub-classes can add to the equals/hashCode and don't need to worry about calling super.equals(). In practice, it's mostly just confusing/inconsistent. And if there are more than two levels, we end up with situations like InternalMappedSignificantTerms which has to call super.doEquals() which defeats the point of having these overridable methods. This PR removes the do versions and just use equals/hashCode ensuring the super when necessary. This PR is part of elastic#41713 refactoring meta * Refactored all subclasses of InternalAggregation to remove doEquals() and doHashCode() * Refactored all subclasses of AbstractPipelineAggregationBuilder to remove doEquals() and doHashCode() * Refactored all subclasses of AbstractAggregationBuilder and CompositeValuesSourceBuilder to remove doEquals() and doHashCode() Backport from v8.0.0
1 parent 2f17340 commit 38ac66a

File tree

105 files changed

+647
-692
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

105 files changed

+647
-692
lines changed

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/stats/InternalMatrixStats.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,16 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
256256
}
257257

258258
@Override
259-
protected int doHashCode() {
260-
return Objects.hash(stats, results);
259+
public int hashCode() {
260+
return Objects.hash(super.hashCode(), stats, results);
261261
}
262262

263263
@Override
264-
protected boolean doEquals(Object obj) {
264+
public boolean equals(Object obj) {
265+
if (this == obj) return true;
266+
if (obj == null || getClass() != obj.getClass()) return false;
267+
if (super.equals(obj) == false) return false;
268+
265269
InternalMatrixStats other = (InternalMatrixStats) obj;
266270
return Objects.equals(this.stats, other.stats) &&
267271
Objects.equals(this.results, other.results);

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/stats/MatrixStatsAggregationBuilder.java

-10
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,6 @@ public XContentBuilder doXContentBody(XContentBuilder builder, ToXContent.Params
9191
return builder;
9292
}
9393

94-
@Override
95-
protected int innerHashCode() {
96-
return 0;
97-
}
98-
99-
@Override
100-
protected boolean innerEquals(Object obj) {
101-
return true;
102-
}
103-
10494
@Override
10595
public String getType() {
10696
return NAME;

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/ArrayValuesSourceAggregationBuilder.java

+12-21
Original file line numberDiff line numberDiff line change
@@ -354,30 +354,21 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa
354354
protected abstract XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException;
355355

356356
@Override
357-
protected final int doHashCode() {
358-
return Objects.hash(fields, format, missing, targetValueType, valueType, valuesSourceType,
359-
innerHashCode());
357+
public int hashCode() {
358+
return Objects.hash(super.hashCode(), fields, format, missing, targetValueType, valueType, valuesSourceType);
360359
}
361360

362-
protected abstract int innerHashCode();
363-
364361
@Override
365-
protected final boolean doEquals(Object obj) {
362+
public boolean equals(Object obj) {
363+
if (this == obj) return true;
364+
if (obj == null || getClass() != obj.getClass()) return false;
365+
if (super.equals(obj) == false) return false;
366366
ArrayValuesSourceAggregationBuilder<?, ?> other = (ArrayValuesSourceAggregationBuilder<?, ?>) obj;
367-
if (!Objects.equals(fields, other.fields))
368-
return false;
369-
if (!Objects.equals(format, other.format))
370-
return false;
371-
if (!Objects.equals(missing, other.missing))
372-
return false;
373-
if (!Objects.equals(targetValueType, other.targetValueType))
374-
return false;
375-
if (!Objects.equals(valueType, other.valueType))
376-
return false;
377-
if (!Objects.equals(valuesSourceType, other.valuesSourceType))
378-
return false;
379-
return innerEquals(obj);
367+
return Objects.equals(fields, other.fields)
368+
&& Objects.equals(format, other.format)
369+
&& Objects.equals(missing, other.missing)
370+
&& Objects.equals(targetValueType, other.targetValueType)
371+
&& Objects.equals(valueType, other.valueType)
372+
&& Objects.equals(valuesSourceType, other.valuesSourceType);
380373
}
381-
382-
protected abstract boolean innerEquals(Object obj);
383374
}

‎modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,15 @@ public static ChildrenAggregationBuilder parse(String aggregationName, XContentP
159159
}
160160

161161
@Override
162-
protected int innerHashCode() {
163-
return Objects.hash(childType);
162+
public int hashCode() {
163+
return Objects.hash(super.hashCode(), childType);
164164
}
165165

166166
@Override
167-
protected boolean innerEquals(Object obj) {
167+
public boolean equals(Object obj) {
168+
if (this == obj) return true;
169+
if (obj == null || getClass() != obj.getClass()) return false;
170+
if (super.equals(obj) == false) return false;
168171
ChildrenAggregationBuilder other = (ChildrenAggregationBuilder) obj;
169172
return Objects.equals(childType, other.childType);
170173
}

‎modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentAggregationBuilder.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,15 @@ public static ParentAggregationBuilder parse(String aggregationName, XContentPar
159159
}
160160

161161
@Override
162-
protected int innerHashCode() {
163-
return Objects.hash(childType);
162+
public int hashCode() {
163+
return Objects.hash(super.hashCode(), childType);
164164
}
165165

166166
@Override
167-
protected boolean innerEquals(Object obj) {
167+
public boolean equals(Object obj) {
168+
if (this == obj) return true;
169+
if (obj == null || getClass() != obj.getClass()) return false;
170+
if (super.equals(obj) == false) return false;
168171
ParentAggregationBuilder other = (ParentAggregationBuilder) obj;
169172
return Objects.equals(childType, other.childType);
170173
}

‎server/src/main/java/org/elasticsearch/search/aggregations/AbstractAggregationBuilder.java

+7-18
Original file line numberDiff line numberDiff line change
@@ -164,28 +164,17 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)
164164

165165
@Override
166166
public int hashCode() {
167-
return Objects.hash(factoriesBuilder, metaData, name, doHashCode());
167+
return Objects.hash(factoriesBuilder, metaData, name);
168168
}
169169

170-
protected abstract int doHashCode();
171-
172170
@Override
173171
public boolean equals(Object obj) {
174-
if (obj == null)
175-
return false;
176-
if (getClass() != obj.getClass())
177-
return false;
178-
@SuppressWarnings("unchecked")
172+
if (this == obj) return true;
173+
if (obj == null || getClass() != obj.getClass()) return false;
179174
AbstractAggregationBuilder<AB> other = (AbstractAggregationBuilder<AB>) obj;
180-
if (!Objects.equals(name, other.name))
181-
return false;
182-
if (!Objects.equals(metaData, other.metaData))
183-
return false;
184-
if (!Objects.equals(factoriesBuilder, other.factoriesBuilder))
185-
return false;
186-
return doEquals(obj);
187-
}
188-
189-
protected abstract boolean doEquals(Object obj);
190175

176+
return Objects.equals(name, other.name)
177+
&& Objects.equals(metaData, other.metaData)
178+
&& Objects.equals(factoriesBuilder, other.factoriesBuilder);
179+
}
191180
}

‎server/src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java

+5-23
Original file line numberDiff line numberDiff line change
@@ -218,40 +218,22 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)
218218

219219
@Override
220220
public int hashCode() {
221-
return Objects.hash(name, metaData, pipelineAggregators, doHashCode());
221+
return Objects.hash(name, metaData, pipelineAggregators);
222222
}
223223

224-
/**
225-
* Opportunity for subclasses to the {@link #hashCode()} for this
226-
* class.
227-
**/
228-
protected abstract int doHashCode();
229-
230224
@Override
231225
public boolean equals(Object obj) {
232-
if (obj == null) {
233-
return false;
234-
}
235-
if (obj.getClass() != getClass()) {
226+
if (obj == null || getClass() != obj.getClass()) {
236227
return false;
237228
}
229+
if (obj == this) { return true; }
230+
238231
InternalAggregation other = (InternalAggregation) obj;
239232
return Objects.equals(name, other.name) &&
240233
Objects.equals(pipelineAggregators, other.pipelineAggregators) &&
241-
Objects.equals(metaData, other.metaData) &&
242-
doEquals(obj);
234+
Objects.equals(metaData, other.metaData);
243235
}
244236

245-
/**
246-
* Opportunity for subclasses to add criteria to the {@link #equals(Object)}
247-
* method for this class.
248-
*
249-
* This method can safely cast <code>obj</code> to the subclass since the
250-
* {@link #equals(Object)} method checks that <code>obj</code> is the same
251-
* class as <code>this</code>
252-
*/
253-
protected abstract boolean doEquals(Object obj);
254-
255237
@Override
256238
public String toString() {
257239
return Strings.toString(this);

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,18 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th
137137
}
138138

139139
@Override
140-
protected boolean doEquals(Object obj) {
140+
public boolean equals(Object obj) {
141+
if (this == obj) return true;
142+
if (obj == null || getClass() != obj.getClass()) return false;
143+
if (super.equals(obj) == false) return false;
144+
141145
InternalSingleBucketAggregation other = (InternalSingleBucketAggregation) obj;
142146
return Objects.equals(docCount, other.docCount) &&
143147
Objects.equals(aggregations, other.aggregations);
144148
}
145149

146150
@Override
147-
protected int doHashCode() {
148-
return Objects.hash(docCount, aggregations);
151+
public int hashCode() {
152+
return Objects.hash(super.hashCode(), docCount, aggregations);
149153
}
150154
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,15 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
230230
}
231231

232232
@Override
233-
protected int doHashCode() {
234-
return Objects.hash(filters, separator);
233+
public int hashCode() {
234+
return Objects.hash(super.hashCode(), filters, separator);
235235
}
236236

237237
@Override
238-
protected boolean doEquals(Object obj) {
238+
public boolean equals(Object obj) {
239+
if (this == obj) return true;
240+
if (obj == null || getClass() != obj.getClass()) return false;
241+
if (super.equals(obj) == false) return false;
239242
AdjacencyMatrixAggregationBuilder other = (AdjacencyMatrixAggregationBuilder) obj;
240243
return Objects.equals(filters, other.filters) && Objects.equals(separator, other.separator);
241244
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/InternalAdjacencyMatrix.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,16 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th
239239
}
240240

241241
@Override
242-
protected int doHashCode() {
243-
return Objects.hash(buckets);
242+
public int hashCode() {
243+
return Objects.hash(super.hashCode(), buckets);
244244
}
245245

246246
@Override
247-
protected boolean doEquals(Object obj) {
247+
public boolean equals(Object obj) {
248+
if (this == obj) return true;
249+
if (obj == null || getClass() != obj.getClass()) return false;
250+
if (super.equals(obj) == false) return false;
251+
248252
InternalAdjacencyMatrix that = (InternalAdjacencyMatrix) obj;
249253
return Objects.equals(buckets, that.buckets);
250254
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,15 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
256256
}
257257

258258
@Override
259-
protected int doHashCode() {
260-
return Objects.hash(sources, size, after);
259+
public int hashCode() {
260+
return Objects.hash(super.hashCode(), sources, size, after);
261261
}
262262

263263
@Override
264-
protected boolean doEquals(Object obj) {
264+
public boolean equals(Object obj) {
265+
if (this == obj) return true;
266+
if (obj == null || getClass() != obj.getClass()) return false;
267+
if (super.equals(obj) == false) return false;
265268
CompositeAggregationBuilder other = (CompositeAggregationBuilder) obj;
266269
return size == other.size &&
267270
Objects.equals(sources, other.sources) &&

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java

+3-8
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,10 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)
138138
}
139139

140140
@Override
141-
public final int hashCode() {
142-
return Objects.hash(field, missingBucket, script, valueType, order, format, innerHashCode());
141+
public int hashCode() {
142+
return Objects.hash(field, missingBucket, script, valueType, order, format);
143143
}
144144

145-
protected abstract int innerHashCode();
146-
147145
@Override
148146
public boolean equals(Object o) {
149147
if (this == o) return true;
@@ -156,12 +154,9 @@ public boolean equals(Object o) {
156154
Objects.equals(valueType, that.valueType()) &&
157155
Objects.equals(missingBucket, that.missingBucket()) &&
158156
Objects.equals(order, that.order()) &&
159-
Objects.equals(format, that.format()) &&
160-
innerEquals(that);
157+
Objects.equals(format, that.format());
161158
}
162159

163-
protected abstract boolean innerEquals(AB builder);
164-
165160
public String name() {
166161
return name;
167162
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,16 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE
9797
}
9898

9999
@Override
100-
protected int innerHashCode() {
101-
return Objects.hash(dateHistogramInterval, timeZone);
100+
public int hashCode() {
101+
return Objects.hash(super.hashCode(), dateHistogramInterval, timeZone);
102102
}
103103

104104
@Override
105-
protected boolean innerEquals(DateHistogramValuesSourceBuilder other) {
105+
public boolean equals(Object obj) {
106+
if (this == obj) return true;
107+
if (obj == null || getClass() != obj.getClass()) return false;
108+
if (super.equals(obj) == false) return false;
109+
DateHistogramValuesSourceBuilder other = (DateHistogramValuesSourceBuilder) obj;
106110
return Objects.equals(dateHistogramInterval, other.dateHistogramInterval)
107111
&& Objects.equals(timeZone, other.timeZone);
108112
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/HistogramValuesSourceBuilder.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,16 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE
7373
}
7474

7575
@Override
76-
protected int innerHashCode() {
77-
return Objects.hash(interval);
76+
public int hashCode() {
77+
return Objects.hash(super.hashCode(), interval);
7878
}
7979

8080
@Override
81-
protected boolean innerEquals(HistogramValuesSourceBuilder other) {
81+
public boolean equals(Object obj) {
82+
if (this == obj) return true;
83+
if (obj == null || getClass() != obj.getClass()) return false;
84+
if (super.equals(obj) == false) return false;
85+
HistogramValuesSourceBuilder other = (HistogramValuesSourceBuilder) obj;
8286
return Objects.equals(interval, other.interval);
8387
}
8488

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/InternalComposite.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,11 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
195195
}
196196

197197
@Override
198-
protected boolean doEquals(Object obj) {
198+
public boolean equals(Object obj) {
199+
if (this == obj) return true;
200+
if (obj == null || getClass() != obj.getClass()) return false;
201+
if (super.equals(obj) == false) return false;
202+
199203
InternalComposite that = (InternalComposite) obj;
200204
return Objects.equals(size, that.size) &&
201205
Objects.equals(buckets, that.buckets) &&
@@ -204,8 +208,8 @@ protected boolean doEquals(Object obj) {
204208
}
205209

206210
@Override
207-
protected int doHashCode() {
208-
return Objects.hash(size, buckets, afterKey, Arrays.hashCode(reverseMuls));
211+
public int hashCode() {
212+
return Objects.hash(super.hashCode(), size, buckets, afterKey, Arrays.hashCode(reverseMuls));
209213
}
210214

211215
private static class BucketIterator implements Comparable<BucketIterator> {

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/TermsValuesSourceBuilder.java

-10
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,6 @@ protected void innerWriteTo(StreamOutput out) throws IOException {}
6464
@Override
6565
protected void doXContentBody(XContentBuilder builder, Params params) throws IOException {}
6666

67-
@Override
68-
protected int innerHashCode() {
69-
return 0;
70-
}
71-
72-
@Override
73-
protected boolean innerEquals(TermsValuesSourceBuilder builder) {
74-
return true;
75-
}
76-
7767
@Override
7868
public String type() {
7969
return TYPE;

0 commit comments

Comments
 (0)