From 2e48dd966aa56be5c42a12507757a89ba9f31ce0 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 4 Mar 2025 21:06:12 +0900 Subject: [PATCH] Address the comment from @ikhoon --- .../server/grpc/GrpcServiceBuilder.java | 15 ++---- .../server/grpc/GrpcServiceServerTest.java | 53 +++---------------- .../InternalTestingBlockHoundIntegration.java | 2 - 3 files changed, 13 insertions(+), 57 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java index 479bfdbc2e0..772b6cf454a 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java @@ -438,18 +438,13 @@ private GrpcServiceBuilder addProtoReflectionService(boolean hasDecorators, "Attempting to add a ProtoReflectionService but one is already present. " + "ProtoReflectionService must only be added once."); protoReflectionServiceInterceptor = new ProtoReflectionServiceInterceptor(); + if (protoReflectionService instanceof ProtoReflectionService) { + logger.warn("Use {} instead of {}.", ProtoReflectionServiceV1.class.getSimpleName(), + protoReflectionService); + } final ServerServiceDefinition intercept = ServerInterceptors.intercept(protoReflectionService, protoReflectionServiceInterceptor); - addProtoReflectionIntercept(intercept, path, methodDescriptor); - if (!(protoReflectionService instanceof ProtoReflectionService)) { - return this; - } - logger.warn("Use {} instead of {}.", ProtoReflectionServiceV1.class.getSimpleName(), - protoReflectionService); - final ServerServiceDefinition interceptV1 = - ServerInterceptors.intercept(ProtoReflectionServiceV1.newInstance(), - protoReflectionServiceInterceptor); - return addProtoReflectionIntercept(interceptV1, path, methodDescriptor); + return addProtoReflectionIntercept(intercept, path, methodDescriptor); } private GrpcServiceBuilder addProtoReflectionIntercept(ServerServiceDefinition intercept, diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java index 6287ceb4a1a..7535c1581ab 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcServiceServerTest.java @@ -109,11 +109,11 @@ import io.grpc.Status.Code; import io.grpc.StatusRuntimeException; import io.grpc.protobuf.ProtoUtils; -import io.grpc.protobuf.services.ProtoReflectionService; -import io.grpc.reflection.v1alpha.ServerReflectionGrpc; -import io.grpc.reflection.v1alpha.ServerReflectionGrpc.ServerReflectionStub; -import io.grpc.reflection.v1alpha.ServerReflectionRequest; -import io.grpc.reflection.v1alpha.ServerReflectionResponse; +import io.grpc.protobuf.services.ProtoReflectionServiceV1; +import io.grpc.reflection.v1.ServerReflectionGrpc; +import io.grpc.reflection.v1.ServerReflectionGrpc.ServerReflectionStub; +import io.grpc.reflection.v1.ServerReflectionRequest; +import io.grpc.reflection.v1.ServerReflectionResponse; import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; import io.netty.buffer.ByteBuf; @@ -453,8 +453,7 @@ protected void configure(ServerBuilder sb) throws Exception { sb.service( GrpcService.builder() - // ProtoReflectionServiceV1 is added by default. - .addService(ProtoReflectionService.newInstance()) + .addService(ProtoReflectionServiceV1.newInstance()) .build(), service -> service.decorate(LoggingService.newDecorator())); } @@ -1230,7 +1229,8 @@ void longMaxRequestLimit() throws Exception { void reflectionService() throws Exception { final ServerReflectionStub stub = ServerReflectionGrpc.newStub(channel); - final AtomicReference response = new AtomicReference<>(); + final AtomicReference response = + new AtomicReference<>(); final StreamObserver request = stub.serverReflectionInfo( new StreamObserver() { @@ -1261,43 +1261,6 @@ public void onCompleted() {} }); } - @Test - void reflectionServiceV1() throws Exception { - final io.grpc.reflection.v1.ServerReflectionGrpc.ServerReflectionStub stub = - io.grpc.reflection.v1.ServerReflectionGrpc.newStub(channel); - - final AtomicReference response = - new AtomicReference<>(); - - final StreamObserver request = stub.serverReflectionInfo( - new StreamObserver() { - @Override - public void onNext(io.grpc.reflection.v1.ServerReflectionResponse value) { - response.set(value); - } - - @Override - public void onError(Throwable t) {} - - @Override - public void onCompleted() {} - }); - request.onNext(io.grpc.reflection.v1.ServerReflectionRequest.newBuilder() - .setListServices("") - .build()); - request.onCompleted(); - - await().untilAsserted( - () -> { - assertThat(response).doesNotHaveValue(null); - // Instead of making this test depend on every other one, just check that there is at - // least two services returned corresponding to UnitTestService and - // ProtoReflectionService. - assertThat(response.get().getListServicesResponse().getServiceList()) - .hasSizeGreaterThanOrEqualTo(2); - }); - } - @ParameterizedTest @ArgumentsSource(BlockingClientProvider.class) void replaceException(UnitTestServiceBlockingStub blockingClient) throws Exception { diff --git a/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/InternalTestingBlockHoundIntegration.java b/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/InternalTestingBlockHoundIntegration.java index 89585565087..3d62508a2b6 100644 --- a/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/InternalTestingBlockHoundIntegration.java +++ b/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/InternalTestingBlockHoundIntegration.java @@ -76,8 +76,6 @@ public void applyTo(Builder builder) { builder.allowBlockingCallsInside("com.linecorp.armeria.client.ClientFactory", "ofDefault"); builder.allowBlockingCallsInside("io.envoyproxy.controlplane.cache.SimpleCache", "createWatch"); builder.allowBlockingCallsInside("io.grpc.netty.shaded.io.netty.util.Version", "identify"); - builder.allowBlockingCallsInside("io.fabric8.kubernetes.client.server.mock.WatchEventsListener", - "onClosed"); // prints the exception which makes it easier to debug issues builder.blockingMethodCallback(this::writeBlockingMethod);