Skip to content
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

[Kotlin][Retrofit] Change OkHttpClient parameter to Call.Factory #9448

Closed
shanselm-ergon opened this issue May 11, 2021 · 2 comments
Closed

Comments

@shanselm-ergon
Copy link
Contributor

With OpenApi version 5 a new constructor parameter got added to pass an OkHttpClient to the ApiClient ([#6855]). We thought this is good enough for us, but now we run into some problems with this approach. The parameter should be changed to a Call.Factory instead.

  • Smaller interface
  • Only the Call.Factory is used (see retrofitBuilder.client(usedClient), this just sets the Call.Factory)
  • It is more like it was before version 5 of OpenApi (the exposed adapterBuilder had a callFactory method)
  • In our use case, we need to replace the underlying http-client during runtime. If we only have to pass the Call.Factory we can easily do this, but if we have to pass the client itself we can't achieve this properly.

Describe the solution you'd like

Change the parameter from OkHttpClient to Call.Factory. Each OkHttpClient is also a Call.Factory, so minimal effort to adopt this change.

Describe alternatives you've considered

With the knowledge that only the Call.Factory interface is used by the ApiClient we could also create our own version of a OkHttpClient and only overwrite the newCall(request) method. But this is based on internal knowledge, which should not be part of the ApiClient API.

I will create a pull request suggestion for this improvement.

@shanselm-ergon
Copy link
Contributor Author

See pull request [#9451]

wing328 pushed a commit that referenced this issue May 28, 2021
* [Kotlin][Retrofit][#9448] Replace OkHttpClient parameter with Call.Factory.

* [Kotlin][Retrofit][#9448] Update sample project.
@shanselm-ergon
Copy link
Contributor Author

Resolved with [#9451]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant