Skip to content

Commit 85a160f

Browse files
not-napoleonnot-napoleon
authored
Make ValuesSourceRegistry immutable after initilization (#55493)
1 parent 0645341 commit 85a160f

File tree

64 files changed

+488
-444
lines changed

Some content is hidden

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

64 files changed

+488
-444
lines changed

‎server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ default List<AggregationSpec> getAggregations() {
117117
/**
118118
* Allows plugins to register new aggregations using aggregation names that are already defined
119119
* in Core, as long as the new aggregations target different ValuesSourceTypes
120+
* @return A list of the new registrar functions
120121
*/
121-
default List<Consumer<ValuesSourceRegistry>> getBareAggregatorRegistrar() {
122+
default List<Consumer<ValuesSourceRegistry.Builder>> getAggregationExtentions() {
122123
return emptyList();
123124
}
124125
/**
@@ -260,7 +261,7 @@ public QuerySpec(String name, Writeable.Reader<T> reader, QueryParser<T> parser)
260261
*/
261262
class AggregationSpec extends SearchExtensionSpec<AggregationBuilder, ContextParser<String, ? extends AggregationBuilder>> {
262263
private final Map<String, Writeable.Reader<? extends InternalAggregation>> resultReaders = new TreeMap<>();
263-
private Consumer<ValuesSourceRegistry> aggregatorRegistrar;
264+
private Consumer<ValuesSourceRegistry.Builder> aggregatorRegistrar;
264265

265266
/**
266267
* Specification for an {@link Aggregation}.
@@ -348,15 +349,15 @@ public Map<String, Writeable.Reader<? extends InternalAggregation>> getResultRea
348349
* Get the function to register the {@link org.elasticsearch.search.aggregations.support.ValuesSource} to aggregator mappings for
349350
* this aggregation
350351
*/
351-
public Consumer<ValuesSourceRegistry> getAggregatorRegistrar() {
352+
public Consumer<ValuesSourceRegistry.Builder> getAggregatorRegistrar() {
352353
return aggregatorRegistrar;
353354
}
354355

355356
/**
356357
* Set the function to register the {@link org.elasticsearch.search.aggregations.support.ValuesSource} to aggregator mappings for
357358
* this aggregation
358359
*/
359-
public AggregationSpec setAggregatorRegistrar(Consumer<ValuesSourceRegistry> aggregatorRegistrar) {
360+
public AggregationSpec setAggregatorRegistrar(Consumer<ValuesSourceRegistry.Builder> aggregatorRegistrar) {
360361
this.aggregatorRegistrar = aggregatorRegistrar;
361362
return this;
362363
}

‎server/src/main/java/org/elasticsearch/search/SearchModule.java

Lines changed: 52 additions & 53 deletions
Large diffs are not rendered by default.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public GeoHashGridAggregationBuilder(StreamInput in) throws IOException {
5353
super(in);
5454
}
5555

56-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
57-
GeoHashGridAggregatorFactory.registerAggregators(valuesSourceRegistry);
56+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
57+
GeoHashGridAggregatorFactory.registerAggregators(builder);
5858
}
5959

6060
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ protected Aggregator doCreateInternal(final ValuesSource valuesSource,
9292
requiredSize, shardSize, searchContext, parent, metadata);
9393
}
9494

95-
static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
96-
valuesSourceRegistry.register(GeoHashGridAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT,
95+
static void registerAggregators(ValuesSourceRegistry.Builder builder) {
96+
builder.register(GeoHashGridAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT,
9797
(GeoGridAggregatorSupplier) (name, factories, valuesSource, precision, geoBoundingBox, requiredSize, shardSize,
9898
aggregationContext, parent, metadata) -> {
9999
CellIdSource cellIdSource = new CellIdSource((ValuesSource.GeoPoint) valuesSource, precision, geoBoundingBox,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ public GeoTileGridAggregationBuilder(StreamInput in) throws IOException {
5252
super(in);
5353
}
5454

55-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
56-
GeoTileGridAggregatorFactory.registerAggregators(valuesSourceRegistry);
55+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
56+
GeoTileGridAggregatorFactory.registerAggregators(builder);
5757
}
5858

5959
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ protected Aggregator doCreateInternal(final ValuesSource valuesSource,
9090
requiredSize, shardSize, searchContext, parent, metadata);
9191
}
9292

93-
static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
94-
valuesSourceRegistry.register(GeoTileGridAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT,
93+
static void registerAggregators(ValuesSourceRegistry.Builder builder) {
94+
builder.register(GeoTileGridAggregationBuilder.NAME, CoreValuesSourceType.GEOPOINT,
9595
(GeoGridAggregatorSupplier) (name, factories, valuesSource, precision, geoBoundingBox, requiredSize, shardSize,
9696
aggregationContext, parent, metadata) -> {
9797
CellIdSource cellIdSource = new CellIdSource((ValuesSource.GeoPoint) valuesSource, precision, geoBoundingBox,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import org.elasticsearch.index.mapper.MappedFieldType.Relation;
3838
import org.elasticsearch.index.query.QueryShardContext;
3939
import org.elasticsearch.search.aggregations.AggregationBuilder;
40-
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
40+
import org.elasticsearch.search.aggregations.AggregatorFactories;
4141
import org.elasticsearch.search.aggregations.AggregatorFactory;
4242
import org.elasticsearch.search.aggregations.BucketOrder;
4343
import org.elasticsearch.search.aggregations.InternalOrder;
@@ -111,8 +111,8 @@ public class DateHistogramAggregationBuilder extends ValuesSourceAggregationBuil
111111
Histogram.ORDER_FIELD);
112112
}
113113

114-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
115-
DateHistogramAggregatorFactory.registerAggregators(valuesSourceRegistry);
114+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
115+
DateHistogramAggregatorFactory.registerAggregators(builder);
116116
}
117117

118118
private DateIntervalWrapper dateHistogramInterval = new DateIntervalWrapper();
@@ -128,7 +128,7 @@ public DateHistogramAggregationBuilder(String name) {
128128
}
129129

130130
protected DateHistogramAggregationBuilder(DateHistogramAggregationBuilder clone,
131-
Builder factoriesBuilder, Map<String, Object> metadata) {
131+
AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metadata) {
132132
super(clone, factoriesBuilder, metadata);
133133
this.dateHistogramInterval = clone.dateHistogramInterval;
134134
this.offset = clone.offset;
@@ -139,7 +139,7 @@ protected DateHistogramAggregationBuilder(DateHistogramAggregationBuilder clone,
139139
}
140140

141141
@Override
142-
protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, Object> metadata) {
142+
protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metadata) {
143143
return new DateHistogramAggregationBuilder(this, factoriesBuilder, metadata);
144144
}
145145

@@ -520,7 +520,7 @@ ZoneId rewriteTimeZone(QueryShardContext context) throws IOException {
520520
protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext,
521521
ValuesSourceConfig config,
522522
AggregatorFactory parent,
523-
Builder subFactoriesBuilder) throws IOException {
523+
AggregatorFactories.Builder subFactoriesBuilder) throws IOException {
524524
final ZoneId tz = timeZone();
525525
final Rounding rounding = dateHistogramInterval.createRounding(tz, offset);
526526
final ZoneId rewrittenTimeZone = rewriteTimeZone(queryShardContext);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@
4343

4444
public final class DateHistogramAggregatorFactory extends ValuesSourceAggregatorFactory {
4545

46-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
47-
valuesSourceRegistry.register(DateHistogramAggregationBuilder.NAME,
46+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
47+
builder.register(DateHistogramAggregationBuilder.NAME,
4848
List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN),
4949
(DateHistogramAggregationSupplier) (String name,
5050
AggregatorFactories factories,
@@ -62,7 +62,7 @@ public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry
6262
factories, rounding, shardRounding, order, keyed, minDocCount, extendedBounds, (ValuesSource.Numeric) valuesSource,
6363
formatter, aggregationContext, parent, metadata));
6464

65-
valuesSourceRegistry.register(DateHistogramAggregationBuilder.NAME,
65+
builder.register(DateHistogramAggregationBuilder.NAME,
6666
CoreValuesSourceType.RANGE,
6767
(DateHistogramAggregationSupplier) (String name,
6868
AggregatorFactories factories,

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.index.query.QueryShardContext;
2828
import org.elasticsearch.search.aggregations.AggregationBuilder;
29-
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
29+
import org.elasticsearch.search.aggregations.AggregatorFactories;
3030
import org.elasticsearch.search.aggregations.AggregatorFactory;
3131
import org.elasticsearch.search.aggregations.BucketOrder;
3232
import org.elasticsearch.search.aggregations.InternalOrder;
@@ -79,8 +79,8 @@ public class HistogramAggregationBuilder extends ValuesSourceAggregationBuilder<
7979
Histogram.ORDER_FIELD);
8080
}
8181

82-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
83-
HistogramAggregatorFactory.registerAggregators(valuesSourceRegistry);
82+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
83+
HistogramAggregatorFactory.registerAggregators(builder);
8484
}
8585

8686
private double interval;
@@ -101,7 +101,9 @@ public HistogramAggregationBuilder(String name) {
101101
super(name);
102102
}
103103

104-
protected HistogramAggregationBuilder(HistogramAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metadata) {
104+
protected HistogramAggregationBuilder(HistogramAggregationBuilder clone,
105+
AggregatorFactories.Builder factoriesBuilder,
106+
Map<String, Object> metadata) {
105107
super(clone, factoriesBuilder, metadata);
106108
this.interval = clone.interval;
107109
this.offset = clone.offset;
@@ -113,7 +115,7 @@ protected HistogramAggregationBuilder(HistogramAggregationBuilder clone, Builder
113115
}
114116

115117
@Override
116-
protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, Object> metadata) {
118+
protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metadata) {
117119
return new HistogramAggregationBuilder(this, factoriesBuilder, metadata);
118120
}
119121

@@ -304,7 +306,7 @@ public String getType() {
304306
protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext,
305307
ValuesSourceConfig config,
306308
AggregatorFactory parent,
307-
Builder subFactoriesBuilder) throws IOException {
309+
AggregatorFactories.Builder subFactoriesBuilder) throws IOException {
308310
return new HistogramAggregatorFactory(name, config, interval, offset, order, keyed, minDocCount, minBound, maxBound,
309311
queryShardContext, parent, subFactoriesBuilder, metadata);
310312
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,8 @@ public final class HistogramAggregatorFactory extends ValuesSourceAggregatorFact
5050
private final long minDocCount;
5151
private final double minBound, maxBound;
5252

53-
// TODO: Registration should happen on the actual aggregator classes, but I don't want to set up the whole dynamic loading thing yet
54-
static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
55-
valuesSourceRegistry.register(HistogramAggregationBuilder.NAME, CoreValuesSourceType.RANGE,
53+
static void registerAggregators(ValuesSourceRegistry.Builder builder) {
54+
builder.register(HistogramAggregationBuilder.NAME, CoreValuesSourceType.RANGE,
5655
new HistogramAggregatorSupplier() {
5756
@Override
5857
public Aggregator build(String name, AggregatorFactories factories, double interval, double offset,
@@ -70,7 +69,7 @@ public Aggregator build(String name, AggregatorFactories factories, double inter
7069
}
7170
);
7271

73-
valuesSourceRegistry.register(HistogramAggregationBuilder.NAME,
72+
builder.register(HistogramAggregationBuilder.NAME,
7473
List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN),
7574
new HistogramAggregatorSupplier() {
7675
@Override

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.index.query.QueryShardContext;
2828
import org.elasticsearch.search.aggregations.AggregationBuilder;
29-
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
29+
import org.elasticsearch.search.aggregations.AggregatorFactories;
3030
import org.elasticsearch.search.aggregations.AggregatorFactory;
3131
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3232
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
@@ -47,15 +47,17 @@ public class MissingAggregationBuilder extends ValuesSourceAggregationBuilder<Mi
4747
ValuesSourceAggregationBuilder.declareFields(PARSER, true, true, false);
4848
}
4949

50-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
51-
MissingAggregatorFactory.registerAggregators(valuesSourceRegistry);
50+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
51+
MissingAggregatorFactory.registerAggregators(builder);
5252
}
5353

5454
public MissingAggregationBuilder(String name) {
5555
super(name);
5656
}
5757

58-
protected MissingAggregationBuilder(MissingAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metadata) {
58+
protected MissingAggregationBuilder(MissingAggregationBuilder clone,
59+
AggregatorFactories.Builder factoriesBuilder,
60+
Map<String, Object> metadata) {
5961
super(clone, factoriesBuilder, metadata);
6062
}
6163

@@ -65,7 +67,7 @@ protected ValuesSourceType defaultValueSourceType() {
6567
}
6668

6769
@Override
68-
protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, Object> metadata) {
70+
protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metadata) {
6971
return new MissingAggregationBuilder(this, factoriesBuilder, metadata);
7072
}
7173

@@ -94,7 +96,7 @@ public BucketCardinality bucketCardinality() {
9496
protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext,
9597
ValuesSourceConfig config,
9698
AggregatorFactory parent,
97-
Builder subFactoriesBuilder) throws IOException {
99+
AggregatorFactories.Builder subFactoriesBuilder) throws IOException {
98100
return new MissingAggregatorFactory(name, config, queryShardContext, parent, subFactoriesBuilder, metadata);
99101
}
100102

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838

3939
public class MissingAggregatorFactory extends ValuesSourceAggregatorFactory {
4040

41-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
42-
valuesSourceRegistry.register(
41+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
42+
builder.register(
4343
MissingAggregationBuilder.NAME,
4444
List.of(
4545
CoreValuesSourceType.NUMERIC,

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ public class AbstractRangeAggregatorFactory<R extends Range> extends ValuesSourc
4747
private final boolean keyed;
4848
private final String aggregationTypeName;
4949

50-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry, String aggregationName) {
51-
valuesSourceRegistry.register(aggregationName,
50+
public static void registerAggregators(ValuesSourceRegistry.Builder builder,
51+
String aggregationName) {
52+
builder.register(aggregationName,
5253
List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN),
5354
new RangeAggregatorSupplier() {
5455
@Override

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.elasticsearch.index.query.QueryShardContext;
2626
import org.elasticsearch.search.DocValueFormat;
2727
import org.elasticsearch.search.aggregations.AggregationBuilder;
28-
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
28+
import org.elasticsearch.search.aggregations.AggregatorFactories;
2929
import org.elasticsearch.search.aggregations.AggregatorFactory;
3030
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3131
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
@@ -58,20 +58,22 @@ private static RangeAggregator.Range parseRange(XContentParser parser) throws IO
5858
return RangeAggregator.Range.fromXContent(parser);
5959
}
6060

61-
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
62-
AbstractRangeAggregatorFactory.registerAggregators(valuesSourceRegistry, NAME);
61+
public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
62+
AbstractRangeAggregatorFactory.registerAggregators(builder, NAME);
6363
}
6464

6565
public DateRangeAggregationBuilder(String name) {
6666
super(name, InternalDateRange.FACTORY);
6767
}
6868

69-
protected DateRangeAggregationBuilder(DateRangeAggregationBuilder clone, Builder factoriesBuilder, Map<String, Object> metadata) {
69+
protected DateRangeAggregationBuilder(DateRangeAggregationBuilder clone,
70+
AggregatorFactories.Builder factoriesBuilder,
71+
Map<String, Object> metadata) {
7072
super(clone, factoriesBuilder, metadata);
7173
}
7274

7375
@Override
74-
protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map<String, Object> metadata) {
76+
protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map<String, Object> metadata) {
7577
return new DateRangeAggregationBuilder(this, factoriesBuilder, metadata);
7678
}
7779

@@ -297,7 +299,8 @@ public DateRangeAggregationBuilder addUnboundedFrom(ZonedDateTime from) {
297299

298300
@Override
299301
protected DateRangeAggregatorFactory innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig config,
300-
AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException {
302+
AggregatorFactory parent,
303+
AggregatorFactories.Builder subFactoriesBuilder) throws IOException {
301304
// We need to call processRanges here so they are parsed and we know whether `now` has been used before we make
302305
// the decision of whether to cache the request
303306
RangeAggregator.Range[] ranges = processRanges(range -> {

0 commit comments

Comments
 (0)