From f3d31ad11ea38741a212c91876203ce5ad2cf3c6 Mon Sep 17 00:00:00 2001 From: pkryshtop Date: Mon, 4 Aug 2025 16:22:59 +0300 Subject: [PATCH] feat: set requestId if not provided, log requestId on low level exceptions --- .../api/v2/client/ClientFactory.java | 13 +- .../api/v2/client/DefaultRequestIdFilter.java | 23 ++ .../exception/RestApiExceptionHandler.java | 8 +- .../exception/RestApiRuntimeException.java | 32 ++- .../api/v2/client/request/RequestContext.java | 24 ++ .../client/request/RequestContextFilter.java | 14 ++ .../client/request/RequestContextHolder.java | 21 ++ .../RequestContextInvocationHandler.java | 25 ++ .../v2/client/DefaultRequestIdFilterTest.java | 91 +++++++ .../v2/client/RequestIdIntegrationTest.java | 225 ++++++++++++++++++ .../RestApiExceptionHandlerTest.java | 78 ++++-- .../RestApiRuntimeExceptionTest.java | 88 +++++-- .../request/RequestContextFilterTest.java | 48 ++++ .../RequestContextInvocationHandlerTest.java | 88 +++++++ 14 files changed, 728 insertions(+), 50 deletions(-) create mode 100644 smartling-api-commons/src/main/java/com/smartling/api/v2/client/DefaultRequestIdFilter.java create mode 100644 smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContext.java create mode 100644 smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextFilter.java create mode 100644 smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextHolder.java create mode 100644 smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextInvocationHandler.java create mode 100644 smartling-api-commons/src/test/java/com/smartling/api/v2/client/DefaultRequestIdFilterTest.java create mode 100644 smartling-api-commons/src/test/java/com/smartling/api/v2/client/RequestIdIntegrationTest.java create mode 100644 smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextFilterTest.java create mode 100644 smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextInvocationHandlerTest.java diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/ClientFactory.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/ClientFactory.java index fc0238d8..59a514b3 100644 --- a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/ClientFactory.java +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/ClientFactory.java @@ -7,6 +7,8 @@ import com.smartling.api.v2.client.exception.DefaultRestApiExceptionMapper; import com.smartling.api.v2.client.exception.RestApiExceptionHandler; import com.smartling.api.v2.client.exception.RestApiExceptionMapper; +import com.smartling.api.v2.client.request.RequestContextFilter; +import com.smartling.api.v2.client.request.RequestContextInvocationHandler; import com.smartling.api.v2.client.unmarshal.DetailsDeserializer; import com.smartling.api.v2.client.unmarshal.RestApiContextResolver; import com.smartling.api.v2.client.unmarshal.RestApiResponseReaderInterceptor; @@ -40,6 +42,7 @@ import javax.ws.rs.client.ClientResponseFilter; import javax.ws.rs.ext.ContextResolver; import javax.ws.rs.ext.ReaderInterceptor; +import java.lang.reflect.InvocationHandler; import java.lang.reflect.Proxy; import java.util.Collections; import java.util.HashMap; @@ -252,11 +255,15 @@ T build( for (final ClientResponseFilter filter : clientResponseFilters) client.register(filter); + client.register(new DefaultRequestIdFilter()); + client.register(new RequestContextFilter()); + final T proxy = client.proxy(klass); final RestApiExceptionHandler exceptionHandler = new RestApiExceptionHandler(exceptionMapper != null ? exceptionMapper : new DefaultRestApiExceptionMapper()); - final ExceptionDecoratorInvocationHandler handler = new ExceptionDecoratorInvocationHandler<>(proxy, exceptionHandler); - final CloseClientInvocationHandler closeClientInvocationHandler = new CloseClientInvocationHandler(handler, client.getResteasyClient()); + InvocationHandler handler = new ExceptionDecoratorInvocationHandler<>(proxy, exceptionHandler); + handler = new CloseClientInvocationHandler(handler, client.getResteasyClient()); + handler = new RequestContextInvocationHandler(handler); - return (T) Proxy.newProxyInstance(klass.getClassLoader(), new Class[] { klass }, closeClientInvocationHandler); + return (T) Proxy.newProxyInstance(klass.getClassLoader(), new Class[] { klass }, handler); } } diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/DefaultRequestIdFilter.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/DefaultRequestIdFilter.java new file mode 100644 index 00000000..10955219 --- /dev/null +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/DefaultRequestIdFilter.java @@ -0,0 +1,23 @@ +package com.smartling.api.v2.client; + +import org.apache.commons.lang3.StringUtils; + +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.client.ClientRequestFilter; +import java.io.IOException; +import java.util.UUID; + +public class DefaultRequestIdFilter implements ClientRequestFilter +{ + private static final String REQUEST_ID_HEADER = "X-SL-RequestId"; + + @Override + public void filter(ClientRequestContext requestContext) throws IOException + { + String requestId = requestContext.getHeaderString(REQUEST_ID_HEADER); + if (StringUtils.isBlank(requestId)) + { + requestContext.getHeaders().addFirst(REQUEST_ID_HEADER, UUID.randomUUID().toString()); + } + } +} diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiExceptionHandler.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiExceptionHandler.java index 1ac540a0..b8c2b790 100644 --- a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiExceptionHandler.java +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiExceptionHandler.java @@ -1,10 +1,12 @@ package com.smartling.api.v2.client.exception; +import com.smartling.api.v2.client.request.RequestContextHolder; import com.smartling.api.v2.response.ErrorResponse; import lombok.extern.slf4j.Slf4j; import javax.ws.rs.ProcessingException; import javax.ws.rs.WebApplicationException; +import javax.ws.rs.client.ResponseProcessingException; import javax.ws.rs.core.Response; import java.lang.reflect.InvocationTargetException; @@ -39,9 +41,13 @@ else if (throwable instanceof ProcessingException && throwable.getCause() instan { restApiRuntimeException = (RestApiRuntimeException)throwable.getCause(); } + else if (throwable instanceof ResponseProcessingException) + { + restApiRuntimeException = new RestApiRuntimeException(throwable, ((ResponseProcessingException) throwable).getResponse(), null); + } else { - restApiRuntimeException = new RestApiRuntimeException(throwable); + restApiRuntimeException = new RestApiRuntimeException(throwable, RequestContextHolder.getContext()); } restApiRuntimeException.setErrorDetails(errorDetails); diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiRuntimeException.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiRuntimeException.java index 7daeb6e8..f6bfa52e 100644 --- a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiRuntimeException.java +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/exception/RestApiRuntimeException.java @@ -1,14 +1,15 @@ package com.smartling.api.v2.client.exception; +import com.smartling.api.v2.client.request.RequestContext; import com.smartling.api.v2.response.Error; import com.smartling.api.v2.response.ErrorResponse; import com.smartling.api.v2.response.ResponseCode; import lombok.extern.slf4j.Slf4j; -import java.util.Collections; -import java.util.List; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; +import java.util.Collections; +import java.util.List; @Slf4j public class RestApiRuntimeException extends WebApplicationException @@ -25,6 +26,12 @@ public RestApiRuntimeException(final Throwable cause) this.errorResponse = null; } + public RestApiRuntimeException(final Throwable cause, RequestContext requestContext) + { + super(cause, buildErrorResponse(requestContext)); + this.errorResponse = null; + } + public RestApiRuntimeException(final Throwable cause, final Response response, final ErrorResponse errorResponse) { super(cause, response); @@ -37,11 +44,7 @@ public void setErrorDetails(String errorDetails) { public int getStatus() { - final Response response = getResponse(); - if (response == null) - return 500; - - return response.getStatus(); + return getResponse().getStatus(); } public ResponseCode getResponseCode() @@ -64,7 +67,7 @@ public List getErrors() @Override public String getMessage() { - final String requestId = getResponse().getHeaderString(REQUEST_ID_HEADER); + String requestId = getResponse().getHeaderString(REQUEST_ID_HEADER); final StringBuilder errorMessage = new StringBuilder(); @@ -89,4 +92,17 @@ public String getMessage() return errorMessage.toString(); } + + private static Response buildErrorResponse(RequestContext requestContext) + { + if (requestContext != null && requestContext.getHeaders() != null) + { + return Response + .status(Response.Status.INTERNAL_SERVER_ERROR) + .header(REQUEST_ID_HEADER, requestContext.getHeaders().getFirst(REQUEST_ID_HEADER)) + .build(); + } + + return Response.serverError().build(); + } } diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContext.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContext.java new file mode 100644 index 00000000..9e56d97e --- /dev/null +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContext.java @@ -0,0 +1,24 @@ +package com.smartling.api.v2.client.request; + +import lombok.Data; +import org.jboss.resteasy.specimpl.MultivaluedMapImpl; + +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.core.MultivaluedMap; +import java.net.URI; + +@Data +public class RequestContext +{ + private final String method; + private final URI uri; + private final MultivaluedMap headers; + + public static RequestContext fromClientRequestContext(ClientRequestContext context) + { + MultivaluedMap headers = new MultivaluedMapImpl<>(); + headers.putAll(context.getHeaders()); + + return new RequestContext(context.getMethod(), context.getUri(), headers); + } +} diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextFilter.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextFilter.java new file mode 100644 index 00000000..29daeb2f --- /dev/null +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextFilter.java @@ -0,0 +1,14 @@ +package com.smartling.api.v2.client.request; + +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.client.ClientRequestFilter; +import java.io.IOException; + +public class RequestContextFilter implements ClientRequestFilter +{ + @Override + public void filter(ClientRequestContext clientRequestContext) throws IOException + { + RequestContextHolder.setContext(RequestContext.fromClientRequestContext(clientRequestContext)); + } +} diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextHolder.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextHolder.java new file mode 100644 index 00000000..816bf187 --- /dev/null +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextHolder.java @@ -0,0 +1,21 @@ +package com.smartling.api.v2.client.request; + +public class RequestContextHolder +{ + private static final ThreadLocal REQUEST_CONTEXT = new ThreadLocal<>(); + + public static void setContext(RequestContext requestContext) + { + REQUEST_CONTEXT.set(requestContext); + } + + public static RequestContext getContext() + { + return REQUEST_CONTEXT.get(); + } + + public static void clearContext() + { + REQUEST_CONTEXT.remove(); + } +} diff --git a/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextInvocationHandler.java b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextInvocationHandler.java new file mode 100644 index 00000000..fda3484e --- /dev/null +++ b/smartling-api-commons/src/main/java/com/smartling/api/v2/client/request/RequestContextInvocationHandler.java @@ -0,0 +1,25 @@ +package com.smartling.api.v2.client.request; + +import lombok.RequiredArgsConstructor; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; + +@RequiredArgsConstructor +public class RequestContextInvocationHandler implements InvocationHandler +{ + private final InvocationHandler delegate; + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable + { + try + { + return delegate.invoke(proxy, method, args); + } + finally + { + RequestContextHolder.clearContext(); + } + } +} diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/DefaultRequestIdFilterTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/DefaultRequestIdFilterTest.java new file mode 100644 index 00000000..eecf9279 --- /dev/null +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/DefaultRequestIdFilterTest.java @@ -0,0 +1,91 @@ +package com.smartling.api.v2.client; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.core.MultivaluedMap; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class DefaultRequestIdFilterTest +{ + private static final String REQUEST_ID_HEADER = "X-SL-RequestId"; + + @Mock + private ClientRequestContext requestContext; + + @Mock + private MultivaluedMap headers; + + private final DefaultRequestIdFilter filter = new DefaultRequestIdFilter(); + + @Before + public void setUp() + { + when(requestContext.getHeaders()).thenReturn(headers); + } + + @Test + public void shouldNotAddHeaderWhenRequestIdHeaderExists() throws Exception + { + // Given + when(requestContext.getHeaderString(REQUEST_ID_HEADER)).thenReturn("existing-request-id"); + + // When + filter.filter(requestContext); + + // Then + verifyNoMoreInteractions(headers); + } + + @Test + public void shouldAddHeaderWhenRequestIdHeaderDoesNotExist() throws Exception + { + // Given + when(requestContext.getHeaderString(REQUEST_ID_HEADER)).thenReturn(null); + + // When + filter.filter(requestContext); + + // Then + verify(headers).addFirst(eq(REQUEST_ID_HEADER), any()); + verifyNoMoreInteractions(headers); + } + + @Test + public void shouldAddHeaderWhenRequestIdHeaderIsEmpty() throws Exception + { + // Given + when(requestContext.getHeaderString(REQUEST_ID_HEADER)).thenReturn(""); + + // When + filter.filter(requestContext); + + // Then + verify(headers).addFirst(eq(REQUEST_ID_HEADER), any()); + verifyNoMoreInteractions(headers); + } + + @Test + public void shouldAddHeaderWhenRequestIdHeaderIsWhitespace() throws Exception + { + // Given + when(requestContext.getHeaderString(REQUEST_ID_HEADER)).thenReturn(" "); + + // When + filter.filter(requestContext); + + // Then + verify(headers).addFirst(eq(REQUEST_ID_HEADER), any()); + verifyNoMoreInteractions(headers); + } +} diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/RequestIdIntegrationTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/RequestIdIntegrationTest.java new file mode 100644 index 00000000..789631d3 --- /dev/null +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/RequestIdIntegrationTest.java @@ -0,0 +1,225 @@ +package com.smartling.api.v2.client; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.smartling.api.v2.client.auth.BearerAuthStaticTokenFilter; +import com.smartling.api.v2.client.exception.RestApiRuntimeException; +import com.smartling.api.v2.client.exception.client.ValidationErrorException; +import lombok.Data; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.ProcessingException; +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.client.ClientRequestFilter; +import javax.ws.rs.client.ResponseProcessingException; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.Arrays; +import java.util.Collections; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.matching; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; +import static java.lang.String.format; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class RequestIdIntegrationTest +{ + private static final String EXTERNAL_REQUEST_ID = "external-request-id"; + private static final String DEFAULT_REQUEST_ID_PATTERN = "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"; + + private interface TestTimeoutApi + { + @GET + @Path("/test/success") + String getQuickResponse(); + + @GET + @Path("/test/error") + String getErrorResponse(); + + @GET + @Path("/test/processing-error") + DTO getProcessingErrorResponse(); + + @GET + @Path("/test/timeout") + String getSlowResponse(); + + @Data + class DTO + { + private String data; + } + } + + @Rule + public WireMockRule wireMockRule = new WireMockRule(0); + + private TestTimeoutApi testApi; + + @Before + public void setUp() throws IOException + { + stubFor(get(urlEqualTo("/test/success")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"response\":{\"code\":\"SUCCESS\",\"data\":\"slow-response\"}}"))); + + stubFor(get(urlEqualTo("/test/error")) + .willReturn(aResponse() + .withStatus(400) + .withHeader("X-SL-RequestId", EXTERNAL_REQUEST_ID) + .withHeader("Content-Type", "application/json") + .withBody("{\"response\":{\"code\":\"VALIDATION_ERROR\",\"errors\":[{\"message\":\"Bad request\"}]}}"))); + + stubFor(get(urlEqualTo("/test/processing-error")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("X-SL-RequestId", EXTERNAL_REQUEST_ID) + .withHeader("Content-Type", "application/json") + .withBody("{\"response\":{\"code\":\"SUCCESS\",\"data\":\"raw string\"}}"))); + + stubFor(get(urlEqualTo("/test/timeout")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"response\":{\"code\":\"SUCCESS\",\"data\":\"slow-response\"}}") + .withFixedDelay(2000))); + } + + @Test + public void shouldSendDefaultRequestId() + { + testApi = new ClientFactory().build( + Collections.singletonList(new BearerAuthStaticTokenFilter("test-token")), + Collections.emptyList(), + "http://localhost:" + wireMockRule.port(), + TestTimeoutApi.class, + new HttpClientConfiguration(), + null, + null + ); + + testApi.getQuickResponse(); + + verify(getRequestedFor(urlEqualTo("/test/success")) + .withHeader("X-SL-RequestId", matching(DEFAULT_REQUEST_ID_PATTERN))); + } + + @Test + public void shouldSendRequestId() + { + testApi = new ClientFactory().build( + Arrays.asList(new BearerAuthStaticTokenFilter("test-token"), new ExternalRequestIdFilter()), + Collections.emptyList(), + "http://localhost:" + wireMockRule.port(), + TestTimeoutApi.class, + new HttpClientConfiguration(), + null, + null + ); + + testApi.getQuickResponse(); + + verify(getRequestedFor(urlEqualTo("/test/success")) + .withHeader("X-SL-RequestId", equalTo(EXTERNAL_REQUEST_ID))); + } + + @Test + public void shouldUseRequestIdFromErrorResponse() + { + testApi = new ClientFactory().build( + Collections.singletonList(new BearerAuthStaticTokenFilter("test-token")), + Collections.emptyList(), + "http://localhost:" + wireMockRule.port(), + TestTimeoutApi.class, + new HttpClientConfiguration(), + null, + null + ); + + try + { + testApi.getErrorResponse(); + fail("Expected 400 error"); + } + catch (RestApiRuntimeException e) + { + String expectedMessage = format("http_status=400, requestId=%s, top errors: 'Bad request'", EXTERNAL_REQUEST_ID); + assertEquals(expectedMessage, e.getMessage()); + assertEquals(ValidationErrorException.class, e.getClass()); + } + } + + @Test + public void shouldUseRequestIdFromErrorResponseOnBodyReadFailure() + { + testApi = new ClientFactory().build( + Collections.singletonList(new BearerAuthStaticTokenFilter("test-token")), + Collections.emptyList(), + "http://localhost:" + wireMockRule.port(), + TestTimeoutApi.class, + new HttpClientConfiguration(), + null, + null + ); + + try + { + testApi.getProcessingErrorResponse(); + fail("Expected deserialization error"); + } + catch (RestApiRuntimeException e) + { + String expectedMessage = format("http_status=200, requestId=%s", EXTERNAL_REQUEST_ID); + assertEquals(expectedMessage, e.getMessage()); + assertEquals(ResponseProcessingException.class, e.getCause().getClass()); + } + } + + @Test + public void shouldPreservesRequestIdOnReadTimeout() + { + testApi = new ClientFactory().build( + Arrays.asList(new BearerAuthStaticTokenFilter("test-token"), new ExternalRequestIdFilter()), + Collections.emptyList(), + "http://localhost:" + wireMockRule.port(), + TestTimeoutApi.class, + new HttpClientConfiguration().setSocketTimeout(1000), + null, + null + ); + + try + { + testApi.getSlowResponse(); + fail("Expected timeout exception"); + } + catch (RestApiRuntimeException e) + { + assertEquals("http_status=500, requestId=" + EXTERNAL_REQUEST_ID, e.getMessage()); + assertEquals(ProcessingException.class, e.getCause().getClass()); + assertEquals(SocketTimeoutException.class, e.getCause().getCause().getClass()); + } + } + + private static class ExternalRequestIdFilter implements ClientRequestFilter + { + @Override + public void filter(ClientRequestContext requestContext) + { + requestContext.getHeaders().add("X-SL-RequestId", EXTERNAL_REQUEST_ID); + } + } +} diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiExceptionHandlerTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiExceptionHandlerTest.java index 34a6372a..0384df77 100644 --- a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiExceptionHandlerTest.java +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiExceptionHandlerTest.java @@ -1,13 +1,20 @@ package com.smartling.api.v2.client.exception; +import com.smartling.api.v2.client.request.RequestContext; +import com.smartling.api.v2.client.request.RequestContextHolder; import com.smartling.api.v2.response.ErrorResponse; +import org.jboss.resteasy.specimpl.MultivaluedMapImpl; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.junit.MockitoJUnitRunner; import javax.ws.rs.ProcessingException; import javax.ws.rs.WebApplicationException; +import javax.ws.rs.client.ResponseProcessingException; +import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import java.lang.reflect.InvocationTargetException; import java.util.Collections; @@ -23,6 +30,7 @@ import static org.mockito.Mockito.when; +@RunWith(MockitoJUnitRunner.class) public class RestApiExceptionHandlerTest { private static final int FAILURE_STATUS = 500; @@ -36,22 +44,20 @@ public class RestApiExceptionHandlerTest @Mock private Response response; + @InjectMocks private RestApiExceptionHandler handler; @Before public void setUp() throws Exception { - MockitoAnnotations.initMocks(this); - when(exceptionMapper.toException((Throwable) any(), (Response) any(), (ErrorResponse) any())) - .thenReturn(restApiRuntimeException); + when(exceptionMapper.toException(any(), any(), any())).thenReturn(restApiRuntimeException); when(restApiRuntimeException.getStatus()).thenReturn(FAILURE_STATUS); when(restApiRuntimeException.getCause()).thenReturn(cause); when(response.getStatusInfo()).thenReturn(mock(Response.StatusType.class)); - handler = new RestApiExceptionHandler(exceptionMapper); } @Test - public void testCreateRestApiExceptionRuntimeException() throws Exception + public void testCreateRestApiExceptionRuntimeException() { final RuntimeException ex = new RuntimeException(); @@ -64,7 +70,29 @@ public void testCreateRestApiExceptionRuntimeException() throws Exception } @Test - public void testCreateRestApiExceptionInvocationTargetException() throws Exception + public void testCreateRestApiExceptionRuntimeExceptionWhenRequestContextAvailable() + { + MultivaluedMap requestHeaders = new MultivaluedMapImpl<>(); + requestHeaders.putSingle("X-SL-RequestId", "test-request-id"); + + RequestContext requestContext = mock(RequestContext.class); + when(requestContext.getHeaders()).thenReturn(requestHeaders); + RequestContextHolder.setContext(requestContext); + + final RuntimeException ex = new RuntimeException(); + + final RestApiRuntimeException restApiRuntimeException = handler.createRestApiException(ex); + assertEquals(ex, restApiRuntimeException.getCause()); + assertEquals(500, restApiRuntimeException.getStatus()); + assertEquals("http_status=500, requestId=test-request-id", restApiRuntimeException.getMessage()); + assertEquals(Collections.emptyList(), restApiRuntimeException.getErrors()); + verify(exceptionMapper, never()).toException(any(Throwable.class), any(Response.class), any(ErrorResponse.class)); + + RequestContextHolder.clearContext(); + } + + @Test + public void testCreateRestApiExceptionInvocationTargetException() { final RuntimeException ex = new RuntimeException(); final InvocationTargetException invocationTargetException = new InvocationTargetException(ex); @@ -78,7 +106,7 @@ public void testCreateRestApiExceptionInvocationTargetException() throws Excepti } @Test - public void testCreateRestApiExceptionWebApplicationExceptionNullResponse() throws Exception + public void testCreateRestApiExceptionWebApplicationExceptionNullResponse() { final WebApplicationException ex = new WebApplicationException((Response)null); final InvocationTargetException invocationTargetException = new InvocationTargetException(ex); @@ -87,11 +115,11 @@ public void testCreateRestApiExceptionWebApplicationExceptionNullResponse() thro assertEquals(cause, restApiRuntimeException.getCause()); assertEquals(500, restApiRuntimeException.getStatus()); assertEquals(Collections.emptyList(), restApiRuntimeException.getErrors()); - verify(exceptionMapper, times(1)).toException(eq(ex), any(Response.class), (ErrorResponse)isNull()); + verify(exceptionMapper, times(1)).toException(eq(ex), any(Response.class), isNull()); } @Test - public void testCreateRestApiExceptionWebApplicationException() throws Exception + public void testCreateRestApiExceptionWebApplicationException() { final WebApplicationException ex = new WebApplicationException(response); final InvocationTargetException invocationTargetException = new InvocationTargetException(ex); @@ -100,11 +128,11 @@ public void testCreateRestApiExceptionWebApplicationException() throws Exception assertEquals(cause, restApiRuntimeException.getCause()); assertEquals(FAILURE_STATUS, restApiRuntimeException.getStatus()); assertEquals(Collections.emptyList(), restApiRuntimeException.getErrors()); - verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), (ErrorResponse)isNull()); + verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), isNull()); } @Test - public void testCreateRestApiExceptionWebApplicationExceptionWithErrorsEntity() throws Exception + public void testCreateRestApiExceptionWebApplicationExceptionWithErrorsEntity() { final ErrorResponse errorResponse = mock(ErrorResponse.class); when(response.readEntity(ErrorResponse.class)).thenReturn(errorResponse); @@ -120,7 +148,7 @@ public void testCreateRestApiExceptionWebApplicationExceptionWithErrorsEntity() } @Test - public void testCreateRestApiExceptionWebApplicationExceptionIllegalStateException() throws Exception + public void testCreateRestApiExceptionWebApplicationExceptionIllegalStateException() { final ErrorResponse errorResponse = mock(ErrorResponse.class); when(response.readEntity(ErrorResponse.class)).thenThrow(new IllegalStateException()); @@ -129,24 +157,24 @@ public void testCreateRestApiExceptionWebApplicationExceptionIllegalStateExcepti final InvocationTargetException invocationTargetException = new InvocationTargetException(ex); final RestApiRuntimeException restApiRuntimeException = handler.createRestApiException(invocationTargetException); - verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), (ErrorResponse)isNull()); + verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), isNull()); } @Test - public void testCreateRestApiExceptionWebApplicationExceptionProcessingException() throws Exception + public void testCreateRestApiExceptionWebApplicationExceptionProcessingException() { - final ErrorResponse errorResponse = mock(ErrorResponse.class); when(response.readEntity(ErrorResponse.class)).thenThrow(new ProcessingException("foo")); final WebApplicationException ex = new WebApplicationException(response); final InvocationTargetException invocationTargetException = new InvocationTargetException(ex); - final RestApiRuntimeException restApiRuntimeException = handler.createRestApiException(invocationTargetException); - verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), (ErrorResponse)isNull()); + handler.createRestApiException(invocationTargetException); + + verify(exceptionMapper, times(1)).toException(eq(ex), eq(response), isNull()); } @Test - public void shouldUnwrapRestApiException() + public void shouldUnwrapProcessingException() { RestApiRuntimeException original = new RestApiRuntimeException(new Exception()); ProcessingException processingException = new ProcessingException(original); @@ -157,7 +185,7 @@ public void shouldUnwrapRestApiException() } @Test - public void shouldntUnwrapNullCause() + public void shouldntUnwrapProcessingExceptionWithNullCause() { ProcessingException processingException = new ProcessingException("No Cause"); final InvocationTargetException invocationTargetException = new InvocationTargetException(processingException); @@ -165,4 +193,14 @@ public void shouldntUnwrapNullCause() final RestApiRuntimeException restApiRuntimeException = handler.createRestApiException(invocationTargetException); assertEquals(restApiRuntimeException.getCause(), processingException); } + + @Test + public void testCreateRestApiExceptionOnResponseProcessingException() + { + ResponseProcessingException ex = new ResponseProcessingException(response, "error"); + + handler.createRestApiException(ex); + + verify(exceptionMapper, never()).toException(eq(ex), eq(response), isNull()); + } } diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiRuntimeExceptionTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiRuntimeExceptionTest.java index b8cac91e..0efe96d8 100644 --- a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiRuntimeExceptionTest.java +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/exception/RestApiRuntimeExceptionTest.java @@ -1,27 +1,36 @@ package com.smartling.api.v2.client.exception; +import com.smartling.api.v2.client.request.RequestContext; import com.smartling.api.v2.response.Error; import com.smartling.api.v2.response.ErrorResponse; import com.smartling.api.v2.response.ResponseCode; +import org.jboss.resteasy.specimpl.MultivaluedMapImpl; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.core.Response; +import java.net.URI; import java.util.Collections; import java.util.LinkedList; import java.util.List; -import javax.ws.rs.core.Response; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.StringContains.containsString; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +@RunWith(MockitoJUnitRunner.class) public class RestApiRuntimeExceptionTest { private static final int DEFAULT_STATUS = 500; private static final int FAILURE_STATUS = 400; + private static final String REQUEST_ID = "test-request-id"; private static final ResponseCode DEFAULT_CODE = ResponseCode.GENERAL_ERROR; @Mock @@ -30,32 +39,79 @@ public class RestApiRuntimeExceptionTest @Before public void setUp() { - MockitoAnnotations.initMocks(this); when(response.getStatus()).thenReturn(FAILURE_STATUS); when(response.getStatusInfo()).thenReturn(mock(Response.StatusType.class)); when(response.getStatusInfo().getStatusCode()).thenReturn(FAILURE_STATUS); + when(response.getHeaderString("X-SL-RequestId")).thenReturn(REQUEST_ID); } @Test - public void testExceptionWithNoResponse() throws Exception + public void testExceptionWithNoResponse() { final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException()); assertEquals(DEFAULT_STATUS, exception.getStatus()); assertEquals(DEFAULT_CODE, exception.getResponseCode()); assertEquals(Collections.emptyList(), exception.getErrors()); + assertEquals("http_status=500", exception.getMessage()); + } + + @Test + public void testExceptionWithRequestContext() + { + MultivaluedMap headers = new MultivaluedMapImpl<>(); + headers.add("X-SL-RequestId", REQUEST_ID); + + RequestContext requestContext = new RequestContext( + "POST", + URI.create("https://api.smartling.com/test"), + headers + ); + + final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException(), requestContext); + assertEquals(DEFAULT_STATUS, exception.getStatus()); + assertEquals(DEFAULT_CODE, exception.getResponseCode()); + assertEquals(Collections.emptyList(), exception.getErrors()); + assertEquals("http_status=500, requestId=test-request-id", exception.getMessage()); } @Test - public void testExceptionWithResponseNullEntity() throws Exception + public void testExceptionWithRequestContextAndEmptyRequestIdHeader() + { + RequestContext requestContext = new RequestContext( + "POST", + URI.create("https://api.smartling.com/test"), + new MultivaluedMapImpl() {{ putSingle("key", "value"); }} + ); + + final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException(), requestContext); + assertEquals(DEFAULT_STATUS, exception.getStatus()); + assertEquals(DEFAULT_CODE, exception.getResponseCode()); + assertEquals(Collections.emptyList(), exception.getErrors()); + assertEquals("http_status=500", exception.getMessage()); + } + + @Test + public void testExceptionWithNullRequestContext() + { + final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException(), null); + assertEquals(DEFAULT_STATUS, exception.getStatus()); + assertEquals(DEFAULT_CODE, exception.getResponseCode()); + assertEquals(Collections.emptyList(), exception.getErrors()); + assertEquals("http_status=500", exception.getMessage()); + } + + @Test + public void testExceptionWithResponseAndNullEntity() { final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException(), response, null); assertEquals(FAILURE_STATUS, exception.getStatus()); assertEquals(DEFAULT_CODE, exception.getResponseCode()); assertEquals(Collections.emptyList(), exception.getErrors()); + assertEquals("http_status=400, requestId=test-request-id", exception.getMessage()); } @Test - public void testExceptionWithResponse() throws Exception + public void testExceptionWithResponse() { final List errors = new LinkedList<>(); errors.add(new Error("foo")); @@ -65,10 +121,11 @@ public void testExceptionWithResponse() throws Exception assertEquals(FAILURE_STATUS, exception.getStatus()); assertEquals(ResponseCode.VALIDATION_ERROR, exception.getResponseCode()); assertEquals(errors, exception.getErrors()); + assertEquals("http_status=400, requestId=test-request-id, top errors: 'foo'", exception.getMessage()); } @Test - public void customErrorMessage() throws Exception + public void messageShouldIncludeTop3Errors() { final List errors = new LinkedList<>(); errors.add(new Error("foo1")); @@ -77,18 +134,13 @@ public void customErrorMessage() throws Exception errors.add(new Error("foo4")); final ErrorResponse errorResponse = new ErrorResponse(ResponseCode.VALIDATION_ERROR, errors); - when(response.getHeaderString(anyString())).thenReturn("some request id"); - final RestApiRuntimeException exception = new RestApiRuntimeException(new RuntimeException(), response, errorResponse); String message = exception.getMessage(); + assertThat(message, containsString("http_status=400, requestId=test-request-id, top errors: ")); assertThat(message, containsString("foo1")); assertThat(message, containsString("foo2")); assertThat(message, containsString("foo3")); assertThat(message, not(containsString("foo4"))); - assertThat(message, containsString("some request id")); - assertThat(message, containsString("400")); - - verify(response).getHeaderString("X-SL-RequestId"); } } diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextFilterTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextFilterTest.java new file mode 100644 index 00000000..274b92d5 --- /dev/null +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextFilterTest.java @@ -0,0 +1,48 @@ +package com.smartling.api.v2.client.request; + +import org.jboss.resteasy.specimpl.MultivaluedMapImpl; +import org.junit.After; +import org.junit.Test; + +import javax.ws.rs.client.ClientRequestContext; +import javax.ws.rs.core.MultivaluedMap; +import java.net.URI; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RequestContextFilterTest +{ + private final RequestContextFilter filter = new RequestContextFilter(); + + @After + public void tearDown() + { + RequestContextHolder.clearContext(); + } + + @Test + public void shouldSetRequestContextFromClientRequestContext() throws Exception + { + // Given + final ClientRequestContext clientRequestContext = mock(ClientRequestContext.class); + final MultivaluedMap headers = new MultivaluedMapImpl<>(); + headers.add("Content-Type", "application/json"); + + when(clientRequestContext.getMethod()).thenReturn("POST"); + when(clientRequestContext.getUri()).thenReturn(URI.create("https://api.smartling.com/test")); + when(clientRequestContext.getHeaders()).thenReturn(headers); + + // When + filter.filter(clientRequestContext); + + // Then + RequestContext requestContext = RequestContextHolder.getContext(); + assertNotNull(requestContext); + assertEquals("POST", requestContext.getMethod()); + assertEquals(URI.create("https://api.smartling.com/test"), requestContext.getUri()); + assertEquals("application/json", requestContext.getHeaders().getFirst("Content-Type")); + } +} diff --git a/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextInvocationHandlerTest.java b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextInvocationHandlerTest.java new file mode 100644 index 00000000..c16b0b20 --- /dev/null +++ b/smartling-api-commons/src/test/java/com/smartling/api/v2/client/request/RequestContextInvocationHandlerTest.java @@ -0,0 +1,88 @@ +package com.smartling.api.v2.client.request; + +import org.jboss.resteasy.specimpl.MultivaluedMapImpl; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.net.URI; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RequestContextInvocationHandlerTest +{ + @Mock + private InvocationHandler mockDelegate; + + @InjectMocks + private RequestContextInvocationHandler handler; + + private Object mockProxy; + private Method mockMethod; + private Object[] mockArgs; + + @Before + public void setUp() throws NoSuchMethodException + { + RequestContext context = new RequestContext("POST", URI.create("https://test.com"), new MultivaluedMapImpl<>()); + RequestContextHolder.setContext(context); + + mockProxy = new TestProxy(); + mockMethod = TestProxy.class.getMethod("foo"); + mockArgs = new Object[0]; + } + + @After + public void tearDown() + { + RequestContextHolder.clearContext(); + } + + @Test + public void shouldDelegateInvocationAndClearContext() throws Throwable + { + // Given + final String expectedResult = "test result"; + when(mockDelegate.invoke(mockProxy, mockMethod, mockArgs)).thenReturn(expectedResult); + + // When + Object result = handler.invoke(mockProxy, mockMethod, mockArgs); + + // Then + assertEquals(expectedResult, result); + verify(mockDelegate, times(1)).invoke(mockProxy, mockMethod, mockArgs); + assertNull(RequestContextHolder.getContext()); + } + + @Test + public void shouldClearContextEvenWhenDelegateThrowsException() throws Throwable + { + // Given + when(mockDelegate.invoke(mockProxy, mockMethod, mockArgs)).thenThrow(new RuntimeException()); + + // When-Then + assertThrows(RuntimeException.class, () -> handler.invoke(mockProxy, mockMethod, mockArgs)); + + verify(mockDelegate, times(1)).invoke(mockProxy, mockMethod, mockArgs); + assertNull(RequestContextHolder.getContext()); + } + + private static class TestProxy + { + public void foo() + { + } + } +}