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

XmlBasedHttpClientProvider in Jetty 10 #3341

Closed
sbordet opened this issue Feb 8, 2019 · 1 comment
Closed

XmlBasedHttpClientProvider in Jetty 10 #3341

sbordet opened this issue Feb 8, 2019 · 1 comment
Assignees

Comments

@sbordet
Copy link
Contributor

sbordet commented Feb 8, 2019

XmlBasedHttpClientProvider is loaded via reflection by HttpClientProvider (only usage).
HttpClientProvider is unused.
DefaultHttpClientProvider is only used by HttpClientProvider, which is unused.

@lachlan-roberts I guess all those classes were an attempt to allow applications to provide their own, custom, HttpClient instance to be used in WebSocket.
If this mechanism is present but implemented differently in Jetty 10, then those classes should be removed.

Otherwise a similar mechanism should be provided, based on ServiceLoader rather than reflection.

@joakime other inputs?

@lachlan-roberts
Copy link
Contributor

We create the HttpClient at only one place, in the WebSocketCoreClient constructor if we were not given a HttpClient.
https://github.com/eclipse/jetty.project/blob/477d7cf1da534ecfb5c1087cdbc445adb4028e7f/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java#L62-L70

Using the HttpClientProvider here would work for the javax client side because it is only created with the default constructor, however on the server side we first search for a HttpClient as attributes on the ServletContext and on the Server, so it would use the HttpClientProvider only if it did not find a HttpClient here first.

Also XmlBasedHttpClientProvider currently lives in jetty-websocket-client, but we would need to bring it into websocket-core to use it in the WebSocketCoreClient.

lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 13, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 14, 2019
- HttpClientProvider is now an interface which defines a default method
newHttpClient, its static get() method get will attempt to use the
XmlHttpClientProvider to create a client, and if this fails to give a
non null client it will be created with the default newHttpClient method

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 14, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 14, 2019
- HttpClientProvider is now an interface which defines a default method
newHttpClient, its static get() method get will attempt to use the
XmlHttpClientProvider to create a client, and if this fails to give a
non null client it will be created with the default newHttpClient method

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 14, 2019
- HttpClientProvider is now an interface which defines a default method
newHttpClient, its static get() method get will attempt to use the
XmlHttpClientProvider to create a client, and if this fails to give a
non null client it will be created with the default newHttpClient method

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 14, 2019
- HttpClientProvider is now an interface which defines a default method
newHttpClient, its static get() method get will attempt to use the
XmlHttpClientProvider to create a client, and if this fails to give a
non null client it will be created with the default newHttpClient method

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 15, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 15, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
sbordet added a commit that referenced this issue Feb 18, 2019
…lientProvider

Issue #3341 - WebSocket HttpClientProvider
sbordet added a commit that referenced this issue Feb 18, 2019
Updated comment referencing old class name.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet closed this as completed Feb 18, 2019
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

2 participants