refactor(retrofit/test): rebase with master branch

spinnaker · Aug 11, 2023 · b4d1b59 · b4d1b59
1 parent 63e8a6e
commit b4d1b59
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 63 deletions.
diff --git a/.../src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java b/.../src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java
@@ -50,9 +50,6 @@ public class SpinnakerHttpException extends SpinnakerServerException {
 
  private final Map<String, Object> responseBody;
  private final retrofit2.Retrofit retrofit;
- private final String retrofit2DefaultResponseError = "Response.error()";
- private final String errorMessageFormat = "Status: %s, URL: %s, Message: %s";
- private final String messageKey = "message";
 
  public SpinnakerHttpException(RetrofitError e) {
  super(e);
@@ -108,12 +105,15 @@ public SpinnakerHttpException(
  this.retrofit2Response = retrofit2Response;
  this.response = null;
  this.retrofit = retrofit;
- this.rawMessage = null;
  if ((retrofit2Response.code() == HttpStatus.NOT_FOUND.value())
  || (retrofit2Response.code() == HttpStatus.BAD_REQUEST.value())) {
  setRetryable(false);
  }
  responseBody = (Map<String, Object>) this.getErrorBodyAs(HashMap.class);
+ this.rawMessage =
+ responseBody != null
+ ? (String) responseBody.getOrDefault("message", getMessage())
+ : getMessage();
  }
 
  public int getResponseCode() {
@@ -144,32 +144,24 @@ public HttpHeaders getHeaders() {
 
  @Override
  public String getMessage() {
- if (retrofit2Response != null) {
- String defaultMessage = retrofit2Response.code() + " " + retrofit2Response.message();
- String msg =
- responseBody != null
- ? (String) responseBody.getOrDefault(messageKey, defaultMessage)
- : defaultMessage;
-
- if (!retrofit2DefaultResponseError.equals(retrofit2Response.message())) {
- msg = defaultMessage; // means message property is not set in the error response body
- }
+ // 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();
+ }
 
+ if (retrofit2Response != null) {
  return String.format(
- errorMessageFormat,
+ "Status: %s, URL: %s, Message: %s",
  retrofit2Response.code(),
- retrofit2Response.raw().request().url(),
- msg);
+ retrofit2Response.raw().request().url().toString(),
+ getRawMessage());
  } else {
- // 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(
- errorMessageFormat, response.getStatus(), response.getUrl(), getRawMessage());
+ "Status: %s, URL: %s, Message: %s",
+ response.getStatus(), response.getUrl(), getRawMessage());
  }
  }
 

diff --git a/.../test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpExceptionTest.java b/.../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<String, Object> 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/...est/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerServerExceptionTest.java b/...est/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");