Skip to content

Commit

Permalink
Deprecate gRPC client filters (#1890)
Browse files Browse the repository at this point in the history
Motivation:

gRPC client filters are not as flexible as HTTP client filters. Their
reuse is limited due to the strict typing with gRPC definitions. HTTP
filters are the best choice most of the time. If required, users can
embed their custom logic in the actual client implementation where the
decoded protos can be accessed.

Modifications:

- Deprecated `GrpcClientFilterFactory`,
- Deprecated `GrpcClientFactory#appendClientFilter`,
  `GrpcClientFactory#appendClientFilterFactory`, and
  `GrpcClientFactory#newFilter`  methods,
- `Generator` adds deprecations to corresponding generated
  classes/interfaces and methods,
- Added deprecation warning in the project documentation.

Result:

Client filters have been deprecated and are prepared for removal in a
future release.
  • Loading branch information
Dariusz Jedrzejczyk committed Oct 18, 2021
1 parent 4083567 commit 1a2cc15
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 8 deletions.
9 changes: 7 additions & 2 deletions servicetalk-grpc-api/docs/modules/ROOT/pages/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ which provides additional context
into the `Connection`/transport details for each request/response. This means that a `GrpcService` method may be invoked
for multiple connections, from different threads, and even concurrently.

[#HttpFilters]
[#ServerHttpFilters]
=== HTTP Filters

As gRPC module is built on top of xref:{page-version}@servicetalk-http-api::index.adoc[HTTP module], one can use the
Expand All @@ -98,7 +98,7 @@ HTTP layer.
=== gRPC Filters

WARNING: gRPC Service Filters have been deprecated and will be removed in a future release. Please use
<<index#HttpFilters, HTTP service filters>> or implement the interception logic in the particular service definition
<<index#ServerHttpFilters, HTTP service filters>> or implement the interception logic in the particular service definition
if decoded protos are required.

In addition to HTTP filters, gRPC users can also add gRPC filters which follow the same interface definition as the
Expand Down Expand Up @@ -130,13 +130,18 @@ The control flow of a request/response can be visualized in the below diagram:
The xref:{page-version}@servicetalk-loadbalancer::index.adoc[LoadBalancer] is consulted for each request to determine
which connection should be used.

[#ClientHttpFilters]
=== HTTP Filters
As gRPC module is built on top of xref:{page-version}@servicetalk-http-api::index.adoc[HTTP module], one can use the
xref:{page-version}@servicetalk-http-api::index.adoc#client-filters[HTTP client filters] if required to intercept the
HTTP layer.

=== gRPC Filters

WARNING: gRPC Client Filters have been deprecated and will be removed in a future release. Please use
<<index#ClientHttpFilters, HTTP service filters>> or implement the interception logic in the particular
service definition if decoded protos are required.

In addition to HTTP filters, gRPC users can also add gRPC filters which follow the same interface definition as the
service and can be composed using the generated
link:{source-root}/servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcClientFactory.java[GrpcClientFactory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,15 @@ final BlockingClient newBlockingClientForCallFactory(GrpcClientCallFactory clien
*
* @param before the factory to apply before this factory is applied
* @return {@code this}
* @deprecated gRPC Client Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link io.servicetalk.http.api.StreamingHttpClientFilterFactory} and if the access to the decoded payload is
* necessary, then performing that logic can be done in the particular {@link GrpcClient client implementation}.
* Please use {@link io.servicetalk.http.api.SingleAddressHttpClientBuilder#appendClientFilter(
* io.servicetalk.http.api.StreamingHttpClientFilterFactory)} upon the {@code builder} obtained using
* {@link io.servicetalk.grpc.api.GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)}
* if HTTP filters are acceptable in your use case.
*/
@Deprecated
public GrpcClientFactory<Client, BlockingClient, Filter, FilterableClient, FilterFactory>
appendClientFilter(FilterFactory before) {
if (filterFactory == null) {
Expand Down Expand Up @@ -137,7 +145,15 @@ protected List<ContentCodec> supportedMessageCodings() {
* @param append {@link FilterFactory} to append to {@code existing}.
* @return a composed factory that first applies the {@code before} factory and then applies {@code existing}
* factory
* @deprecated gRPC Client Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link io.servicetalk.http.api.StreamingHttpClientFilterFactory} and if the access to the decoded payload is
* necessary, then performing that logic can be done in the particular {@link GrpcClient client implementation}.
* Please use {@link io.servicetalk.http.api.SingleAddressHttpClientBuilder#appendClientFilter(
* io.servicetalk.http.api.StreamingHttpClientFilterFactory)} upon the {@code builder} obtained using
* {@link io.servicetalk.grpc.api.GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)}
* if HTTP filters are acceptable in your use case.
*/
@Deprecated
protected abstract FilterFactory appendClientFilterFactory(FilterFactory existing, FilterFactory append);

/**
Expand All @@ -157,7 +173,15 @@ protected List<ContentCodec> supportedMessageCodings() {
* @param client {@link Client} to use for creating a {@link Filter} through the {@link FilterFactory}.
* @param filterFactory {@link FilterFactory}
* @return A {@link Filter} filtering the passed {@link Client}.
* @deprecated gRPC Client Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link io.servicetalk.http.api.StreamingHttpClientFilterFactory} and if the access to the decoded payload is
* necessary, then performing that logic can be done in the particular {@link GrpcClient client implementation}.
* Please use {@link io.servicetalk.http.api.SingleAddressHttpClientBuilder#appendClientFilter(
* io.servicetalk.http.api.StreamingHttpClientFilterFactory)} upon the {@code builder} obtained using
* {@link io.servicetalk.grpc.api.GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)}
* if HTTP filters are acceptable in your use case.
*/
@Deprecated
protected abstract Filter newFilter(Client client, FilterFactory filterFactory);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@
*
* @param <Filter> Type for client filter
* @param <FilterableClient> Type of filterable client.
* @deprecated gRPC Client Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link io.servicetalk.http.api.StreamingHttpClientFilterFactory} and if the access to the decoded payload is
* necessary, then performing that logic can be done in the particular {@link GrpcClient client implementation}.
* Please use {@link io.servicetalk.http.api.SingleAddressHttpClientBuilder#appendClientFilter(
* io.servicetalk.http.api.StreamingHttpClientFilterFactory)} upon the {@code builder} obtained using
* {@link io.servicetalk.grpc.api.GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)}
* if HTTP filters are acceptable in your use case.
*/
@Deprecated
public interface GrpcClientFilterFactory<Filter extends FilterableClient,
FilterableClient extends FilterableGrpcClient> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
import static io.servicetalk.grpc.protoc.Types.FilterableGrpcClient;
import static io.servicetalk.grpc.protoc.Types.GrpcBindableService;
import static io.servicetalk.grpc.protoc.Types.GrpcClient;
import static io.servicetalk.grpc.protoc.Types.GrpcClientBuilder;
import static io.servicetalk.grpc.protoc.Types.GrpcClientBuilderHttpInitializer;
import static io.servicetalk.grpc.protoc.Types.GrpcClientCallFactory;
import static io.servicetalk.grpc.protoc.Types.GrpcClientFactory;
import static io.servicetalk.grpc.protoc.Types.GrpcClientFilterFactory;
Expand All @@ -74,13 +76,13 @@
import static io.servicetalk.grpc.protoc.Types.GrpcRoutes;
import static io.servicetalk.grpc.protoc.Types.GrpcSerializationProvider;
import static io.servicetalk.grpc.protoc.Types.GrpcServerBuilder;
import static io.servicetalk.grpc.protoc.Types.GrpcServerBuilderHttpInitializer;
import static io.servicetalk.grpc.protoc.Types.GrpcService;
import static io.servicetalk.grpc.protoc.Types.GrpcServiceContext;
import static io.servicetalk.grpc.protoc.Types.GrpcServiceFactory;
import static io.servicetalk.grpc.protoc.Types.GrpcServiceFilterFactory;
import static io.servicetalk.grpc.protoc.Types.GrpcStatusException;
import static io.servicetalk.grpc.protoc.Types.GrpcSupportedCodings;
import static io.servicetalk.grpc.protoc.Types.HttpInitializer;
import static io.servicetalk.grpc.protoc.Types.HttpServerBuilder;
import static io.servicetalk.grpc.protoc.Types.ProtoBufSerializationProviderBuilder;
import static io.servicetalk.grpc.protoc.Types.Publisher;
Expand All @@ -90,7 +92,9 @@
import static io.servicetalk.grpc.protoc.Types.ResponseStreamingRoute;
import static io.servicetalk.grpc.protoc.Types.Route;
import static io.servicetalk.grpc.protoc.Types.Single;
import static io.servicetalk.grpc.protoc.Types.SingleAddressHttpClientBuilder;
import static io.servicetalk.grpc.protoc.Types.StreamingClientCall;
import static io.servicetalk.grpc.protoc.Types.StreamingHttpClientFilterFactory;
import static io.servicetalk.grpc.protoc.Types.StreamingHttpServiceFilterFactory;
import static io.servicetalk.grpc.protoc.Types.StreamingRoute;
import static io.servicetalk.grpc.protoc.Words.Blocking;
Expand Down Expand Up @@ -415,7 +419,7 @@ private TypeSpec.Builder addServiceFilter(final State state, final TypeSpec.Buil
" upon the {@code builder} obtained using {@link $T#initializeHttp($T)}" +
" if HTTP filters are acceptable in your use case." + lineSeparator(),
StreamingHttpServiceFilterFactory, GrpcService, HttpServerBuilder,
StreamingHttpServiceFilterFactory, GrpcServerBuilder, HttpInitializer)
StreamingHttpServiceFilterFactory, GrpcServerBuilder, GrpcServerBuilderHttpInitializer)
.addAnnotation(Deprecated.class);

