From 73ca8743691c96ed94f1cae54a319df8ceda0a34 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Mon, 22 Nov 2021 16:50:04 -0500 Subject: [PATCH 1/3] fix: update exception mapping on HTTP error responses. Interprets HTTP error reponse codes as defined by the canonical error code mapping. Not marking as breaking changes as it affects only httpjson that is not GA-ed. --- .../httpjson/HttpJsonExceptionCallable.java | 2 +- .../httpjson/HttpJsonOperationSnapshot.java | 3 +- .../api/gax/httpjson/HttpJsonStatusCode.java | 91 ++++++-------- .../HttpJsonOperationSnapshotTest.java | 2 +- .../gax/httpjson/HttpJsonStatusCodeTest.java | 89 ++++---------- .../google/api/gax/httpjson/RetryingTest.java | 115 +++++++----------- 6 files changed, 106 insertions(+), 196 deletions(-) diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonExceptionCallable.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonExceptionCallable.java index b70899be7..14be14332 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonExceptionCallable.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonExceptionCallable.java @@ -95,7 +95,7 @@ public void onSuccess(ResponseT r) { public void onFailure(Throwable throwable) { if (throwable instanceof HttpResponseException) { HttpResponseException e = (HttpResponseException) throwable; - StatusCode statusCode = HttpJsonStatusCode.of(e.getStatusCode(), e.getMessage()); + StatusCode statusCode = HttpJsonStatusCode.of(e.getStatusCode()); boolean canRetry = retryableCodes.contains(statusCode.getCode()); String message = e.getStatusMessage(); ApiException newException = diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java index 97cb55dfb..358573fee 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java @@ -140,8 +140,7 @@ public Builder setResponse(Object response) { public Builder setError(int errorCode, String errorMessage) { this.errorCode = - HttpJsonStatusCode.of( - errorCode == 0 ? Code.OK.getHttpStatusCode() : errorCode, errorMessage); + errorCode == 0 ? HttpJsonStatusCode.of(Code.OK) : HttpJsonStatusCode.of(errorCode); this.errorMessage = errorMessage; return this; } diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonStatusCode.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonStatusCode.java index 638b3d3ae..617496eb5 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonStatusCode.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonStatusCode.java @@ -32,26 +32,18 @@ import com.google.api.core.BetaApi; import com.google.api.core.InternalExtensionOnly; import com.google.api.gax.rpc.StatusCode; -import com.google.api.gax.rpc.StatusCode.Code; -import com.google.common.base.Strings; import java.util.Objects; /** A failure code specific to an HTTP call. */ @BetaApi @InternalExtensionOnly public class HttpJsonStatusCode implements StatusCode { - static final String FAILED_PRECONDITION = "FAILED_PRECONDITION"; - static final String OUT_OF_RANGE = "OUT_OF_RANGE"; - static final String ALREADY_EXISTS = "ALREADY_EXISTS"; - static final String DATA_LOSS = "DATA_LOSS"; - static final String UNKNOWN = "UNKNOWN"; - private final int httpStatus; private final Code statusCode; /** Creates a new instance with the given status code. */ - public static HttpJsonStatusCode of(int httpStatus, String errorMessage) { - return new HttpJsonStatusCode(httpStatus, httpStatusToStatusCode(httpStatus, errorMessage)); + public static HttpJsonStatusCode of(int httpStatus) { + return new HttpJsonStatusCode(httpStatus, httpStatusToStatusCode(httpStatus)); } public static HttpJsonStatusCode of(Code statusCode) { @@ -103,56 +95,43 @@ static Code rpcCodeToStatusCode(com.google.rpc.Code rpcCode) { } } - static Code httpStatusToStatusCode(int httpStatus, String errorMessage) { - String causeMessage = Strings.nullToEmpty(errorMessage).toUpperCase(); - switch (httpStatus) { - case 200: - return Code.OK; - case 400: - if (causeMessage.contains(OUT_OF_RANGE)) { - return Code.OUT_OF_RANGE; - } else if (causeMessage.contains(FAILED_PRECONDITION)) { - return Code.FAILED_PRECONDITION; - } else { + static Code httpStatusToStatusCode(int httpStatus) { + if (200 <= httpStatus && httpStatus < 300) { + return Code.OK; + } else if (400 <= httpStatus && httpStatus < 500) { + switch (httpStatus) { + case 400: return Code.INVALID_ARGUMENT; - } - case 401: - return Code.UNAUTHENTICATED; - case 403: - return Code.PERMISSION_DENIED; - case 404: - return Code.NOT_FOUND; - case 409: - if (causeMessage.contains(ALREADY_EXISTS)) { - return Code.ALREADY_EXISTS; - } else { + case 401: + return Code.UNAUTHENTICATED; + case 403: + return Code.PERMISSION_DENIED; + case 404: + return Code.NOT_FOUND; + case 409: return Code.ABORTED; - } - case 411: - throw new IllegalStateException( - "411 status code received (Content-Length header not given.) Please file a bug against https://github.com/googleapis/gax-java/\n" - + httpStatus); - case 429: - return Code.RESOURCE_EXHAUSTED; - case 499: - return Code.CANCELLED; - case 500: - if (causeMessage.contains(DATA_LOSS)) { - return Code.DATA_LOSS; - } else if (causeMessage.contains(UNKNOWN)) { - return Code.UNKNOWN; - } else { + case 416: + return Code.OUT_OF_RANGE; + case 429: + return Code.RESOURCE_EXHAUSTED; + case 499: + return Code.CANCELLED; + default: + return Code.FAILED_PRECONDITION; + } + } else if (500 <= httpStatus && httpStatus < 600) { + switch (httpStatus) { + case 501: + return Code.UNIMPLEMENTED; + case 503: + return Code.UNAVAILABLE; + case 504: + return Code.DEADLINE_EXCEEDED; + default: return Code.INTERNAL; - } - case 501: - return Code.UNIMPLEMENTED; - case 503: - return Code.UNAVAILABLE; - case 504: - return Code.DEADLINE_EXCEEDED; - default: - throw new IllegalArgumentException("Unrecognized http status code: " + httpStatus); + } } + return Code.UNKNOWN; } @Override diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java index 20561fe17..6ff331c2c 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshotTest.java @@ -67,7 +67,7 @@ public void newBuilderTestWithError() { .build(); assertEquals(testOperationSnapshot.getErrorMessage(), "Forbidden"); - assertEquals(testOperationSnapshot.getErrorCode(), HttpJsonStatusCode.of(403, "Forbidden")); + assertEquals(testOperationSnapshot.getErrorCode(), HttpJsonStatusCode.of(403)); assertTrue(testOperationSnapshot.isDone()); } diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java index 7117026e8..6d90d25d1 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java @@ -33,7 +33,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; -import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.StatusCode.Code; import java.util.Arrays; import java.util.HashSet; import java.util.Set; @@ -43,9 +43,9 @@ public class HttpJsonStatusCodeTest { @Test public void rpcCodeToStatusCodeTest() { - Set allCodes = new HashSet<>(); + Set allCodes = new HashSet<>(); for (com.google.rpc.Code rpcCode : com.google.rpc.Code.values()) { - StatusCode.Code statusCode; + Code statusCode; try { statusCode = HttpJsonStatusCode.rpcCodeToStatusCode(rpcCode); } catch (IllegalArgumentException e) { @@ -59,73 +59,30 @@ public void rpcCodeToStatusCodeTest() { allCodes.add(statusCode); } - assertEquals(allCodes, new HashSet<>(Arrays.asList(StatusCode.Code.values()))); + assertEquals(allCodes, new HashSet<>(Arrays.asList(Code.values()))); } @Test public void httpStatusToStatusCodeTest() { - // The HTTP status code conversion logic is currently in the process of being standardized, - // the tested logic may change in nearest future. - final String defaultMessage = "anything"; - assertEquals( - StatusCode.Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(200, defaultMessage)); - assertEquals( - StatusCode.Code.OUT_OF_RANGE, - HttpJsonStatusCode.httpStatusToStatusCode(400, HttpJsonStatusCode.OUT_OF_RANGE)); - assertEquals( - StatusCode.Code.FAILED_PRECONDITION, - HttpJsonStatusCode.httpStatusToStatusCode(400, HttpJsonStatusCode.FAILED_PRECONDITION)); - assertEquals( - StatusCode.Code.INVALID_ARGUMENT, - HttpJsonStatusCode.httpStatusToStatusCode(400, defaultMessage)); - assertEquals( - StatusCode.Code.UNAUTHENTICATED, - HttpJsonStatusCode.httpStatusToStatusCode(401, defaultMessage)); - assertEquals( - StatusCode.Code.PERMISSION_DENIED, - HttpJsonStatusCode.httpStatusToStatusCode(403, defaultMessage)); - assertEquals( - StatusCode.Code.NOT_FOUND, HttpJsonStatusCode.httpStatusToStatusCode(404, defaultMessage)); - assertEquals( - StatusCode.Code.ALREADY_EXISTS, - HttpJsonStatusCode.httpStatusToStatusCode(409, HttpJsonStatusCode.ALREADY_EXISTS)); - assertEquals( - StatusCode.Code.ABORTED, HttpJsonStatusCode.httpStatusToStatusCode(409, defaultMessage)); - assertEquals( - StatusCode.Code.RESOURCE_EXHAUSTED, - HttpJsonStatusCode.httpStatusToStatusCode(429, defaultMessage)); - assertEquals( - StatusCode.Code.CANCELLED, HttpJsonStatusCode.httpStatusToStatusCode(499, defaultMessage)); - assertEquals( - StatusCode.Code.DATA_LOSS, - HttpJsonStatusCode.httpStatusToStatusCode(500, HttpJsonStatusCode.DATA_LOSS)); - assertEquals( - StatusCode.Code.UNKNOWN, - HttpJsonStatusCode.httpStatusToStatusCode(500, HttpJsonStatusCode.UNKNOWN)); - assertEquals( - StatusCode.Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(500, defaultMessage)); - assertEquals( - StatusCode.Code.UNIMPLEMENTED, - HttpJsonStatusCode.httpStatusToStatusCode(501, defaultMessage)); - assertEquals( - StatusCode.Code.UNAVAILABLE, - HttpJsonStatusCode.httpStatusToStatusCode(503, defaultMessage)); - assertEquals( - StatusCode.Code.DEADLINE_EXCEEDED, - HttpJsonStatusCode.httpStatusToStatusCode(504, defaultMessage)); + assertEquals(Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(200)); + assertEquals(Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(201)); + assertEquals(Code.INVALID_ARGUMENT, HttpJsonStatusCode.httpStatusToStatusCode(400)); + assertEquals(Code.UNAUTHENTICATED, HttpJsonStatusCode.httpStatusToStatusCode(401)); + assertEquals(Code.PERMISSION_DENIED, HttpJsonStatusCode.httpStatusToStatusCode(403)); + assertEquals(Code.NOT_FOUND, HttpJsonStatusCode.httpStatusToStatusCode(404)); + assertEquals(Code.ABORTED, HttpJsonStatusCode.httpStatusToStatusCode(409)); + assertEquals(Code.OUT_OF_RANGE, HttpJsonStatusCode.httpStatusToStatusCode(416)); + assertEquals(Code.RESOURCE_EXHAUSTED, HttpJsonStatusCode.httpStatusToStatusCode(429)); + assertEquals(Code.CANCELLED, HttpJsonStatusCode.httpStatusToStatusCode(499)); + assertEquals(Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(500)); + assertEquals(Code.UNIMPLEMENTED, HttpJsonStatusCode.httpStatusToStatusCode(501)); + assertEquals(Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(502)); + assertEquals(Code.UNAVAILABLE, HttpJsonStatusCode.httpStatusToStatusCode(503)); + assertEquals(Code.DEADLINE_EXCEEDED, HttpJsonStatusCode.httpStatusToStatusCode(504)); - try { - HttpJsonStatusCode.httpStatusToStatusCode(411, defaultMessage); - fail(); - } catch (IllegalStateException e) { - // expected - } - - try { - HttpJsonStatusCode.httpStatusToStatusCode(666, defaultMessage); - fail(); - } catch (IllegalArgumentException e) { - // expected - } + assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(100)); + assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(300)); + assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(302)); + assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(600)); } } diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java index c6a68b661..d03d7e57f 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java @@ -29,6 +29,8 @@ */ package com.google.api.gax.httpjson; +import static org.junit.Assert.assertThrows; + import com.google.api.client.http.HttpHeaders; import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; @@ -53,7 +55,6 @@ import com.google.common.util.concurrent.UncheckedExecutionException; import java.util.Set; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -65,17 +66,13 @@ public class RetryingTest { @SuppressWarnings("unchecked") - private UnaryCallable callInt = Mockito.mock(UnaryCallable.class); + private final UnaryCallable callInt = Mockito.mock(UnaryCallable.class); private RecordingScheduler executor; private FakeApiClock fakeClock; private ClientContext clientContext; - private static int STATUS_SERVER_ERROR = 500; - private static int STATUS_DEADLINE_EXCEEDED = 504; - private static int STATUS_FAILED_PRECONDITION = 400; - - private static String DEADLINE_EXCEEDED = "DEADLINE_EXCEEDED"; + private static final int HTTP_CODE_PRECONDITION_FAILED = 412; private HttpResponseException HTTP_SERVICE_UNAVAILABLE_EXCEPTION = new HttpResponseException.Builder( @@ -112,17 +109,13 @@ public void teardown() { executor.shutdownNow(); } - static ApiFuture immediateFailedFuture(Throwable t) { - return ApiFutures.immediateFailedFuture(t); - } - @Test public void retry() { ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) .thenReturn(ApiFutures.immediateFuture(2)); UnaryCallSettings callSettings = @@ -132,7 +125,7 @@ public void retry() { Truth.assertThat(callable.call(1)).isEqualTo(2); } - @Test(expected = ApiException.class) + @Test public void retryTotalTimeoutExceeded() { ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); HttpResponseException httpResponseException = @@ -148,7 +141,7 @@ public void retryTotalTimeoutExceeded() { HttpJsonStatusCode.of(Code.FAILED_PRECONDITION), false); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(apiException)) + .thenReturn(ApiFutures.immediateFailedFuture(apiException)) .thenReturn(ApiFutures.immediateFuture(2)); RetrySettings retrySettings = @@ -160,30 +153,30 @@ public void retryTotalTimeoutExceeded() { UnaryCallSettings callSettings = createSettings(retryable, retrySettings); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - callable.call(1); + assertThrows(ApiException.class, () -> callable.call(1)); } - @Test(expected = ApiException.class) + @Test public void retryMaxAttemptsExceeded() { ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) .thenReturn(ApiFutures.immediateFuture(2)); RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(2).build(); UnaryCallSettings callSettings = createSettings(retryable, retrySettings); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - callable.call(1); + assertThrows(ApiException.class, () -> callable.call(1)); } @Test public void retryWithinMaxAttempts() { ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) - .thenReturn(RetryingTest.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) + .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) .thenReturn(ApiFutures.immediateFuture(2)); RetrySettings retrySettings = FAST_RETRY_SETTINGS.toBuilder().setMaxAttempts(3).build(); @@ -199,13 +192,14 @@ public void retryOnStatusUnknown() { ImmutableSet retryable = ImmutableSet.of(Code.UNKNOWN); HttpResponseException throwable = new HttpResponseException.Builder( - STATUS_SERVER_ERROR, "server unavailable", new HttpHeaders()) - .setMessage("UNKNOWN") + HttpStatusCodes.STATUS_CODE_TEMPORARY_REDIRECT, + "temporary redirect", + new HttpHeaders()) .build(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)) + .thenReturn(ApiFutures.immediateFailedFuture(throwable)) + .thenReturn(ApiFutures.immediateFailedFuture(throwable)) + .thenReturn(ApiFutures.immediateFailedFuture(throwable)) .thenReturn(ApiFutures.immediateFuture(2)); UnaryCallSettings callSettings = createSettings(retryable, FAST_RETRY_SETTINGS); @@ -219,24 +213,21 @@ public void retryOnUnexpectedException() { ImmutableSet retryable = ImmutableSet.of(Code.UNKNOWN); Throwable throwable = new RuntimeException("foobar"); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)); + .thenReturn(ApiFutures.immediateFailedFuture(throwable)); UnaryCallSettings callSettings = createSettings(retryable, FAST_RETRY_SETTINGS); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - try { - callable.call(1); - Assert.fail("Callable should have thrown an exception"); - } catch (ApiException expected) { - Truth.assertThat(expected).hasCauseThat().isSameInstanceAs(throwable); - } + ApiException exception = assertThrows(ApiException.class, () -> callable.call(1)); + Truth.assertThat(exception).hasCauseThat().isSameInstanceAs(throwable); } @Test public void retryNoRecover() { ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); HttpResponseException httpResponseException = - new HttpResponseException.Builder(STATUS_FAILED_PRECONDITION, "foobar", new HttpHeaders()) + new HttpResponseException.Builder( + HTTP_CODE_PRECONDITION_FAILED, "foobar", new HttpHeaders()) .build(); ApiException apiException = ApiExceptionFactory.createException( @@ -245,18 +236,14 @@ public void retryNoRecover() { HttpJsonStatusCode.of(Code.FAILED_PRECONDITION), false); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(apiException)) + .thenReturn(ApiFutures.immediateFailedFuture(apiException)) .thenReturn(ApiFutures.immediateFuture(2)); UnaryCallSettings callSettings = createSettings(retryable, FAST_RETRY_SETTINGS); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - try { - callable.call(1); - Assert.fail("Callable should have thrown an exception"); - } catch (ApiException expected) { - Truth.assertThat(expected).isSameInstanceAs(apiException); - } + ApiException exception = assertThrows(ApiException.class, () -> callable.call(1)); + Truth.assertThat(exception).isSameInstanceAs(apiException); } @Test @@ -267,7 +254,7 @@ public void retryKeepFailing() { HttpStatusCodes.STATUS_CODE_SERVICE_UNAVAILABLE, "Unavailable", new HttpHeaders()) .build(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)); + .thenReturn(ApiFutures.immediateFailedFuture(throwable)); UnaryCallSettings callSettings = createSettings(retryable, FAST_RETRY_SETTINGS); UnaryCallable callable = @@ -275,13 +262,10 @@ public void retryKeepFailing() { // Need to advance time inside the call. ApiFuture future = callable.futureCall(1); - try { - Futures.getUnchecked(future); - Assert.fail("Callable should have thrown an exception"); - } catch (UncheckedExecutionException expected) { - Truth.assertThat(expected).hasCauseThat().isInstanceOf(ApiException.class); - Truth.assertThat(expected).hasCauseThat().hasMessageThat().contains("Unavailable"); - } + UncheckedExecutionException exception = + assertThrows(UncheckedExecutionException.class, () -> Futures.getUnchecked(future)); + Truth.assertThat(exception).hasCauseThat().isInstanceOf(ApiException.class); + Truth.assertThat(exception).hasCauseThat().hasMessageThat().contains("Unavailable"); } @Test @@ -302,46 +286,37 @@ public void testKnownStatusCode() { + "}"; HttpResponseException throwable = new HttpResponseException.Builder( - STATUS_FAILED_PRECONDITION, - HttpJsonStatusCode.FAILED_PRECONDITION, - new HttpHeaders()) + HTTP_CODE_PRECONDITION_FAILED, "precondition failed", new HttpHeaders()) .setMessage(throwableMessage) .build(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(throwable)); + .thenReturn(ApiFutures.immediateFailedFuture(throwable)); UnaryCallSettings callSettings = UnaryCallSettings.newUnaryCallSettingsBuilder() .setRetryableCodes(retryable) .build(); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - try { - callable.call(1); - Assert.fail("Callable should have thrown an exception"); - } catch (FailedPreconditionException expected) { - Truth.assertThat(((HttpJsonStatusCode) expected.getStatusCode()).getTransportCode()) - .isEqualTo(STATUS_FAILED_PRECONDITION); - Truth.assertThat(expected.getMessage()).contains(HttpJsonStatusCode.FAILED_PRECONDITION); - } + ApiException exception = + assertThrows(FailedPreconditionException.class, () -> callable.call(1)); + Truth.assertThat(exception.getStatusCode().getTransportCode()) + .isEqualTo(HTTP_CODE_PRECONDITION_FAILED); + Truth.assertThat(exception).hasMessageThat().contains("precondition failed"); } @Test public void testUnknownStatusCode() { ImmutableSet retryable = ImmutableSet.of(); Mockito.when(callInt.futureCall((Integer) Mockito.any(), (ApiCallContext) Mockito.any())) - .thenReturn(RetryingTest.immediateFailedFuture(new RuntimeException("unknown"))); + .thenReturn(ApiFutures.immediateFailedFuture(new RuntimeException("unknown"))); UnaryCallSettings callSettings = UnaryCallSettings.newUnaryCallSettingsBuilder() .setRetryableCodes(retryable) .build(); UnaryCallable callable = HttpJsonCallableFactory.createUnaryCallable(callInt, callSettings, clientContext); - try { - callable.call(1); - Assert.fail("Callable should have thrown an exception"); - } catch (UnknownException expected) { - Truth.assertThat(expected.getMessage()).isEqualTo("java.lang.RuntimeException: unknown"); - } + UnknownException exception = assertThrows(UnknownException.class, () -> callable.call(1)); + Truth.assertThat(exception).hasMessageThat().isEqualTo("java.lang.RuntimeException: unknown"); } public static UnaryCallSettings createSettings( From 68bf8654f681f6f1d960fef21956942c730ceb15 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Mon, 22 Nov 2021 17:00:38 -0500 Subject: [PATCH 2/3] chore: rename parameter --- .../google/api/gax/httpjson/HttpJsonOperationSnapshot.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java index 358573fee..471cff04d 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonOperationSnapshot.java @@ -138,9 +138,9 @@ public Builder setResponse(Object response) { return this; } - public Builder setError(int errorCode, String errorMessage) { + public Builder setError(int httpStatus, String errorMessage) { this.errorCode = - errorCode == 0 ? HttpJsonStatusCode.of(Code.OK) : HttpJsonStatusCode.of(errorCode); + httpStatus == 0 ? HttpJsonStatusCode.of(Code.OK) : HttpJsonStatusCode.of(httpStatus); this.errorMessage = errorMessage; return this; } From 10cd6f296f3d455e9548d9fec87bdfbdcda5a5ab Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Tue, 23 Nov 2021 16:06:14 -0500 Subject: [PATCH 3/3] chore: enhance test --- .../gax/httpjson/HttpJsonStatusCodeTest.java | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java index 6d90d25d1..7c7664c3b 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonStatusCodeTest.java @@ -29,12 +29,10 @@ */ package com.google.api.gax.httpjson; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; import com.google.api.gax.rpc.StatusCode.Code; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; import org.junit.Test; @@ -55,34 +53,39 @@ public void rpcCodeToStatusCodeTest() { continue; } - assertNotNull(statusCode); + assertThat(statusCode).isNotNull(); allCodes.add(statusCode); } - assertEquals(allCodes, new HashSet<>(Arrays.asList(Code.values()))); + assertThat(Code.values()).asList().containsExactlyElementsIn(allCodes); } @Test public void httpStatusToStatusCodeTest() { - assertEquals(Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(200)); - assertEquals(Code.OK, HttpJsonStatusCode.httpStatusToStatusCode(201)); - assertEquals(Code.INVALID_ARGUMENT, HttpJsonStatusCode.httpStatusToStatusCode(400)); - assertEquals(Code.UNAUTHENTICATED, HttpJsonStatusCode.httpStatusToStatusCode(401)); - assertEquals(Code.PERMISSION_DENIED, HttpJsonStatusCode.httpStatusToStatusCode(403)); - assertEquals(Code.NOT_FOUND, HttpJsonStatusCode.httpStatusToStatusCode(404)); - assertEquals(Code.ABORTED, HttpJsonStatusCode.httpStatusToStatusCode(409)); - assertEquals(Code.OUT_OF_RANGE, HttpJsonStatusCode.httpStatusToStatusCode(416)); - assertEquals(Code.RESOURCE_EXHAUSTED, HttpJsonStatusCode.httpStatusToStatusCode(429)); - assertEquals(Code.CANCELLED, HttpJsonStatusCode.httpStatusToStatusCode(499)); - assertEquals(Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(500)); - assertEquals(Code.UNIMPLEMENTED, HttpJsonStatusCode.httpStatusToStatusCode(501)); - assertEquals(Code.INTERNAL, HttpJsonStatusCode.httpStatusToStatusCode(502)); - assertEquals(Code.UNAVAILABLE, HttpJsonStatusCode.httpStatusToStatusCode(503)); - assertEquals(Code.DEADLINE_EXCEEDED, HttpJsonStatusCode.httpStatusToStatusCode(504)); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(200)).isEqualTo(Code.OK); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(201)).isEqualTo(Code.OK); - assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(100)); - assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(300)); - assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(302)); - assertEquals(Code.UNKNOWN, HttpJsonStatusCode.httpStatusToStatusCode(600)); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(400)).isEqualTo(Code.INVALID_ARGUMENT); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(401)).isEqualTo(Code.UNAUTHENTICATED); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(403)).isEqualTo(Code.PERMISSION_DENIED); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(404)).isEqualTo(Code.NOT_FOUND); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(409)).isEqualTo(Code.ABORTED); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(416)).isEqualTo(Code.OUT_OF_RANGE); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(429)).isEqualTo(Code.RESOURCE_EXHAUSTED); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(499)).isEqualTo(Code.CANCELLED); + + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(405)).isEqualTo(Code.FAILED_PRECONDITION); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(408)).isEqualTo(Code.FAILED_PRECONDITION); + + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(500)).isEqualTo(Code.INTERNAL); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(501)).isEqualTo(Code.UNIMPLEMENTED); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(502)).isEqualTo(Code.INTERNAL); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(503)).isEqualTo(Code.UNAVAILABLE); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(504)).isEqualTo(Code.DEADLINE_EXCEEDED); + + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(100)).isEqualTo(Code.UNKNOWN); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(300)).isEqualTo(Code.UNKNOWN); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(302)).isEqualTo(Code.UNKNOWN); + assertThat(HttpJsonStatusCode.httpStatusToStatusCode(600)).isEqualTo(Code.UNKNOWN); } }