Spring 6+ upgrades trigger migration to HttpClient 5 – how to deal with undeclared dependency on HttpClient 4?

[timtebeek]

Discussed in https://github.com/openrewrite/rewrite/discussions/4584

^{Originally posted by **@DidierLoiseau** October 16, 2024} I noticed an issue with this migration, which is triggered transitively by Spring Boot 3+ upgrades since openrewrite/rewrite-spring#566: HttpClient 4 is a relatively common dependency, so people often use it without even realizing it is pulled transitively by another dependency, and they don’t declare HttpClient as an explicit dependency. The problem with this migration is that it will update the code without adding the HttpClient 5 dependency if HttpClient 4 was missing. Is there a way around this?

[timtebeek]

hi! Thanks for reporting this issue; it appears indeed an oversight for transitive dependencies; might help to add an AddDependency step to the migration recipe: https://github.com/openrewrite/rewrite-apache/blob/6dd876da0ddd9a3140bed2b9789e437ae05f0524/src/main/resources/META-INF/rewrite/apache-httpclient-5.yml#L21-L48

If we add an appropriate onlyIfUsing & acceptTransitive there then it should not add the dependency when not in use, or already available transitively. A couple quick unit tests could confirm this is the case when there is a Spring Boot 3 parent, as UpgradeApacheHttpClient_5 runs after UpgradeSpringFramework_6_0.

[DidierLoiseau]

The following seems to work if applied before the upgrade:

  - org.openrewrite.maven.AddDependency:
      groupId: org.apache.httpcomponents
      artifactId: httpclient
      version: '*'
      onlyIfUsing: org.apache.http..*
      acceptTransitive: false

I had a hard time figuring out about the .. syntax as it isn’t mentioned in the documentation.

Unfortunately, the version appears to be mandatory even though we don’t want it (since it is managed), so I don’t know what would happen if applied after the upgrade (with the proper HttpClient5 groupId/artifactId).

I suppose the same would be needed for Gradle.

On our side, I realized that our usage is sometimes together with JMeter/jmeter-java-dsl, and they still depend on HttpClient4, so we can’t really do the upgrade anyway in those cases.

[timtebeek]

Ah sorry to hear that .. gave you trouble; it's documented on this page on method patterns: https://docs.openrewrite.org/reference/method-patterns#anatomy-of-a-method-pattern

I wonder if we should instead add httpclient5 after all the other recipe steps, such that we can acceptTransitive: true, and potentially even use a managed version. Did you plan a PR for this or was this just a passing suggestion? Either way it's appreciated!

[DidierLoiseau]

it's documented on this page on method patterns

That’s not really the place I would look for for package patterns 😅 I actually found the answer through debugging, seeing that it uses UsesType which has specific logic in the construction for dotdot and is used by FindTypes which has documentation for this behavior.

Did you plan a PR for this or was this just a passing suggestion?

Not for the moment as it does not solve our issue – switching to HttpClient5 even though it’s actually not compatible with the API we are using.

In fact I wonder if it wouldn’t be good to revert openrewrite/rewrite-spring#566. I don’t find it very safe since it can’t detect whether the code will work with HttpClient5. I could create a PR for that if desired. Of course there is the problem that SB 3.1 also removes the dependency management for HttpClient4, as described in that PR. Maybe only the code that uses it together with RestTemplate should be upgraded?