From 0d2e18cb4f370fdc3f19d2368019c54e2ddc3f33 Mon Sep 17 00:00:00 2001 From: Santiago Pericas-Geertsen Date: Thu, 1 Aug 2024 11:27:37 -0400 Subject: [PATCH] Introduces a new interface implemented by all gRPC client proxies that can be used to update the server's listening port dynamically. This is very useful during testing. Removes reflective code from the client gRPC library and updates tests. --- .../microprofile/grpc/StringServiceTest.java | 13 +++++++ .../grpc/client/GrpcChannelsProvider.java | 32 ----------------- .../grpc/client/GrpcClientBuilder.java | 2 +- .../grpc/client/GrpcConfigurablePort.java | 35 +++++++++++++++++++ .../grpc/client/GrpcProxyBuilder.java | 2 +- .../grpc/client/GrpcServiceClient.java | 12 +++++++ .../grpc/client/EchoServiceTest.java | 8 +++++ .../helidon/webclient/grpc/GrpcChannel.java | 11 +++++- .../io/helidon/webclient/grpc/GrpcClient.java | 7 ++++ .../webclient/grpc/GrpcClientImpl.java | 5 +++ 10 files changed, 92 insertions(+), 35 deletions(-) create mode 100644 microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcConfigurablePort.java diff --git a/examples/microprofile/grpc/src/test/java/io/helidon/examples/microprofile/grpc/StringServiceTest.java b/examples/microprofile/grpc/src/test/java/io/helidon/examples/microprofile/grpc/StringServiceTest.java index 0d37d16cde3..b6e9410f134 100644 --- a/examples/microprofile/grpc/src/test/java/io/helidon/examples/microprofile/grpc/StringServiceTest.java +++ b/examples/microprofile/grpc/src/test/java/io/helidon/examples/microprofile/grpc/StringServiceTest.java @@ -22,10 +22,13 @@ import java.util.stream.Stream; import io.helidon.grpc.api.Grpc; +import io.helidon.microprofile.grpc.client.GrpcConfigurablePort; import io.helidon.microprofile.testing.junit5.HelidonTest; import io.grpc.stub.StreamObserver; import jakarta.inject.Inject; +import jakarta.ws.rs.client.WebTarget; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.is; @@ -36,10 +39,20 @@ @HelidonTest class StringServiceTest { + @Inject + private WebTarget webTarget; + @Inject @Grpc.GrpcProxy private StringServiceClient client; + @BeforeEach + void updatePort() { + if (client instanceof GrpcConfigurablePort c) { + c.channelPort(webTarget.getUri().getPort()); + } + } + @Test void testUnaryUpper() { Strings.StringMessage res = client.upper(newMessage("hello")); diff --git a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcChannelsProvider.java b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcChannelsProvider.java index 85fd849bd54..aacbc1f25d6 100644 --- a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcChannelsProvider.java +++ b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcChannelsProvider.java @@ -16,7 +16,6 @@ package io.helidon.microprofile.grpc.client; -import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; @@ -25,8 +24,6 @@ import io.helidon.webclient.grpc.GrpcClient; import io.grpc.Channel; -import jakarta.enterprise.inject.spi.CDI; -import jakarta.enterprise.inject.spi.Extension; /** * GrpcChannelsProvider is a factory for pre-configured gRPC Channel instances. @@ -43,11 +40,6 @@ public class GrpcChannelsProvider { */ public static final String DEFAULT_HOST = "localhost"; - /** - * The configuration key for the channels' configuration. - */ - public static final String CFG_KEY_CHANNELS = "channels"; - /** * A constant for holding the default port (which is "1408"). */ @@ -136,9 +128,6 @@ private Channel createChannel(String name, GrpcChannelDescriptor descriptor) { throw new IllegalArgumentException("Client TLS must be configured for gRPC proxy client"); } int port = descriptor.port(); - if (port <= 0) { - port = discoverServerPort(name); - } GrpcClient grpcClient = GrpcClient.builder() .tls(clientTls) .baseUri("https://" + descriptor.host() + ":" + port) @@ -146,27 +135,6 @@ private Channel createChannel(String name, GrpcChannelDescriptor descriptor) { return grpcClient.channel(); } - /** - * Used for unit testing: if port not set, try to obtain port from server CDI - * extension via reflection, without introducing a static dependency. - * - * @param name the channel name - * @return server port - * @throws java.lang.IllegalArgumentException if unable to discover port - */ - @SuppressWarnings("unchecked") - private static int discoverServerPort(String name) { - try { - Class extClass = (Class) Class - .forName("io.helidon.microprofile.server.ServerCdiExtension"); - Extension extension = CDI.current().getBeanManager().getExtension(extClass); - Method m = extension.getClass().getMethod("port", String.class); - return (int) m.invoke(extension, new Object[] {"@default"}); - } catch (ReflectiveOperationException e) { - throw new IllegalArgumentException("Unable to determine server port for channel " + name); - } - } - /** * Builder builds an instance of {@link GrpcChannelsProvider}. */ diff --git a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcClientBuilder.java b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcClientBuilder.java index baf4ce5ebaf..b391de0116a 100644 --- a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcClientBuilder.java +++ b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcClientBuilder.java @@ -32,7 +32,7 @@ import static java.lang.System.Logger.Level; /** - * A builder for constructing a {@link io.helidon.microprofile.grpc.client.ClientServiceDescriptor.Builder} instances + * A builder for constructing a {@link io.helidon.microprofile.grpc.client.ClientServiceDescriptor} instances * from an annotated POJO. */ class GrpcClientBuilder extends AbstractServiceBuilder diff --git a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcConfigurablePort.java b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcConfigurablePort.java new file mode 100644 index 00000000000..9742daf7bac --- /dev/null +++ b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcConfigurablePort.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * 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.helidon.microprofile.grpc.client; + +/** + * Interface implemented by all gRPC client proxies. The method {@link #channelPort} can be + * called at runtime to override the client URI port from config. Typically used for testing. + */ +public interface GrpcConfigurablePort { + + /** + * Name of single setter method on this interface. + */ + String CHANNEL_PORT = "channelPort"; + + /** + * Overrides client URI port. + * + * @param value the new port value + */ + void channelPort(int value); +} diff --git a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcProxyBuilder.java b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcProxyBuilder.java index a1fb6b29151..82d9bea7f00 100644 --- a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcProxyBuilder.java +++ b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcProxyBuilder.java @@ -68,7 +68,7 @@ public static GrpcProxyBuilder create(Channel channel, Class type) { */ @Override public T build() { - return client.proxy(type); + return client.proxy(type, GrpcConfigurablePort.class); } private static ClientServiceDescriptor createDescriptor(Class type) { diff --git a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcServiceClient.java b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcServiceClient.java index 8bb65eb4b03..022a2293a36 100644 --- a/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcServiceClient.java +++ b/microprofile/grpc/client/src/main/java/io/helidon/microprofile/grpc/client/GrpcServiceClient.java @@ -30,6 +30,7 @@ import io.helidon.grpc.core.InterceptorWeights; import io.helidon.grpc.core.MethodHandler; import io.helidon.grpc.core.WeightedBag; +import io.helidon.webclient.api.ClientUri; import io.grpc.CallOptions; import io.grpc.Channel; @@ -45,6 +46,8 @@ */ public class GrpcServiceClient { + private final Channel channel; + private final HashMap> methodStubs; private final ClientServiceDescriptor clientServiceDescriptor; @@ -76,6 +79,7 @@ public static GrpcServiceClient create(Channel channel, ClientServiceDescriptor private GrpcServiceClient(Channel channel, CallOptions callOptions, ClientServiceDescriptor clientServiceDescriptor) { + this.channel = channel; this.clientServiceDescriptor = clientServiceDescriptor; this.methodStubs = new HashMap<>(); @@ -137,6 +141,13 @@ public String serviceName() { Object invoke(String name, Object[] args) { GrpcMethodStub stub = methodStubs.get(name); if (stub == null) { + // may be used during testing to override a channel's port + if (name.equals(GrpcConfigurablePort.CHANNEL_PORT)) { + io.helidon.webclient.grpc.GrpcChannel grpcChannel = (io.helidon.webclient.grpc.GrpcChannel) channel; + ClientUri uri = grpcChannel.grpcClient().clientConfig().baseUri().orElseThrow(); + uri.port((int) args[0]); + return null; + } throw Status.INTERNAL.withDescription("gRPC method '" + name + "' does not exist").asRuntimeException(); } ClientMethodDescriptor descriptor = stub.descriptor(); @@ -164,6 +175,7 @@ Object invoke(String name, Object[] args) { @SuppressWarnings("unchecked") public T proxy(Class type, Class... extraTypes) { Map names = new HashMap<>(); + names.put(GrpcConfigurablePort.CHANNEL_PORT, GrpcConfigurablePort.CHANNEL_PORT); // for testing for (ClientMethodDescriptor methodDescriptor : clientServiceDescriptor.methods()) { MethodHandler methodHandler = methodDescriptor.methodHandler(); if (methodHandler != null) { diff --git a/microprofile/grpc/client/src/test/java/io/helidon/microprofile/grpc/client/EchoServiceTest.java b/microprofile/grpc/client/src/test/java/io/helidon/microprofile/grpc/client/EchoServiceTest.java index 7d46dd392b5..68db09e9b91 100644 --- a/microprofile/grpc/client/src/test/java/io/helidon/microprofile/grpc/client/EchoServiceTest.java +++ b/microprofile/grpc/client/src/test/java/io/helidon/microprofile/grpc/client/EchoServiceTest.java @@ -33,6 +33,7 @@ import io.grpc.stub.StreamObserver; import jakarta.inject.Inject; import jakarta.ws.rs.client.WebTarget; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static io.helidon.grpc.core.ResponseHelper.complete; @@ -53,6 +54,13 @@ class EchoServiceTest { @Grpc.GrpcProxy private EchoServiceClient proxyClient; + @BeforeEach + void updatePort() { + if (proxyClient instanceof GrpcConfigurablePort client) { + client.channelPort(webTarget.getUri().getPort()); + } + } + @Test void testEcho() throws InterruptedException, ExecutionException, TimeoutException { Tls clientTls = Tls.builder() diff --git a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcChannel.java b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcChannel.java index 3efb29e47f7..f4d5e006cb7 100644 --- a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcChannel.java +++ b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcChannel.java @@ -26,7 +26,7 @@ /** * Helidon's implementation of a gRPC {@link Channel}. */ -class GrpcChannel extends Channel { +public class GrpcChannel extends Channel { private final GrpcClientImpl grpcClient; @@ -39,6 +39,15 @@ class GrpcChannel extends Channel { this.grpcClient = (GrpcClientImpl) grpcClient; } + /** + * Underlying gRPC Client for this channel. + * + * @return the gRPC client + */ + public GrpcClient grpcClient() { + return grpcClient; + } + @Override public ClientCall newCall( MethodDescriptor methodDescriptor, CallOptions callOptions) { diff --git a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClient.java b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClient.java index d786f7840f0..b4ffdc582cf 100644 --- a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClient.java +++ b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClient.java @@ -114,4 +114,11 @@ static GrpcClient create() { default Channel channel(Collection interceptors) { return channel(interceptors.toArray(new ClientInterceptor[]{})); } + + /** + * Configuration for this gRPC client. + * + * @return the configuration + */ + GrpcClientConfig clientConfig(); } diff --git a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClientImpl.java b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClientImpl.java index 7a8ce6dfa91..077b8e9a83f 100644 --- a/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClientImpl.java +++ b/webclient/grpc/src/main/java/io/helidon/webclient/grpc/GrpcClientImpl.java @@ -61,4 +61,9 @@ public Channel channel() { public Channel channel(ClientInterceptor... interceptors) { return ClientInterceptors.intercept(channel(), interceptors); } + + @Override + public GrpcClientConfig clientConfig() { + return clientConfig; + } }