[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
hms-dbmi · Oct 25, 2023 · ae6aa61 · ae6aa61
1 parent f2dc837
commit ae6aa61
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 22 deletions.
diff --git a/...gh-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java b/...gh-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/.../src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java b/.../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/.../test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java b/.../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
@@ -61,7 +61,12 @@
           <artifactId>swagger-annotations</artifactId>
           <version>2.2.8</version>
       </dependency>
-
+      <dependency>
+          <groupId>org.junit.vintage</groupId>
+          <artifactId>junit-vintage-engine</artifactId>
+          <version>5.9.3</version>
+          <scope>test</scope>
+      </dependency>
 
   </dependencies>
 </project>
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
@@ -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
 	 *