From 4f7462d8340083714675957442793870a96ba311 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 23 Apr 2020 14:58:16 -0700 Subject: [PATCH 01/16] Eliminate InternalNotifyOnServerBuilder interface. --- .../io/grpc/InternalNotifyOnServerBuild.java | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 api/src/main/java/io/grpc/InternalNotifyOnServerBuild.java diff --git a/api/src/main/java/io/grpc/InternalNotifyOnServerBuild.java b/api/src/main/java/io/grpc/InternalNotifyOnServerBuild.java deleted file mode 100644 index b52acfaa7cc..00000000000 --- a/api/src/main/java/io/grpc/InternalNotifyOnServerBuild.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2016 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc; - -/** - * Provides a callback method for a service to receive a reference to its server. The contract with - * {@link ServerBuilder} is that this method will be called on all registered services implementing - * the interface after build() has been called and before the {@link Server} instance is returned. - */ -@Internal -public interface InternalNotifyOnServerBuild { - /** Notifies the service that the server has been built. */ - void notifyOnBuild(Server server); -} From 7d8547aa5f8a050789f22288bd59292d654bc2c8 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 23 Apr 2020 14:58:38 -0700 Subject: [PATCH 02/16] Expose server reference on ServerCall interface. --- api/src/main/java/io/grpc/PartialForwardingServerCall.java | 6 ++++++ api/src/main/java/io/grpc/ServerCall.java | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/api/src/main/java/io/grpc/PartialForwardingServerCall.java b/api/src/main/java/io/grpc/PartialForwardingServerCall.java index 8b95d7ae3fe..bd2e2e93fc2 100644 --- a/api/src/main/java/io/grpc/PartialForwardingServerCall.java +++ b/api/src/main/java/io/grpc/PartialForwardingServerCall.java @@ -76,6 +76,12 @@ public String getAuthority() { return delegate().getAuthority(); } + // TODO(chengyuanzhang): add ExperimentalApi annotation. + @Override + public Server getServer() { + return delegate().getServer(); + } + @Override public String toString() { return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString(); diff --git a/api/src/main/java/io/grpc/ServerCall.java b/api/src/main/java/io/grpc/ServerCall.java index 15ac0efdc93..e372808fec3 100644 --- a/api/src/main/java/io/grpc/ServerCall.java +++ b/api/src/main/java/io/grpc/ServerCall.java @@ -229,4 +229,10 @@ public String getAuthority() { * The {@link MethodDescriptor} for the call. */ public abstract MethodDescriptor getMethodDescriptor(); + + /** + * Returns the {@link Server} that dispatches the call. + */ + // TODO(chengyuanzhang): add ExperimentalApi annotation. + public abstract Server getServer(); } From 19529270d93a81fff621703e6c4ee70ddf66933f Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 23 Apr 2020 15:01:07 -0700 Subject: [PATCH 03/16] Add getServer() API implementation to subclasses. --- .../test/java/io/grpc/alts/AuthorizationUtilTest.java | 6 ++++++ .../src/main/java/io/grpc/internal/ServerCallImpl.java | 10 +++++++++- core/src/main/java/io/grpc/internal/ServerImpl.java | 1 + .../test/java/io/grpc/internal/ServerCallImplTest.java | 9 +++++++-- stub/src/test/java/io/grpc/stub/ServerCallsTest.java | 6 ++++++ .../src/main/java/io/grpc/internal/NoopServerCall.java | 6 ++++++ 6 files changed, 35 insertions(+), 3 deletions(-) diff --git a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java index dfbe8b5ff9f..3f924cf38ad 100644 --- a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java +++ b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java @@ -22,6 +22,7 @@ import io.grpc.Attributes; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.grpc.alts.internal.AltsAuthContext; @@ -110,5 +111,10 @@ public String getAuthority() { public MethodDescriptor getMethodDescriptor() { throw new AssertionError("Should not be called"); } + + @Override + public Server getServer() { + throw new AssertionError("Should not be called"); + } } } diff --git a/core/src/main/java/io/grpc/internal/ServerCallImpl.java b/core/src/main/java/io/grpc/internal/ServerCallImpl.java index 6f123e76678..d16c975eead 100644 --- a/core/src/main/java/io/grpc/internal/ServerCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerCallImpl.java @@ -35,6 +35,7 @@ import io.grpc.InternalDecompressorRegistry; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.perfmark.PerfMark; @@ -52,6 +53,7 @@ final class ServerCallImpl extends ServerCall { @VisibleForTesting static final String MISSING_RESPONSE = "Completed without a response"; + private final Server server; private final ServerStream stream; private final MethodDescriptor method; private final Tag tag; @@ -68,10 +70,11 @@ final class ServerCallImpl extends ServerCall { private Compressor compressor; private boolean messageSent; - ServerCallImpl(ServerStream stream, MethodDescriptor method, + ServerCallImpl(Server server, ServerStream stream, MethodDescriptor method, Metadata inboundHeaders, Context.CancellableContext context, DecompressorRegistry decompressorRegistry, CompressorRegistry compressorRegistry, CallTracer serverCallTracer, Tag tag) { + this.server = server; this.stream = stream; this.method = method; this.context = context; @@ -245,6 +248,11 @@ public MethodDescriptor getMethodDescriptor() { return method; } + @Override + public Server getServer() { + return server; + } + /** * Close the {@link ServerStream} because an internal error occurred. Allow the application to * run until completion, but silently ignore interactions with the {@link ServerStream} from now diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index 6e9cb9bf5ec..765121ce67a 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -636,6 +636,7 @@ private ServerStreamListener startWrappedCall( Tag tag) { ServerCallImpl call = new ServerCallImpl<>( + ServerImpl.this, stream, methodDef.getMethodDescriptor(), headers, diff --git a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java index a4c0da2d69d..02d5d4a0b0c 100644 --- a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java @@ -39,6 +39,7 @@ import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.Marshaller; import io.grpc.MethodDescriptor.MethodType; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.grpc.internal.ServerCallImpl.ServerStreamListenerImpl; @@ -60,6 +61,7 @@ @RunWith(JUnit4.class) public class ServerCallImplTest { @Rule public final ExpectedException thrown = ExpectedException.none(); + @Mock private Server server; @Mock private ServerStream stream; @Mock private ServerCall.Listener callListener; @@ -89,7 +91,7 @@ public class ServerCallImplTest { public void setUp() { MockitoAnnotations.initMocks(this); context = Context.ROOT.withCancellation(); - call = new ServerCallImpl<>(stream, UNARY_METHOD, requestHeaders, context, + call = new ServerCallImpl<>(server, stream, UNARY_METHOD, requestHeaders, context, DecompressorRegistry.getDefaultInstance(), CompressorRegistry.getDefaultInstance(), serverCallTracer, PerfMark.createTag()); } @@ -112,7 +114,7 @@ private void callTracer0(Status status) { assertEquals(0, before.callsStarted); assertEquals(0, before.lastCallStartedNanos); - call = new ServerCallImpl<>(stream, UNARY_METHOD, requestHeaders, context, + call = new ServerCallImpl<>(server, stream, UNARY_METHOD, requestHeaders, context, DecompressorRegistry.getDefaultInstance(), CompressorRegistry.getDefaultInstance(), tracer, PerfMark.createTag()); @@ -219,6 +221,7 @@ public void sendMessage_serverSendsOne_closeOnSecondCall_clientStreaming() { private void sendMessage_serverSendsOne_closeOnSecondCall( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( + server, stream, method, requestHeaders, @@ -254,6 +257,7 @@ public void sendMessage_serverSendsOne_closeOnSecondCall_appRunToCompletion_clie private void sendMessage_serverSendsOne_closeOnSecondCall_appRunToCompletion( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( + server, stream, method, requestHeaders, @@ -292,6 +296,7 @@ public void serverSendsOne_okFailsOnMissingResponse_clientStreaming() { private void serverSendsOne_okFailsOnMissingResponse( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( + server, stream, method, requestHeaders, diff --git a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java index 04f88755429..0d98549b83a 100644 --- a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java +++ b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java @@ -28,6 +28,7 @@ import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerServiceDefinition; @@ -573,5 +574,10 @@ public boolean isReady() { public MethodDescriptor getMethodDescriptor() { return methodDescriptor; } + + @Override + public Server getServer() { + return null; + } } } diff --git a/testing/src/main/java/io/grpc/internal/NoopServerCall.java b/testing/src/main/java/io/grpc/internal/NoopServerCall.java index 45038198fa9..08c1490cd41 100644 --- a/testing/src/main/java/io/grpc/internal/NoopServerCall.java +++ b/testing/src/main/java/io/grpc/internal/NoopServerCall.java @@ -18,6 +18,7 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; @@ -57,4 +58,9 @@ public boolean isCancelled() { public MethodDescriptor getMethodDescriptor() { return null; } + + @Override + public Server getServer() { + return null; + } } From 5b51f87e512d67d1a1de7b3b69ec43a78d372eed Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 23 Apr 2020 15:01:42 -0700 Subject: [PATCH 04/16] Obtain server via a server interceptor. --- .../services/ProtoReflectionService.java | 49 ++++++++++++++----- .../services/ProtoReflectionServiceTest.java | 3 +- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index beadb0f1eba..5d4c9149be8 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -24,10 +24,14 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; -import io.grpc.BindableService; import io.grpc.ExperimentalApi; -import io.grpc.InternalNotifyOnServerBuild; +import io.grpc.Metadata; import io.grpc.Server; +import io.grpc.ServerCall; +import io.grpc.ServerCall.Listener; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.ServerInterceptors; import io.grpc.ServerServiceDefinition; import io.grpc.Status; import io.grpc.protobuf.ProtoFileDescriptorSupplier; @@ -50,6 +54,7 @@ import java.util.Map; import java.util.Queue; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; @@ -61,26 +66,30 @@ * extension. */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2222") -public final class ProtoReflectionService extends ServerReflectionGrpc.ServerReflectionImplBase - implements InternalNotifyOnServerBuild { +public final class ProtoReflectionService extends ServerReflectionGrpc.ServerReflectionImplBase { private final Object lock = new Object(); + private AtomicReference serverRef; @GuardedBy("lock") private ServerReflectionIndex serverReflectionIndex; - private Server server; - private ProtoReflectionService() {} - public static BindableService newInstance() { - return new ProtoReflectionService(); + /** + * Creates a instance of {@link ProtoReflectionService}. Intended usage is one instance per + * {@link Server}. + */ + public static ServerServiceDefinition newInstance() { + AtomicReference serverCaptor = new AtomicReference<>(); + ProtoReflectionService protoReflection = new ProtoReflectionService(); + protoReflection.init(serverCaptor); + return ServerInterceptors.intercept( + protoReflection, new ServerCaptureInterceptor(serverCaptor)); } - /** Receives a reference to the server at build time. */ - @Override - public void notifyOnBuild(Server server) { - this.server = checkNotNull(server); + private void init(AtomicReference serverRef) { + this.serverRef = serverRef; } /** @@ -92,6 +101,7 @@ public void notifyOnBuild(Server server) { */ private ServerReflectionIndex updateIndexIfNecessary() { synchronized (lock) { + Server server = serverRef.get(); if (serverReflectionIndex == null) { serverReflectionIndex = new ServerReflectionIndex(server.getImmutableServices(), server.getMutableServices()); @@ -140,6 +150,21 @@ public StreamObserver serverReflectionInfo( return requestObserver; } + private static final class ServerCaptureInterceptor implements ServerInterceptor { + private final AtomicReference captor; + + ServerCaptureInterceptor(AtomicReference captor) { + this.captor = captor; + } + + @Override + public Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + captor.set(call.getServer()); + return next.startCall(call, headers); + } + } + private static class ProtoReflectionStreamObserver implements Runnable, StreamObserver { private final ServerReflectionIndex serverReflectionIndex; diff --git a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java index 00cc42ca3bd..26603e02f47 100644 --- a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java +++ b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java @@ -22,7 +22,6 @@ import static org.junit.Assert.fail; import com.google.protobuf.ByteString; -import io.grpc.BindableService; import io.grpc.ManagedChannel; import io.grpc.Server; import io.grpc.ServerServiceDefinition; @@ -64,7 +63,7 @@ public class ProtoReflectionServiceTest { private static final String TEST_HOST = "localhost"; private MutableHandlerRegistry handlerRegistry = new MutableHandlerRegistry(); - private BindableService reflectionService; + private ServerServiceDefinition reflectionService; private ServerServiceDefinition dynamicService = new DynamicServiceGrpc.DynamicServiceImplBase() {}.bindService(); private ServerServiceDefinition anotherDynamicService = From 9273cdd24397fd30e339d3a66c41a04d9e1af69a Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 23 Apr 2020 15:02:10 -0700 Subject: [PATCH 05/16] Eliminate callback for server build. --- .../grpc/internal/AbstractServerImplBuilder.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java index 6928434df3c..21a1becc284 100644 --- a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java @@ -29,7 +29,6 @@ import io.grpc.DecompressorRegistry; import io.grpc.HandlerRegistry; import io.grpc.InternalChannelz; -import io.grpc.InternalNotifyOnServerBuild; import io.grpc.Server; import io.grpc.ServerBuilder; import io.grpc.ServerInterceptor; @@ -77,7 +76,6 @@ public static ServerBuilder forPort(int port) { new InternalHandlerRegistry.Builder(); final List transportFilters = new ArrayList<>(); final List interceptors = new ArrayList<>(); - private final List notifyOnBuildList = new ArrayList<>(); private final List streamTracerFactories = new ArrayList<>(); HandlerRegistry fallbackRegistry = DEFAULT_FALLBACK_REGISTRY; ObjectPool executorPool = DEFAULT_EXECUTOR_POOL; @@ -114,9 +112,6 @@ public final T addService(ServerServiceDefinition service) { @Override public final T addService(BindableService bindableService) { - if (bindableService instanceof InternalNotifyOnServerBuild) { - notifyOnBuildList.add((InternalNotifyOnServerBuild) bindableService); - } return addService(checkNotNull(bindableService, "bindableService").bindService()); } @@ -222,14 +217,7 @@ protected void setDeadlineTicker(Deadline.Ticker ticker) { @Override public final Server build() { - ServerImpl server = new ServerImpl( - this, - buildTransportServers(getTracerFactories()), - Context.ROOT); - for (InternalNotifyOnServerBuild notifyTarget : notifyOnBuildList) { - notifyTarget.notifyOnBuild(server); - } - return server; + return new ServerImpl(this, buildTransportServers(getTracerFactories()), Context.ROOT); } @VisibleForTesting From d26801272485926a6a04e31b3bbb79fc4c1f9c73 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Wed, 29 Apr 2020 14:29:44 -0700 Subject: [PATCH 06/16] Add default implementation for ServerCall.getServer(). --- alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java | 5 ----- api/src/main/java/io/grpc/ServerCall.java | 7 +++++-- stub/src/test/java/io/grpc/stub/ServerCallsTest.java | 5 ----- testing/src/main/java/io/grpc/internal/NoopServerCall.java | 5 ----- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java index 3f924cf38ad..8088c0c9aa5 100644 --- a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java +++ b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java @@ -111,10 +111,5 @@ public String getAuthority() { public MethodDescriptor getMethodDescriptor() { throw new AssertionError("Should not be called"); } - - @Override - public Server getServer() { - throw new AssertionError("Should not be called"); - } } } diff --git a/api/src/main/java/io/grpc/ServerCall.java b/api/src/main/java/io/grpc/ServerCall.java index e372808fec3..8beaeae9f76 100644 --- a/api/src/main/java/io/grpc/ServerCall.java +++ b/api/src/main/java/io/grpc/ServerCall.java @@ -231,8 +231,11 @@ public String getAuthority() { public abstract MethodDescriptor getMethodDescriptor(); /** - * Returns the {@link Server} that dispatches the call. + * Returns the {@link Server} that dispatches the call. {@code null} if the implementation + * choose to not expose the server. */ // TODO(chengyuanzhang): add ExperimentalApi annotation. - public abstract Server getServer(); + public Server getServer() { + return null; + } } diff --git a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java index 0d98549b83a..eb65f3aec91 100644 --- a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java +++ b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java @@ -574,10 +574,5 @@ public boolean isReady() { public MethodDescriptor getMethodDescriptor() { return methodDescriptor; } - - @Override - public Server getServer() { - return null; - } } } diff --git a/testing/src/main/java/io/grpc/internal/NoopServerCall.java b/testing/src/main/java/io/grpc/internal/NoopServerCall.java index 08c1490cd41..7fe4f325a4f 100644 --- a/testing/src/main/java/io/grpc/internal/NoopServerCall.java +++ b/testing/src/main/java/io/grpc/internal/NoopServerCall.java @@ -58,9 +58,4 @@ public boolean isCancelled() { public MethodDescriptor getMethodDescriptor() { return null; } - - @Override - public Server getServer() { - return null; - } } From 933d0992993a8573a39e4c9f8292b496e14526a3 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Wed, 29 Apr 2020 14:31:51 -0700 Subject: [PATCH 07/16] Add ExperimentalApi annotation. --- api/src/main/java/io/grpc/PartialForwardingServerCall.java | 2 +- api/src/main/java/io/grpc/ServerCall.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/PartialForwardingServerCall.java b/api/src/main/java/io/grpc/PartialForwardingServerCall.java index bd2e2e93fc2..644a3a17a74 100644 --- a/api/src/main/java/io/grpc/PartialForwardingServerCall.java +++ b/api/src/main/java/io/grpc/PartialForwardingServerCall.java @@ -76,8 +76,8 @@ public String getAuthority() { return delegate().getAuthority(); } - // TODO(chengyuanzhang): add ExperimentalApi annotation. @Override + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6989") public Server getServer() { return delegate().getServer(); } diff --git a/api/src/main/java/io/grpc/ServerCall.java b/api/src/main/java/io/grpc/ServerCall.java index 8beaeae9f76..a4502f3a9f6 100644 --- a/api/src/main/java/io/grpc/ServerCall.java +++ b/api/src/main/java/io/grpc/ServerCall.java @@ -234,7 +234,7 @@ public String getAuthority() { * Returns the {@link Server} that dispatches the call. {@code null} if the implementation * choose to not expose the server. */ - // TODO(chengyuanzhang): add ExperimentalApi annotation. + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6989") public Server getServer() { return null; } From 130adde5b13209a6aa5d14e36b2329c66d9d79d7 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Wed, 29 Apr 2020 16:11:15 -0700 Subject: [PATCH 08/16] Remove comment requirement for using one reflection service instance per server. --- alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java | 1 - .../java/io/grpc/protobuf/services/ProtoReflectionService.java | 3 +-- stub/src/test/java/io/grpc/stub/ServerCallsTest.java | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java index 8088c0c9aa5..dfbe8b5ff9f 100644 --- a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java +++ b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java @@ -22,7 +22,6 @@ import io.grpc.Attributes; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.grpc.alts.internal.AltsAuthContext; diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index 5d4c9149be8..cfedd6f01e7 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -77,8 +77,7 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef private ProtoReflectionService() {} /** - * Creates a instance of {@link ProtoReflectionService}. Intended usage is one instance per - * {@link Server}. + * Creates a instance of {@link ProtoReflectionService}. */ public static ServerServiceDefinition newInstance() { AtomicReference serverCaptor = new AtomicReference<>(); diff --git a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java index eb65f3aec91..04f88755429 100644 --- a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java +++ b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java @@ -28,7 +28,6 @@ import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerServiceDefinition; From f50789b05b52a9985c8c2eda5cefc016ba6bfec7 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Wed, 29 Apr 2020 17:53:10 -0700 Subject: [PATCH 09/16] Add a simple test case covering a single reflection service instance can be shared for multiple servers. --- .../AnotherReflectableServiceGrpc.java | 288 ++++++++++++++++++ .../services/ProtoReflectionService.java | 81 +++-- .../services/ProtoReflectionServiceTest.java | 62 +++- .../reflection/testing/reflection_test.proto | 4 + .../java/io/grpc/internal/NoopServerCall.java | 1 - 5 files changed, 378 insertions(+), 58 deletions(-) create mode 100644 services/src/generated/test/grpc/io/grpc/reflection/testing/AnotherReflectableServiceGrpc.java diff --git a/services/src/generated/test/grpc/io/grpc/reflection/testing/AnotherReflectableServiceGrpc.java b/services/src/generated/test/grpc/io/grpc/reflection/testing/AnotherReflectableServiceGrpc.java new file mode 100644 index 00000000000..cae08a3fcf0 --- /dev/null +++ b/services/src/generated/test/grpc/io/grpc/reflection/testing/AnotherReflectableServiceGrpc.java @@ -0,0 +1,288 @@ +package io.grpc.reflection.testing; + +import static io.grpc.MethodDescriptor.generateFullMethodName; +import static io.grpc.stub.ClientCalls.asyncBidiStreamingCall; +import static io.grpc.stub.ClientCalls.asyncClientStreamingCall; +import static io.grpc.stub.ClientCalls.asyncServerStreamingCall; +import static io.grpc.stub.ClientCalls.asyncUnaryCall; +import static io.grpc.stub.ClientCalls.blockingServerStreamingCall; +import static io.grpc.stub.ClientCalls.blockingUnaryCall; +import static io.grpc.stub.ClientCalls.futureUnaryCall; +import static io.grpc.stub.ServerCalls.asyncBidiStreamingCall; +import static io.grpc.stub.ServerCalls.asyncClientStreamingCall; +import static io.grpc.stub.ServerCalls.asyncServerStreamingCall; +import static io.grpc.stub.ServerCalls.asyncUnaryCall; +import static io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall; +import static io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall; + +/** + */ +@javax.annotation.Generated( + value = "by gRPC proto compiler", + comments = "Source: io/grpc/reflection/testing/reflection_test.proto") +public final class AnotherReflectableServiceGrpc { + + private AnotherReflectableServiceGrpc() {} + + public static final String SERVICE_NAME = "grpc.reflection.testing.AnotherReflectableService"; + + // Static method descriptors that strictly reflect the proto. + private static volatile io.grpc.MethodDescriptor getMethodMethod; + + @io.grpc.stub.annotations.RpcMethod( + fullMethodName = SERVICE_NAME + '/' + "Method", + requestType = io.grpc.reflection.testing.Request.class, + responseType = io.grpc.reflection.testing.Reply.class, + methodType = io.grpc.MethodDescriptor.MethodType.UNARY) + public static io.grpc.MethodDescriptor getMethodMethod() { + io.grpc.MethodDescriptor getMethodMethod; + if ((getMethodMethod = AnotherReflectableServiceGrpc.getMethodMethod) == null) { + synchronized (AnotherReflectableServiceGrpc.class) { + if ((getMethodMethod = AnotherReflectableServiceGrpc.getMethodMethod) == null) { + AnotherReflectableServiceGrpc.getMethodMethod = getMethodMethod = + io.grpc.MethodDescriptor.newBuilder() + .setType(io.grpc.MethodDescriptor.MethodType.UNARY) + .setFullMethodName(generateFullMethodName(SERVICE_NAME, "Method")) + .setSampledToLocalTracing(true) + .setRequestMarshaller(io.grpc.protobuf.ProtoUtils.marshaller( + io.grpc.reflection.testing.Request.getDefaultInstance())) + .setResponseMarshaller(io.grpc.protobuf.ProtoUtils.marshaller( + io.grpc.reflection.testing.Reply.getDefaultInstance())) + .setSchemaDescriptor(new AnotherReflectableServiceMethodDescriptorSupplier("Method")) + .build(); + } + } + } + return getMethodMethod; + } + + /** + * Creates a new async stub that supports all call types for the service + */ + public static AnotherReflectableServiceStub newStub(io.grpc.Channel channel) { + io.grpc.stub.AbstractStub.StubFactory factory = + new io.grpc.stub.AbstractStub.StubFactory() { + @java.lang.Override + public AnotherReflectableServiceStub newStub(io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceStub(channel, callOptions); + } + }; + return AnotherReflectableServiceStub.newStub(factory, channel); + } + + /** + * Creates a new blocking-style stub that supports unary and streaming output calls on the service + */ + public static AnotherReflectableServiceBlockingStub newBlockingStub( + io.grpc.Channel channel) { + io.grpc.stub.AbstractStub.StubFactory factory = + new io.grpc.stub.AbstractStub.StubFactory() { + @java.lang.Override + public AnotherReflectableServiceBlockingStub newStub(io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceBlockingStub(channel, callOptions); + } + }; + return AnotherReflectableServiceBlockingStub.newStub(factory, channel); + } + + /** + * Creates a new ListenableFuture-style stub that supports unary calls on the service + */ + public static AnotherReflectableServiceFutureStub newFutureStub( + io.grpc.Channel channel) { + io.grpc.stub.AbstractStub.StubFactory factory = + new io.grpc.stub.AbstractStub.StubFactory() { + @java.lang.Override + public AnotherReflectableServiceFutureStub newStub(io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceFutureStub(channel, callOptions); + } + }; + return AnotherReflectableServiceFutureStub.newStub(factory, channel); + } + + /** + */ + public static abstract class AnotherReflectableServiceImplBase implements io.grpc.BindableService { + + /** + */ + public void method(io.grpc.reflection.testing.Request request, + io.grpc.stub.StreamObserver responseObserver) { + asyncUnimplementedUnaryCall(getMethodMethod(), responseObserver); + } + + @java.lang.Override public final io.grpc.ServerServiceDefinition bindService() { + return io.grpc.ServerServiceDefinition.builder(getServiceDescriptor()) + .addMethod( + getMethodMethod(), + asyncUnaryCall( + new MethodHandlers< + io.grpc.reflection.testing.Request, + io.grpc.reflection.testing.Reply>( + this, METHODID_METHOD))) + .build(); + } + } + + /** + */ + public static final class AnotherReflectableServiceStub extends io.grpc.stub.AbstractAsyncStub { + private AnotherReflectableServiceStub( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + super(channel, callOptions); + } + + @java.lang.Override + protected AnotherReflectableServiceStub build( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceStub(channel, callOptions); + } + + /** + */ + public void method(io.grpc.reflection.testing.Request request, + io.grpc.stub.StreamObserver responseObserver) { + asyncUnaryCall( + getChannel().newCall(getMethodMethod(), getCallOptions()), request, responseObserver); + } + } + + /** + */ + public static final class AnotherReflectableServiceBlockingStub extends io.grpc.stub.AbstractBlockingStub { + private AnotherReflectableServiceBlockingStub( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + super(channel, callOptions); + } + + @java.lang.Override + protected AnotherReflectableServiceBlockingStub build( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceBlockingStub(channel, callOptions); + } + + /** + */ + public io.grpc.reflection.testing.Reply method(io.grpc.reflection.testing.Request request) { + return blockingUnaryCall( + getChannel(), getMethodMethod(), getCallOptions(), request); + } + } + + /** + */ + public static final class AnotherReflectableServiceFutureStub extends io.grpc.stub.AbstractFutureStub { + private AnotherReflectableServiceFutureStub( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + super(channel, callOptions); + } + + @java.lang.Override + protected AnotherReflectableServiceFutureStub build( + io.grpc.Channel channel, io.grpc.CallOptions callOptions) { + return new AnotherReflectableServiceFutureStub(channel, callOptions); + } + + /** + */ + public com.google.common.util.concurrent.ListenableFuture method( + io.grpc.reflection.testing.Request request) { + return futureUnaryCall( + getChannel().newCall(getMethodMethod(), getCallOptions()), request); + } + } + + private static final int METHODID_METHOD = 0; + + private static final class MethodHandlers implements + io.grpc.stub.ServerCalls.UnaryMethod, + io.grpc.stub.ServerCalls.ServerStreamingMethod, + io.grpc.stub.ServerCalls.ClientStreamingMethod, + io.grpc.stub.ServerCalls.BidiStreamingMethod { + private final AnotherReflectableServiceImplBase serviceImpl; + private final int methodId; + + MethodHandlers(AnotherReflectableServiceImplBase serviceImpl, int methodId) { + this.serviceImpl = serviceImpl; + this.methodId = methodId; + } + + @java.lang.Override + @java.lang.SuppressWarnings("unchecked") + public void invoke(Req request, io.grpc.stub.StreamObserver responseObserver) { + switch (methodId) { + case METHODID_METHOD: + serviceImpl.method((io.grpc.reflection.testing.Request) request, + (io.grpc.stub.StreamObserver) responseObserver); + break; + default: + throw new AssertionError(); + } + } + + @java.lang.Override + @java.lang.SuppressWarnings("unchecked") + public io.grpc.stub.StreamObserver invoke( + io.grpc.stub.StreamObserver responseObserver) { + switch (methodId) { + default: + throw new AssertionError(); + } + } + } + + private static abstract class AnotherReflectableServiceBaseDescriptorSupplier + implements io.grpc.protobuf.ProtoFileDescriptorSupplier, io.grpc.protobuf.ProtoServiceDescriptorSupplier { + AnotherReflectableServiceBaseDescriptorSupplier() {} + + @java.lang.Override + public com.google.protobuf.Descriptors.FileDescriptor getFileDescriptor() { + return io.grpc.reflection.testing.ReflectionTestProto.getDescriptor(); + } + + @java.lang.Override + public com.google.protobuf.Descriptors.ServiceDescriptor getServiceDescriptor() { + return getFileDescriptor().findServiceByName("AnotherReflectableService"); + } + } + + private static final class AnotherReflectableServiceFileDescriptorSupplier + extends AnotherReflectableServiceBaseDescriptorSupplier { + AnotherReflectableServiceFileDescriptorSupplier() {} + } + + private static final class AnotherReflectableServiceMethodDescriptorSupplier + extends AnotherReflectableServiceBaseDescriptorSupplier + implements io.grpc.protobuf.ProtoMethodDescriptorSupplier { + private final String methodName; + + AnotherReflectableServiceMethodDescriptorSupplier(String methodName) { + this.methodName = methodName; + } + + @java.lang.Override + public com.google.protobuf.Descriptors.MethodDescriptor getMethodDescriptor() { + return getServiceDescriptor().findMethodByName(methodName); + } + } + + private static volatile io.grpc.ServiceDescriptor serviceDescriptor; + + public static io.grpc.ServiceDescriptor getServiceDescriptor() { + io.grpc.ServiceDescriptor result = serviceDescriptor; + if (result == null) { + synchronized (AnotherReflectableServiceGrpc.class) { + result = serviceDescriptor; + if (result == null) { + serviceDescriptor = result = io.grpc.ServiceDescriptor.newBuilder(SERVICE_NAME) + .setSchemaDescriptor(new AnotherReflectableServiceFileDescriptorSupplier()) + .addMethod(getMethodMethod()) + .build(); + } + } + } + return result; + } +} diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index cfedd6f01e7..6270c5040a9 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -54,9 +54,9 @@ import java.util.Map; import java.util.Queue; import java.util.Set; +import java.util.WeakHashMap; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; -import javax.annotation.concurrent.GuardedBy; /** * Provides a reflection service for Protobuf services (including the reflection service itself). @@ -68,11 +68,8 @@ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2222") public final class ProtoReflectionService extends ServerReflectionGrpc.ServerReflectionImplBase { - private final Object lock = new Object(); - + private final Map serverReflectionIndexes = new WeakHashMap<>(); private AtomicReference serverRef; - @GuardedBy("lock") - private ServerReflectionIndex serverReflectionIndex; private ProtoReflectionService() {} @@ -92,48 +89,48 @@ private void init(AtomicReference serverRef) { } /** - * Checks for updates to the server's mutable services and updates the index if any changes are + * Retrieves the index for services of the server that dispatches the current call. Computes + * one if not exist. The index is updated if any changes to the server's mutable services are * detected. A change is any addition or removal in the set of file descriptors attached to the * mutable services or a change in the service names. - * - * @return The (potentially updated) index. */ - private ServerReflectionIndex updateIndexIfNecessary() { - synchronized (lock) { - Server server = serverRef.get(); - if (serverReflectionIndex == null) { - serverReflectionIndex = - new ServerReflectionIndex(server.getImmutableServices(), server.getMutableServices()); - return serverReflectionIndex; - } - - Set serverFileDescriptors = new HashSet<>(); - Set serverServiceNames = new HashSet<>(); - List serverMutableServices = server.getMutableServices(); - for (ServerServiceDefinition mutableService : serverMutableServices) { - io.grpc.ServiceDescriptor serviceDescriptor = mutableService.getServiceDescriptor(); - if (serviceDescriptor.getSchemaDescriptor() instanceof ProtoFileDescriptorSupplier) { - String serviceName = serviceDescriptor.getName(); - FileDescriptor fileDescriptor = - ((ProtoFileDescriptorSupplier) serviceDescriptor.getSchemaDescriptor()) - .getFileDescriptor(); - serverFileDescriptors.add(fileDescriptor); - serverServiceNames.add(serviceName); - } - } - - // Replace the index if the underlying mutable services have changed. Check both the file - // descriptors and the service names, because one file descriptor can define multiple - // services. - FileDescriptorIndex mutableServicesIndex = serverReflectionIndex.getMutableServicesIndex(); - if (!mutableServicesIndex.getServiceFileDescriptors().equals(serverFileDescriptors) - || !mutableServicesIndex.getServiceNames().equals(serverServiceNames)) { - serverReflectionIndex = - new ServerReflectionIndex(server.getImmutableServices(), serverMutableServices); + private synchronized ServerReflectionIndex getRefreshedIndex() { + Server server = serverRef.get(); + ServerReflectionIndex index = serverReflectionIndexes.get(server); + if (index == null) { + index = + new ServerReflectionIndex(server.getImmutableServices(), server.getMutableServices()); + serverReflectionIndexes.put(server, index); + return index; + } + + Set serverFileDescriptors = new HashSet<>(); + Set serverServiceNames = new HashSet<>(); + List serverMutableServices = server.getMutableServices(); + for (ServerServiceDefinition mutableService : serverMutableServices) { + io.grpc.ServiceDescriptor serviceDescriptor = mutableService.getServiceDescriptor(); + if (serviceDescriptor.getSchemaDescriptor() instanceof ProtoFileDescriptorSupplier) { + String serviceName = serviceDescriptor.getName(); + FileDescriptor fileDescriptor = + ((ProtoFileDescriptorSupplier) serviceDescriptor.getSchemaDescriptor()) + .getFileDescriptor(); + serverFileDescriptors.add(fileDescriptor); + serverServiceNames.add(serviceName); } + } - return serverReflectionIndex; + // Replace the index if the underlying mutable services have changed. Check both the file + // descriptors and the service names, because one file descriptor can define multiple + // services. + FileDescriptorIndex mutableServicesIndex = index.getMutableServicesIndex(); + if (!mutableServicesIndex.getServiceFileDescriptors().equals(serverFileDescriptors) + || !mutableServicesIndex.getServiceNames().equals(serverServiceNames)) { + index = + new ServerReflectionIndex(server.getImmutableServices(), serverMutableServices); + serverReflectionIndexes.put(server, index); } + + return index; } @Override @@ -142,7 +139,7 @@ public StreamObserver serverReflectionInfo( final ServerCallStreamObserver serverCallStreamObserver = (ServerCallStreamObserver) responseObserver; ProtoReflectionStreamObserver requestObserver = - new ProtoReflectionStreamObserver(updateIndexIfNecessary(), serverCallStreamObserver); + new ProtoReflectionStreamObserver(getRefreshedIndex(), serverCallStreamObserver); serverCallStreamObserver.setOnReadyHandler(requestObserver); serverCallStreamObserver.disableAutoInboundFlowControl(); serverCallStreamObserver.request(1); diff --git a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java index 26603e02f47..333e97c6a7c 100644 --- a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java +++ b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java @@ -29,6 +29,7 @@ import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.internal.testing.StreamRecorder; import io.grpc.reflection.testing.AnotherDynamicServiceGrpc; +import io.grpc.reflection.testing.AnotherReflectableServiceGrpc; import io.grpc.reflection.testing.DynamicReflectionTestDepthTwoProto; import io.grpc.reflection.testing.DynamicServiceGrpc; import io.grpc.reflection.testing.ReflectableServiceGrpc; @@ -46,14 +47,17 @@ import io.grpc.stub.ClientCallStreamObserver; import io.grpc.stub.ClientResponseObserver; import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcCleanupRule; import io.grpc.util.MutableHandlerRegistry; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; -import org.junit.After; +import java.util.concurrent.ExecutionException; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -61,6 +65,9 @@ /** Tests for {@link ProtoReflectionService}. */ @RunWith(JUnit4.class) public class ProtoReflectionServiceTest { + @Rule + public GrpcCleanupRule grpcCleanupRule = new GrpcCleanupRule(); + private static final String TEST_HOST = "localhost"; private MutableHandlerRegistry handlerRegistry = new MutableHandlerRegistry(); private ServerServiceDefinition reflectionService; @@ -68,14 +75,12 @@ public class ProtoReflectionServiceTest { new DynamicServiceGrpc.DynamicServiceImplBase() {}.bindService(); private ServerServiceDefinition anotherDynamicService = new AnotherDynamicServiceGrpc.AnotherDynamicServiceImplBase() {}.bindService(); - private Server server; - private ManagedChannel channel; private ServerReflectionGrpc.ServerReflectionStub stub; @Before public void setUp() throws Exception { reflectionService = ProtoReflectionService.newInstance(); - server = + Server server = InProcessServerBuilder.forName("proto-reflection-test") .directExecutor() .addService(reflectionService) @@ -83,20 +88,13 @@ public void setUp() throws Exception { .fallbackHandlerRegistry(handlerRegistry) .build() .start(); - channel = InProcessChannelBuilder.forName("proto-reflection-test").directExecutor().build(); + grpcCleanupRule.register(server); + ManagedChannel channel = + grpcCleanupRule.register( + InProcessChannelBuilder.forName("proto-reflection-test").directExecutor().build()); stub = ServerReflectionGrpc.newStub(channel); } - @After - public void tearDown() { - if (server != null) { - server.shutdownNow(); - } - if (channel != null) { - channel.shutdownNow(); - } - } - @Test public void listServices() throws Exception { Set originalServices = @@ -523,6 +521,40 @@ public void allExtensionNumbersOfTypeForMutableServices() throws Exception { responseObserver3.firstValue().get().getMessageResponseCase()); } + @Test + public void sharedServiceBetweenServers() + throws IOException, ExecutionException, InterruptedException { + Server anotherServer = InProcessServerBuilder.forName("proto-reflection-test-2") + .directExecutor() + .addService(reflectionService) + .addService(new AnotherReflectableServiceGrpc.AnotherReflectableServiceImplBase() {}) + .build() + .start(); + grpcCleanupRule.register(anotherServer); + ManagedChannel anotherChannel = grpcCleanupRule.register( + InProcessChannelBuilder.forName("proto-reflection-test-2").directExecutor().build()); + ServerReflectionGrpc.ServerReflectionStub stub2 = ServerReflectionGrpc.newStub(anotherChannel); + + ServerReflectionRequest request = + ServerReflectionRequest.newBuilder().setHost(TEST_HOST).setListServices("services").build(); + StreamRecorder responseObserver = StreamRecorder.create(); + StreamObserver requestObserver = + stub2.serverReflectionInfo(responseObserver); + requestObserver.onNext(request); + requestObserver.onCompleted(); + List response = + responseObserver.firstValue().get().getListServicesResponse().getServiceList(); + assertEquals(new HashSet<>( + Arrays.asList( + ServiceResponse.newBuilder() + .setName("grpc.reflection.v1alpha.ServerReflection") + .build(), + ServiceResponse.newBuilder() + .setName("grpc.reflection.testing.AnotherReflectableService") + .build())), + new HashSet<>(response)); + } + @Test public void flowControl() throws Exception { FlowControlClientResponseObserver clientResponseObserver = diff --git a/services/src/test/proto/io/grpc/reflection/testing/reflection_test.proto b/services/src/test/proto/io/grpc/reflection/testing/reflection_test.proto index 12f396964b2..3d0cd020eeb 100644 --- a/services/src/test/proto/io/grpc/reflection/testing/reflection_test.proto +++ b/services/src/test/proto/io/grpc/reflection/testing/reflection_test.proto @@ -32,3 +32,7 @@ extend ThirdLevelType { service ReflectableService { rpc Method (Request) returns (Reply) {} } + +service AnotherReflectableService { + rpc Method (Request) returns (Reply) {} +} diff --git a/testing/src/main/java/io/grpc/internal/NoopServerCall.java b/testing/src/main/java/io/grpc/internal/NoopServerCall.java index 7fe4f325a4f..45038198fa9 100644 --- a/testing/src/main/java/io/grpc/internal/NoopServerCall.java +++ b/testing/src/main/java/io/grpc/internal/NoopServerCall.java @@ -18,7 +18,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; From acb2a44e07c91d634a59b2e81083af75de240c07 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 10:25:36 -0700 Subject: [PATCH 10/16] Use explicit lock --- .../services/ProtoReflectionService.java | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index 6270c5040a9..06defb5534a 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -57,6 +57,7 @@ import java.util.WeakHashMap; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; /** * Provides a reflection service for Protobuf services (including the reflection service itself). @@ -68,6 +69,9 @@ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2222") public final class ProtoReflectionService extends ServerReflectionGrpc.ServerReflectionImplBase { + private final Object lock = new Object(); + + @GuardedBy("lock") private final Map serverReflectionIndexes = new WeakHashMap<>(); private AtomicReference serverRef; @@ -94,43 +98,45 @@ private void init(AtomicReference serverRef) { * detected. A change is any addition or removal in the set of file descriptors attached to the * mutable services or a change in the service names. */ - private synchronized ServerReflectionIndex getRefreshedIndex() { - Server server = serverRef.get(); - ServerReflectionIndex index = serverReflectionIndexes.get(server); - if (index == null) { - index = - new ServerReflectionIndex(server.getImmutableServices(), server.getMutableServices()); - serverReflectionIndexes.put(server, index); - return index; - } + private ServerReflectionIndex getRefreshedIndex() { + synchronized (lock) { + Server server = serverRef.get(); + ServerReflectionIndex index = serverReflectionIndexes.get(server); + if (index == null) { + index = + new ServerReflectionIndex(server.getImmutableServices(), server.getMutableServices()); + serverReflectionIndexes.put(server, index); + return index; + } - Set serverFileDescriptors = new HashSet<>(); - Set serverServiceNames = new HashSet<>(); - List serverMutableServices = server.getMutableServices(); - for (ServerServiceDefinition mutableService : serverMutableServices) { - io.grpc.ServiceDescriptor serviceDescriptor = mutableService.getServiceDescriptor(); - if (serviceDescriptor.getSchemaDescriptor() instanceof ProtoFileDescriptorSupplier) { - String serviceName = serviceDescriptor.getName(); - FileDescriptor fileDescriptor = - ((ProtoFileDescriptorSupplier) serviceDescriptor.getSchemaDescriptor()) - .getFileDescriptor(); - serverFileDescriptors.add(fileDescriptor); - serverServiceNames.add(serviceName); + Set serverFileDescriptors = new HashSet<>(); + Set serverServiceNames = new HashSet<>(); + List serverMutableServices = server.getMutableServices(); + for (ServerServiceDefinition mutableService : serverMutableServices) { + io.grpc.ServiceDescriptor serviceDescriptor = mutableService.getServiceDescriptor(); + if (serviceDescriptor.getSchemaDescriptor() instanceof ProtoFileDescriptorSupplier) { + String serviceName = serviceDescriptor.getName(); + FileDescriptor fileDescriptor = + ((ProtoFileDescriptorSupplier) serviceDescriptor.getSchemaDescriptor()) + .getFileDescriptor(); + serverFileDescriptors.add(fileDescriptor); + serverServiceNames.add(serviceName); + } } - } - // Replace the index if the underlying mutable services have changed. Check both the file - // descriptors and the service names, because one file descriptor can define multiple - // services. - FileDescriptorIndex mutableServicesIndex = index.getMutableServicesIndex(); - if (!mutableServicesIndex.getServiceFileDescriptors().equals(serverFileDescriptors) - || !mutableServicesIndex.getServiceNames().equals(serverServiceNames)) { - index = - new ServerReflectionIndex(server.getImmutableServices(), serverMutableServices); - serverReflectionIndexes.put(server, index); - } + // Replace the index if the underlying mutable services have changed. Check both the file + // descriptors and the service names, because one file descriptor can define multiple + // services. + FileDescriptorIndex mutableServicesIndex = index.getMutableServicesIndex(); + if (!mutableServicesIndex.getServiceFileDescriptors().equals(serverFileDescriptors) + || !mutableServicesIndex.getServiceNames().equals(serverServiceNames)) { + index = + new ServerReflectionIndex(server.getImmutableServices(), serverMutableServices); + serverReflectionIndexes.put(server, index); + } - return index; + return index; + } } @Override From 19fd73030f715f534d53d9dcc671b42172b7b354 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 13:10:00 -0700 Subject: [PATCH 11/16] Put server into Context. --- core/src/main/java/io/grpc/internal/ServerImpl.java | 7 ++++++- core/src/test/java/io/grpc/internal/ServerImplTest.java | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index 765121ce67a..a5eff4b700a 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -45,6 +45,7 @@ import io.grpc.InternalLogId; import io.grpc.InternalServerInterceptors; import io.grpc.Metadata; +import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; @@ -91,6 +92,7 @@ public final class ServerImpl extends io.grpc.Server implements InternalInstrume private static final Logger log = Logger.getLogger(ServerImpl.class.getName()); private static final ServerStreamListener NOOP_LISTENER = new NoopListener(); + static final Context.Key SERVER_CONTEXT_KEY = Context.key("io.grpc.ServerImpl"); private final InternalLogId logId; private final ObjectPool executorPool; /** Executor for application processing. Safe to read after {@link #start()}. */ @@ -593,7 +595,10 @@ private Context.CancellableContext createContext( Metadata headers, StatsTraceContext statsTraceCtx) { Long timeoutNanos = headers.get(TIMEOUT_KEY); - Context baseContext = statsTraceCtx.serverFilterContext(rootContext); + Context baseContext = + statsTraceCtx + .serverFilterContext(rootContext) + .withValue(SERVER_CONTEXT_KEY, ServerImpl.this); if (timeoutNanos == null) { return baseContext.withCancellation(); diff --git a/core/src/test/java/io/grpc/internal/ServerImplTest.java b/core/src/test/java/io/grpc/internal/ServerImplTest.java index 2fe2692bcdb..481556d44f5 100644 --- a/core/src/test/java/io/grpc/internal/ServerImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerImplTest.java @@ -561,6 +561,7 @@ public ServerCall.Listener startCall( Context callContext = callContextReference.get(); assertNotNull(callContext); assertEquals("context added by tracer", SERVER_TRACER_ADDED_KEY.get(callContext)); + assertEquals(server, ServerImpl.SERVER_CONTEXT_KEY.get(callContext)); streamListener.messagesAvailable(new SingleMessageProducer(STRING_MARSHALLER.stream(request))); assertEquals(1, executor.runDueTasks()); From b0d15a6e40eebf727f972d37c4f5345b09690fdb Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 13:10:23 -0700 Subject: [PATCH 12/16] Add an internal accessor for accessing the Context key. --- .../grpc/internal/InternalServerAccessor.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 core/src/main/java/io/grpc/internal/InternalServerAccessor.java diff --git a/core/src/main/java/io/grpc/internal/InternalServerAccessor.java b/core/src/main/java/io/grpc/internal/InternalServerAccessor.java new file mode 100644 index 00000000000..d06def425b4 --- /dev/null +++ b/core/src/main/java/io/grpc/internal/InternalServerAccessor.java @@ -0,0 +1,36 @@ +/* + * Copyright 2020 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.internal; + +import io.grpc.Context; +import io.grpc.Internal; +import io.grpc.Server; + +/** + * Internal accessor for getting the {@link Server} instance inside server RPC {@link Context}. + * This is intended for usage internal to the gRPC team, as it's unclear to us what users would + * need. If you *really* think you need to use this, please file an issue for us to discuss a + * public API. + */ +@Internal +public class InternalServerAccessor { + public static final Context.Key SERVER_KEY = ServerImpl.SERVER_CONTEXT_KEY; + + // Prevent instantiation. + private InternalServerAccessor() { + } +} From d3935faa87c7bfc19fe601f0b130c97ecfadf818 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 13:10:50 -0700 Subject: [PATCH 13/16] Change ProtoReflectionService to obtain Server from Context. --- .../services/ProtoReflectionService.java | 39 +++---------------- .../services/ProtoReflectionServiceTest.java | 3 +- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index 06defb5534a..2ad226e2948 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -24,16 +24,12 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; +import io.grpc.BindableService; import io.grpc.ExperimentalApi; -import io.grpc.Metadata; import io.grpc.Server; -import io.grpc.ServerCall; -import io.grpc.ServerCall.Listener; -import io.grpc.ServerCallHandler; -import io.grpc.ServerInterceptor; -import io.grpc.ServerInterceptors; import io.grpc.ServerServiceDefinition; import io.grpc.Status; +import io.grpc.internal.InternalServerAccessor; import io.grpc.protobuf.ProtoFileDescriptorSupplier; import io.grpc.reflection.v1alpha.ErrorResponse; import io.grpc.reflection.v1alpha.ExtensionNumberResponse; @@ -55,7 +51,6 @@ import java.util.Queue; import java.util.Set; import java.util.WeakHashMap; -import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; @@ -73,23 +68,14 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef @GuardedBy("lock") private final Map serverReflectionIndexes = new WeakHashMap<>(); - private AtomicReference serverRef; private ProtoReflectionService() {} /** * Creates a instance of {@link ProtoReflectionService}. */ - public static ServerServiceDefinition newInstance() { - AtomicReference serverCaptor = new AtomicReference<>(); - ProtoReflectionService protoReflection = new ProtoReflectionService(); - protoReflection.init(serverCaptor); - return ServerInterceptors.intercept( - protoReflection, new ServerCaptureInterceptor(serverCaptor)); - } - - private void init(AtomicReference serverRef) { - this.serverRef = serverRef; + public static BindableService newInstance() { + return new ProtoReflectionService(); } /** @@ -100,7 +86,7 @@ private void init(AtomicReference serverRef) { */ private ServerReflectionIndex getRefreshedIndex() { synchronized (lock) { - Server server = serverRef.get(); + Server server = InternalServerAccessor.SERVER_KEY.get(); ServerReflectionIndex index = serverReflectionIndexes.get(server); if (index == null) { index = @@ -152,21 +138,6 @@ public StreamObserver serverReflectionInfo( return requestObserver; } - private static final class ServerCaptureInterceptor implements ServerInterceptor { - private final AtomicReference captor; - - ServerCaptureInterceptor(AtomicReference captor) { - this.captor = captor; - } - - @Override - public Listener interceptCall(ServerCall call, - Metadata headers, ServerCallHandler next) { - captor.set(call.getServer()); - return next.startCall(call, headers); - } - } - private static class ProtoReflectionStreamObserver implements Runnable, StreamObserver { private final ServerReflectionIndex serverReflectionIndex; diff --git a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java index 333e97c6a7c..1e36f54c87d 100644 --- a/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java +++ b/services/src/test/java/io/grpc/protobuf/services/ProtoReflectionServiceTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.fail; import com.google.protobuf.ByteString; +import io.grpc.BindableService; import io.grpc.ManagedChannel; import io.grpc.Server; import io.grpc.ServerServiceDefinition; @@ -70,7 +71,7 @@ public class ProtoReflectionServiceTest { private static final String TEST_HOST = "localhost"; private MutableHandlerRegistry handlerRegistry = new MutableHandlerRegistry(); - private ServerServiceDefinition reflectionService; + private BindableService reflectionService; private ServerServiceDefinition dynamicService = new DynamicServiceGrpc.DynamicServiceImplBase() {}.bindService(); private ServerServiceDefinition anotherDynamicService = From e4a6b9ba19f297fb85f2e66f740b22ede32b65d8 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 13:21:00 -0700 Subject: [PATCH 14/16] Revert changes for exposing Server via ServerCall. --- .../main/java/io/grpc/PartialForwardingServerCall.java | 6 ------ api/src/main/java/io/grpc/ServerCall.java | 9 --------- .../src/main/java/io/grpc/internal/ServerCallImpl.java | 10 +--------- core/src/main/java/io/grpc/internal/ServerImpl.java | 1 - .../test/java/io/grpc/internal/ServerCallImplTest.java | 9 ++------- 5 files changed, 3 insertions(+), 32 deletions(-) diff --git a/api/src/main/java/io/grpc/PartialForwardingServerCall.java b/api/src/main/java/io/grpc/PartialForwardingServerCall.java index 644a3a17a74..8b95d7ae3fe 100644 --- a/api/src/main/java/io/grpc/PartialForwardingServerCall.java +++ b/api/src/main/java/io/grpc/PartialForwardingServerCall.java @@ -76,12 +76,6 @@ public String getAuthority() { return delegate().getAuthority(); } - @Override - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6989") - public Server getServer() { - return delegate().getServer(); - } - @Override public String toString() { return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString(); diff --git a/api/src/main/java/io/grpc/ServerCall.java b/api/src/main/java/io/grpc/ServerCall.java index a4502f3a9f6..15ac0efdc93 100644 --- a/api/src/main/java/io/grpc/ServerCall.java +++ b/api/src/main/java/io/grpc/ServerCall.java @@ -229,13 +229,4 @@ public String getAuthority() { * The {@link MethodDescriptor} for the call. */ public abstract MethodDescriptor getMethodDescriptor(); - - /** - * Returns the {@link Server} that dispatches the call. {@code null} if the implementation - * choose to not expose the server. - */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6989") - public Server getServer() { - return null; - } } diff --git a/core/src/main/java/io/grpc/internal/ServerCallImpl.java b/core/src/main/java/io/grpc/internal/ServerCallImpl.java index d16c975eead..6f123e76678 100644 --- a/core/src/main/java/io/grpc/internal/ServerCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerCallImpl.java @@ -35,7 +35,6 @@ import io.grpc.InternalDecompressorRegistry; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.perfmark.PerfMark; @@ -53,7 +52,6 @@ final class ServerCallImpl extends ServerCall { @VisibleForTesting static final String MISSING_RESPONSE = "Completed without a response"; - private final Server server; private final ServerStream stream; private final MethodDescriptor method; private final Tag tag; @@ -70,11 +68,10 @@ final class ServerCallImpl extends ServerCall { private Compressor compressor; private boolean messageSent; - ServerCallImpl(Server server, ServerStream stream, MethodDescriptor method, + ServerCallImpl(ServerStream stream, MethodDescriptor method, Metadata inboundHeaders, Context.CancellableContext context, DecompressorRegistry decompressorRegistry, CompressorRegistry compressorRegistry, CallTracer serverCallTracer, Tag tag) { - this.server = server; this.stream = stream; this.method = method; this.context = context; @@ -248,11 +245,6 @@ public MethodDescriptor getMethodDescriptor() { return method; } - @Override - public Server getServer() { - return server; - } - /** * Close the {@link ServerStream} because an internal error occurred. Allow the application to * run until completion, but silently ignore interactions with the {@link ServerStream} from now diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index a5eff4b700a..1bcf12d3a8f 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -641,7 +641,6 @@ private ServerStreamListener startWrappedCall( Tag tag) { ServerCallImpl call = new ServerCallImpl<>( - ServerImpl.this, stream, methodDef.getMethodDescriptor(), headers, diff --git a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java index 02d5d4a0b0c..a4c0da2d69d 100644 --- a/core/src/test/java/io/grpc/internal/ServerCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerCallImplTest.java @@ -39,7 +39,6 @@ import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.Marshaller; import io.grpc.MethodDescriptor.MethodType; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.Status; import io.grpc.internal.ServerCallImpl.ServerStreamListenerImpl; @@ -61,7 +60,6 @@ @RunWith(JUnit4.class) public class ServerCallImplTest { @Rule public final ExpectedException thrown = ExpectedException.none(); - @Mock private Server server; @Mock private ServerStream stream; @Mock private ServerCall.Listener callListener; @@ -91,7 +89,7 @@ public class ServerCallImplTest { public void setUp() { MockitoAnnotations.initMocks(this); context = Context.ROOT.withCancellation(); - call = new ServerCallImpl<>(server, stream, UNARY_METHOD, requestHeaders, context, + call = new ServerCallImpl<>(stream, UNARY_METHOD, requestHeaders, context, DecompressorRegistry.getDefaultInstance(), CompressorRegistry.getDefaultInstance(), serverCallTracer, PerfMark.createTag()); } @@ -114,7 +112,7 @@ private void callTracer0(Status status) { assertEquals(0, before.callsStarted); assertEquals(0, before.lastCallStartedNanos); - call = new ServerCallImpl<>(server, stream, UNARY_METHOD, requestHeaders, context, + call = new ServerCallImpl<>(stream, UNARY_METHOD, requestHeaders, context, DecompressorRegistry.getDefaultInstance(), CompressorRegistry.getDefaultInstance(), tracer, PerfMark.createTag()); @@ -221,7 +219,6 @@ public void sendMessage_serverSendsOne_closeOnSecondCall_clientStreaming() { private void sendMessage_serverSendsOne_closeOnSecondCall( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( - server, stream, method, requestHeaders, @@ -257,7 +254,6 @@ public void sendMessage_serverSendsOne_closeOnSecondCall_appRunToCompletion_clie private void sendMessage_serverSendsOne_closeOnSecondCall_appRunToCompletion( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( - server, stream, method, requestHeaders, @@ -296,7 +292,6 @@ public void serverSendsOne_okFailsOnMissingResponse_clientStreaming() { private void serverSendsOne_okFailsOnMissingResponse( MethodDescriptor method) { ServerCallImpl serverCall = new ServerCallImpl<>( - server, stream, method, requestHeaders, From 9a979c5c6d3ecbd3377867424e75fabcaf330030 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 14:45:07 -0700 Subject: [PATCH 15/16] Move the Context key definition and accessor to io.grpc. --- .../src/main/java/io/grpc/InternalServer.java | 17 ++++++----------- api/src/main/java/io/grpc/Server.java | 8 ++++++++ .../main/java/io/grpc/internal/ServerImpl.java | 2 -- .../java/io/grpc/internal/ServerImplTest.java | 2 +- .../services/ProtoReflectionService.java | 4 ++-- 5 files changed, 17 insertions(+), 16 deletions(-) rename core/src/main/java/io/grpc/internal/InternalServerAccessor.java => api/src/main/java/io/grpc/InternalServer.java (61%) diff --git a/core/src/main/java/io/grpc/internal/InternalServerAccessor.java b/api/src/main/java/io/grpc/InternalServer.java similarity index 61% rename from core/src/main/java/io/grpc/internal/InternalServerAccessor.java rename to api/src/main/java/io/grpc/InternalServer.java index d06def425b4..8a28c91fbec 100644 --- a/core/src/main/java/io/grpc/internal/InternalServerAccessor.java +++ b/api/src/main/java/io/grpc/InternalServer.java @@ -14,23 +14,18 @@ * limitations under the License. */ -package io.grpc.internal; - -import io.grpc.Context; -import io.grpc.Internal; -import io.grpc.Server; +package io.grpc; /** * Internal accessor for getting the {@link Server} instance inside server RPC {@link Context}. - * This is intended for usage internal to the gRPC team, as it's unclear to us what users would - * need. If you *really* think you need to use this, please file an issue for us to discuss a - * public API. + * This is intended for usage internal to the gRPC team. If you think you need to use + * this, contact the gRPC team first. */ @Internal -public class InternalServerAccessor { - public static final Context.Key SERVER_KEY = ServerImpl.SERVER_CONTEXT_KEY; +public class InternalServer { + public static final Context.Key SERVER_CONTEXT_KEY = Server.SERVER_CONTEXT_KEY; // Prevent instantiation. - private InternalServerAccessor() { + private InternalServer() { } } diff --git a/api/src/main/java/io/grpc/Server.java b/api/src/main/java/io/grpc/Server.java index fc98fe242e0..f5b4e811f8e 100644 --- a/api/src/main/java/io/grpc/Server.java +++ b/api/src/main/java/io/grpc/Server.java @@ -30,6 +30,14 @@ @ThreadSafe public abstract class Server { + /** + * Key for accessing the {@link Server} instance inside server RPC {@link Context}. It's + * unclear to us what users would need. If you think you need to use this, please file an + * issue for us to discuss a public API. + */ + protected static final Context.Key SERVER_CONTEXT_KEY = + Context.key("io.grpc.Server"); + /** * Bind and start the server. After this call returns, clients may begin connecting to the * listening socket(s). diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index 1bcf12d3a8f..a528061847f 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -45,7 +45,6 @@ import io.grpc.InternalLogId; import io.grpc.InternalServerInterceptors; import io.grpc.Metadata; -import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; @@ -92,7 +91,6 @@ public final class ServerImpl extends io.grpc.Server implements InternalInstrume private static final Logger log = Logger.getLogger(ServerImpl.class.getName()); private static final ServerStreamListener NOOP_LISTENER = new NoopListener(); - static final Context.Key SERVER_CONTEXT_KEY = Context.key("io.grpc.ServerImpl"); private final InternalLogId logId; private final ObjectPool executorPool; /** Executor for application processing. Safe to read after {@link #start()}. */ diff --git a/core/src/test/java/io/grpc/internal/ServerImplTest.java b/core/src/test/java/io/grpc/internal/ServerImplTest.java index 481556d44f5..d76e714523b 100644 --- a/core/src/test/java/io/grpc/internal/ServerImplTest.java +++ b/core/src/test/java/io/grpc/internal/ServerImplTest.java @@ -561,7 +561,7 @@ public ServerCall.Listener startCall( Context callContext = callContextReference.get(); assertNotNull(callContext); assertEquals("context added by tracer", SERVER_TRACER_ADDED_KEY.get(callContext)); - assertEquals(server, ServerImpl.SERVER_CONTEXT_KEY.get(callContext)); + assertEquals(server, io.grpc.InternalServer.SERVER_CONTEXT_KEY.get(callContext)); streamListener.messagesAvailable(new SingleMessageProducer(STRING_MARSHALLER.stream(request))); assertEquals(1, executor.runDueTasks()); diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index 2ad226e2948..fbf6bf745bf 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -26,10 +26,10 @@ import com.google.protobuf.Descriptors.ServiceDescriptor; import io.grpc.BindableService; import io.grpc.ExperimentalApi; +import io.grpc.InternalServer; import io.grpc.Server; import io.grpc.ServerServiceDefinition; import io.grpc.Status; -import io.grpc.internal.InternalServerAccessor; import io.grpc.protobuf.ProtoFileDescriptorSupplier; import io.grpc.reflection.v1alpha.ErrorResponse; import io.grpc.reflection.v1alpha.ExtensionNumberResponse; @@ -86,7 +86,7 @@ public static BindableService newInstance() { */ private ServerReflectionIndex getRefreshedIndex() { synchronized (lock) { - Server server = InternalServerAccessor.SERVER_KEY.get(); + Server server = InternalServer.SERVER_CONTEXT_KEY.get(); ServerReflectionIndex index = serverReflectionIndexes.get(server); if (index == null) { index = From 6277cac648680d5de348817a9efd9ddaaaefffb5 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 30 Apr 2020 15:00:55 -0700 Subject: [PATCH 16/16] Makee context key package private. --- api/src/main/java/io/grpc/Server.java | 2 +- core/src/main/java/io/grpc/internal/ServerImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/Server.java b/api/src/main/java/io/grpc/Server.java index f5b4e811f8e..781455b18ee 100644 --- a/api/src/main/java/io/grpc/Server.java +++ b/api/src/main/java/io/grpc/Server.java @@ -35,7 +35,7 @@ public abstract class Server { * unclear to us what users would need. If you think you need to use this, please file an * issue for us to discuss a public API. */ - protected static final Context.Key SERVER_CONTEXT_KEY = + static final Context.Key SERVER_CONTEXT_KEY = Context.key("io.grpc.Server"); /** diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java index a528061847f..5db33a226b2 100644 --- a/core/src/main/java/io/grpc/internal/ServerImpl.java +++ b/core/src/main/java/io/grpc/internal/ServerImpl.java @@ -596,7 +596,7 @@ private Context.CancellableContext createContext( Context baseContext = statsTraceCtx .serverFilterContext(rootContext) - .withValue(SERVER_CONTEXT_KEY, ServerImpl.this); + .withValue(io.grpc.InternalServer.SERVER_CONTEXT_KEY, ServerImpl.this); if (timeoutNanos == null) { return baseContext.withCancellation();