Skip to content

Commit 0bf6448

Browse files
jvercejverce
committed
Copilot feedback
1 parent 23067a4 commit 0bf6448

File tree

3 files changed

+62
-57
lines changed

3 files changed

+62
-57
lines changed

‎src/main/java/com/pipedream/api/resources/proxy/AsyncRawProxyClient.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private static void handleSuccessResponse(
5757
ProxyResponse proxyBody;
5858
if (isJsonContentType(contentType)) {
5959
try {
60-
String responseBodyString = responseBody != null ? responseBody.string() : "null";
60+
String responseBodyString = responseBody.string();
6161
Object parsed = ObjectMappers.JSON_MAPPER.readValue(responseBodyString, Object.class);
6262
proxyBody = ProxyResponse.json(parsed, contentTypeString);
6363
} catch (JsonProcessingException e) {
@@ -75,6 +75,7 @@ private static void handleSuccessResponse(
7575
proxyBody = ProxyResponse.stream(new ResponseBodyInputStream(response), contentTypeString);
7676
} catch (IOException e) {
7777
future.completeExceptionally(new BaseClientException("Error creating response stream", e));
78+
response.close();
7879
return;
7980
}
8081
}

‎src/main/java/com/pipedream/api/resources/proxy/RawProxyClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ private static ProxyResponse parseSuccessResponse(Response response, ResponseBod
4848
MediaType contentType = responseBody != null ? responseBody.contentType() : null;
4949
String contentTypeString = contentType != null ? contentType.toString() : null;
5050
if (isJsonContentType(contentType)) {
51-
String responseBodyString = responseBody != null ? responseBody.string() : "null";
51+
String responseBodyString = responseBody.string();
5252
try {
5353
Object parsed = ObjectMappers.JSON_MAPPER.readValue(responseBodyString, Object.class);
5454
return ProxyResponse.json(parsed, contentTypeString);

‎src/test/java/com/pipedream/api/ProxyClientWireTest.java‎

Lines changed: 59 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
package com.pipedream.api;
22

3+
import static org.junit.jupiter.api.Assertions.*;
4+
35
import com.pipedream.api.resources.proxy.requests.ProxyGetRequest;
46
import com.pipedream.api.resources.proxy.types.ProxyResponse;
7+
import java.io.ByteArrayOutputStream;
8+
import java.io.IOException;
9+
import java.io.InputStream;
10+
import java.util.Map;
11+
import java.util.concurrent.CompletableFuture;
12+
import java.util.concurrent.TimeUnit;
513
import okhttp3.mockwebserver.MockResponse;
614
import okhttp3.mockwebserver.MockWebServer;
715
import okhttp3.mockwebserver.RecordedRequest;
@@ -10,15 +18,6 @@
1018
import org.junit.jupiter.api.BeforeEach;
1119
import org.junit.jupiter.api.Test;
1220

13-
import java.io.ByteArrayOutputStream;
14-
import java.io.IOException;
15-
import java.io.InputStream;
16-
import java.util.Map;
17-
import java.util.concurrent.CompletableFuture;
18-
import java.util.concurrent.TimeUnit;
19-
20-
import static org.junit.jupiter.api.Assertions.*;
21-
2221
public class ProxyClientWireTest {
2322
private MockWebServer server;
2423
private BaseClient client;
@@ -84,25 +83,26 @@ public void testGetJsonResponse() throws Exception {
8483
.setHeader("Content-Type", "application/json")
8584
.setBody(jsonBody));
8685

87-
ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest());
86+
try (ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest())) {
8887

89-
// Verify response type
90-
assertTrue(response.isJson(), "Response should be JSON");
91-
assertFalse(response.isStream(), "Response should not be a stream");
88+
// Verify response type
89+
assertTrue(response.isJson(), "Response should be JSON");
90+
assertFalse(response.isStream(), "Response should not be a stream");
9291

93-
// Verify parsed JSON content
94-
Object json = response.json();
95-
assertNotNull(json, "JSON body should not be null");
96-
assertInstanceOf(Map.class, json, "JSON body should be a Map");
92+
// Verify parsed JSON content
93+
Object json = response.json();
94+
assertNotNull(json, "JSON body should not be null");
95+
assertInstanceOf(Map.class, json, "JSON body should be a Map");
9796

98-
@SuppressWarnings("unchecked")
99-
Map<String, Object> jsonMap = (Map<String, Object>) json;
100-
assertEquals("value", jsonMap.get("key"));
101-
assertEquals(42, jsonMap.get("number"));
97+
@SuppressWarnings("unchecked")
98+
Map<String, Object> jsonMap = (Map<String, Object>) json;
99+
assertEquals("value", jsonMap.get("key"));
100+
assertEquals(42, jsonMap.get("number"));
102101

103-
// Verify content type is preserved
104-
assertTrue(response.getContentType().isPresent());
105-
assertEquals("application/json", response.getContentType().get());
102+
// Verify content type is preserved
103+
assertTrue(response.getContentType().isPresent());
104+
assertEquals("application/json", response.getContentType().get());
105+
}
106106

107107
// Verify request was made correctly
108108
RecordedRequest request = server.takeRequest();
@@ -111,26 +111,27 @@ public void testGetJsonResponse() throws Exception {
111111
}
112112

113113
@Test
114-
public void testGetJsonResponseWithCharset() {
114+
public void testGetJsonResponseWithCharset() throws Exception {
115115
String jsonBody = "{\"message\":\"hello\"}";
116116
server.enqueue(new MockResponse()
117117
.setResponseCode(200)
118118
.setHeader("Content-Type", "application/json; charset=utf-8")
119119
.setBody(jsonBody));
120120

121-
ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest());
121+
try (ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest())) {
122122

123-
// Should still be detected as JSON even with charset parameter
124-
assertTrue(response.isJson(), "Response should be JSON even with charset");
123+
// Should still be detected as JSON even with charset parameter
124+
assertTrue(response.isJson(), "Response should be JSON even with charset");
125125

126-
@SuppressWarnings("unchecked")
127-
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
128-
assertEquals("hello", jsonMap.get("message"));
126+
@SuppressWarnings("unchecked")
127+
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
128+
assertEquals("hello", jsonMap.get("message"));
129+
}
129130
}
130131

131132
@Test
132133
public void testGetOctetStreamResponse() throws Exception {
133-
byte[] binaryData = new byte[]{0x00, 0x01, 0x02, 0x03, 0x04, 0x05};
134+
byte[] binaryData = new byte[] {0x00, 0x01, 0x02, 0x03, 0x04, 0x05};
134135
Buffer buffer = new Buffer();
135136
buffer.write(binaryData);
136137

@@ -176,15 +177,16 @@ public void testGetRedirectFollowed() throws Exception {
176177
.setHeader("Content-Type", "application/json")
177178
.setBody(jsonBody));
178179

179-
ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest());
180+
try (ProxyResponse response = client.proxy().get(TEST_URL, createGetRequest())) {
180181

181-
// Verify we got the final response (redirect was followed)
182-
assertTrue(response.isJson(), "Response should be JSON after redirect");
182+
// Verify we got the final response (redirect was followed)
183+
assertTrue(response.isJson(), "Response should be JSON after redirect");
183184

184-
@SuppressWarnings("unchecked")
185-
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
186-
assertEquals(true, jsonMap.get("redirected"));
187-
assertEquals("success", jsonMap.get("status"));
185+
@SuppressWarnings("unchecked")
186+
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
187+
assertEquals(true, jsonMap.get("redirected"));
188+
assertEquals("success", jsonMap.get("status"));
189+
}
188190

189191
// Verify both requests were made (original + redirect)
190192
RecordedRequest firstRequest = server.takeRequest();
@@ -209,17 +211,18 @@ public void testAsyncGetJsonResponse() throws Exception {
209211
.setBody(jsonBody));
210212

211213
CompletableFuture<ProxyResponse> future = asyncClient.proxy().get(TEST_URL, createGetRequest());
212-
ProxyResponse response = future.get(10, TimeUnit.SECONDS);
214+
try (ProxyResponse response = future.get(10, TimeUnit.SECONDS)) {
213215

214-
// Verify response type
215-
assertTrue(response.isJson(), "Response should be JSON");
216-
assertFalse(response.isStream(), "Response should not be a stream");
216+
// Verify response type
217+
assertTrue(response.isJson(), "Response should be JSON");
218+
assertFalse(response.isStream(), "Response should not be a stream");
217219

218-
// Verify parsed JSON content
219-
@SuppressWarnings("unchecked")
220-
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
221-
assertEquals(true, jsonMap.get("async"));
222-
assertEquals("test", jsonMap.get("data"));
220+
// Verify parsed JSON content
221+
@SuppressWarnings("unchecked")
222+
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
223+
assertEquals(true, jsonMap.get("async"));
224+
assertEquals("test", jsonMap.get("data"));
225+
}
223226

224227
// Verify request was made correctly
225228
RecordedRequest request = server.takeRequest();
@@ -229,7 +232,7 @@ public void testAsyncGetJsonResponse() throws Exception {
229232

230233
@Test
231234
public void testAsyncGetOctetStreamResponse() throws Exception {
232-
byte[] binaryData = new byte[]{(byte) 0xFF, (byte) 0xFE, (byte) 0xFD};
235+
byte[] binaryData = new byte[] {(byte) 0xFF, (byte) 0xFE, (byte) 0xFD};
233236
Buffer buffer = new Buffer();
234237
buffer.write(binaryData);
235238

@@ -273,14 +276,15 @@ public void testAsyncGetRedirectFollowed() throws Exception {
273276
.setBody(jsonBody));
274277

275278
CompletableFuture<ProxyResponse> future = asyncClient.proxy().get(TEST_URL, createGetRequest());
276-
ProxyResponse response = future.get(10, TimeUnit.SECONDS);
279+
try (ProxyResponse response = future.get(10, TimeUnit.SECONDS)) {
277280

278-
// Verify we got the final response (redirect was followed)
279-
assertTrue(response.isJson(), "Response should be JSON after redirect");
281+
// Verify we got the final response (redirect was followed)
282+
assertTrue(response.isJson(), "Response should be JSON after redirect");
280283

281-
@SuppressWarnings("unchecked")
282-
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
283-
assertEquals(true, jsonMap.get("asyncRedirected"));
284+
@SuppressWarnings("unchecked")
285+
Map<String, Object> jsonMap = (Map<String, Object>) response.json();
286+
assertEquals(true, jsonMap.get("asyncRedirected"));
287+
}
284288

285289
// Verify both requests were made (original + redirect)
286290
RecordedRequest firstRequest = server.takeRequest();

0 commit comments

Comments
 (0)