Skip to content

Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues #6100

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
May 22, 2025

Conversation

joviegas
Copy link
Contributor

This PR is continuation of migrating the Apache 4 client

  • In this PR we are fixing the compilation error which occured during baseling
  • Making sure basic test cases are passing

Motivation and Context

Modifications

Testing

  • maven build with checkstyle and spotbugs

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner May 13, 2025 17:23
@@ -152,7 +152,9 @@ public void run() {

for (Map.Entry<HttpClientConnectionManager, Long> entry : connectionManagers.entrySet()) {
try {
entry.getKey().closeIdleConnections(entry.getValue(), TimeUnit.MILLISECONDS);
entry.getKey().close(CloseMode.GRACEFUL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add a todo here to keep track of close_idle_connection_timeout config. We will need to figure out how to support it with Apache5 if we can't do it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added JAVA-8272 will handle all IM issues in upcoming PR before starting working on test suites

@@ -250,20 +250,28 @@ public void abort() {
public void close() {
HttpClientConnectionManager cm = httpClient.getHttpClientConnectionManager();
IdleConnectionReaper.getInstance().deregisterConnectionManager(cm);
cm.shutdown();
// TODO : need to add test cases for this
cm.close(CloseMode.IMMEDIATE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what are the other options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CloseMode.java

public enum CloseMode {

    IMMEDIATE, GRACEFUL

}

Comment on lines +263 to +265
// TODO : This is required since Apache5 closes streams immediately, check memory impacts because of this.
if (response.getEntity() != null) {
response.setEntity(new BufferedHttpEntity(response.getEntity()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this seems odd. We should not buffer the entire stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap I agree , but as mentioned in

The use of an HTTP response handler guarantees that the underlying HTTP connection will be released back to the connection manager automatically in all cases.

https://hc.apache.org/httpcomponents-client-5.4.x/examples.html

Thus we need to buffer the stream , we need to find a way around this or some better approach.

@@ -596,12 +606,12 @@ public void setDnsResolver(DnsResolver dnsResolver) {
}

@Override
public Builder socketFactory(ConnectionSocketFactory socketFactory) {
public Builder socketFactory(SSLConnectionSocketFactory socketFactory) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we are changing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/apache/httpcomponents-client/blob/4021b7c47914efaf9cf30b8d00bbd129295c0639/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java#L122-L127

public final PoolingHttpClientConnectionManagerBuilder setSSLSocketFactory(
        final org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory sslSocketFactory) {
    this.tlsSocketStrategy = (socket, target, port, attachment, context) ->
            (SSLSocket) sslSocketFactory.createLayeredSocket(socket, target, port, context);
    return this;
}

PoolingHttpClientConnectionManagerBuilder only accepts LayeredConnectionSocketFactory thus we need to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use LayeredConnectionSocketFactory instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline, it's deprecated. Will be addressed in further PR.

if (!firstAttempt && isRepeatable()) {
content.reset();
if (!firstAttempt && isRepeatableStream()) {
getContent().reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we are calling getContent() instead of saving it as a member variable? ContentStreamProvider#newStream could be very expensive for some custom implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@joviegas joviegas merged commit 6e9939f into feature/master/apache5x May 22, 2025
6 of 16 checks passed
@joviegas joviegas deleted the joviegas/phase2_apache5 branch May 29, 2025 15:06
github-merge-queue bot pushed a commit that referenced this pull request Jul 15, 2025
* Add initial empty module for Apache5x for seting up package (#6075)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline all the classes from Apache4 SDK client to the new Apache5 module

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100)

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues

* Handle comments from review

* Handle comments from Zoe

* Update the snap shot

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132)

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability

* Upated test cases

* Handled comments

* Updated snap shot after merge from master

* Revert "Updated snap shot after merge from master"

This reverts commit 64f7d33.

* Updated snap shot after merge from master

* Fix architecture test failures for apache5.x (#6140)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Updated snap shot after merge from master

* Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix Apache5 HTTP client retry failures with non-resettable streams (#6154)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Merge PR#6165 #6165

* Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Update snapshots

* Do not buffer the Response stream using BufferedHttpEntity (#6200)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Merge from master

* Apache5x SDkBenhmark Tests (#6206)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211)

* Clean up unused APIs and add test to make sure it can be handled with alternatives

* Added NTCredentials to keep backward compatibilty with Apache4.x

* Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview API annotation added for Public APIs and TODOs addressed (#6215)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview ready , addressing open TODOs

* Added PublicApi since checkstyle was failing

* Updated the snapshot

* Updated thr Brazil package nma e to have preview as suffix

* Updated Version as -PREVIEW to release apache5 as preview release (#6219)

* Updated Version as -PREVIEW

* japi cmp needs to be disables since this is a new version and we dont have old version to compare

* Handled Surface API review comments (#6224)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* update pom.xml for apache5.x

* Handled commed for merge to master PR 6220 (#6240)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* Handled comment for merge to master PR for apache 5.x

* Added change logs

* Review comment

* Handled Review comments
github-merge-queue bot pushed a commit that referenced this pull request Jul 22, 2025
* Add initial empty module for Apache5x for seting up package (#6075)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline all the classes from Apache4 SDK client to the new Apache5 module

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100)

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues

* Handle comments from review

* Handle comments from Zoe

* Update the snap shot

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132)

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability

* Upated test cases

* Handled comments

* Updated snap shot after merge from master

* Revert "Updated snap shot after merge from master"

This reverts commit 64f7d33.

* Updated snap shot after merge from master

* Fix architecture test failures for apache5.x (#6140)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Updated snap shot after merge from master

* Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix Apache5 HTTP client retry failures with non-resettable streams (#6154)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Merge PR#6165 #6165

* Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Update snapshots

* Do not buffer the Response stream using BufferedHttpEntity (#6200)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Merge from master

* Apache5x SDkBenhmark Tests (#6206)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211)

* Clean up unused APIs and add test to make sure it can be handled with alternatives

* Added NTCredentials to keep backward compatibilty with Apache4.x

* Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview API annotation added for Public APIs and TODOs addressed (#6215)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview ready , addressing open TODOs

* Added PublicApi since checkstyle was failing

* Updated the snapshot

* Updated thr Brazil package nma e to have preview as suffix

* Updated Version as -PREVIEW to release apache5 as preview release (#6219)

* Updated Version as -PREVIEW

* japi cmp needs to be disables since this is a new version and we dont have old version to compare

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* Handled Surface API review comments (#6224)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* update pom.xml for apache5.x

* Handled comment for merge to master PR for apache 5.x

* Handled commed for merge to master PR 6220 (#6240)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* Handled comment for merge to master PR for apache 5.x

* Added change logs

* Add Http client bench marks with S3 Get/Put

* Removed some dependencies

* updated performance comparison logic

* updated readme

* updated warmup time and iterations

* remove performance comparison since we already have printed performamnce and one sent to cw

* Review from github

* Review comment

* updatded prints

* Handled Review comments

* Added performance comparison for Apache

* updated README

* remove preview buid

* added changed to skpi http client benchmark

* updated the snapshot version

* updated to run on jdk8

* merge from master

* updated README.md

* fixed checkbuild issues

* fixed checksyle issues

* fixed issue where secondary benchmarks were not found

* Revert "fixed issue where secondary benchmarks were not found"

This reverts commit b536571.

* Revert "fixed checksyle issues"

This reverts commit 99e697e.

* Fixed checkstyle issues

* Handled review comments

* Updated snapshot

* Add skipping of http-client-benchmarks in validate-brazil-config.yml

* updated snapshot
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2025
…ted code (#6312)

* Add initial empty module for Apache5x for seting up package (#6075)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088)

* Add initial module for Apache5x for seting up package

* Add based on new module checklist

* Baseline all the classes from Apache4 SDK client to the new Apache5 module

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100)

* Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues

* Handle comments from review

* Handle comments from Zoe

* Update the snap shot

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132)

* Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability

* Upated test cases

* Handled comments

* Updated snap shot after merge from master

* Revert "Updated snap shot after merge from master"

This reverts commit 64f7d33.

* Updated snap shot after merge from master

* Fix architecture test failures for apache5.x (#6140)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Updated snap shot after merge from master

* Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix Apache5 HTTP client retry failures with non-resettable streams (#6154)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Merge PR#6165 #6165

* Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Update snapshots

* Do not buffer the Response stream using BufferedHttpEntity (#6200)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Merge from master

* Apache5x SDkBenhmark Tests (#6206)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211)

* Clean up unused APIs and add test to make sure it can be handled with alternatives

* Added NTCredentials to keep backward compatibilty with Apache4.x

* Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview API annotation added for Public APIs and TODOs addressed (#6215)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

* Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x

* Fix compilation issues

* Fix checkstyle  issues

* Remove test which are specific to apache http

* Add benchmark for Apache5 and add Streaming Api test cases

* Update Apache5 to 5.5

* Preview ready , addressing open TODOs

* Added PublicApi since checkstyle was failing

* Updated the snapshot

* Updated thr Brazil package nma e to have preview as suffix

* Updated Version as -PREVIEW to release apache5 as preview release (#6219)

* Updated Version as -PREVIEW

* japi cmp needs to be disables since this is a new version and we dont have old version to compare

* Handled Surface API review comments (#6224)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* update pom.xml for apache5.x

* Handled commed for merge to master PR 6220 (#6240)

* Handled Surface API review comments

* Added a single test for localaddress , handled review comments

* Removing internal package name as -preview after internal discussion

* Fix transient text case failures

* Handled comment for merge to master PR for apache 5.x

* Added change logs

* Review comment

* Handled Review comments

* Move connection timeout configuration from RequestConfig to ConnectionConfig in Apache HttpClient 5 (#6293)

* Replacing deprecated API like connectionTimeout on RequestConfig and passing it via defaultconnectionconfigs

* Handle review comments

* Replace httpclient.execute call with httpclient.executeOpen as mentioned in deprecation notes of httpclient.execute (#6298)

* Replacing deprecated API like connectionTimeout on RequestConfig and passing it via defaultconnectionconfigs

* Handle review comments

* replace httpclient.execute call with httpclient.executeOpen as mentioned in deprecation notes of httpclient.execute

* nit updates

* fixed sonar quebe issues

* Replace deprecated SSLConnectionSocketFactory with recommended API (#6281)

* Replace deprecated SSLConnectionSocketFactory with recommended API

* Fixed checkstyle issues

* Changed name tlsSocketStrategy on builder

* Removed warning log

* added more test cases

* updated after review

* Added ConnectionSocketFactory to Apache5Client builder same as Apache4

* handled PR comments

* Removed unused classes after moving to SSL sockets

* Added change logs

* removed old change logs

* remove unused imports

* inermittent port used in wiremock fixed

* update change logs
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants