From b868bc0148205945542c924c49e855aade4cb765 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Mon, 12 Jun 2023 15:01:20 +0900 Subject: [PATCH] Address comments by @be-hase --- .../grpc/kotlin/CoroutineServerInterceptor.kt | 8 -------- .../armeria/server/grpc/GrpcServiceBuilder.java | 14 +++++++------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/grpc-kotlin/src/main/kotlin/com/linecorp/armeria/server/grpc/kotlin/CoroutineServerInterceptor.kt b/grpc-kotlin/src/main/kotlin/com/linecorp/armeria/server/grpc/kotlin/CoroutineServerInterceptor.kt index 5af2461cd3c0..04b6eb7e0a70 100644 --- a/grpc-kotlin/src/main/kotlin/com/linecorp/armeria/server/grpc/kotlin/CoroutineServerInterceptor.kt +++ b/grpc-kotlin/src/main/kotlin/com/linecorp/armeria/server/grpc/kotlin/CoroutineServerInterceptor.kt @@ -65,20 +65,12 @@ interface CoroutineServerInterceptor : AsyncServerInterceptor { headers: Metadata, next: ServerCallHandler ): CompletableFuture> { - check(call is AbstractServerCall) { - throw IllegalArgumentException( - "Cannot use ${AsyncServerInterceptor::class.java.name} with a non-Armeria gRPC server" - ) - } - val executor = call.blockingExecutor() ?: call.eventLoop() - // COROUTINE_CONTEXT_KEY.get(): // It is necessary to propagate the CoroutineContext set by the previous CoroutineContextServerInterceptor. // (The ArmeriaRequestCoroutineContext is also propagated by CoroutineContextServerInterceptor) // GrpcContextElement.current(): // In gRPC-kotlin, the Coroutine Context is propagated using the gRPC Context. return CoroutineScope( - executor.asCoroutineDispatcher() + ArmeriaRequestCoroutineContext(call.ctx()) + COROUTINE_CONTEXT_KEY.get() + GrpcContextElement.current() ).future { suspendedInterceptCall(call, headers, next) 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 5500506b5d45..38b66d976645 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 @@ -948,11 +948,6 @@ static GrpcStatusFunction toGrpcStatusFunction( private ImmutableList.Builder interceptors() { if (interceptors == null) { interceptors = ImmutableList.builder(); - if (USE_COROUTINE_CONTEXT_INTERCEPTOR) { - final ServerInterceptor coroutineContextInterceptor = - new ArmeriaCoroutineContextInterceptor(useBlockingTaskExecutor); - interceptors.add(coroutineContextInterceptor); - } } return interceptors; } @@ -966,6 +961,11 @@ private ImmutableList.Builder interceptors() { */ public GrpcService build() { final HandlerRegistry handlerRegistry; + if (USE_COROUTINE_CONTEXT_INTERCEPTOR) { + final ServerInterceptor coroutineContextInterceptor = + new ArmeriaCoroutineContextInterceptor(useBlockingTaskExecutor); + interceptors().add(coroutineContextInterceptor); + } if (!enableUnframedRequests && unframedGrpcErrorHandler != null) { throw new IllegalStateException( "'unframedGrpcErrorHandler' can only be set if unframed requests are enabled"); @@ -981,9 +981,9 @@ public GrpcService build() { if (grpcHealthCheckService != null) { registryBuilder.addService(grpcHealthCheckService.bindService(), null, ImmutableList.of()); } - final ImmutableList interceptors = interceptors().build(); - if (!interceptors.isEmpty()) { + if (interceptors != null) { final HandlerRegistry.Builder newRegistryBuilder = new HandlerRegistry.Builder(); + final ImmutableList interceptors = this.interceptors.build(); for (Entry entry : registryBuilder.entries()) { final MethodDescriptor methodDescriptor = entry.method(); final ServerServiceDefinition intercepted =