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

Add portable Http Connector Engine interface #1282

Open
hantsy opened this issue Aug 31, 2024 · 12 comments
Open

Add portable Http Connector Engine interface #1282

hantsy opened this issue Aug 31, 2024 · 12 comments

Comments

@hantsy
Copy link

hantsy commented Aug 31, 2024

Currently, we(the end developers) do not know which the rest clients use which HTTP Client library in the background, like the issue I mentioned in #1276, we need to add more provider-specific properties to use it.

I would like use a more fine-grained configuration on the background HttpClient itself directly.

Expose a HttpConnector interface to allow developers to customize it as expected.

ClientBuilder.httpConnector(
custom JdkHttpClientHttpConnector
or ApacheHttpClientHttpConnector
or JettyClientHttpConnector
or NettyClientHttpConnector
etc...
)

For example, JdkHttpClientHttpConnector can be instantize simply as the following.

new JdkHttpClientHttpConnector(); // create a built-in JDK 11 HttpClient internally.

Or accept a your custom JDK 11 HttpClient.

var executorService = Executors.newFixedThreadPool(5);
var httpClient = HttpClient.newBuilder()
            .executor(executorService)
            .version(HttpClient.Version.HTTP_2)
            .build();

new JdkHttpClientHttpConnector(httpClient);
@mkarg
Copy link
Contributor

mkarg commented Aug 31, 2024

What shall vendors do that currently are JAX-RS compatible, but that do not have replaceable backends, but just one hardcoded backend? Shall they not be JAX-RS compatible anymore or do you pay their costs for reworking their kernel to support replaceable backends?

Having said that, wouldn't it be enough to mandata that IF a vendor supports replaceable backends, then they MUST support predefined properties to configure them?

@hantsy
Copy link
Author

hantsy commented Sep 2, 2024

I hope the spec provider can provide their HttpConnector implementations freely, but at least give an implementation based on JDK 11 HttpClient, and make it the default implementation(for the end users, we do not need to add more dependencies).

The end user can also implement its own HttpConnector impl to replace the one in the provider. For example, in one project, we use OkHttp as an HTTP client to handle all client cases, but this Jaxrs provider does not provide an OKHttp version, to align with our codes and we do not want to introduce new dependencies, we can impl a HttpConnector based on OkHttp.

@jansupol
Copy link
Contributor

jansupol commented Sep 2, 2024

@hantsy Can you try to prototype some way to hook in the connector? It is not clear to me what simply new JdkHttpClientHttpConnector(httpClient); would do to deliver the response. Do you mean some static approach or some other way?

@mkarg
Copy link
Contributor

mkarg commented Sep 2, 2024

I hope the spec provider can provide their HttpConnector implementations freely, but at least give an implementation based on JDK 11 HttpClient, and make it the default implementation(for the end users, we do not need to add more dependencies).

The end user can also implement its own HttpConnector impl to replace the one in the provider. For example, in one project, we use OkHttp as an HTTP client to handle all client cases, but this Jaxrs provider does not provide an OKHttp version, to align with our codes and we do not want to introduce new dependencies, we can impl a HttpConnector based on OkHttp.

This imposes strong restrictions on how a JAX-RS implementation is actually designed -- something that an API shall never do.

@hantsy
Copy link
Author

hantsy commented Sep 3, 2024

something that an API shall never do.
Like the Jakarta Rest spec, there are several impls, such as Eclipse Jersey, Resteasy etc.

It is not just an API, in the end user's mind, it is a framework, that should be provide more flexible options.

@hantsy
Copy link
Author

hantsy commented Sep 3, 2024

@hantsy Can you try to prototype some way to hook in the connector? It is not clear to me what simply new JdkHttpClientHttpConnector(httpClient); would do to deliver the response. Do you mean some static approach or some other way?

The new Spring 6.x provides a RestClient, and Spring also provided a WebClient since 5.x. Unlike the former client tools(eg. restTemplate), they offer a configurable option to switch the background HTTP Client engine and allow developers to configure HTTP Message codecs details.

Check the following docs for more details.

@mkarg
Copy link
Contributor

mkarg commented Sep 4, 2024

It is not just an API, in the end user's mind, it is a framework, that should be provide more flexible options.

Jakarta REST is not a framework, it is just an API.

@mkarg
Copy link
Contributor

mkarg commented Sep 4, 2024

I think if we adopt this idea, then there should be a separate Jakarta subprojekt for defining such an API that we can build upon.

@jamezp
Copy link
Contributor

jamezp commented Sep 11, 2024

One thing to point out is the JDK HTTP Client doesn't implement everything needed to work with the jakara.ws.rs.client.Client. For example, the Jakarta REST Client allows the a hostname verifier to be set which you cannot do on the JDK's HTTP Client. I proposed we deprecate that, but it's a breaking change and was rejected.

FWIW RESTEasy does this with an SPI already, so it would work for RESTEasy.

That said, I don't think the Jakarta REST API should provide any implementations, nor should they be required to even implement one. I can see this useful to an end user where they might want to use some custom HTTP client.

@hantsy
Copy link
Author

hantsy commented Sep 12, 2024

I think if we adopt this idea, then there should be a separate Jakarta subprojekt for defining such an API that we can build upon.

I do not think so.

Jakarta Rest Client itself can use switchable serialization/deserialization providers, filters, portable rx invokers, etc., so why the HTTP client should be a subproject?

@hantsy
Copy link
Author

hantsy commented Sep 12, 2024

FWIW RESTEasy does this with an SPI already, so it would work for RESTEasy.

If possible to port this to the public Rest specification?

@mkarg
Copy link
Contributor

mkarg commented Sep 13, 2024

I think if we adopt this idea, then there should be a separate Jakarta subprojekt for defining such an API that we can build upon.

I do not think so.

Jakarta Rest Client itself can use switchable serialization/deserialization providers, filters, portable rx invokers, etc., so why the HTTP client should be a subproject?

Because it makes sense to have the same interface whereever pure HTTP services are needed. Jakarta REST is just a REST API, not a generic HTTP client. What we need here is not a REST-only API, but a pure HTTP API. It makes sense that other projects (even non-Jakarta-projects) make use of this. In fact, it should be defined by the vendors of these HTTP Clients, not the the consuming REST engines.

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

No branches or pull requests

5 participants
@jamezp @hantsy @mkarg @jansupol and others