From f9b5929c75ec401d97558fa5b3d742b63d437b42 Mon Sep 17 00:00:00 2001 From: cmwigard Date: Mon, 1 May 2023 02:38:50 +0200 Subject: [PATCH 1/2] Streamlined the creation of GRPC Exceptions by adding convenience methods. Motivation: In reference to issue #2502, there should be a more convenient way to initialize GRPC Exceptions analogous to other GRPC implementations. Modifications: - Removed the deprecation from GrpcStatus.asException as it is required for this implementation of desired functionality. - Added additional constructor for GrpcStatus to support initialization from codeValue and description. - Added convenience method GrpcStatusCode.withDescription to support desired functionality. - Added GrpcStatusTest.testGrpcStatusExceptionFromGrpcStatusCode for test-driven development and better test coverage. Result: GRPC Exceptions can now be initialized and thrown more conveniently. What is the result of this change? --- .../io/servicetalk/grpc/api/GrpcStatus.java | 19 ++++++++++++++++--- .../servicetalk/grpc/api/GrpcStatusCode.java | 10 ++++++++++ .../servicetalk/grpc/api/GrpcStatusTest.java | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java index 1132293875..f2ed7d695c 100644 --- a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java +++ b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java @@ -124,6 +124,20 @@ public static GrpcStatus fromCodeValue(int codeValue) { new GrpcStatus(UNKNOWN, "Unknown code: " + codeValue) : INT_TO_GRPC_STATUS_MAP[codeValue]; } + /** + * Obtains the status given an integer code value and a description String. + * @param codeValue integer code value. + * @param description description for the given status. + * @return status associated with the code value, or {@link GrpcStatusCode#UNKNOWN}. + */ + public static GrpcStatus fromCodeValueAndDescription(int codeValue, String description) { + if (codeValue < 0 || codeValue >= INT_TO_GRPC_STATUS_MAP.length) { + return new GrpcStatus(UNKNOWN, "Unknown code: " + codeValue); + } else { + return new GrpcStatus(GrpcStatusCode.fromCodeValue(codeValue), description); + } + } + /** * Translates a throwable into a status. * @@ -152,14 +166,13 @@ public static GrpcStatus fromThrowableNullable(Throwable t) { // FIXME: 0.43 - r return exception == null ? null : exception.status(); } + /** * Returns the current status wrapped in a {@link GrpcStatusException}. * * @return the current status wrapped in a {@link GrpcStatusException}. - * @deprecated Use {@link GrpcStatusException#GrpcStatusException(GrpcStatus)}. */ - @Deprecated - public GrpcStatusException asException() { // FIXME: 0.43 - remove deprecated method + public GrpcStatusException asException() { return new GrpcStatusException(this, () -> null); } diff --git a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatusCode.java b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatusCode.java index 99beca234f..9a362c094e 100644 --- a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatusCode.java +++ b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatusCode.java @@ -164,4 +164,14 @@ public static GrpcStatusCode fromHttp2ErrorCode(Http2ErrorCode errorCode) { return h2ErrorCode < 0 || h2ErrorCode >= H2_ERROR_TO_STATUS_CODE_MAP.length ? UNKNOWN : H2_ERROR_TO_STATUS_CODE_MAP[h2ErrorCode]; } + + /** + * Returns a standard {@link GrpcStatus} with description as part of a {@link GrpcStatusException} builder pattern. + * @param description the Description for pending {@link GrpcStatusException}. + * @return the GrpcStatus with set description + */ + public GrpcStatus withDescription(String description) { + GrpcStatus grpcStatus = GrpcStatus.fromCodeValueAndDescription(this.value(), description); + return grpcStatus; + } } diff --git a/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java b/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java index b4d7b5fbb1..b0c3a84bf1 100644 --- a/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java +++ b/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java @@ -16,8 +16,11 @@ package io.servicetalk.grpc.api; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestClassOrder; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class GrpcStatusTest { @@ -56,4 +59,16 @@ void testGrpcStatusUnknownFromUnparsableStringCodeValue() { assertEquals(GrpcStatusCode.UNKNOWN, grpcStatus.code()); assertEquals("Status code value not a number: " + unknownCode, grpcStatus.description()); } + + @Test + void testGrpcStatusExceptionFromGrpcStatusCode() { + final String exceptionMessage = "denied!"; + GrpcStatusException grpcStatusException = GrpcStatusCode.PERMISSION_DENIED.withDescription(exceptionMessage).asException(); + Exception thrownGrpcStatusException = assertThrows(GrpcStatusException.class, () -> { + throw grpcStatusException; + }); + final String expectedExceptionMessage = exceptionMessage; + final String actualMessage = thrownGrpcStatusException.getMessage(); + assertTrue(actualMessage.contains(expectedExceptionMessage)); + } } From 0302d3236470bb8655d89a1d15f09d7c6fe3db9a Mon Sep 17 00:00:00 2001 From: cmwigard Date: Mon, 1 May 2023 02:53:57 +0200 Subject: [PATCH 2/2] Fixed minor unwanted formatting and import errors. Motivation: Cleaner code and commit Modifications: - Removed an unnecessary empty line from GrpcStatus as well as an unused import from GrpcStatusTest Result: Cleaned up for pull request --- .../src/main/java/io/servicetalk/grpc/api/GrpcStatus.java | 1 - .../src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java | 1 - 2 files changed, 2 deletions(-) diff --git a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java index f2ed7d695c..7eee1bc8b5 100644 --- a/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java +++ b/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatus.java @@ -166,7 +166,6 @@ public static GrpcStatus fromThrowableNullable(Throwable t) { // FIXME: 0.43 - r return exception == null ? null : exception.status(); } - /** * Returns the current status wrapped in a {@link GrpcStatusException}. * diff --git a/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java b/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java index b0c3a84bf1..8c3a76ca43 100644 --- a/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java +++ b/servicetalk-grpc-api/src/test/java/io/servicetalk/grpc/api/GrpcStatusTest.java @@ -16,7 +16,6 @@ package io.servicetalk.grpc.api; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestClassOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows;