From ae6aa614e56e6086fd6662709520e9cbc61b5253 Mon Sep 17 00:00:00 2001 From: Luke Sikina Date: Mon, 23 Oct 2023 15:51:10 -0400 Subject: [PATCH] [ALS-5216] - 500 for 401 on sites - Change 401 errors to 500 errors for auth issues coming from remote instances - Add junit vintage engine to make tests runnable in intellij --- .../resource/passthru/HttpClient.java | 4 +++ .../passthru/PassThroughResourceRS.java | 14 +++++----- .../passthru/PassThroughResourceRSTest.java | 27 +++++++++---------- .../pic-sure-resource-api/pom.xml | 7 ++++- .../dbmi/avillach/util/HttpClientUtil.java | 16 +++++++++++ 5 files changed, 46 insertions(+), 22 deletions(-) diff --git a/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java b/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java index 2e186c62..f929f91e 100644 --- a/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java +++ b/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java @@ -56,4 +56,8 @@ public HttpResponse retrievePostResponse(String uri, Header[] headers, String bo public void throwResponseError(HttpResponse response, String baseURL) { edu.harvard.dbmi.avillach.util.HttpClientUtil.throwResponseError(response, baseURL); } + + public void throwInternalResponseError(HttpResponse response, String baseURL) { + edu.harvard.dbmi.avillach.util.HttpClientUtil.throwInternalResponseError(response, baseURL); + } } diff --git a/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java b/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java index e251a759..67b2a13a 100644 --- a/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java +++ b/pic-sure-resources/pic-sure-passthrough-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java @@ -66,7 +66,7 @@ public ResourceInfo info(QueryRequest infoRequest) { if (response.getStatusLine().getStatusCode() != 200) { logger.error("{}{} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } ResourceInfo resourceInfo = httpClient.readObjectFromResponse(response, ResourceInfo.class); @@ -110,7 +110,7 @@ public QueryStatus query(QueryRequest queryRequest) { logger.error("{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } QueryStatus queryStatus = httpClient.readObjectFromResponse(response, QueryStatus.class); queryStatus.setResourceID(queryRequest.getResourceUUID()); @@ -146,7 +146,7 @@ public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryR logger.error("{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } return Response.ok(response.getEntity().getContent()).build(); @@ -182,7 +182,7 @@ public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, Que logger.error("{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } QueryStatus queryStatus = httpClient.readObjectFromResponse(response, QueryStatus.class); queryStatus.setResourceID(statusRequest.getResourceUUID()); @@ -223,7 +223,7 @@ public Response querySync(QueryRequest queryRequest) { logger.error("{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } if (response.containsHeader(QUERY_METADATA_FIELD)) { @@ -267,7 +267,7 @@ public SearchResults search(QueryRequest searchRequest) { if (response.getStatusLine().getStatusCode() != 200) { logger.error("{}{} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } return httpClient.readObjectFromResponse(response, SearchResults.class); } catch (IOException e) { @@ -303,7 +303,7 @@ public Response queryFormat(QueryRequest queryRequest) { logger.error("{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName, chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()); - httpClient.throwResponseError(response, properties.getTargetPicsureUrl()); + httpClient.throwInternalResponseError(response, properties.getTargetPicsureUrl()); } return Response.ok(response.getEntity().getContent()).build(); diff --git a/pic-sure-resources/pic-sure-passthrough-resource/src/test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java b/pic-sure-resources/pic-sure-passthrough-resource/src/test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java index 578fba9e..286538da 100644 --- a/pic-sure-resources/pic-sure-passthrough-resource/src/test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java +++ b/pic-sure-resources/pic-sure-passthrough-resource/src/test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java @@ -14,8 +14,6 @@ import java.util.Map; import java.util.UUID; -import javax.ws.rs.NotAuthorizedException; - import org.apache.commons.io.IOUtils; import org.apache.http.Header; import org.apache.http.HeaderElement; @@ -59,6 +57,7 @@ void init() { lenient().doCallRealMethod().when(httpClient).composeURL(anyString(), anyString()); lenient().doCallRealMethod().when(httpClient).readObjectFromResponse(any(HttpResponse.class), any()); lenient().doCallRealMethod().when(httpClient).throwResponseError(any(HttpResponse.class), anyString()); + lenient().doCallRealMethod().when(httpClient).throwInternalResponseError(any(HttpResponse.class), anyString()); resource = new PassThroughResourceRS(appProperties, httpClient); } @@ -79,9 +78,9 @@ void testInfo() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.info(new QueryRequest()); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(200); ResourceInfo resourceInfo = newResourceInfo(); @@ -119,9 +118,9 @@ void testQuery() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.query(newQueryRequest("test")); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(200); QueryStatus queryStatus = newQueryStatus(null); @@ -157,9 +156,9 @@ void testQueryResult() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.queryResult(UUID.randomUUID().toString(), newQueryRequest(null)); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(200); String resultId = UUID.randomUUID().toString(); @@ -198,9 +197,9 @@ void testQueryStatus() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.queryStatus(UUID.randomUUID().toString(), newQueryRequest(null)); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(200); UUID queryId = UUID.randomUUID(); @@ -239,9 +238,9 @@ void testQuerySync() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.querySync(newQueryRequest("test")); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(200); String resultId = UUID.randomUUID().toString(); @@ -279,9 +278,9 @@ void testSearch() throws Exception { }, "Downstream Resource returned 500 and should cause 'ResourceInterfaceException'"); when(statusLine.getStatusCode()).thenReturn(401); - assertThrows(NotAuthorizedException.class, () -> { + assertThrows(ResourceInterfaceException.class, () -> { resource.search(newQueryRequest("test")); - }, "Downstream Resource returned 401 and should cause 'NotAuthorizedException'"); + }, "Downstream Resource returned 401 and should cause 'ResourceInterfaceException'"); String queryText = "test"; when(statusLine.getStatusCode()).thenReturn(200); diff --git a/pic-sure-resources/pic-sure-resource-api/pom.xml b/pic-sure-resources/pic-sure-resource-api/pom.xml index 98a5014b..e6a913ea 100755 --- a/pic-sure-resources/pic-sure-resource-api/pom.xml +++ b/pic-sure-resources/pic-sure-resource-api/pom.xml @@ -61,7 +61,12 @@ swagger-annotations 2.2.8 - + + org.junit.vintage + junit-vintage-engine + 5.9.3 + test + diff --git a/pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/HttpClientUtil.java b/pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/HttpClientUtil.java index b912ed33..a506b94d 100644 --- a/pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/HttpClientUtil.java +++ b/pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/HttpClientUtil.java @@ -186,6 +186,22 @@ public static void throwResponseError(HttpResponse response, String baseURL) { throw new ResourceInterfaceException(errorMessage); } + public static void throwInternalResponseError(HttpResponse response, String baseURL) { + // We don't want to propagate 401s. A site 401ing is a server side error and should + // 500 in the common area. + String errorMessage = baseURL + " " + response.getStatusLine().getStatusCode() + " " + + response.getStatusLine().getReasonPhrase(); + try { + JsonNode responseNode = json.readTree(response.getEntity().getContent()); + if (responseNode != null && responseNode.has("message")) { + errorMessage += "/n" + responseNode.get("message").asText(); + } + } catch (IOException e) { + // That's fine, there's no message + } + throw new ResourceInterfaceException(errorMessage); + } + /** * Basic and general post function using Apache Http Client *