Improve httpclient connect timeout migration - #1112
Open
Ouweshs28 wants to merge 13 commits into
Open
Conversation
Ouweshs28
force-pushed
the
improve-httpclient-connect-timeout-migration
branch
from
July 31, 2026 08:07
8e96c02 to
c877928
Compare
timtebeek
self-requested a review
July 31, 2026 08:21
Ouweshs28
marked this pull request as draft
July 31, 2026 09:36
Ouweshs28
force-pushed
the
improve-httpclient-connect-timeout-migration
branch
from
July 31, 2026 09:45
edbf0f8 to
87a938c
Compare
Ouweshs28
marked this pull request as ready for review
July 31, 2026 10:42
Contributor
Author
|
@timtebeek ready |
…nnect-timeout-migration
The httpclient5 bump to 5.5 was committed without regenerating classpath.tsv.gz, which still held 5.1.4; ConnectionConfig and setDefaultConnectionConfig only exist from 5.2. That missing type information is what required the TypeValidation opt-out and the fully qualified ConnectionConfig in the template. - Regenerate the type table (httpclient5 5.5.2, httpcore5 5.3.6) - Type the template receiver and import ConnectionConfig, so the tests pass under full type validation - Key migrations by the connection manager declaration, dropping the UUID map, the findTimeout rescan and the nested removal visitor - Match setConnectionManager and setDefaultConnectionConfig with MethodMatchers rather than simple names - Hoist the template and parser classpath out of the loop - Run the migration before AddCommentToMethodInvocations - Give the recipe a display name distinct from the composite - Share one parser across tests, pinning the ambiguous spring-web-6
Member
|
Thanks @Ouweshs28 ; I'm behind on reviews, but what prompted you to close this one? |
Contributor
Author
|
i was planning out my branches i think i deleted this one by mistake hahaahaha |
Contributor
Author
|
@timtebeek ready for review |
…nnect-timeout-migration # Conflicts: # src/main/resources/META-INF/rewrite/recipes.csv
…ectTimeout - Iterate all variables in a declaration so the second (or later) manager in a multi-variable declaration is recognized as the wired one - Recognize `new HttpComponentsClientHttpRequestFactory(HttpClients.custom() .setConnectionManager(cm).build())` inline construction via the same connectionManagerName walk used for client initializers - When a factory has multiple setConnectTimeout calls, migrate the last (matches Java last-write-wins) and drop earlier calls only when no intervening statement references the factory, client, or manager - Split the overloaded wiredTo map into clientToManager and factoryToManager - Key migrations by manager name so two managers in one declaration do not collide, and target the specific variable in the inserted template Adds four tests covering each new shape.
steve-aom-elliott
approved these changes
Aug 14, 2026
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
Contributor
Author
|
thanks @steve-aom-elliott |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's changed?
setConnectTimeout(int)configurations to Apache HttpClientConnectionConfig.What's your motivation?
Spring Framework 7 removes
HttpComponentsClientHttpRequestFactory#setConnectTimeout. This provides a safe automated migration where the pooling manager is wired to the request factory.Anything in particular you'd like reviewers to focus on?
The safety guard proving the local manager → HttpClient → request factory flow.
Have you considered any alternatives or workarounds?
A TODO-only migration was retained for cases where the connection-manager configuration cannot be safely inferred.
Checklist