Skip to content

Commit ec5d7c0

Browse files
not-napoleonnot-napoleon
committed
(refactor) Extract Empty/Script/Missing ValuesSource behavior to an interface (elastic#48320)
This is a pure code rearrangement refactor. Logic for what specific ValuesSource instance to use for a given type (e.g. script or field) moved out of ValuesSourceConfig and into CoreValuesSourceType (previously just ValueSourceType; we extract an interface for future extensibility). ValueSourceConfig still selects which case to use, and then the ValuesSourceType instance knows how to construct the ValuesSource for that case. Conflicts: server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregationBuilder.java x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java
1 parent c4c8a7a commit ec5d7c0

File tree

42 files changed

+558
-403
lines changed

Some content is hidden

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

42 files changed

+558
-403
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@
2828
import org.elasticsearch.search.aggregations.AggregatorFactories;
2929
import org.elasticsearch.search.aggregations.AggregatorFactory;
3030
import org.elasticsearch.search.aggregations.support.ArrayValuesSourceAggregationBuilder;
31+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3132
import org.elasticsearch.search.aggregations.support.ValueType;
3233
import org.elasticsearch.search.aggregations.support.ValuesSource;
3334
import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric;
3435
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
35-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
3636

3737
import java.io.IOException;
3838
import java.util.Map;
@@ -44,7 +44,7 @@ public class MatrixStatsAggregationBuilder
4444
private MultiValueMode multiValueMode = MultiValueMode.AVG;
4545

4646
public MatrixStatsAggregationBuilder(String name) {
47-
super(name, ValuesSourceType.NUMERIC, ValueType.NUMERIC);
47+
super(name, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
4848
}
4949

5050
protected MatrixStatsAggregationBuilder(MatrixStatsAggregationBuilder clone,
@@ -62,7 +62,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu
6262
* Read from a stream.
6363
*/
6464
public MatrixStatsAggregationBuilder(StreamInput in) throws IOException {
65-
super(in, ValuesSourceType.NUMERIC, ValueType.NUMERIC);
65+
super(in, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
6666
}
6767

6868
@Override

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -266,16 +266,16 @@ public ValuesSourceConfig<VS> config(QueryShardContext queryShardContext, String
266266

267267
if (field == null) {
268268
if (script == null) {
269-
ValuesSourceConfig<VS> config = new ValuesSourceConfig<>(ValuesSourceType.ANY);
269+
ValuesSourceConfig<VS> config = new ValuesSourceConfig<>(CoreValuesSourceType.ANY);
270270
return config.format(resolveFormat(null, valueType));
271271
}
272272
ValuesSourceType valuesSourceType = valueType != null ? valueType.getValuesSourceType() : this.valuesSourceType;
273-
if (valuesSourceType == null || valuesSourceType == ValuesSourceType.ANY) {
273+
if (valuesSourceType == null || valuesSourceType == CoreValuesSourceType.ANY) {
274274
// the specific value source type is undefined, but for scripts,
275275
// we need to have a specific value source
276276
// type to know how to handle the script values, so we fallback
277277
// on Bytes
278-
valuesSourceType = ValuesSourceType.BYTES;
278+
valuesSourceType = CoreValuesSourceType.BYTES;
279279
}
280280
ValuesSourceConfig<VS> config = new ValuesSourceConfig<>(valuesSourceType);
281281
config.missing(missingMap.get(field));
@@ -294,13 +294,13 @@ public ValuesSourceConfig<VS> config(QueryShardContext queryShardContext, String
294294
IndexFieldData<?> indexFieldData = queryShardContext.getForField(fieldType);
295295

296296
ValuesSourceConfig<VS> config;
297-
if (valuesSourceType == ValuesSourceType.ANY) {
297+
if (valuesSourceType == CoreValuesSourceType.ANY) {
298298
if (indexFieldData instanceof IndexNumericFieldData) {
299-
config = new ValuesSourceConfig<>(ValuesSourceType.NUMERIC);
299+
config = new ValuesSourceConfig<>(CoreValuesSourceType.NUMERIC);
300300
} else if (indexFieldData instanceof IndexGeoPointFieldData) {
301-
config = new ValuesSourceConfig<>(ValuesSourceType.GEOPOINT);
301+
config = new ValuesSourceConfig<>(CoreValuesSourceType.GEOPOINT);
302302
} else {
303-
config = new ValuesSourceConfig<>(ValuesSourceType.BYTES);
303+
config = new ValuesSourceConfig<>(CoreValuesSourceType.BYTES);
304304
}
305305
} else {
306306
config = new ValuesSourceConfig<>(valuesSourceType);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,28 @@ public abstract class ArrayValuesSourceParser<VS extends ValuesSource> implement
3838
public abstract static class AnyValuesSourceParser extends ArrayValuesSourceParser<ValuesSource> {
3939

4040
protected AnyValuesSourceParser(boolean formattable) {
41-
super(formattable, ValuesSourceType.ANY, null);
41+
super(formattable, CoreValuesSourceType.ANY, null);
4242
}
4343
}
4444

4545
public abstract static class NumericValuesSourceParser extends ArrayValuesSourceParser<ValuesSource.Numeric> {
4646

4747
protected NumericValuesSourceParser(boolean formattable) {
48-
super(formattable, ValuesSourceType.NUMERIC, ValueType.NUMERIC);
48+
super(formattable, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
4949
}
5050
}
5151

5252
public abstract static class BytesValuesSourceParser extends ArrayValuesSourceParser<ValuesSource.Bytes> {
5353

5454
protected BytesValuesSourceParser(boolean formattable) {
55-
super(formattable, ValuesSourceType.BYTES, ValueType.STRING);
55+
super(formattable, CoreValuesSourceType.BYTES, ValueType.STRING);
5656
}
5757
}
5858

5959
public abstract static class GeoPointValuesSourceParser extends ArrayValuesSourceParser<ValuesSource.GeoPoint> {
6060

6161
protected GeoPointValuesSourceParser(boolean formattable) {
62-
super(formattable, ValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
62+
super(formattable, CoreValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
6363
}
6464
}
6565

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
import org.elasticsearch.search.aggregations.AggregationBuilder;
3434
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
3535
import org.elasticsearch.search.aggregations.AggregatorFactory;
36+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3637
import org.elasticsearch.search.aggregations.support.FieldContext;
3738
import org.elasticsearch.search.aggregations.support.ValueType;
3839
import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.WithOrdinals;
3940
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
4041
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
4142
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
42-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
4343

4444
import java.io.IOException;
4545
import java.util.Map;
@@ -61,7 +61,7 @@ public class ChildrenAggregationBuilder
6161
* the type of children documents
6262
*/
6363
public ChildrenAggregationBuilder(String name, String childType) {
64-
super(name, ValuesSourceType.BYTES, ValueType.STRING);
64+
super(name, CoreValuesSourceType.BYTES, ValueType.STRING);
6565
if (childType == null) {
6666
throw new IllegalArgumentException("[childType] must not be null: [" + name + "]");
6767
}
@@ -85,7 +85,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, O
8585
* Read from a stream.
8686
*/
8787
public ChildrenAggregationBuilder(StreamInput in) throws IOException {
88-
super(in, ValuesSourceType.BYTES, ValueType.STRING);
88+
super(in, CoreValuesSourceType.BYTES, ValueType.STRING);
8989
childType = in.readString();
9090
}
9191

@@ -105,7 +105,7 @@ protected ValuesSourceAggregatorFactory<WithOrdinals> innerBuild(QueryShardConte
105105

106106
@Override
107107
protected ValuesSourceConfig<WithOrdinals> resolveConfig(QueryShardContext queryShardContext) {
108-
ValuesSourceConfig<WithOrdinals> config = new ValuesSourceConfig<>(ValuesSourceType.BYTES);
108+
ValuesSourceConfig<WithOrdinals> config = new ValuesSourceConfig<>(CoreValuesSourceType.BYTES);
109109
joinFieldResolveConfig(queryShardContext, config);
110110
return config;
111111
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
import org.elasticsearch.search.aggregations.AggregationBuilder;
3434
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
3535
import org.elasticsearch.search.aggregations.AggregatorFactory;
36+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3637
import org.elasticsearch.search.aggregations.support.FieldContext;
3738
import org.elasticsearch.search.aggregations.support.ValueType;
3839
import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.WithOrdinals;
3940
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
4041
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
4142
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
42-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
4343

4444
import java.io.IOException;
4545
import java.util.Map;
@@ -61,7 +61,7 @@ public class ParentAggregationBuilder
6161
* the type of children documents
6262
*/
6363
public ParentAggregationBuilder(String name, String childType) {
64-
super(name, ValuesSourceType.BYTES, ValueType.STRING);
64+
super(name, CoreValuesSourceType.BYTES, ValueType.STRING);
6565
if (childType == null) {
6666
throw new IllegalArgumentException("[childType] must not be null: [" + name + "]");
6767
}
@@ -85,7 +85,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, O
8585
* Read from a stream.
8686
*/
8787
public ParentAggregationBuilder(StreamInput in) throws IOException {
88-
super(in, ValuesSourceType.BYTES, ValueType.STRING);
88+
super(in, CoreValuesSourceType.BYTES, ValueType.STRING);
8989
childType = in.readString();
9090
}
9191

@@ -105,7 +105,7 @@ protected ValuesSourceAggregatorFactory<WithOrdinals> innerBuild(QueryShardConte
105105

106106
@Override
107107
protected ValuesSourceConfig<WithOrdinals> resolveConfig(QueryShardContext queryShardContext) {
108-
ValuesSourceConfig<WithOrdinals> config = new ValuesSourceConfig<>(ValuesSourceType.BYTES);
108+
ValuesSourceConfig<WithOrdinals> config = new ValuesSourceConfig<>(CoreValuesSourceType.BYTES);
109109
joinFieldResolveConfig(queryShardContext, config);
110110
return config;
111111
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@
3131
import org.elasticsearch.search.aggregations.AggregatorFactory;
3232
import org.elasticsearch.search.aggregations.bucket.BucketUtils;
3333
import org.elasticsearch.search.aggregations.bucket.MultiBucketAggregationBuilder;
34+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3435
import org.elasticsearch.search.aggregations.support.ValueType;
3536
import org.elasticsearch.search.aggregations.support.ValuesSource;
3637
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
3738
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
3839
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
3940
import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
40-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
4141

4242
import java.io.IOException;
4343
import java.util.Map;
@@ -70,7 +70,7 @@ public static ObjectParser<GeoGridAggregationBuilder, Void> createParser(String
7070
}
7171

7272
public GeoGridAggregationBuilder(String name) {
73-
super(name, ValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
73+
super(name, CoreValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
7474
}
7575

7676
protected GeoGridAggregationBuilder(GeoGridAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metaData) {
@@ -85,7 +85,7 @@ protected GeoGridAggregationBuilder(GeoGridAggregationBuilder clone, Builder fac
8585
* Read from a stream.
8686
*/
8787
public GeoGridAggregationBuilder(StreamInput in) throws IOException {
88-
super(in, ValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
88+
super(in, CoreValuesSourceType.GEOPOINT, ValueType.GEOPOINT);
8989
precision = in.readVInt();
9090
requiredSize = in.readVInt();
9191
shardSize = in.readVInt();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@
3434
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
3535
import org.elasticsearch.search.aggregations.AggregatorFactory;
3636
import org.elasticsearch.search.aggregations.MultiBucketConsumerService;
37+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3738
import org.elasticsearch.search.aggregations.support.ValueType;
3839
import org.elasticsearch.search.aggregations.support.ValuesSource;
3940
import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric;
4041
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
4142
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
4243
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
4344
import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
44-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
4545

4646
import java.io.IOException;
4747
import java.time.ZoneId;
@@ -134,12 +134,12 @@ public AutoDateHistogramAggregationBuilder setMinimumIntervalExpression(String m
134134

135135
/** Create a new builder with the given name. */
136136
public AutoDateHistogramAggregationBuilder(String name) {
137-
super(name, ValuesSourceType.NUMERIC, ValueType.DATE);
137+
super(name, CoreValuesSourceType.NUMERIC, ValueType.DATE);
138138
}
139139

140140
/** Read from a stream, for internal use only. */
141141
public AutoDateHistogramAggregationBuilder(StreamInput in) throws IOException {
142-
super(in, ValuesSourceType.NUMERIC, ValueType.DATE);
142+
super(in, CoreValuesSourceType.NUMERIC, ValueType.DATE);
143143
numBuckets = in.readVInt();
144144
if (in.getVersion().onOrAfter(Version.V_7_3_0)) {
145145
minimumIntervalExpression = in.readOptionalString();

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.elasticsearch.search.aggregations.InternalOrder;
4646
import org.elasticsearch.search.aggregations.InternalOrder.CompoundOrder;
4747
import org.elasticsearch.search.aggregations.bucket.MultiBucketAggregationBuilder;
48+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
4849
import org.elasticsearch.search.aggregations.support.ValueType;
4950
import org.elasticsearch.search.aggregations.support.ValuesSource;
5051
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
@@ -136,7 +137,7 @@ public static DateHistogramAggregationBuilder parse(String aggregationName, XCon
136137

137138
/** Create a new builder with the given name. */
138139
public DateHistogramAggregationBuilder(String name) {
139-
super(name, ValuesSourceType.ANY, ValueType.DATE);
140+
super(name, CoreValuesSourceType.ANY, ValueType.DATE);
140141
}
141142

142143
protected DateHistogramAggregationBuilder(DateHistogramAggregationBuilder clone,
@@ -157,7 +158,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, O
157158

158159
/** Read from a stream, for internal use only. */
159160
public DateHistogramAggregationBuilder(StreamInput in) throws IOException {
160-
super(in, ValuesSourceType.ANY, ValueType.DATE);
161+
super(in, CoreValuesSourceType.ANY, ValueType.DATE);
161162
order = InternalOrder.Streams.readHistogramOrder(in, true);
162163
keyed = in.readBoolean();
163164
minDocCount = in.readVLong();
@@ -169,7 +170,7 @@ public DateHistogramAggregationBuilder(StreamInput in) throws IOException {
169170
@Override
170171
protected ValuesSourceType resolveScriptAny(Script script) {
171172
// TODO: No idea how we'd support Range scripts here.
172-
return ValuesSourceType.NUMERIC;
173+
return CoreValuesSourceType.NUMERIC;
173174
}
174175

175176

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.search.aggregations.InternalOrder;
3535
import org.elasticsearch.search.aggregations.InternalOrder.CompoundOrder;
3636
import org.elasticsearch.search.aggregations.bucket.MultiBucketAggregationBuilder;
37+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3738
import org.elasticsearch.search.aggregations.support.ValueType;
3839
import org.elasticsearch.search.aggregations.support.ValuesSource;
3940
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
@@ -99,12 +100,12 @@ public static HistogramAggregationBuilder parse(String aggregationName, XContent
99100
@Override
100101
protected ValuesSourceType resolveScriptAny(Script script) {
101102
// TODO: No idea how we'd support Range scripts here.
102-
return ValuesSourceType.NUMERIC;
103+
return CoreValuesSourceType.NUMERIC;
103104
}
104105

105106
/** Create a new builder with the given name. */
106107
public HistogramAggregationBuilder(String name) {
107-
super(name, ValuesSourceType.ANY, ValueType.NUMERIC);
108+
super(name, CoreValuesSourceType.ANY, ValueType.NUMERIC);
108109
}
109110

110111
protected HistogramAggregationBuilder(HistogramAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metaData) {
@@ -125,7 +126,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, O
125126

126127
/** Read from a stream, for internal use only. */
127128
public HistogramAggregationBuilder(StreamInput in) throws IOException {
128-
super(in, ValuesSourceType.ANY, ValueType.NUMERIC);
129+
super(in, CoreValuesSourceType.ANY, ValueType.NUMERIC);
129130
order = InternalOrder.Streams.readHistogramOrder(in, true);
130131
keyed = in.readBoolean();
131132
minDocCount = in.readVLong();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@
2929
import org.elasticsearch.search.aggregations.AggregationBuilder;
3030
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
3131
import org.elasticsearch.search.aggregations.AggregatorFactory;
32+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3233
import org.elasticsearch.search.aggregations.support.ValueType;
3334
import org.elasticsearch.search.aggregations.support.ValuesSource;
3435
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
3536
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
3637
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
3738
import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
38-
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
3939

4040
import java.io.IOException;
4141
import java.util.Map;
@@ -54,7 +54,7 @@ public static MissingAggregationBuilder parse(String aggregationName, XContentPa
5454
}
5555

5656
public MissingAggregationBuilder(String name, ValueType targetValueType) {
57-
super(name, ValuesSourceType.ANY, targetValueType);
57+
super(name, CoreValuesSourceType.ANY, targetValueType);
5858
}
5959

6060
protected MissingAggregationBuilder(MissingAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metaData) {
@@ -70,7 +70,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, O
7070
* Read from a stream.
7171
*/
7272
public MissingAggregationBuilder(StreamInput in) throws IOException {
73-
super(in, ValuesSourceType.ANY);
73+
super(in, CoreValuesSourceType.ANY);
7474
}
7575

7676
@Override

0 commit comments

Comments
 (0)