Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ALS-5216] - 500 for 401 on sites #151

Merged
merged 1 commit into from
Oct 26, 2023
Merged

[ALS-5216] - 500 for 401 on sites #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...gh-resource/src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/HttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
14 changes: 7 additions & 7 deletions .../src/main/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down
27 changes: 13 additions & 14 deletions .../test/java/edu/harvard/hms/dbmi/avillach/resource/passthru/PassThroughResourceRSTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion pic-sure-resources/pic-sure-resource-api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
16 changes: 16 additions & 0 deletions pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/HttpClientUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down