fix MultiValuesSourceFieldConfig toXContent

[talevy]

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

Closes #36474.

[elasticmachine]

Pinging @elastic/es-analytics-geo

[talevy]

@polyfractal I was interested in adding actual tests for the aggregation as well since I couldn't find any. Do we have to/from xcontent tests somewhere for the weighted_avg aggregation?

I tried my hand, but
ran into issues where it isn't clear how to parse the toXContent version of things. Looks like things are inconsistent?

For example, here is a snippet of the generated xcontent:

{“vOdlQXAnYr”:{“weighted_avg”:{“fields”:{“weight”:{“field”:“weight_field”},“value”:{“field”:“value_field”}}}}}

There is an extra fields field that differs from the vanilla query-dsl format.

[talevy]

I assume I am missing some context, but I went ahead and added a second commit that demonstrates what I meant by my previous comment

[polyfractal]

Thanks @talevy! Left a few comments, otherwise LGTM.

Should this target master, not just 6.x?

[polyfractal]

Should we snag the code from BaseAggregatorTestCase#parse() so that we have a similar set of assertions here?

[talevy]

oh woah, yeah, I'll lift that...

thanks for the pointer, was looking for something like that to re-use

[polyfractal]

I think that if we want to change this, we might need to wrap this in version checks? readOptionalString() encodes an extra byte for the boolean

[talevy]

good catch. yes.

[talevy]

OK. maybe we can do this in a separate PR. I think Docs need clean up too. We claim that field is required, but it can be replaced with script.

I see this as a bug, but want to attack this separately since the original issue was only with XContent.

[polyfractal]

++ makes sense to me

[talevy]

Hey @polyfractal, I know you approved this, but I've reverted the serialization changes, mind taking another look?

As I said previously. the bug in the serialization is not the concern of this PR.

[polyfractal]

👍 looks good! ++ to doing the serialization stuff in a followup PR (agree it's a bug and the docs should be updated to match)

[talevy]

thanks for the review @polyfractal!

[RussellSpitzer]

This is also broken on the 6.4 branch, would it be possible to back port the fix there as well?