Undeprecate "aggs" in search request bodies

[dakrone]

"aggs" accidentally got deprecated as a field name in requests, we should un-deprecate it since it is a supported parameter in search requests.

HTTP/1.1 500 Internal Server Error
Warning: Deprecated field [aggs] used, expected [aggregations] instead
Content-Type: application/json; charset=UTF-8
Content-Encoding: gzip
Content-Length: 556

[nik9000]

I think it'd be nice to wait until #19509 is merged so we can assert things about headers in the REST tests.

[colings86]

Oops, I think this was me sorry. The problem is here:

elasticsearch/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

Line 94 in dec620c

public static final ParseField AGGREGATIONS_FIELD = new ParseField("aggregations", "aggs");

The issue is that currently ParseField expects there to be only one acceptable name for the field and all other names are taken as deprecated. So the question here is do we want to deprecate either aggs or aggregations or do we want to add the ability to specify alternative names in ParseField.

If it is the latter I would suggest keeping the constructor in ParseField as is so only the first name is taken as an acceptable name and the rest are deprecated, and then add a addAlternateName(String) method for the few cases where we have alternative acceptable name.

As for #19509 it would be great if post that PR we could add a check to all calls made in rest tests to ensure they do not contain a warning header and if so fail the test (since most tests should not be using deprecated options). For any tests that do need to test deprecated options we could add a accept_deprecated:true to the YAML tests to bypass this warning header check.

wdyt?

[colings86]

Actually we should also set the REST tests to have strict parsing anyway so using deprecated functionality causes an exception and the request to be rejected (unless the accept_deprecated:true option is set on the YAML test)

[colings86]

I opened #19533 to fix this using the ParseField changes I mentioned above. I decided to keep ParseField immutable so the alternative names are passed into a constructor rather than in an add method.

[pickypg]

the request to be rejected (unless the accept_deprecated:true option is set on the YAML test)

I think it might be better to call it expect_deprecated:true and then have the same semantics, but also fail if it doesn't get a deprecation warning returned.

[nik9000]

I think it might be better to call it expect_deprecated:true and then have the same semantics, but also fail if it doesn't get a deprecation warning returned.

Yes, that sounds good. I'd honestly really like to make that change.