Array, passed to Objects.hash(), should be wrapped into Arrays.hashCode()

[dk2k]

Array, passed to Objects.hash(), should be wrapped into Arrays.hashCode()

[github-actions]

✅ Gradle check result for 9abc6d8: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 85.96974% with 102 lines in your changes missing coverage. Please review.

Project coverage is 72.00%. Comparing base (dc8a435) to head (ce74105).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...search/cache/common/tier/TieredSpilloverCache.java	93.21%	13 Missing and 6 partials ⚠️
...opensearch/identity/shiro/ShiroIdentityPlugin.java	0.00%	16 Missing ⚠️
...common/io/stream/BufferedChecksumStreamOutput.java	72.09%	12 Missing ⚠️
...ch/plugin/wlm/rest/RestUpdateQueryGroupAction.java	0.00%	8 Missing ⚠️
...search/gradle/testfixtures/TestFixturesPlugin.java	0.00%	6 Missing ⚠️
.../opensearch/cache/store/disk/EhcacheDiskCache.java	75.00%	3 Missing and 3 partials ⚠️
...in/wlm/action/TransportUpdateQueryGroupAction.java	0.00%	5 Missing ⚠️
...rch/plugin/wlm/action/UpdateQueryGroupRequest.java	73.33%	4 Missing ⚠️
...ugin/wlm/service/QueryGroupPersistenceService.java	92.85%	2 Missing and 2 partials ⚠️
...opensearch/gradle/docker/DockerSupportService.java	0.00%	3 Missing ⚠️
... and 13 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #15383      +/-   ##
============================================
+ Coverage     71.96%   72.00%   +0.04%     
- Complexity    64791    64816      +25     
============================================
  Files          5307     5307              
  Lines        302714   302725      +11     
  Branches      43733    43734       +1     
============================================
+ Hits         217836   217991     +155     
+ Misses        67013    66823     -190     
- Partials      17865    17911      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dblock]

How does this change the behavior of this code? Needs a test?

[sandeshkr419]

We are increasing time complexity by doing this change for sure. Is there a specific reason for doing this?

[dbwiddis]

@sandeshkr419 the hash of operations here is the array object's hash, but does not hash the contents. You could have a case of two different arrays containing the same elements, which would produce a different hashCode here but match the .equals() check which uses Arrays.equals(). This is a good change.

[dbwiddis]

Code change LGTM.

@dk2k please add a test to ResyncReplicationRequestTests to validate that equals() and hashCode() are consistent for two different arrays with the same elements. It should fail before this change and succeed after it.

Also as a bugfix this needs a change log.

[dk2k]

Sorry, this rebase mess happened again.

[dbwiddis]

Sorry, this rebase mess happened again.

@dk2k It actually took me a few years to figure out how to do it. A rebase is different than a merge. Here's some simple steps.

1. On your local machine, switch to the main branch: git checkout main
2. Get the latest from main: git pull upstream main
3. Switch back to your pr branch: git checkout dk_array_hashcode
4. Type git rebase main

At this point you may get merge conflicts. You can usually resolve them in your editor. Add/stage each file you fix the conflicts in, and then type git rebase --continue.

5. Once you've got everything rebased, force push to your fork PR branch: git push -f.

[github-actions]

✅ Gradle check result for ce74105: SUCCESS

[dblock]

@dk2k It actually took me a few years to figure out how to do it. A rebase is different than a merge. Here's some simple steps.

Sometimes that rebase is nasty. Here's an even less sophisticated way to get out of this: https://code.dblock.org/2015/08/31/getting-out-of-your-first-git-mess.html.