-
Notifications
You must be signed in to change notification settings - Fork 83
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.
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
Migrate HttpComponentsClientHttpRequestFactory.setReadTimeout
to SocketConfig.setSoTimeout
#572
Conversation
15ca5be
to
dae1505
Compare
...org/openrewrite/java/spring/framework/HttpComponentsClientHttpRequestFactoryReadTimeout.java
Outdated
Show resolved
Hide resolved
...org/openrewrite/java/spring/framework/HttpComponentsClientHttpRequestFactoryReadTimeout.java
Outdated
Show resolved
Hide resolved
...openrewrite/java/spring/framework/HttpComponentsClientHttpRequestFactoryReadTimeoutTest.java
Outdated
Show resolved
Hide resolved
HttpComponentsClientHttpRequestFactory.setReadTimeout
to SocketConfig.setSoTimeout
HttpComponentsClientHttpRequestFactory.setReadTimeout
to SocketConfig.setSoTimeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a thing to note here is that these classpath resources end up in the packaged jar we send out to folks ; right now we're adding four jars, together weighing in at 2.8 MB; there's also alternatives that do not require any addition to the classpath for tests:
rewrite-spring/src/test/java/org/openrewrite/java/spring/test/SpringRulesToJUnitExtensionTest.java
Line 34 in 729164f
.dependsOn("package org.junit; public @interface ClassRule {}", "package org.junit; public @interface Rule {}") |
Let's strive to be conservative into what we add as jars, and how we add those. Preferably in src/test when we can, and automatically through parserClasspath
entries and the ./gradlew downloadRecipeDependencies
task, as opposed to manually.
...openrewrite/java/spring/framework/HttpComponentsClientHttpRequestFactoryReadTimeoutTest.java
Outdated
Show resolved
Hide resolved
...org/openrewrite/java/spring/framework/HttpComponentsClientHttpRequestFactoryReadTimeout.java
Outdated
Show resolved
Hide resolved
Add precondition to doAfterVisit so we don't duplicate `setDefaultSocketConfig` call
What's changed?
Added a conservative recipe to migrate from
HttpComponentsClientHttpRequestFactory.setReadTimeout
toSocketConfig.setSoTimeout
.What's your motivation?
HttpComponentsClientHttpRequestFactory.setReadTimeout
was removed in Spring 6.1.0 #540Anything in particular you'd like reviewers to focus on?
Test coverage
Anyone you would like to review specifically?
@timtebeek
Have you considered any alternatives or workarounds?
Any additional context
For now the recipe assumes a
PoolingHttpClientConnectionManager
is defined within the same class as theHttpComponentsClientHttpRequestFactory
. It doesn't take into account that aPoolingHttpClientConnectionManagerBuilder
exists nor does it refactor to this.Checklist