OkHttp implementations of ClientHttpRequestFactory / AsyncClientHttpRequestFactory [SPR-12893]

[spring-projects-issues]

Luciano Leggieri opened SPR-12893 and commented

Alternative for existing implementation that uses the lightweight OkHttp library.

---

Affects: 4.1.6

Issue Links:

• ANDROID-138 Implement OkHttp RequestFactory
• Support for OkHttp 3 [SPR-14024] #18596 Support for OkHttp 3
• Deprecate OkHttp 2.x support [SPR-14344] #18916 Deprecate OkHttp 2.x support

Referenced from: pull request #800, and commits 6468aa7, 69fc2a8

1 votes, 7 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Pull request: #772

[spring-projects-issues]

Rossen Stoyanchev commented

I know there is interest in in this from the side of the Spring Android project (see ANDROID-138). Roy Clarkson can you comment on the state of that, it looks like some code is already added? I guess we'd have to reconcile that and the pull request here. Either way it seems like something to consider adding to spring-web.

[spring-projects-issues]

Rossen Stoyanchev commented

We'll try and target 4.2.

[spring-projects-issues]

Juergen Hoeller commented

According to http://square.github.io/okhttp/ and its usage explanation, OkHttp seems to implement both HttpURLConnection and the Apache HttpClient APIs through adapter modules... So from that perspective, it can be used with our existing ClientHttpRequestFactory implementations already. I wonder what value a direct OkHttp based implementation would add? Increased configurability maybe?

Juergen

[spring-projects-issues]

Luciano Leggieri commented

The HttpURLConnection wrapper (currently used for Android I think) is quite limiting. The Apache one has some drawbacks like:

1. You need the HttpClient JARs which add more weight to a project (consider that OkHttp is lightweight).
2. This one implements the asynchronous interface using the callbacks and future that OkHttp provides, but OKHttp-Apache only implements the synchronous HttpClient interface.
3. OKHttp-Apache uses HttpClient 4.2.2, but the Spring interface requires 4.3 so not sure if it works out of the box.

[spring-projects-issues]

Rossen Stoyanchev commented

Maybe "try out OkHttp without rewriting your network code" was exactly what they meant, nothing more. It's a bit misleading having that on the home page with no further qualification but the README for the Apache adapter does say is quite limited .

[spring-projects-issues]

Roy Clarkson commented

The implementation in Spring for Android uses the HttpURLConnection wrapper. I recommend against this, since it restricts the use of the OkHttp API features. For example, OkHttp offers its own interceptors which is a key feature missing in the android impl. Spring should expose a hook for these. I discussed with Arjen Poutsma this morning, and that sounds like the route he's taking.

[spring-projects-issues]

Juergen Hoeller commented

Thanks for the updated pull request, Arjen, just in time for 4.2! Assigning to Rossen for review...

Juergen

[spring-projects-issues]

Arjen Poutsma commented

I've reworked Luciano Leggieri's PR into #800. I've merged the eight original classes into the three standard classes: request, response and factory, and also made some other minor changes.

[spring-projects-issues]

Luciano Leggieri commented

On the new PR, it seems that the request body is buffered, as it extends AbstractBufferingAsyncClientHttpRequest. Was there a logic for that?

[spring-projects-issues]

Arjen Poutsma commented

yes, there was: when the StreamingHttpOutputMessage was used, I ran into issues with the (Async)RestTemplateIntegrationTests that I could not solve. So I opted for this approach instead.

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks Arjen Poutsma, I've merged the updated the PR into master.

Regarding body buffering, I see for Apache HttpComponents we have HttpComponentsAsyncClientHttpRequest (buffering) next to HttpComponentsStreamingClientHttpRequest. Would it make sense to have the same for OkHttp? It seems the one from the original PR was along the lines of HttpComponentsStreamingClientHttpRequest in terms of the upstream hieararchy.