state.serviceProto.getMethodList().forEach(methodProto ->
Expand All @@ -440,7 +444,7 @@ private static TypeSpec.Builder addServiceFilterFactory(final State state,
" upon the {@code builder} obtained using {@link $T#initializeHttp($T)}" +
" if HTTP filters are acceptable in your use case." + lineSeparator(),
StreamingHttpServiceFilterFactory, GrpcService, HttpServerBuilder,
StreamingHttpServiceFilterFactory, GrpcServerBuilder, HttpInitializer)
StreamingHttpServiceFilterFactory, GrpcServerBuilder, GrpcServerBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addSuperinterface(ParameterizedTypeName.get(GrpcServiceFilterFactory, state.serviceFilterClass,
Expand Down Expand Up @@ -628,7 +632,7 @@ supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, true
" upon the {@code builder} obtained using {@link $T#initializeHttp($T)}" +
" if HTTP filters are acceptable in your use case." + lineSeparator(),
StreamingHttpServiceFilterFactory, GrpcService, HttpServerBuilder,
StreamingHttpServiceFilterFactory, GrpcServerBuilder, HttpInitializer)
StreamingHttpServiceFilterFactory, GrpcServerBuilder, GrpcServerBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addAnnotation(Override.class)
Expand All @@ -646,7 +650,7 @@ supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, true
" upon the {@code builder} obtained using {@link $T#initializeHttp($T)}" +
" if HTTP filters are acceptable in your use case." + lineSeparator(),
StreamingHttpServiceFilterFactory, GrpcService, HttpServerBuilder,
StreamingHttpServiceFilterFactory, GrpcServerBuilder, HttpInitializer)
StreamingHttpServiceFilterFactory, GrpcServerBuilder, GrpcServerBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addModifiers(PROTECTED)
.addAnnotation(Override.class)
Expand Down Expand Up @@ -790,6 +794,16 @@ private TypeSpec.Builder addClientInterfaces(final State state, final TypeSpec.B
private TypeSpec.Builder addClientFilter(final State state, final TypeSpec.Builder serviceClassBuilder) {
final TypeSpec.Builder classSpecBuilder = newFilterDelegateCommonMethods(state.clientFilterClass,
state.filterableClientClass)
.addJavadoc("gRPC Client Filters will be removed in future release of ServiceTalk." +
" We encourage the use of {@link $T} and if the access to the decoded payload" +
" is necessary, then performing that logic can be done in the particular" +
" {@link $T client implementation}. Please use" +
" {@link $T#appendClientFilter($T)} upon the {@code builder} obtained using" +
" {@link $T#initializeHttp($T)} if HTTP filters are acceptable in your" +
" use case." + lineSeparator(),
StreamingHttpClientFilterFactory, GrpcClient, SingleAddressHttpClientBuilder,
StreamingHttpClientFilterFactory, GrpcClientBuilder, GrpcClientBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addMethod(newDelegatingCompletableMethodSpec(onClose, delegate))
.addMethod(newDelegatingMethodSpec(executionContext, delegate, GrpcExecutionContext, null));

Expand All @@ -807,6 +821,16 @@ private TypeSpec.Builder addClientFilter(final State state, final TypeSpec.Build
private static TypeSpec.Builder addClientFilterFactory(final State state,
final TypeSpec.Builder serviceClassBuilder) {
serviceClassBuilder.addType(interfaceBuilder(state.clientFilterFactoryClass)
.addJavadoc("gRPC Client Filters will be removed in future release of ServiceTalk." +
" We encourage the use of {@link $T} and if the access to the decoded payload" +
" is necessary, then performing that logic can be done in the particular" +
" {@link $T client implementation}. Please use" +
" {@link $T#appendClientFilter($T)} upon the {@code builder} obtained using" +
" {@link $T#initializeHttp($T)} if HTTP filters are acceptable in your" +
" use case." + lineSeparator(),
StreamingHttpClientFilterFactory, GrpcClient, SingleAddressHttpClientBuilder,
StreamingHttpClientFilterFactory, GrpcClientBuilder, GrpcClientBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addSuperinterface(ParameterizedTypeName.get(GrpcClientFilterFactory, state.clientFilterClass,
state.filterableClientClass))
Expand All @@ -830,6 +854,16 @@ private TypeSpec.Builder addClientFactory(final State state, final TypeSpec.Buil
.superclass(ParameterizedTypeName.get(GrpcClientFactory, state.clientClass, state.blockingClientClass,
state.clientFilterClass, state.filterableClientClass, state.clientFilterFactoryClass))
.addMethod(methodBuilder("appendClientFilterFactory")
.addJavadoc("gRPC Client Filters will be removed in future release of ServiceTalk." +
" We encourage the use of {@link $T} and if the access to the decoded payload" +
" is necessary, then performing that logic can be done in the particular" +
" {@link $T client implementation}. Please use" +
" {@link $T#appendClientFilter($T)} upon the {@code builder} obtained using" +
" {@link $T#initializeHttp($T)} if HTTP filters are acceptable in your" +
" use case." + lineSeparator(),
StreamingHttpClientFilterFactory, GrpcClient, SingleAddressHttpClientBuilder,
StreamingHttpClientFilterFactory, GrpcClientBuilder, GrpcClientBuilderHttpInitializer)
.addAnnotation(Deprecated.class)
.addModifiers(PROTECTED)
.addAnnotation(Override.class)
.returns(state.clientFilterFactoryClass)
Expand All @@ -845,7 +879,17 @@ private TypeSpec.Builder addClientFactory(final State state, final TypeSpec.Buil
.addStatement("return new $T($L, $L())", defaultClientClass, factory, supportedMessageCodings)
.build())
.addMethod(methodBuilder("newFilter")
.addJavadoc("gRPC Client Filters will be removed in future release of ServiceTalk." +
" We encourage the use of {@link $T} and if the access to the decoded payload" +
" is necessary, then performing that logic can be done in the particular" +
" {@link $T client implementation}. Please use" +
" {@link $T#appendClientFilter($T)} upon the {@code builder} obtained using" +
" {@link $T#initializeHttp($T)} if HTTP filters are acceptable in your" +
" use case." + lineSeparator(),
StreamingHttpClientFilterFactory, GrpcClient, SingleAddressHttpClientBuilder,
StreamingHttpClientFilterFactory, GrpcClientBuilder, GrpcClientBuilderHttpInitializer)
.addModifiers(PROTECTED)
.addAnnotation(Deprecated.class)
.addAnnotation(Override.class)
.returns(state.clientFilterClass)
.addParameter(state.clientClass, client, FINAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,16 @@ final class Types {

static final ClassName StreamingHttpServiceFilterFactory =
bestGuess(httpApiPkg + ".StreamingHttpServiceFilterFactory");
static final ClassName StreamingHttpClientFilterFactory =
bestGuess(httpApiPkg + ".StreamingHttpClientFilterFactory");
static final ClassName HttpServerBuilder = bestGuess(httpApiPkg + ".HttpServerBuilder");
static final ClassName SingleAddressHttpClientBuilder = bestGuess(httpApiPkg + ".SingleAddressHttpClientBuilder");

static final ClassName BlockingGrpcClient = bestGuess(grpcApiPkg + ".BlockingGrpcClient");
static final ClassName BlockingGrpcService = bestGuess(grpcApiPkg + ".BlockingGrpcService");
static final ClassName DefaultGrpcClientMetadata = bestGuess(grpcApiPkg + ".DefaultGrpcClientMetadata");
static final ClassName GrpcClient = bestGuess(grpcApiPkg + ".GrpcClient");
static final ClassName GrpcClientBuilder = bestGuess(grpcApiPkg + ".GrpcClientBuilder");
static final ClassName GrpcClientCallFactory = bestGuess(grpcApiPkg + ".GrpcClientCallFactory");
static final ClassName GrpcClientFactory = bestGuess(grpcApiPkg + ".GrpcClientFactory");
static final ClassName GrpcClientFilterFactory = bestGuess(grpcApiPkg + ".GrpcClientFilterFactory");
Expand All @@ -70,7 +74,8 @@ final class Types {
static final ClassName GrpcBindableService = bestGuess(grpcApiPkg + ".GrpcBindableService");
static final ClassName GrpcService = bestGuess(grpcApiPkg + ".GrpcService");
static final ClassName GrpcServerBuilder = bestGuess(grpcApiPkg + ".GrpcServerBuilder");
static final ClassName HttpInitializer = bestGuess(GrpcServerBuilder + ".HttpInitializer");
static final ClassName GrpcServerBuilderHttpInitializer = bestGuess(GrpcServerBuilder + ".HttpInitializer");
static final ClassName GrpcClientBuilderHttpInitializer = bestGuess(GrpcClientBuilder + ".HttpInitializer");
static final ClassName GrpcServiceContext = bestGuess(grpcApiPkg + ".GrpcServiceContext");
static final ClassName GrpcServiceFactory = bestGuess(grpcApiPkg + ".GrpcServiceFactory");
static final ClassName GrpcServiceFilterFactory = bestGuess(grpcApiPkg + ".GrpcServiceFilterFactory");
Expand Down

0 comments on commit 1a2cc15

Please sign in to comment.