Skip to content
This repository has been archived by the owner on Feb 7, 2025. It is now read-only.

Apache Client Bug Fix #694

Closed
wants to merge 3 commits into from
Closed

Apache Client Bug Fix #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f812d23
multiple changes:
jorg3lopez Nov 30, 2023
88d10b5
Merge branch 'main' into bug/apache-client
jorg3lopez Nov 30, 2023
e83bc75
Merge branch 'main' into bug/apache-client
jorg3lopez Nov 30, 2023
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: 2 additions & 2 deletions e2e/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ java {

dependencies {
//client
implementation 'org.apache.httpcomponents.client5:httpclient5:5.2.2'
implementation 'org.apache.httpcomponents.client5:httpclient5-fluent:5.2.2'
implementation 'org.apache.httpcomponents.client5:httpclient5:5.2.1'
Copy link
Member

Choose a reason for hiding this comment

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

If we changed our code, do we need to also downgrade? Or can we keep the latest version?

Copy link
Contributor Author

@jorg3lopez jorg3lopez Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Edit: I misinterpreted the question the first time. The answer is Yes. The reason is that there were two bugs. One which involved the incompatibility of the import, which blew the POST request when executed. And two, the invalid proxy error that the client throws. The invalid proxy error was discovered after I fixed the issue with the import.

Copy link
Member

@halprin halprin Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I see.

I'm a little confused about the header issue. What exactly is wrong with the headers? Maybe I could pair with you on this?

implementation 'org.apache.httpcomponents.client5:httpclient5-fluent:5.2.1'

//jackson
implementation 'com.fasterxml.jackson.core:jackson-core:2.16.0'
Expand Down
4 changes: 2 additions & 2 deletions shared/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ dependencies {
api 'org.fhir:ucum:1.0.8'

// Apache Client
implementation 'org.apache.httpcomponents.client5:httpclient5:5.2.2'
implementation 'org.apache.httpcomponents.client5:httpclient5-fluent:5.2.2'
implementation 'org.apache.httpcomponents.client5:httpclient5:5.2.1'
implementation 'org.apache.httpcomponents.client5:httpclient5-fluent:5.2.1'

// jjwt
implementation 'io.jsonwebtoken:jjwt-api:0.12.3'
Expand Down
25 changes: 5 additions & 20 deletions shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/apache/ApacheClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
import java.io.IOException;
import java.util.Map;
import org.apache.hc.client5.http.fluent.Request;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.message.BasicHeader;

/**
* This class implements HttpClient and is a "humble object" for the Apache Client 5 library. Using
Expand All @@ -27,29 +25,16 @@ public static ApacheClient getInstance() {
@Override
public String post(String url, Map<String, String> headerMap, String body)
throws HttpClientException {
Header[] headers = convertMapToHeader(headerMap);

try {
return Request.post(url)
.setHeaders(headers)
.body(new StringEntity(body))
.execute()
.returnContent()
.asString();
Request request = Request.post(url).body(new StringEntity(body));
if (headerMap != null) {
headerMap.forEach(request::addHeader);
}
return request.execute().returnContent().asString();
} catch (IOException e) {
throw new HttpClientException(
"Error occurred while making HTTP request to [" + url + "]", e);
}
}

protected Header[] convertMapToHeader(Map<String, String> headerMap) {

if (headerMap == null) {
return new Header[0];
}

return headerMap.entrySet().stream()
.map(entry -> new BasicHeader(entry.getKey(), entry.getValue()))
.toArray(Header[]::new);
}
}
20 changes: 0 additions & 20 deletions ...d/src/test/groovy/gov/hhs/cdc/trustedintermediary/external/apache/ApacheClientTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,6 @@ import spock.lang.Specification

class ApacheClientTest extends Specification {

def "convertMapToHeader works"() {
given:
def headerMap = [
"key": "value",
"name": "dogCow",
"first": "last"
]
def bhArr = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth commenting these tests and having an item to uncomment them in the devex/opex backlog in the future when we get bumped to a bug free version?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jcrichlake I think that's a good idea

Copy link
Contributor Author

@jorg3lopez jorg3lopez Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Good thought. Since the org.apache.hc.core5.http.Header is not supported in the 5.3.x version, we will not be utilizing the convertMapToHeader(Map<String, String> headerMap) helper function. This unit test was designed to test the functionality of the helper function. Once the helper function gets deleted, there is no need for this test case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks for clarifying

new BasicHeader("key", "value"),
new BasicHeader("name", "dogCow"),
new BasicHeader("first", "last")
]

when:
def actual = ApacheClient.getInstance().convertMapToHeader(headerMap)

then:
actual.toString() == bhArr.toString()
}

def "Http request with error"() {
given:
def httpClient = ApacheClient.getInstance()
Expand Down
Loading