Skip to content

Commit b820d7c

Browse files
talevytalevy
authored
fix MultiValuesSourceFieldConfig toXContent (elastic#36525)
This commit turns MultiValuesSourceFieldConfig into a proper ToXContentObject for easy testing and verification of its to/from XContent methods. Closes elastic#36474.
1 parent e3cf642 commit b820d7c

File tree

4 files changed

+135
-6
lines changed

4 files changed

+135
-6
lines changed

‎server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregationBuilder.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,9 @@ protected boolean serializeTargetValueType() {
207207
public final XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException {
208208
builder.startObject();
209209
if (fields != null) {
210-
builder.field(CommonFields.FIELDS.getPreferredName(), fields);
210+
for (Map.Entry<String, MultiValuesSourceFieldConfig> fieldEntry : fields.entrySet()) {
211+
builder.field(fieldEntry.getKey(), fieldEntry.getValue());
212+
}
211213
}
212214
if (format != null) {
213215
builder.field(CommonFields.FORMAT.getPreferredName(), format);

‎server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,17 @@
2525
import org.elasticsearch.common.io.stream.StreamOutput;
2626
import org.elasticsearch.common.io.stream.Writeable;
2727
import org.elasticsearch.common.xcontent.ObjectParser;
28-
import org.elasticsearch.common.xcontent.ToXContentFragment;
28+
import org.elasticsearch.common.xcontent.ToXContentObject;
2929
import org.elasticsearch.common.xcontent.XContentBuilder;
3030
import org.elasticsearch.common.xcontent.XContentParser;
3131
import org.elasticsearch.script.Script;
3232
import org.joda.time.DateTimeZone;
3333

3434
import java.io.IOException;
35+
import java.util.Objects;
3536
import java.util.function.BiFunction;
3637

37-
public class MultiValuesSourceFieldConfig implements Writeable, ToXContentFragment {
38+
public class MultiValuesSourceFieldConfig implements Writeable, ToXContentObject {
3839
private String fieldName;
3940
private Object missing;
4041
private Script script;
@@ -110,6 +111,7 @@ public void writeTo(StreamOutput out) throws IOException {
110111

111112
@Override
112113
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
114+
builder.startObject();
113115
if (missing != null) {
114116
builder.field(ParseField.CommonFields.MISSING.getPreferredName(), missing);
115117
}
@@ -120,11 +122,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
120122
builder.field(ParseField.CommonFields.FIELD.getPreferredName(), fieldName);
121123
}
122124
if (timeZone != null) {
123-
builder.field(ParseField.CommonFields.TIME_ZONE.getPreferredName(), timeZone);
125+
builder.field(ParseField.CommonFields.TIME_ZONE.getPreferredName(), timeZone.getID());
124126
}
127+
builder.endObject();
125128
return builder;
126129
}
127130

131+
@Override
132+
public boolean equals(Object o) {
133+
if (this == o) return true;
134+
if (o == null || getClass() != o.getClass()) return false;
135+
MultiValuesSourceFieldConfig that = (MultiValuesSourceFieldConfig) o;
136+
return Objects.equals(fieldName, that.fieldName)
137+
&& Objects.equals(missing, that.missing)
138+
&& Objects.equals(script, that.script)
139+
&& Objects.equals(timeZone, that.timeZone);
140+
}
141+
142+
@Override
143+
public int hashCode() {
144+
return Objects.hash(fieldName, missing, script, timeZone);
145+
}
146+
147+
@Override
148+
public String toString() {
149+
return Strings.toString(this);
150+
}
151+
128152
public static class Builder {
129153
private String fieldName;
130154
private Object missing = null;

‎server/src/test/java/org/elasticsearch/search/aggregations/metrics/weighted_avg/WeightedAvgAggregationBuilderTests.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.search.aggregations.metrics.weighted_avg;
21+
22+
import org.elasticsearch.common.io.stream.Writeable;
23+
import org.elasticsearch.common.settings.Settings;
24+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
25+
import org.elasticsearch.common.xcontent.XContentParser;
26+
import org.elasticsearch.search.SearchModule;
27+
import org.elasticsearch.search.aggregations.AggregatorFactories;
28+
import org.elasticsearch.search.aggregations.metrics.WeightedAvgAggregationBuilder;
29+
import org.elasticsearch.search.aggregations.support.MultiValuesSourceFieldConfig;
30+
import org.elasticsearch.test.AbstractSerializingTestCase;
31+
import org.junit.Before;
32+
33+
import java.io.IOException;
34+
import java.util.Collections;
35+
36+
import static org.hamcrest.Matchers.hasSize;
37+
38+
public class WeightedAvgAggregationBuilderTests extends AbstractSerializingTestCase<WeightedAvgAggregationBuilder> {
39+
String aggregationName;
40+
41+
@Before
42+
public void setupName() {
43+
aggregationName = randomAlphaOfLength(10);
44+
}
45+
46+
@Override
47+
protected NamedXContentRegistry xContentRegistry() {
48+
SearchModule searchModule = new SearchModule(Settings.EMPTY, false, Collections.emptyList());
49+
return new NamedXContentRegistry(searchModule.getNamedXContents());
50+
}
51+
52+
@Override
53+
protected WeightedAvgAggregationBuilder doParseInstance(XContentParser parser) throws IOException {
54+
assertSame(XContentParser.Token.START_OBJECT, parser.nextToken());
55+
AggregatorFactories.Builder parsed = AggregatorFactories.parseAggregators(parser);
56+
assertThat(parsed.getAggregatorFactories(), hasSize(1));
57+
assertThat(parsed.getPipelineAggregatorFactories(), hasSize(0));
58+
WeightedAvgAggregationBuilder agg = (WeightedAvgAggregationBuilder) parsed.getAggregatorFactories().iterator().next();
59+
assertNull(parser.nextToken());
60+
assertNotNull(agg);
61+
return agg;
62+
}
63+
64+
@Override
65+
protected WeightedAvgAggregationBuilder createTestInstance() {
66+
MultiValuesSourceFieldConfig valueConfig = new MultiValuesSourceFieldConfig.Builder().setFieldName("value_field").build();
67+
MultiValuesSourceFieldConfig weightConfig = new MultiValuesSourceFieldConfig.Builder().setFieldName("weight_field").build();
68+
WeightedAvgAggregationBuilder aggregationBuilder = new WeightedAvgAggregationBuilder(aggregationName)
69+
.value(valueConfig)
70+
.weight(weightConfig);
71+
return aggregationBuilder;
72+
}
73+
74+
@Override
75+
protected Writeable.Reader<WeightedAvgAggregationBuilder> instanceReader() {
76+
return WeightedAvgAggregationBuilder::new;
77+
}
78+
}

‎server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,37 @@
1919

2020
package org.elasticsearch.search.aggregations.support;
2121

22+
import org.elasticsearch.common.io.stream.Writeable;
23+
import org.elasticsearch.common.xcontent.XContentParser;
2224
import org.elasticsearch.script.Script;
23-
import org.elasticsearch.test.ESTestCase;
25+
import org.elasticsearch.test.AbstractSerializingTestCase;
26+
import org.joda.time.DateTimeZone;
27+
28+
import java.io.IOException;
2429

2530
import static org.hamcrest.Matchers.equalTo;
2631

27-
public class MultiValuesSourceFieldConfigTests extends ESTestCase {
32+
public class MultiValuesSourceFieldConfigTests extends AbstractSerializingTestCase<MultiValuesSourceFieldConfig> {
33+
34+
@Override
35+
protected MultiValuesSourceFieldConfig doParseInstance(XContentParser parser) throws IOException {
36+
return MultiValuesSourceFieldConfig.PARSER.apply(true, true).apply(parser, null).build();
37+
}
38+
39+
@Override
40+
protected MultiValuesSourceFieldConfig createTestInstance() {
41+
String field = randomAlphaOfLength(10);
42+
Object missing = randomBoolean() ? randomAlphaOfLength(10) : null;
43+
DateTimeZone timeZone = randomBoolean() ? randomDateTimeZone() : null;
44+
return new MultiValuesSourceFieldConfig.Builder()
45+
.setFieldName(field).setMissing(missing).setScript(null).setTimeZone(timeZone).build();
46+
}
47+
48+
@Override
49+
protected Writeable.Reader<MultiValuesSourceFieldConfig> instanceReader() {
50+
return MultiValuesSourceFieldConfig::new;
51+
}
52+
2853
public void testMissingFieldScript() {
2954
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new MultiValuesSourceFieldConfig.Builder().build());
3055
assertThat(e.getMessage(), equalTo("[field] and [script] cannot both be null. Please specify one or the other."));

0 commit comments

Comments
 (0)