[BUG-1402] Return HTTP 409 on version confict when creating & updating tenants - added relevant unit test

[prabhask5]

Description

Added new TenantsActionApiTest file to put api unit tests for the /api/tenants api call- additionally wrote new unit test to test for parallel PUT /tenant/{name} calls, that they successfully return a 409 call and retry the call.

• Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
  Test fix

• Why these changes are required?
  There was no test that checked for the behavior mentioned in the linked ticket, so previously we didn't know if the issue still existed or not (it apparently was fixed before), to make sure this doesn't happen again for this particular case, I decided to write a concrete test for it.

• What is the old behavior before changes and new behavior after changes?
  Test was added, there's no behavior change?

Issues Resolved

• [BUG] Return HTTP 409 on version confict when creating & updating tenants #1402 - added unit test to cover bug in this issue

Testing

Added new tenant action unit test to test parallel tenant api calls.

Check List

• New functionality includes testing
• New functionality has been documented
• Commits are signed per the DCO using --signoff

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.

[prabhask5]

@cwperks This PR was created to address the comment you left under #1402

[peternied]

@prabhask5 Thanks for the contribution of the test - much appreciated! I've left feedback please address and let me know and I'll take another look.

[prabhask5]

@peternied Alright I refactored the added test to test only two parallel requests and I'm getting a consistent one 409 and one 201 codes, which should be good. The only thing that changes from test run to test run is which request is the "conflict" one, but I think I account for this when checking the description with the get request. Let me know if you need me to make any other changes! TY!

[codecov]

Codecov Report

Merging #3580 (3b3cf82) into main (905c97d) will increase coverage by 63.14%.
The diff coverage is n/a.

❗ Current head 3b3cf82 differs from pull request most recent head 6791c7a. Consider uploading reports for the commit 6791c7a to get more accurate results

@@             Coverage Diff             @@
##             main    #3580       +/-   ##
===========================================
+ Coverage        0   63.14%   +63.14%     
- Complexity      0     3561     +3561     
===========================================
  Files           0      284      +284     
  Lines           0    20618    +20618     
  Branches        0     3390     +3390     
===========================================
+ Hits            0    13019    +13019     
- Misses          0     5923     +5923     
- Partials        0     1676     +1676     

see 284 files with indirect coverage changes

[DarshitChanpura]

Thank you for adding this test @prabhask5.

[peternied]

Async tests are on my mind today, see this PR [1] for an example of how I've implemented async tests + validation. Those utilities might be useful, feel free to merge my branch onto this change if it helps your development.

• [1] [Refactor] Improve asynchronous test cases format #3601

[prabhask5]

I restructured the test using streams and the AsyncActions.java file from #3601 - let me know of other changes, ty!

[DarshitChanpura]

We can wait until #3601 merge to ensure any conflicts are resolved.

[DarshitChanpura]

Ty for adding the test @prabhask5.

[peternied]

@prabhask5 please take a look at the test failures, I'm going to hold off on another review until those are passing. Please let us know if you need help with an issue.

[davidlago]

@prabhask5 if you are not ready to continue work on this PR, we can go ahead and close it, and re-open at a later time. Please let us know!

[prabhask5]

@prabhask5 please take a look at the test failures, I'm going to hold off on another review until those are passing. Please let us know if you need help with an issue.

@peternied The errors that I was getting in the test were related to java not being able to find import org.opensearch.test.framework.AsyncActions; when it does exist (which is used in your PR as well), I merged main into this PR to try to solve this error, but if it does not fix it, do you have any fixes going forward?

[peternied]

@prabhask5 Checkout this guide [1] on rebasing it might help you get unblocked.

• [1] https://akrabat.com/the-beginners-guide-to-rebasing-your-pr/

[prabhask5]

@peternied I think I rebased so feel free to unblock the tests. TY!

[prabhask5]

I'm going to make a new PR to see if that fixes things.

[peternied]

@prabhask5 Could you link the new PR?

[prabhask5]

@prabhask5 Could you link the new PR?

Here: #3673