[fix][admin] Fix asyncGetRequest to handle 204

[nodece]

Motivation

This pull request improves the handling and testing of asynchronous GET requests in the Pulsar admin client. The main changes ensure that both HTTP 200 (OK) and 204 (No Content) responses are treated as successful, and introduce comprehensive tests to verify correct behavior for various HTTP response scenarios.

Modifications

Enhancements to async GET request handling:

• Modified BaseResource.java so that both HTTP 200 (OK) and 204 (No Content) responses are accepted as successful. For 204 responses, the future is completed with null since there is no response body to process.

Testing improvements:

• Added a new test class AsyncGetRequestTest.java to thoroughly test the async GET request logic. The tests cover successful responses with and without a body (200 and 204), as well as error responses like 404 and 500, and 200 with an empty body.

Testability and code maintenance:

• Exposed the root WebTarget in PulsarAdminImpl via a new @VisibleForTesting method to facilitate direct testing of request paths.
• Added the necessary import for @VisibleForTesting to PulsarAdminImpl.java.

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

[Copilot]

Pull request overview

This pull request fixes the async GET request handling in the Pulsar admin client to properly support HTTP 204 (No Content) responses. The changes ensure that both 200 OK and 204 No Content are treated as successful responses, with 204 completing the future with null since there's no response body.

• Modified BaseResource.asyncGetRequest to accept both 200 and 204 status codes as successful responses
• Added comprehensive test coverage for various HTTP response scenarios (200 with body, 204, 404, 500, and 200 with empty body)
• Exposed the root WebTarget via a package-private test method to facilitate testing

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java	Updated async GET request handler to accept 204 No Content responses and complete the future with null when no body is present
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/PulsarAdminImpl.java	Added package-private getRoot() method with @VisibleForTesting annotation to expose WebTarget for testing
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java	New test class providing comprehensive coverage for async GET request scenarios including 200 OK, 204 No Content, 404, 500, and empty body cases

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lhotari]

LGTM. However, I'm just wondering whether some existing behavior depends on this change.

[nodece]

@lhotari I noticed that sink, source, function, and worker all depend on this method. #16377 fixed the behavior for namespace, topic, and related resources, but this method is still used by other admin resources. Therefore, fixing it here is important to ensure consistent behavior across all dependent components.

org.apache.pulsar.client.admin.internal.BaseResource.FutureCallback should be deprecated.

org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, java.lang.Class<? extends T>) and org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, javax.ws.rs.core.GenericType<T>) can handle most cases.

[lhotari]

@lhotari I noticed that sink, source, function, and worker all depend on this method. #16377 fixed the behavior for namespace, topic, and related resources, but this method is still used by other admin resources. Therefore, fixing it here is important to ensure consistent behavior across all dependent components.

org.apache.pulsar.client.admin.internal.BaseResource.FutureCallback should be deprecated.

org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, java.lang.Class<? extends T>) and org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, javax.ws.rs.core.GenericType<T>) can handle most cases.

@nodece Just wondering if something is currently broken due to the lack of properly handling 204 return code.

[nodece]

I believe most use cases still work today. In my environment, I use the pulsar-admin API to get the replicator dispatch rate of a namespace. This is on a private Pulsar distribution, not upstream Apache Pulsar.

That said, Apache Pulsar itself still relies on this behavior in several admin APIs, so proper handling of HTTP 204 responses is required. This PR ensures consistent and correct behavior in those cases.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.48%. Comparing base (f101811) to head (62ff354).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #25124       +/-   ##
=============================================
+ Coverage     38.75%   74.48%   +35.73%     
- Complexity    13215    33684    +20469     
=============================================
  Files          1842     1899       +57     
  Lines        145545   149716     +4171     
  Branches      16916    17405      +489     
=============================================
+ Hits          56400   111512    +55112     
+ Misses        81460    29322    -52138     
- Partials       7685     8882     +1197     

Flag	Coverage Δ
inttests	26.55% <28.57%> (-0.03%)	⬇️
systests	22.99% <42.85%> (-0.08%)	⬇️
unittests	74.01% <100.00%> (+39.46%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...che/pulsar/client/admin/internal/BaseResource.java	78.82% <100.00%> (+24.60%)	⬆️
.../pulsar/client/admin/internal/PulsarAdminImpl.java	79.24% <100.00%> (+10.67%)	⬆️

... and 1404 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.