From 00fc79f2125d5d73eabfba70aa2c5e98fd8055a6 Mon Sep 17 00:00:00 2001 From: SheetalAtre Date: Fri, 11 Aug 2023 09:39:26 +0530 Subject: [PATCH] refactor(retrofit/test): revert previous commit for tests --- .../exceptions/SpinnakerHttpException.java | 9 +--- .../SpinnakerHttpExceptionTest.java | 45 ++++++++++--------- .../SpinnakerServerExceptionTest.java | 18 -------- 3 files changed, 26 insertions(+), 46 deletions(-) diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java index 0fc5b06c5..7e9a46c2d 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java @@ -153,18 +153,11 @@ public String getMessage() { } if (retrofit2Response != null) { - // If there's no message derived from a response, get the specified message. - // It feels a little backwards to do it this way, but super.getMessage() - // always returns something whether there's a specified message or not, so - // look at getRawMessage instead. - if (getRawMessage() == null) { - return super.getMessage(); - } return String.format( "Status: %s, URL: %s, Message: %s", retrofit2Response.code(), retrofit2Response.raw().request().url().toString(), - retrofit2Response.code() + " " + retrofit2Response.message()); + getRawMessage()); } else { return String.format( "Status: %s, URL: %s, Message: %s", diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java index 0f965b740..d8e8e1f07 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java @@ -12,25 +12,32 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * */ package com.netflix.spinnaker.kork.retrofit.exceptions; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import java.util.List; import java.util.Map; import okhttp3.MediaType; import okhttp3.ResponseBody; import org.junit.jupiter.api.Test; import org.springframework.http.HttpStatus; +import retrofit.RetrofitError; +import retrofit.client.Response; import retrofit2.Retrofit; import retrofit2.converter.jackson.JacksonConverterFactory; public class SpinnakerHttpExceptionTest { + private static final String CUSTOM_MESSAGE = "custom message"; @Test - public void testSpinnakerHttpException() { + public void testSpinnakerHttpExceptionFromRetrofitException() { final String validJsonResponseBodyString = "{\"name\":\"test\"}"; ResponseBody responseBody = ResponseBody.create( @@ -49,25 +56,23 @@ public void testSpinnakerHttpException() { Map errorResponseBody = notFoundException.getResponseBody(); assertEquals(errorResponseBody.get("name"), "test"); assertEquals(HttpStatus.NOT_FOUND.value(), notFoundException.getResponseCode()); + assertTrue( + notFoundException.getMessage().contains(String.valueOf(HttpStatus.NOT_FOUND.value()))); + } + + @Test + public void testSpinnakerHttpException_NewInstance() { + Response response = new Response("http://localhost", 200, "reason", List.of(), null); + try { + RetrofitError error = RetrofitError.httpError("http://localhost", response, null, null); + throw new SpinnakerHttpException(error); + } catch (SpinnakerException e) { + SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - // Note: - // * If error response body has property "message" then return its value: - // eg: if errorResponseBody = {"message":"test"}, then expectedMessage = "Status: 404, URL: - // http://localhost/, Message: test" - // * If error response body does not have any property "message", then return default error - // message: - // eg: if errorResponseBody = {"name":"test"}, then expectedMessage = "Status: 404, URL: - // http://localhost/, Message: 404 Response.error()" - // * TODO: If error response body is an invalid json then the converter fails to convert it and - // returns error message: - // eg: if errorResponseBody = {"name":"test",(no closing bracket at end), then expectedMessage - // ="Failed to parse response". - String expectedMessage = - String.format( - "Status: %s, URL: %s, Message: %s", - HttpStatus.NOT_FOUND.value(), - "http://localhost/", - HttpStatus.NOT_FOUND.value() + " " + "Response.error()"); - assertEquals(expectedMessage, notFoundException.getMessage()); + assertTrue(newException instanceof SpinnakerHttpException); + assertEquals(CUSTOM_MESSAGE, newException.getMessage()); + assertEquals(e, newException.getCause()); + assertEquals(response.getStatus(), ((SpinnakerHttpException) newException).getResponseCode()); + } } } diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java index 9b74866a4..a9b764a93 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java @@ -21,30 +21,12 @@ import com.netflix.spinnaker.kork.exceptions.SpinnakerException; import java.io.IOException; -import java.util.List; import org.junit.jupiter.api.Test; import retrofit.RetrofitError; -import retrofit.client.Response; public class SpinnakerServerExceptionTest { private static final String CUSTOM_MESSAGE = "custom message"; - @Test - public void testSpinnakerHttpException_NewInstance() { - Response response = new Response("http://localhost", 200, "reason", List.of(), null); - try { - RetrofitError error = RetrofitError.httpError("http://localhost", response, null, null); - throw new SpinnakerHttpException(error); - } catch (SpinnakerException e) { - SpinnakerException newException = e.newInstance(CUSTOM_MESSAGE); - - assertTrue(newException instanceof SpinnakerHttpException); - assertEquals(CUSTOM_MESSAGE, newException.getMessage()); - assertEquals(e, newException.getCause()); - assertEquals(response.getStatus(), ((SpinnakerHttpException) newException).getResponseCode()); - } - } - @Test public void testSpinnakerNetworkException_NewInstance() { IOException initialException = new IOException("message");