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-8239: Fix PFB encoding issues #217

Merged
merged 8 commits into from
Jan 31, 2025
Merged

ALS-8239: Fix PFB encoding issues #217

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bcf0f50
Fix character encoding for binary files
ramari16 Jan 27, 2025
fa96cda
Fix test
ramari16 Jan 27, 2025
81cf6ae
Fix character encoding for binary files
ramari16 Jan 28, 2025
9c3658e
Fix character encoding for binary files
ramari16 Jan 29, 2025
a25e4f9
Fix character encoding for binary files
ramari16 Jan 29, 2025
65fa296
ALS-8239: Fix tests
ramari16 Jan 31, 2025
6a6c345
Fix pass through resource
ramari16 Jan 31, 2025
2332ba6
Cleanup debugging
ramari16 Jan 31, 2025
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
3 changes: 2 additions & 1 deletion ...-resource/src/main/java/edu/harvard/hms/dbmi/avillach/AggregateDataSharingResourceRS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -207,7 +208,7 @@ public Response queryResult(@PathParam("resourceQueryId") UUID queryId, QueryReq
checkQuery(resultRequest);
HttpResponse response = postRequest(resultRequest, "/query/" + queryId + "/result");
try {
String responseBody = httpClientUtil.readObjectFromResponse(response);
String responseBody = httpClientUtil.readObjectFromResponse(response, StandardCharsets.UTF_8);
return Response.ok(responseBody).build();
} finally {
HttpClientUtil.closeHttpResponse(response);
Expand Down
18 changes: 9 additions & 9 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
@@ -1,6 +1,7 @@
package edu.harvard.hms.dbmi.avillach.resource.passthru;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.UUID;

import javax.inject.Inject;
Expand All @@ -13,7 +14,6 @@
import org.apache.http.HttpResponse;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -69,7 +69,7 @@ public PassThroughResourceRS(ApplicationProperties properties, HttpClientUtil ht
public ResourceInfo info(QueryRequest infoRequest) {
String pathName = "/info";

HttpResponse response = null;
HttpResponse response = null;
try {
QueryRequest chainRequest = new GeneralQueryRequest();
if (infoRequest != null) {
Expand Down Expand Up @@ -101,8 +101,8 @@ public ResourceInfo info(QueryRequest infoRequest) {
logger.error(e.getMessage());
throw new ProtocolException(ProtocolException.INCORRECTLY_FORMATTED_REQUEST);
} finally {
closeHttpResponse(response);
}
closeHttpResponse(response);
}
}

@POST
Expand Down Expand Up @@ -154,7 +154,7 @@ public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryR

String pathName = "/query/" + queryId + "/result";

HttpResponse response = null;
HttpResponse response = null;
try {
QueryRequest chainRequest = new GeneralQueryRequest();
chainRequest.setQuery(resultRequest.getQuery());
Expand All @@ -164,8 +164,8 @@ public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryR
String payload = objectMapper.writeValueAsString(chainRequest);
response = httpClient
.retrievePostResponse(httpClient.composeURL(properties.getTargetPicsureUrl(), pathName), createAuthHeader(), payload);
String content = httpClient.readObjectFromResponse(response);
if (response.getStatusLine().getStatusCode() != 200) {
byte[] content = httpClient.readBytesFromResponse(response);
if (response.getStatusLine().getStatusCode() != 200) {
logger.error(
"{}{} calling resource with id {} did not return a 200: {} {} ", properties.getTargetPicsureUrl(), pathName,
chainRequest.getResourceUUID(), response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()
Expand All @@ -180,8 +180,8 @@ public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryR
logger.error(e.getMessage());
throw new ProtocolException(ProtocolException.INCORRECTLY_FORMATTED_REQUEST);
} finally {
closeHttpResponse(response);
}
closeHttpResponse(response);
}
}

@POST
Expand Down
11 changes: 5 additions & 6 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
@@ -1,9 +1,7 @@
package edu.harvard.hms.dbmi.avillach.resource.passthru;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -139,7 +137,8 @@ void testQueryResult() throws Exception {
when(httpResponse.getStatusLine()).thenReturn(statusLine);
when(httpResponse.getEntity()).thenReturn(httpResponseEntity);
when(httpClient.retrievePostResponse(anyString(), any(Header[].class), anyString())).thenReturn(httpResponse);
when(httpClient.readObjectFromResponse(any(HttpResponse.class))).thenReturn("4");
byte[] byteResponse = {101};
when(httpClient.readBytesFromResponse(any(HttpResponse.class))).thenReturn(byteResponse);

assertThrows(ProtocolException.class, () -> {
resource.queryResult("", null);
Expand Down Expand Up @@ -167,7 +166,7 @@ void testQueryResult() throws Exception {
when(httpResponse.getEntity()).thenReturn(httpResponseEntity);
GeneralQueryRequest queryRequest = newQueryRequest(null);
javax.ws.rs.core.Response returnVal = resource.queryResult(queryId.toString(), queryRequest);
assertEquals("4", returnVal.getEntity());
assertArrayEquals(byteResponse, (byte[]) returnVal.getEntity());
// assertEquals(resultId, returnVal.getHeaderString("resultId"));
}

Expand Down
24 changes: 17 additions & 7 deletions ...-sure-resource-api/src/main/java/edu/harvard/dbmi/avillach/service/ResourceWebClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.io.*;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Map;

import static edu.harvard.dbmi.avillach.util.HttpClientUtil.closeHttpResponse;
Expand Down Expand Up @@ -240,7 +240,13 @@ public Response queryResult(String rsURL, String queryId, QueryRequest queryRequ
HttpClientUtil.composeURL(rsURL, pathName), createHeaders(queryRequest.getResourceCredentials()), body
);

String content = httpClientUtil.readObjectFromResponse(resourcesResponse);
byte[] content = httpClientUtil.readBytesFromResponse(resourcesResponse);
FileOutputStream fos = new FileOutputStream("/tmp/test.avro");
Copy link
Contributor

@srpiatt srpiatt Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want this (/tmp/test.avro) hardcoded in this file? If yes, can we move it to the top of the class/file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bahhh, this was for debugging. I'll remove it

fos.write(content);

String contentString = new String(content, StandardCharsets.ISO_8859_1);
logger.info("Query result:");
logger.info(contentString);
if (resourcesResponse.getStatusLine().getStatusCode() != 200) {
logger.error("ResourceRS did not return a 200");
HttpClientUtil.throwResponseError(resourcesResponse, rsURL);
Expand All @@ -249,6 +255,10 @@ public Response queryResult(String rsURL, String queryId, QueryRequest queryRequ
} catch (JsonProcessingException e) {
logger.error("Unable to encode resource credentials");
throw new NotAuthorizedException("Unable to encode resource credentials", e);
} catch (FileNotFoundException e) {
throw new RuntimeException(e);
} catch (IOException e) {
throw new RuntimeException(e);
} finally {
closeHttpResponse(resourcesResponse);
}
Expand Down Expand Up @@ -281,7 +291,7 @@ public Response queryResultSignedUrl(String rsURL, String queryId, QueryRequest
httpClientUtil.throwResponseError(resourcesResponse, rsURL);
}

String content = httpClientUtil.readObjectFromResponse(resourcesResponse);
String content = httpClientUtil.readObjectFromResponse(resourcesResponse, StandardCharsets.UTF_8);
return Response.ok(content).build();
} catch (JsonProcessingException e) {
logger.error("Unable to encode resource credentials");
Expand Down Expand Up @@ -311,7 +321,7 @@ public Response queryFormat(String rsURL, QueryRequest queryRequest) {
httpClientUtil.composeURL(rsURL, pathName), createHeaders(queryRequest.getResourceCredentials()), body
);

String content = httpClientUtil.readObjectFromResponse(resourcesResponse);
String content = httpClientUtil.readObjectFromResponse(resourcesResponse, StandardCharsets.UTF_8);
int status = resourcesResponse.getStatusLine().getStatusCode();
if (status != 200) {
logger.error("Query format request did not return a 200: {}", resourcesResponse.getStatusLine().getStatusCode());
Expand Down Expand Up @@ -360,7 +370,7 @@ public Response querySync(String rsURL, QueryRequest queryRequest, String reques
throwError(resourcesResponse, rsURL);
}

String content = httpClientUtil.readObjectFromResponse(resourcesResponse);
String content = httpClientUtil.readObjectFromResponse(resourcesResponse, StandardCharsets.UTF_8);
if (resourcesResponse.containsHeader(QUERY_METADATA_FIELD)) {
Header metadataHeader = ((Header[]) resourcesResponse.getHeaders(QUERY_METADATA_FIELD))[0];
return Response.ok(content).header(QUERY_METADATA_FIELD, metadataHeader.getValue()).build();
Expand Down Expand Up @@ -417,7 +427,7 @@ public Response queryContinuous(String rsURL, QueryRequest queryRequest, String
throwError(resourcesResponse, rsURL);
}

String content = httpClientUtil.readObjectFromResponse(resourcesResponse);
String content = httpClientUtil.readObjectFromResponse(resourcesResponse, StandardCharsets.UTF_8);
return Response.ok(content).build();
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
Expand Down
6 changes: 3 additions & 3 deletions ...e-resource-api/src/test/java/edu/harvard/dbmi/avillach/service/ResourceWebClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public void testQuery() throws JsonProcessingException {
@Test
public void testQueryResult() throws JsonProcessingException {
String testId = "230048";
String mockResult = "{}";
byte[] mockResult = new byte[] {};



Expand Down Expand Up @@ -341,8 +341,8 @@ public void testQueryResult() throws JsonProcessingException {
Response result = cut.queryResult(testURL, testId, queryRequest);
assertNotNull("Result should not be null", result);
// String resultContent = IOUtils.toString((InputStream) result.getEntity(), "UTF-8");
String resultContent = (String) result.getEntity();
assertEquals("Result should match " + mockResult, mockResult, resultContent);
byte[] resultContent = (byte[]) result.getEntity();
assertArrayEquals("Result should match " + mockResult, mockResult, resultContent);

// What if the resource has a problem?
wireMockRule.stubFor(any(urlMatching("/query/.*/result")).willReturn(aResponse().withStatus(500)));
Expand Down
17 changes: 14 additions & 3 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 @@ -7,6 +7,7 @@
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
Expand All @@ -19,7 +20,6 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;

import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
Expand Down Expand Up @@ -139,10 +139,21 @@ public <T> List<T> readListFromResponse(HttpResponse response, Class<T> expected
}
}

public String readObjectFromResponse(HttpResponse response) {
public String readObjectFromResponse(HttpResponse response, Charset charset) {
logger.debug("HttpClientUtil readObjectFromResponse(HttpResponse response)");
try {
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
String responseBody = EntityUtils.toString(response.getEntity(), charset);
logger.debug("readObjectFromResponse() responseBody {}", responseBody);
return responseBody;
} catch (IOException e) {
throw new ApplicationException("Incorrect object type returned", e);
}
}

public byte[] readBytesFromResponse(HttpResponse response) {
logger.debug("HttpClientUtil readObjectFromResponse(HttpResponse response)");
try {
byte[] responseBody = EntityUtils.toByteArray(response.getEntity());
logger.debug("readObjectFromResponse() responseBody {}", responseBody);
return responseBody;
} catch (IOException e) {
Expand Down