[BUG] [feign-java] default client changed; okhttp vs httpclient5 are now the same?

[lesteenman]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Since OpenAPI generator 7.13.0, our patch methods have stopped working. It turns out that #21085 resulted in our client implementations being changed from OkHTTP to the default Feign one, which does not support patch methods. This means that that performance fix has caused a backwards incompatible change in the library.

Furthermore, in the same version, HttpClient5 support was added for feign (#21007). From what I can see, this no longer does anything either - since there are no default clients created, you will never get HttpClient5 nor OkHttp out of the box.

openapi-generator version

Changed in 7.13.0

OpenAPI declaration file content or url

Java-Feign HttpClient5

generatorName: java
outputDir: samples/client/petstore/java/feign-hc5
library: feign-hc5

Java-Feign:

generatorName: java
outputDir: samples/client/petstore/java/feign
library: feign

Under 7.13.0 (broken):

Java-Feign HttpClient5 builder:

    feignBuilder = Feign.builder()
                .encoder(new FormEncoder(new JacksonEncoder(objectMapper)))
                .decoder(new ApiResponseDecoder(objectMapper))
                .errorDecoder(new ApiErrorDecoder())
                .retryer(new Retryer.Default(0, 0, 2))
                .logger(new Slf4jLogger());

Or Java-Feign builder:

    feignBuilder = Feign.builder()
                .encoder(new FormEncoder(new JacksonEncoder(objectMapper)))
                .decoder(new ApiResponseDecoder(objectMapper))
                .errorDecoder(new ApiErrorDecoder())
                .retryer(new Retryer.Default(0, 0, 2))
                .logger(new Slf4jLogger());

Before merging #21085:

Java-Feign HttpClient5 builder:

    feignBuilder = Feign.builder()
                .client(new ApacheHttp5Client())
                .encoder(new FormEncoder(new JacksonEncoder(objectMapper)))
                .decoder(new ApiResponseDecoder(objectMapper))
                .errorDecoder(new ApiErrorDecoder())
                .retryer(new Retryer.Default(0, 0, 2))
                .logger(new Slf4jLogger());

Or Java-Feign builder:

    feignBuilder = Feign.builder()
                .client(new OkHttpClient())
                .encoder(new FormEncoder(new JacksonEncoder(objectMapper)))
                .decoder(new ApiResponseDecoder(objectMapper))
                .errorDecoder(new ApiErrorDecoder())
                .retryer(new Retryer.Default(0, 0, 2))
                .logger(new Slf4jLogger());

Generation Details

See description.

Steps to reproduce

• Build a java client with library feign or feign-hc5
• Check the feign builder

Related issues/PRs

"Broken" in #21085

Suggest a fix

We should either:

Remove the default clients entirely

• remove httpclient5 flavor
• remove the implementation imports for okhttp and httpclient5 from the pom.xml and build.gradle files
• make it clear you need to supply your own client

Support the clients again

• revert the change made in [JAVA][FEIGN] Removing hardcoded HTTP Client which is causing performance issues #21085
• add explicit support for a "client-less" feign builder

[sushma3690]

@lesteenman @wing328 I am beginning to fix this today. In the solution options given above, its best to go with the second one but with a slight change. Putting back the change, puts back the performance issue back and its a significant one, so I will try to create one client builder ahead of time and pass that to the client builder so that its not creating a new() client for every invocation.

If for some reason, if I am unable to do that, then I will go for option 1.

[sushma3690]

@lesteenman fix for this is here: #22905.

Thank you for catching it.