Decouple multi-address and partitioned client builders from HttpClientBuildContext
#2136
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
…tBuildContext Motivation: `DefaultMultiAddressHttpClientBuilder` and `DefaultPartitionedHttpClientBuilder` use `DefaultSingleAddressHttpClientBuilder` as a template for underlying single-address clients. It was helpful before we introduced a `SingleAddressInitializer` in apple#1387. Before that, all builders had the same set of methods and it was nice to reuse the single-address builder instead of maintaining a similar state at higher level builders. Now, `SingleAddressInitializer` helps to avoid duplication between builders. We don't have to use `DefaultSingleAddressHttpClientBuilder` as a template and can simplify its state by using a factory for `SingleAddressHttpClientBuilder`. Modifications: - `DefaultMultiAddressUrlHttpClientBuilder` and `DefaultPartitionedHttpClientBuilder` use a factory for `SingleAddressHttpClientBuilder` instead of `DefaultSingleAddressHttpClientBuilder` as a template and `HttpExecutionContextBuilder` for context-related items; - Remove and hide methods from `DefaultSingleAddressHttpClientBuilder` that are not used by other builders anymore; - Add a test to verify `DefaultMultiAddressUrlHttpClientBuilder` sets execution context items for underlying single-address builders; Result: `DefaultSingleAddressHttpClientBuilder` has less pkg-private API, `DefaultMultiAddressUrlHttpClientBuilder` and `DefaultPartitionedHttpClientBuilder` create a new single-address builder on demand, similar to grpc builders.
|
Another part of the motivation for this change is to be able to intercept when multi-address client initializes a new single-address client builder to make it work for providers in #2137. |
bondolo
approved these changes
Mar 9, 2022
...p-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java
Outdated
Show resolved
Hide resolved
...-http-netty/src/main/java/io/servicetalk/http/netty/DefaultPartitionedHttpClientBuilder.java
Outdated
Show resolved
Hide resolved
servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpExecutionContextBuilder.java
Outdated
Show resolved
Hide resolved
...tty/src/test/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilderTest.java
Outdated
Show resolved
Hide resolved
...netty/src/test/java/io/servicetalk/http/netty/DefaultSingleAddressHttpClientBuilderTest.java
Outdated
Show resolved
Hide resolved
|
Nice! 👍 |
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.
Motivation:
DefaultMultiAddressHttpClientBuilderandDefaultPartitionedHttpClientBuilderuseDefaultSingleAddressHttpClientBuilderas a template for underlyingsingle-address clients. It was helpful before we introduced a
SingleAddressInitializerin #1387. Before that, all builders had thesame set of methods and it was nice to reuse the single-address builder
instead of maintaining a similar state at higher level builders. Now,
SingleAddressInitializerhelps to avoid duplication between builders.We don't have to use
DefaultSingleAddressHttpClientBuilderas atemplate and can simplify its state by using a factory for
SingleAddressHttpClientBuilder.Modifications:
DefaultMultiAddressUrlHttpClientBuilderandDefaultPartitionedHttpClientBuilderuse a factory forSingleAddressHttpClientBuilderinstead ofDefaultSingleAddressHttpClientBuilderas a template andHttpExecutionContextBuilderfor context-related items;DefaultSingleAddressHttpClientBuilderthat are not used by other builders anymore;
DefaultMultiAddressUrlHttpClientBuildersetsexecution context items for underlying single-address builders;
Result:
DefaultSingleAddressHttpClientBuilderhas less pkg-private API,DefaultMultiAddressUrlHttpClientBuilderandDefaultPartitionedHttpClientBuildercreate a new single-addressbuilder on demand, similar to grpc builders.