HDDS-13532. Refactor BucketEndpoint.get() method
What changes were proposed in this pull request?
This PR refactors the BucketEndpoint.get() method to improve code maintainability and readability.
The original method was nearly 200 lines long and contained multiple unrelated logic blocks, making it difficult to understand, test, and maintain.
Changes in this PRr
- Extracted 5 methods from the
get()method - add
BucketListingContextclass to encapsulate parameters - added test coverage (unit, integration, performance)
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13532
How was this patch tested?
- Unit tests:
TestBucketEndpointRefactoredSimple.java - Integration tests:
TestBucketEndpointIntegration.java - Performance tests:
TestBucketEndpointPerformance.java
Thanks, @rich7420, for the patch. Please move the integration tests to the integration module instead of keeping them with the unit tests. Also, it would be helpful if you could add some context explaining the purpose of these test cases.
@peterxcli , @priyeshkaratha Thanks for the review! The three new tests were added mainly to verify the refactored logic in BucketEndpoint.get(). I think that they help ensure that behavior remains consistent before and after the refactor. That said, I can move the integration test to the integration module as suggested and update the comments to clarify the purpose of each test. If you prefer, I can also remove the tests after verifying it locally.
I’ve enabled the build-branch workflow on my fork. https://github.com/rich7420/ozone/actions/runs/18313906976/job/52148987227 Regarding the three tests: I trimmed the change set to keep the PR focused on the refactor. The newly added integration/performance tests have been removed to keep CI green and to avoid duplicating coverage that already exists in the integration suites. If the team prefers, I can reintroduce them in a follow‑up PR.
@jojochuang @hevinhsu @TaiJuWu Thanks a lot for the suggestions! I’ve implemented the updates that switched tests to use EndpointBuilder for endpoint creation, moved the key-listing logic into BucketListingContext with BucketEndpoint.processKeyListing(...) delegating to it, renamed the method to handleGetBucketAcl(...), added throws IOException to the test helper getObjectEndpoint(...), and fixed the trailing newline and related style checks. CI is all green now. If there’s anything else we can refine, I’m happy to follow up.
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
@jojochuang Thanks for the review! I'll fix it as soon as possible