From 5dbc657a4423ed906b8b515b0ca5e605d2437d1f Mon Sep 17 00:00:00 2001 From: Kamil Berdychowski Date: Tue, 11 Oct 2022 15:31:11 +0200 Subject: [PATCH 1/2] chore: removing some todos --- src/main/java/com/box/sdk/BoxAPIRequest.java | 2 -- src/main/java/com/box/sdk/BoxAPIResponse.java | 1 - 2 files changed, 3 deletions(-) diff --git a/src/main/java/com/box/sdk/BoxAPIRequest.java b/src/main/java/com/box/sdk/BoxAPIRequest.java index 0849c2645..824b3848e 100644 --- a/src/main/java/com/box/sdk/BoxAPIRequest.java +++ b/src/main/java/com/box/sdk/BoxAPIRequest.java @@ -105,8 +105,6 @@ protected BoxAPIRequest(BoxAPIConnection api, URL url, String method, String med this.readTimeout = BoxGlobalSettings.getReadTimeout(); } - //TODO: should I add this? Breaks tests as client expets to get GZIP response -// this.addHeader("Accept-Encoding", "gzip"); this.addHeader("Accept-Charset", "utf-8"); } diff --git a/src/main/java/com/box/sdk/BoxAPIResponse.java b/src/main/java/com/box/sdk/BoxAPIResponse.java index ca3213f34..ae388e9e1 100644 --- a/src/main/java/com/box/sdk/BoxAPIResponse.java +++ b/src/main/java/com/box/sdk/BoxAPIResponse.java @@ -102,7 +102,6 @@ public BoxAPIResponse(int code, this.logResponse(); } else { this.logErrorResponse(this.responseCode); - //TODO: log body when error occurs throw new BoxAPIResponseException("The API returned an error code", responseCode, null, headers); } } From ee5532fba40d6e20bcccb4fecc5785988bfe70d5 Mon Sep 17 00:00:00 2001 From: Kamil Berdychowski Date: Wed, 12 Oct 2022 09:06:44 +0200 Subject: [PATCH 2/2] chore: removing empty catch blocks --- build.gradle | 10 ------ src/main/java/com/box/sdk/BoxAPIRequest.java | 32 ++++++++++--------- .../java/com/box/sdk/BoxAPIRequestTest.java | 6 ++-- src/test/java/com/box/sdk/BoxFileTest.java | 4 ++- src/test/java/com/box/sdk/BoxFolderTest.java | 12 ++++--- 5 files changed, 31 insertions(+), 33 deletions(-) diff --git a/build.gradle b/build.gradle index 96ebf1fe2..9a09be450 100644 --- a/build.gradle +++ b/build.gradle @@ -117,16 +117,6 @@ task integrationTest(type: Test) { classpath = sourceSets.intTest.runtimeClasspath } -//TODO: enable JWT tests -//task integrationTestJWT(type: Test) { -// description "Runs the JWT based integration tests." -// group "Verification" -// testLogging.showStandardStreams = true -// useJUnit { -// includeCategories "com.box.sdk.IntegrationTestJWT" -// } -//} - jacoco { reportsDirectory = file("$buildDir/reports/jacoco") } diff --git a/src/main/java/com/box/sdk/BoxAPIRequest.java b/src/main/java/com/box/sdk/BoxAPIRequest.java index 824b3848e..bc94ab437 100644 --- a/src/main/java/com/box/sdk/BoxAPIRequest.java +++ b/src/main/java/com/box/sdk/BoxAPIRequest.java @@ -144,28 +144,30 @@ public static boolean isRequestRetryable(BoxAPIException apiException) { * @return true if the response is one that should be retried, otherwise false */ public static boolean isResponseRetryable(int responseCode, BoxAPIException apiException) { + if (responseCode >= 500 || responseCode == 429) { + return true; + } + return isClockSkewError(responseCode, apiException); + } + + private static boolean isClockSkewError(int responseCode, BoxAPIException apiException) { String response = apiException.getResponse(); + if (response == null || response.length() == 0) { + return false; + } String message = apiException.getMessage(); String errorCode = ""; - //TODO: remove this empty catch and fix failing tests - try { - JsonObject responseBody = Json.parse(response).asObject(); - if (responseBody.get("code") != null) { - errorCode = responseBody.get("code").toString(); - } else if (responseBody.get("error") != null) { - errorCode = responseBody.get("error").toString(); - } - } catch (Exception e) { + JsonObject responseBody = Json.parse(response).asObject(); + if (responseBody.get("code") != null) { + errorCode = responseBody.get("code").toString(); + } else if (responseBody.get("error") != null) { + errorCode = responseBody.get("error").toString(); } - boolean isClockSkewError = responseCode == 400 + return responseCode == 400 && errorCode.contains("invalid_grant") && message.contains("exp"); - - return (isClockSkewError - || responseCode >= 500 - || responseCode == 429); } private static boolean isResponseRedirect(int responseCode) { @@ -457,7 +459,6 @@ BoxFileUploadSessionPart sendForUploadPart(BoxFileUploadSession session, long of throw apiException; } if (apiException.getResponseCode() == 500) { - //TODO: another empty catch block... try { Iterable parts = session.listParts(); for (BoxFileUploadSessionPart part : parts) { @@ -466,6 +467,7 @@ BoxFileUploadSessionPart sendForUploadPart(BoxFileUploadSession session, long of } } } catch (BoxAPIException e) { + // ignoring exception as we are retrying } } LOGGER.warn(format( diff --git a/src/test/java/com/box/sdk/BoxAPIRequestTest.java b/src/test/java/com/box/sdk/BoxAPIRequestTest.java index 86c6eb0b1..2fad37bf5 100644 --- a/src/test/java/com/box/sdk/BoxAPIRequestTest.java +++ b/src/test/java/com/box/sdk/BoxAPIRequestTest.java @@ -34,7 +34,7 @@ public class BoxAPIRequestTest { @Test public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns500() { - stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500))); + stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500).withBody("{}"))); Time mockTime = mock(Time.class); BackoffCounter backoffCounter = new BackoffCounter(mockTime); @@ -50,7 +50,7 @@ public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns500() { @Test public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns429() { - stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(429))); + stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(429).withBody("{}"))); Time mockTime = mock(Time.class); BackoffCounter backoffCounter = new BackoffCounter(mockTime); @@ -87,7 +87,7 @@ public void requestRetriesTheDefaultNumberOfTimesWhenServerReturnsInvalidGrantIn @Test public void requestRetriesTheNumberOfTimesConfiguredInTheAPIConnection() { final int expectedNumRetryAttempts = 1; - stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500))); + stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500).withBody("{}"))); Time mockTime = mock(Time.class); BackoffCounter backoffCounter = new BackoffCounter(mockTime); diff --git a/src/test/java/com/box/sdk/BoxFileTest.java b/src/test/java/com/box/sdk/BoxFileTest.java index 4fd0aaecd..affdb58b8 100644 --- a/src/test/java/com/box/sdk/BoxFileTest.java +++ b/src/test/java/com/box/sdk/BoxFileTest.java @@ -681,6 +681,7 @@ public void testSetClassification() { wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL)) .willReturn(WireMock.aResponse() + .withBody("{}") .withStatus(409))); wireMockRule.stubFor(WireMock.put(WireMock.urlPathEqualTo(metadataURL)) @@ -704,6 +705,7 @@ public void testSetClassificationThrowsException() { wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL)) .willReturn(WireMock.aResponse() + .withBody("{}") .withStatus(403))); BoxFile file = new BoxFile(this.api, fileID); @@ -1038,7 +1040,7 @@ public void setMetadataWorksWhenNoChangesSubmittedAndConflictOccured() { request -> { if (request.getMethod().equals("POST")) { postCounter.incrementAndGet(); - throw new BoxAPIException("Conflict", 409, "Conflict"); + throw new BoxAPIException("Conflict", 409, "{}"); } if (request.getMethod().equals("GET")) { getCounter.incrementAndGet(); diff --git a/src/test/java/com/box/sdk/BoxFolderTest.java b/src/test/java/com/box/sdk/BoxFolderTest.java index 6985d5575..f79ab9616 100644 --- a/src/test/java/com/box/sdk/BoxFolderTest.java +++ b/src/test/java/com/box/sdk/BoxFolderTest.java @@ -466,17 +466,19 @@ public void testGetAllRootFolderItemsSucceeds() { final String modifiedByLogin = "test@user.com"; final String modifiedByName = "Test User"; - String result = TestUtils.getFixture("BoxFolder/GetAllRootFolderItems200"); + String items = TestUtils.getFixture("BoxFolder/GetAllRootFolderItems200"); + String info = TestUtils.getFixture("BoxFolder/GetFolderInfo200"); wireMockRule.stubFor(WireMock.get(WireMock.urlPathEqualTo(rootFolderItemsURL)) .willReturn(WireMock.aResponse() .withHeader("Content-Type", APPLICATION_JSON) - .withStatus(200))); + .withStatus(200).withBody(items))); wireMockRule.stubFor(WireMock.get(WireMock.urlPathEqualTo(folderURL)) .willReturn(WireMock.aResponse() .withHeader("Content-Type", APPLICATION_JSON) - .withBody(result))); + .withBody(info) + .withStatus(200))); BoxFolder rootFolder = BoxFolder.getRootFolder(this.api); BoxFolder.Info rootFolderInfo = rootFolder.getInfo(); @@ -1133,6 +1135,7 @@ public void testSetClassification() { wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL)) .willReturn(WireMock.aResponse() + .withBody("{}") .withStatus(409))); wireMockRule.stubFor(WireMock.put(WireMock.urlPathEqualTo(metadataURL)) @@ -1156,6 +1159,7 @@ public void testSetClassificationThrowsException() { wireMockRule.stubFor(WireMock.post(WireMock.urlPathEqualTo(metadataURL)) .willReturn(WireMock.aResponse() + .withBody("{}") .withStatus(403))); BoxFolder folder = new BoxFolder(this.api, folderID); @@ -1524,7 +1528,7 @@ public void setMetadataWorksWhenNoChangesSubmittedAndConflictOccured() { request -> { if (request.getMethod().equals("POST")) { postCounter.incrementAndGet(); - throw new BoxAPIException("Conflict", 409, "Conflict"); + throw new BoxAPIException("Conflict", 409, "{}"); } if (request.getMethod().equals("GET")) { getCounter.incrementAndGet();