Skip to content

Commit

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

gRPC service filters are not as flexible as HTTP service 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 service implementation where the
decoded protos can be accessed.

Modifications:

- Deprecated `GrpcServiceFilterFactory`,
- Deprecated `GrpcServiceFactory#appendServiceFilter` and
  `GrpcServiceFactory#appendServiceFilterFactory` methods,
- `Generator` adds deprecations to corresponding generated
  classes/interfaces and methods.
- Added deprecation warning in the project documentation.

Result:

Service filters have been deprecated and are prepared for removal in a
future release.
  • Loading branch information
Dariusz Jedrzejczyk committed Oct 18, 2021
1 parent 9b5fe0a commit 4083567
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 1 deletion.
5 changes: 5 additions & 0 deletions servicetalk-grpc-api/docs/modules/ROOT/pages/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +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]
=== 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 @@ -96,6 +97,10 @@ 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
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/GrpcServiceFactory.java[GrpcServiceFactory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.servicetalk.http.api.BlockingStreamingHttpService;
import io.servicetalk.http.api.HttpService;
import io.servicetalk.http.api.StreamingHttpService;
import io.servicetalk.http.api.StreamingHttpServiceFilterFactory;
import io.servicetalk.transport.api.ExecutionContext;
import io.servicetalk.transport.api.ServerContext;

Expand Down Expand Up @@ -98,7 +99,16 @@ public final Single<ServerContext> bind(final ServerBinder binder, final Executi
*
* @param before the factory to apply before this factory is applied
* @return {@code this}
* @deprecated gRPC Service Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link StreamingHttpServiceFilterFactory} and if the access to the decoded payload is necessary, then performing
* that logic can be done in the particular {@link GrpcService service implementation}.
* Please use
* {@link io.servicetalk.http.api.HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)}
* upon the {@code builder} obtained using
* {@link GrpcServerBuilder#initializeHttp(GrpcServerBuilder.HttpInitializer)} if HTTP filters are acceptable
* in your use case.
*/
@Deprecated
public GrpcServiceFactory<Filter, Service, FilterFactory> appendServiceFilter(FilterFactory before) {
requireNonNull(before);
if (filterFactory == null) {
Expand All @@ -116,7 +126,16 @@ public GrpcServiceFactory<Filter, Service, FilterFactory> appendServiceFilter(Fi
* @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 Service Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link StreamingHttpServiceFilterFactory} and if the access to the decoded payload is necessary, then performing
* that logic can be done in the particular {@link GrpcService service implementation}.
* Please use
* {@link io.servicetalk.http.api.HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)}
* upon the {@code builder} obtained using
* {@link GrpcServerBuilder#initializeHttp(GrpcServerBuilder.HttpInitializer)} if HTTP filters are acceptable
* in your use case.
*/
@Deprecated
protected abstract FilterFactory appendServiceFilterFactory(FilterFactory existing, FilterFactory append);

private void applyFilterToRoutes(final FilterFactory filterFactory) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@
*/
package io.servicetalk.grpc.api;

import io.servicetalk.http.api.StreamingHttpServiceFilterFactory;

import static java.util.Objects.requireNonNull;

/**
* A factory to create <a href="https://www.grpc.io">gRPC</a> service filters.
*
* @param <Filter> Type for service filter
* @param <Service> Type for service
* @deprecated gRPC Service Filters will be removed in future release of ServiceTalk. We encourage the use of
* {@link StreamingHttpServiceFilterFactory} and if the access to the decoded payload is necessary, then performing
* that logic can be done in the particular {@link GrpcService service implementation}.
* Please use {@link io.servicetalk.http.api.HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)}
* upon the {@code builder} obtained using {@link GrpcServerBuilder#initializeHttp(GrpcServerBuilder.HttpInitializer)}
* if HTTP filters are acceptable in your use case.
*/
@FunctionalInterface
@Deprecated
public interface GrpcServiceFilterFactory<Filter extends Service, Service> {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,15 @@
import static io.servicetalk.grpc.protoc.Types.GrpcRouteExecutionStrategyFactory;
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.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;
import static io.servicetalk.grpc.protoc.Types.RequestStreamingClientCall;
Expand All @@ -88,6 +91,7 @@
import static io.servicetalk.grpc.protoc.Types.Route;
import static io.servicetalk.grpc.protoc.Types.Single;
import static io.servicetalk.grpc.protoc.Types.StreamingClientCall;
import static io.servicetalk.grpc.protoc.Types.StreamingHttpServiceFilterFactory;
import static io.servicetalk.grpc.protoc.Types.StreamingRoute;
import static io.servicetalk.grpc.protoc.Words.Blocking;
import static io.servicetalk.grpc.protoc.Words.Builder;
Expand All @@ -98,6 +102,7 @@
import static io.servicetalk.grpc.protoc.Words.Factory;
import static io.servicetalk.grpc.protoc.Words.Filter;
import static io.servicetalk.grpc.protoc.Words.INSTANCE;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_DEPRECATED;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_PARAM;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_RETURN;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_THROWS;
Expand Down Expand Up @@ -401,7 +406,17 @@ private TypeSpec.Builder addServiceInterfaces(final State state, final TypeSpec.

private TypeSpec.Builder addServiceFilter(final State state, final TypeSpec.Builder serviceClassBuilder) {
final TypeSpec.Builder classSpecBuilder =
newFilterDelegateCommonMethods(state.serviceFilterClass, state.serviceClass);
newFilterDelegateCommonMethods(state.serviceFilterClass, state.serviceClass)
.addJavadoc(JAVADOC_DEPRECATED + " gRPC Service 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 service implementation}." +
" Please use {@link $T#appendServiceFilter($T)}" +
" 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)
.addAnnotation(Deprecated.class);

state.serviceProto.getMethodList().forEach(methodProto ->
classSpecBuilder.addMethod(newRpcMethodSpec(methodProto, noneOf(NewRpcMethodFlag.class), false,
Expand All @@ -417,6 +432,16 @@ private TypeSpec.Builder addServiceFilter(final State state, final TypeSpec.Buil
private static TypeSpec.Builder addServiceFilterFactory(final State state,
final TypeSpec.Builder serviceClassBuilder) {
serviceClassBuilder.addType(interfaceBuilder(state.serviceFilterFactoryClass)
.addJavadoc(JAVADOC_DEPRECATED + " gRPC Service 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 service implementation}." +
" Please use {@link $T#appendServiceFilter($T)}" +
" 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)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addSuperinterface(ParameterizedTypeName.get(GrpcServiceFilterFactory, state.serviceFilterClass,
state.serviceClass))
Expand Down Expand Up @@ -595,6 +620,16 @@ supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, true
.addStatement("super($L)", builder)
.build())
.addMethod(methodBuilder(appendServiceFilter)
.addJavadoc(JAVADOC_DEPRECATED + " gRPC Service 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 service implementation}." +
" Please use {@link $T#appendServiceFilter($T)}" +
" 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)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addAnnotation(Override.class)
.returns(state.serviceFactoryClass)
Expand All @@ -603,6 +638,16 @@ supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, true
.addStatement("return this")
.build())
.addMethod(methodBuilder(appendServiceFilter + Factory)
.addJavadoc(JAVADOC_DEPRECATED + " gRPC Service 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 service implementation}." +
" Please use {@link $T#appendServiceFilter($T)}" +
" 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)
.addAnnotation(Deprecated.class)
.addModifiers(PROTECTED)
.addAnnotation(Override.class)
.returns(state.serviceFilterFactoryClass)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ final class Types {
private static final String concurrentPkg = basePkg + ".concurrent";
private static final String concurrentApiPkg = basePkg + ".concurrent.api";
private static final String grpcBasePkg = basePkg + ".grpc";
private static final String httpBasePkg = basePkg + ".http";
private static final String encodingBasePkg = basePkg + ".encoding";
private static final String encodingApiPkg = encodingBasePkg + ".api";
private static final String grpcApiPkg = grpcBasePkg + ".api";
private static final String httpApiPkg = httpBasePkg + ".api";
private static final String grpcRoutesFqcn = grpcApiPkg + ".GrpcRoutes";
private static final String grpcProtobufPkg = grpcBasePkg + ".protobuf";
private static final String routerApiPkg = basePkg + ".router.api";
Expand All @@ -45,6 +47,10 @@ final class Types {
static final ClassName Publisher = bestGuess(concurrentApiPkg + ".Publisher");
static final ClassName Single = bestGuess(concurrentApiPkg + ".Single");

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

static final ClassName BlockingGrpcClient = bestGuess(grpcApiPkg + ".BlockingGrpcClient");
static final ClassName BlockingGrpcService = bestGuess(grpcApiPkg + ".BlockingGrpcService");
static final ClassName DefaultGrpcClientMetadata = bestGuess(grpcApiPkg + ".DefaultGrpcClientMetadata");
Expand All @@ -63,6 +69,8 @@ final class Types {
static final ClassName GrpcSerializationProvider = bestGuess(grpcApiPkg + ".GrpcSerializationProvider");
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 GrpcServiceContext = bestGuess(grpcApiPkg + ".GrpcServiceContext");
static final ClassName GrpcServiceFactory = bestGuess(grpcApiPkg + ".GrpcServiceFactory");
static final ClassName GrpcServiceFilterFactory = bestGuess(grpcApiPkg + ".GrpcServiceFilterFactory");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ final class Words {
static final String JAVADOC_PARAM = "@param ";
static final String JAVADOC_RETURN = "@return ";
static final String JAVADOC_THROWS = "@throws ";
static final String JAVADOC_DEPRECATED = "@deprecated";

private Words() {
// no instance
Expand Down

0 comments on commit 4083567

Please sign in to comment.