Add httpClient() method to WebServiceClient.Builder #592
Conversation
This allows users to provide a custom HttpClient for full control over client configuration. When a custom HttpClient is provided, the builder validates that conflicting parameters (connectTimeout, proxy) are not also set, ensuring clear configuration ownership. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
| + "Configure timeout on the provided HttpClient instead."); | ||
| } | ||
| // Check if proxy was changed from default | ||
| if (proxy != ProxySelector.getDefault()) { |
There was a problem hiding this comment.
Should we be using equals here? Does ProxySelector.getDefault() always return the same reference? Perhaps it does, but I didn't dig into it. If it doesn't and we switch to equals, we should make sure it provides an overload that does what we want.
Another option would be to remove the default values above and set these in this method if the values are null.
There was a problem hiding this comment.
It looks like it does return the same reference yeah. https://github.com/openjdk/jdk/blob/040cc7aee03e82e70bcbfcd2dde5cd4b35faeabd/src/java.base/share/classes/java/net/ProxySelector.java#L95
There was a problem hiding this comment.
Happy to change it if you think that's necessary though. Removing the default values might be cleaner generally.
There was a problem hiding this comment.
Either way seems fine in that case, but moving the defaults to build might make it less likely that someone copies the pattern for same case where it is not appropriate.
Use null defaults for connectTimeout and proxy in Builder, making validation cleaner by simply checking for null values. Set defaults only when no custom HttpClient is provided. Remove unnecessary comments. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
| @@ -141,7 +141,6 @@ private WebServiceClient(Builder builder) { | |||
|
|
|||
| requestTimeout = builder.requestTimeout; | |||
|
|
|||
| // Use custom HttpClient if provided, otherwise create a default one | |||
There was a problem hiding this comment.
I'm not sure why this comment came out, but it also doesn't seem necessary to keep.
This allows users to provide a custom HttpClient for full control over client configuration. When a custom HttpClient is provided, the builder validates that conflicting parameters (connectTimeout, proxy) are not also set, ensuring clear configuration ownership.
🤖 Generated with Claude Code