fix MultiValuesSourceFieldConfig toXContent (#36525)

This commit turns MultiValuesSourceFieldConfig into a proper
ToXContentObject for easy testing and verification of its
to/from XContent methods.

Closes #36474.

Author: talevy

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

@@ -225,7 +225,9 @@ protected boolean serializeTargetValueType() {
     public final XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         if (fields != null) {
-            builder.field(CommonFields.FIELDS.getPreferredName(), fields);
+            for (Map.Entry<String, MultiValuesSourceFieldConfig> fieldEntry : fields.entrySet()) {
+                builder.field(fieldEntry.getKey(), fieldEntry.getValue());
+            }
         }
         if (format != null) {
             builder.field(CommonFields.FORMAT.getPreferredName(), format);

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

@@ -25,16 +25,17 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ObjectParser;
-import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.Script;
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
+import java.util.Objects;
 import java.util.function.BiFunction;
 
-public class MultiValuesSourceFieldConfig implements Writeable, ToXContentFragment {
+public class MultiValuesSourceFieldConfig implements Writeable, ToXContentObject {
     private String fieldName;
     private Object missing;
     private Script script;
@@ -110,6 +111,7 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         if (missing != null) {
             builder.field(ParseField.CommonFields.MISSING.getPreferredName(), missing);
         }
@@ -120,11 +122,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             builder.field(ParseField.CommonFields.FIELD.getPreferredName(), fieldName);
         }
         if (timeZone != null) {
-            builder.field(ParseField.CommonFields.TIME_ZONE.getPreferredName(), timeZone);
+            builder.field(ParseField.CommonFields.TIME_ZONE.getPreferredName(), timeZone.getID());
         }
+        builder.endObject();
         return builder;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        MultiValuesSourceFieldConfig that = (MultiValuesSourceFieldConfig) o;
+        return Objects.equals(fieldName, that.fieldName)
+            && Objects.equals(missing, that.missing)
+            && Objects.equals(script, that.script)
+            && Objects.equals(timeZone, that.timeZone);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(fieldName, missing, script, timeZone);
+    }
+
+    @Override
+    public String toString() {
+        return Strings.toString(this);
+    }
+
     public static class Builder {
         private String fieldName;
         private Object missing = null;

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

@@ -0,0 +1,77 @@
+/*
+ * 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.metrics.weighted_avg;
+
+import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.search.SearchModule;
+import org.elasticsearch.search.aggregations.AggregatorFactories;
+import org.elasticsearch.search.aggregations.support.MultiValuesSourceFieldConfig;
+import org.elasticsearch.test.AbstractSerializingTestCase;
+import org.junit.Before;
+
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.hasSize;
+
+public class WeightedAvgAggregationBuilderTests extends AbstractSerializingTestCase<WeightedAvgAggregationBuilder> {
+    String aggregationName;
+
+    @Before
+    public void setupName() {
+        aggregationName = randomAlphaOfLength(10);
+    }
+
+    @Override
+    protected NamedXContentRegistry xContentRegistry() {
+        SearchModule searchModule = new SearchModule(Settings.EMPTY, false, Collections.emptyList());
+        return new NamedXContentRegistry(searchModule.getNamedXContents());
+    }
+
+    @Override
+    protected WeightedAvgAggregationBuilder doParseInstance(XContentParser parser) throws IOException {
+        assertSame(XContentParser.Token.START_OBJECT, parser.nextToken());
+        AggregatorFactories.Builder parsed = AggregatorFactories.parseAggregators(parser);
+        assertThat(parsed.getAggregatorFactories(), hasSize(1));
+        assertThat(parsed.getPipelineAggregatorFactories(), hasSize(0));
+        WeightedAvgAggregationBuilder agg = (WeightedAvgAggregationBuilder) parsed.getAggregatorFactories().iterator().next();
+        assertNull(parser.nextToken());
+        assertNotNull(agg);
+        return agg;
+    }
+
+    @Override
+    protected WeightedAvgAggregationBuilder createTestInstance() {
+        MultiValuesSourceFieldConfig valueConfig = new MultiValuesSourceFieldConfig.Builder().setFieldName("value_field").build();
+        MultiValuesSourceFieldConfig weightConfig = new MultiValuesSourceFieldConfig.Builder().setFieldName("weight_field").build();
+        WeightedAvgAggregationBuilder aggregationBuilder = new WeightedAvgAggregationBuilder(aggregationName)
+            .value(valueConfig)
+            .weight(weightConfig);
+        return aggregationBuilder;
+    }
+
+    @Override
+    protected Writeable.Reader<WeightedAvgAggregationBuilder> instanceReader() {
+        return WeightedAvgAggregationBuilder::new;
+    }
+}

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

@@ -19,12 +19,37 @@
 
 package org.elasticsearch.search.aggregations.support;
 
+import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.Script;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.AbstractSerializingTestCase;
+import org.joda.time.DateTimeZone;
+
+import java.io.IOException;
 
 import static org.hamcrest.Matchers.equalTo;
 
-public class MultiValuesSourceFieldConfigTests extends ESTestCase {
+public class MultiValuesSourceFieldConfigTests extends AbstractSerializingTestCase<MultiValuesSourceFieldConfig> {
+
+    @Override
+    protected MultiValuesSourceFieldConfig doParseInstance(XContentParser parser) throws IOException {
+        return MultiValuesSourceFieldConfig.PARSER.apply(true, true).apply(parser, null).build();
+    }
+
+    @Override
+    protected MultiValuesSourceFieldConfig createTestInstance() {
+        String field = randomAlphaOfLength(10);
+        Object missing = randomBoolean() ? randomAlphaOfLength(10) : null;
+        DateTimeZone timeZone = randomBoolean() ? randomDateTimeZone() : null;
+        return new MultiValuesSourceFieldConfig.Builder()
+            .setFieldName(field).setMissing(missing).setScript(null).setTimeZone(timeZone).build();
+    }
+
+    @Override
+    protected Writeable.Reader<MultiValuesSourceFieldConfig> instanceReader() {
+        return MultiValuesSourceFieldConfig::new;
+    }
+
     public void testMissingFieldScript() {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new MultiValuesSourceFieldConfig.Builder().build());
         assertThat(e.getMessage(), equalTo("[field] and [script] cannot both be null.  Please specify one or the other."));