Issue #3341 - WebSocket HttpClientProvider #3357
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lachlan-roberts
force-pushed
the
jetty-10.0.x-3341-HttpClientProvider
branch
from
caf79b2 to
db5dcbd
Compare
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts
force-pushed
the
jetty-10.0.x-3341-HttpClientProvider
branch
2 times, most recently
from
4e5aea1 to
b0d9a36
Compare
- 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
force-pushed
the
jetty-10.0.x-3341-HttpClientProvider
branch
from
b0d9a36 to
1496205
Compare
sbordet
requested changes
Feb 14, 2019
| Log.getLogger(HttpClientProvider.class).ignore(ignore); | ||
| } | ||
|
|
||
| return new HttpClientProvider(){}.newHttpClient(); |
There was a problem hiding this comment.
Let's use one more feature of interfaces.
They can now have private static methods, so:
- Move the implementation of
newHttpClient()to a private static method callednewDefaultHttpClient(). - Here, rather than creating a new anonymous implementation, just call the private static method.
sbordet
requested changes
Feb 15, 2019
| { | ||
| HttpClient client = new HttpClient(new SslContextFactory()); | ||
| QueuedThreadPool threadPool = new QueuedThreadPool(); | ||
| threadPool.setName("WebSocketClient@" + client.hashCode()); | ||
| client.setExecutor(threadPool); | ||
| return client; | ||
| } | ||
|
|
||
| HttpClient newHttpClient(); |
There was a problem hiding this comment.
This method should be implemented by calling newDefaultHttpClient().
In this way, subclasses can call super, obtain the default instance and further customize it.
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts
force-pushed
the
jetty-10.0.x-3341-HttpClientProvider
branch
from
0104e37 to
d8b5f0e
Compare
sbordet
approved these changes
Feb 18, 2019
gregw
reviewed
Feb 20, 2019
|
|
||
| private static HttpClient newDefaultHttpClient() | ||
| { | ||
| HttpClient client = new HttpClient(new SslContextFactory()); |
There was a problem hiding this comment.
Note that we were previously setting setEndpointIdentificationAlgorithm("HTTPS"). Should we still do that or has it been changed to the default now?
There was a problem hiding this comment.
Yes the endpointIdentificationAlgorithm now defaults to "HTTPS" in the SslContextFactory.
I had this line in previously but @sbordet said it was not needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #3341 - XmlBasedHttpClientProvider in Jetty 10
HttpClientProviderclasses were moved from jetty-websocket-client to websocket-coreWebSocketCoreClientnow defers toHttpClientProviderto create itsHttpClientif it is not given one in its constructorHttpClientProvidernow and interface which implements a static methodget()and a default methodnewHttpClient()XmlHttpClientProvidernow implementsHttpClientProvider