Skip to content

Commit bf98773

Browse files
nodecenodece
authored
[fix][admin] Fix asyncGetRequest to handle 204 (#25124)
1 parent b1019ce commit bf98773

File tree

3 files changed

+184
-2
lines changed

3 files changed

+184
-2
lines changed

‎pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,19 @@ private <T> CompletableFuture<T> asyncGetRequest(final WebTarget target, Functio
202202
new InvocationCallback<Response>() {
203203
@Override
204204
public void completed(Response response) {
205-
if (response.getStatus() != Response.Status.OK.getStatusCode()) {
205+
int status = response.getStatus();
206+
// Accept both 200 OK and 204 No Content as success
207+
if (status != Response.Status.OK.getStatusCode()
208+
&& status != Response.Status.NO_CONTENT.getStatusCode()) {
206209
future.completeExceptionally(getApiException(response));
207210
} else {
208211
try {
209-
future.complete(readResponse.apply(response));
212+
// Handle 204 No Content - no response body to read
213+
if (status == Response.Status.NO_CONTENT.getStatusCode()) {
214+
future.complete(null);
215+
} else {
216+
future.complete(readResponse.apply(response));
217+
}
210218
} catch (Exception e) {
211219
future.completeExceptionally(getApiException(e));
212220
}

‎pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/PulsarAdminImpl.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.pulsar.client.admin.internal;
2020

2121
import static com.google.common.base.Preconditions.checkArgument;
22+
import com.google.common.annotations.VisibleForTesting;
2223
import java.io.IOException;
2324
import java.net.URL;
2425
import java.util.Map;
@@ -445,4 +446,9 @@ public void close() {
445446

446447
asyncHttpConnector.close();
447448
}
449+
450+
@VisibleForTesting
451+
WebTarget getRoot() {
452+
return root;
453+
}
448454
}

‎pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.pulsar.client.admin.internal;
20+
21+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
24+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
import com.github.tomakehurst.wiremock.WireMockServer;
27+
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
28+
import java.io.IOException;
29+
import java.util.concurrent.CompletableFuture;
30+
import java.util.concurrent.TimeUnit;
31+
import lombok.Cleanup;
32+
import org.apache.pulsar.client.admin.PulsarAdmin;
33+
import org.apache.pulsar.client.admin.PulsarAdminException;
34+
import org.testng.annotations.AfterClass;
35+
import org.testng.annotations.BeforeClass;
36+
import org.testng.annotations.Test;
37+
38+
public class AsyncGetRequestTest {
39+
40+
WireMockServer server;
41+
String adminUrl;
42+
43+
@BeforeClass(alwaysRun = true)
44+
void beforeClass() throws IOException {
45+
server = new WireMockServer(WireMockConfiguration.wireMockConfig()
46+
.containerThreads(8)
47+
.port(0));
48+
server.start();
49+
adminUrl = "http://localhost:" + server.port();
50+
}
51+
52+
@AfterClass(alwaysRun = true)
53+
void afterClass() {
54+
if (server != null) {
55+
server.stop();
56+
}
57+
}
58+
59+
@Test
60+
public void testAsyncGetRequest_200OK_WithBody() throws Exception {
61+
// Mock successful response with body
62+
server.stubFor(get(urlEqualTo("/200-with-body"))
63+
.willReturn(aResponse()
64+
.withStatus(200)
65+
.withHeader("Content-Type", "application/json")
66+
.withBody("{\"name\":\"test-namespace\"}")));
67+
68+
@Cleanup
69+
PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
70+
71+
NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
72+
CompletableFuture<Object> getRequest =
73+
namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
74+
75+
assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
76+
77+
// Verify the request was made
78+
server.verify(getRequestedFor(urlEqualTo("/200-with-body")));
79+
}
80+
81+
@Test
82+
public void testAsyncGetRequest_204NoContent() throws Exception {
83+
// Mock 204 No Content response
84+
server.stubFor(get(urlEqualTo("/204"))
85+
.willReturn(aResponse()
86+
.withStatus(204)));
87+
88+
@Cleanup
89+
PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
90+
91+
NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
92+
CompletableFuture<Object> getRequest =
93+
namespaces.asyncGetRequest(admin.getRoot().path("/204"), Object.class);
94+
95+
assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
96+
97+
// Verify the request was made
98+
server.verify(getRequestedFor(urlEqualTo("/204")));
99+
}
100+
101+
@Test
102+
public void testAsyncGetRequest_404NotFound() throws Exception {
103+
// Mock 404 Not Found response
104+
server.stubFor(get(urlEqualTo("/404"))
105+
.willReturn(aResponse()
106+
.withStatus(404)
107+
.withHeader("Content-Type", "application/json")
108+
.withBody("{\"reason\":\"Not found\"}")));
109+
110+
@Cleanup
111+
PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
112+
113+
NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
114+
CompletableFuture<Object> getRequest =
115+
namespaces.asyncGetRequest(admin.getRoot().path("/404"), Object.class);
116+
117+
assertThat(getRequest)
118+
.failsWithin(3, TimeUnit.SECONDS)
119+
.withThrowableOfType(java.util.concurrent.ExecutionException.class)
120+
.withCauseInstanceOf(PulsarAdminException.class);
121+
122+
server.verify(getRequestedFor(urlEqualTo("/404")));
123+
}
124+
125+
@Test
126+
public void testAsyncGetRequest_500InternalServerError() throws Exception {
127+
// Mock 500 Internal Server Error response
128+
server.stubFor(get(urlEqualTo("/500"))
129+
.willReturn(aResponse()
130+
.withStatus(500)
131+
.withBody("Internal Server Error")));
132+
133+
@Cleanup
134+
PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
135+
136+
NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
137+
CompletableFuture<Object> getRequest =
138+
namespaces.asyncGetRequest(admin.getRoot().path("/500"), Object.class);
139+
140+
assertThat(getRequest)
141+
.failsWithin(3, TimeUnit.SECONDS)
142+
.withThrowableOfType(java.util.concurrent.ExecutionException.class)
143+
.withCauseInstanceOf(PulsarAdminException.class);
144+
145+
server.verify(getRequestedFor(urlEqualTo("/500")));
146+
}
147+
148+
@Test
149+
public void testAsyncGetRequest_EmptyResponseBody_With200() throws Exception {
150+
// Mock 200 with empty body
151+
server.stubFor(get(urlEqualTo("/200-empty"))
152+
.willReturn(aResponse()
153+
.withStatus(200)
154+
.withBody("")));
155+
156+
@Cleanup
157+
PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
158+
159+
NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
160+
CompletableFuture<Object> getRequest =
161+
namespaces.asyncGetRequest(admin.getRoot().path("/200-empty"), Object.class);
162+
163+
// Should handle empty body gracefully
164+
assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
165+
166+
server.verify(getRequestedFor(urlEqualTo("/200-empty")));
167+
}
168+
}

0 commit comments

Comments
 (0)