[java] Provide default implementations for native HttpClient methods#16781
[java] Provide default implementations for native HttpClient methods#16781manuelsblanco wants to merge 2 commits intoSeleniumHQ:trunkfrom
Conversation
This prevents compilation errors in HttpClient implementations that do not support native Java HTTP operations. Fixes SeleniumHQ#16673
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
diemol
left a comment
There was a problem hiding this comment.
Please run the script formatter, at ./scripts/format.sh.
@joerg1985, can you please review as well?
There was a problem hiding this comment.
A default implementation should not just throw an exception, it should delegate it to the standard methods.
But i think we should not add these methods to the interface at all, as this is implementation specific stuff.
e.g. if you like to implement you own client and you are using the module system you are forced to include the java.net.http module. (we might currently ship the client in the same module as the interface, but this might change over time)
So in my mind the correct way of handling the need to access the native methods - if there is one at all - is to add a public method to the JdkHttpClient to get the underlying HttpClient. So if there is the need to access the methods you could just do what ever you like, e.g. open a socket:
if (client instanceof JdkHttpClient jdkClient) {
jdkClient.httpClient().newWebSocketBuilder()
} else {
throw new WeAreDoomedException()
}
@diemol what do you think about extending the JdkHttpClient instead of the interface?
diemol
left a comment
There was a problem hiding this comment.
I think @joerg1985 is correct. This was my mistake because I reviewed and approved the original PR without thinking it through.
@manuelsblanco could you please follow the suggestion?
|
|
||
|
|
User description
This change prevents compilation errors in
HttpClientimplementations that do not supportnative Java HTTP operations by providing default implementations for the newly added methods.
🔗 Related Issues
Fixes #16673
💥 What does this PR do?
This PR makes the newly introduced native HTTP client methods (
sendNativeandsendAsyncNative) optional by providing default implementations in theHttpClientinterface.The default implementations throw
UnsupportedOperationException, preserving theprevious behavior for clients that do not support Java 11 native HTTP APIs, while
allowing implementations such as
JdkHttpClientto override and provide fullsupport.
🔧 Implementation Notes
defaultin theHttpClientinterface to avoidbreaking existing implementations.
UnsupportedOperationExceptionwere removedfrom internal and test HTTP client implementations.
JdkHttpClient) remainunchanged and continue to provide concrete behavior.
Alternative approaches considered:
added complexity and limited benefit).
💡 Additional Considerations
HttpClientimplementations.HTTP methods.
🔄 Types of changes
PR Type
Bug fix
Description
Adds default implementations for native HTTP methods in
HttpClientinterfaceRemoves redundant method overrides from multiple implementations
Methods now throw
UnsupportedOperationExceptionby defaultAllows implementations to optionally override native HTTP support
Diagram Walkthrough
File Walkthrough
HttpClient.java
Add default implementations for native HTTP methodsjava/src/org/openqa/selenium/remote/http/HttpClient.java
sendAsyncNative()from abstract to default method withUnsupportedOperationExceptionsendNative()from abstract to default method withUnsupportedOperationExceptionRoutableHttpClientFactory.java
Remove redundant native HTTP method overridesjava/src/org/openqa/selenium/grid/web/RoutableHttpClientFactory.java
sendAsyncNative()override that only threw exceptionsendNative()override that only threw exceptionHttpClientinterfaceRemoteWebDriverBuilder.java
Remove redundant native HTTP method overridesjava/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java
sendAsyncNative()override that only threw exceptionsendNative()override that only threw exceptionHttpClientinterfacePassthroughHttpClient.java
Remove redundant native HTTP method overridesjava/test/org/openqa/selenium/grid/testing/PassthroughHttpClient.java
sendAsyncNative()override that only threw exceptionsendNative()override that only threw exceptionHttpClientinterfaceProtocolHandshakeTest.java
Remove redundant native HTTP method overridesjava/test/org/openqa/selenium/remote/ProtocolHandshakeTest.java
sendAsyncNative()override that only threw exceptionsendNative()override that only threw exceptionHttpClientinterfaceNativeHttpClientMethodsTest.java
Minor formatting adjustmentsjava/test/org/openqa/selenium/remote/http/NativeHttpClientMethodsTest.java