Return null for aggregations factory even if empty aggs is present in search source

[jed326]

Description

Whenever "aggs" is passed into the request body, even if it is empty (ex. "aggs": {},), then this line will evaluate to true:

OpenSearch/server/src/main/java/org/opensearch/action/search/QueryPhaseResultConsumer.java

Line 117 in 8eea7b9

this.hasAggs = source != null && source.aggregations() != null;

Causing us to go down the wrong path and hit the NPE.

This behavior exists from before #7514 and was handled by various checks for the aggregators, for example like here: https://github.com/opensearch-project/OpenSearch/blob/2.8/server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java#L68-L74 where collectors are not added if the aggregators length is not 0.

Related Issues

Resolves #8115

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/18125/
• CommitID: 4443ff0
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.indices.replication.SegmentReplicationIT.testReplicationPostDeleteAndForceMerge

• URL: https://build.ci.opensearch.org/job/gradle-check/18127/
• CommitID: f319f8d
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

Merging #8206 (121df44) into main (68c1c86) will increase coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 121df44 differs from pull request most recent head e97ed89. Consider uploading reports for the commit e97ed89 to get more accurate results

@@             Coverage Diff              @@
##               main    #8206      +/-   ##
============================================
+ Coverage     70.93%   70.95%   +0.01%     
- Complexity    56636    56668      +32     
============================================
  Files          4719     4719              
  Lines        267559   267559              
  Branches      39206    39207       +1     
============================================
+ Hits         189805   189850      +45     
- Misses        61714    61731      +17     
+ Partials      16040    15978      -62     

Impacted Files	Coverage Δ
...earch/search/aggregations/AggregatorFactories.java	75.09% <0.00%> (-0.39%)	⬇️

... and 475 files with indirect coverage changes

[jed326]

@andrross @reta Fixed the changelog and moved the test to a new file with a basic hits.total match. Let me know what you think!

[jed326]

Flakey test from here is due to the change needing to be backported to 2.x as well.
From logs https://build.ci.opensearch.org/job/gradle-check/18133/consoleText we can see that this test was for 2.9 which we would expect to fail at this point.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/18146/
• CommitID: 121df44
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[jed326]

The test failures seem unrelated to these changes. @reta @andrross do you think we could get this merged tonight to unblock OpenSearch Dashboards builds?

[wbeckler]

Nice way to add a test. Thank you for getting this in. I hope it gets approved ASAP.

I do not have approval rights on this repo, even though I am an Admin and accidentally approved it.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

• URL: https://build.ci.opensearch.org/job/gradle-check/18150/
• CommitID: 121df44
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[jed326]

@reta Applied your suggestion, I think we're good to merge then? Thanks!

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      2 org.opensearch.cluster.service.MasterServiceTests.classMethod
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchShardTaskCancellationWithHighCpu
      1 org.opensearch.cluster.service.MasterServiceTests.testThrottlingForMultipleTaskTypes

• URL: https://build.ci.opensearch.org/job/gradle-check/18184/
• CommitID: e97ed89
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.