Check for duplicate services - mutiny and plain; from quarkusio#44326

alesj · Dec 12, 2024 · e39381b · e39381b
1 parent 45fa5d3
commit e39381b
Show file tree

Hide file tree

Showing 9 changed files with 136 additions and 11 deletions.
diff --git a/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java b/extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java
@@ -148,17 +148,18 @@ void processGeneratedBeans(CombinedIndexBuildItem index, BuildProducer<Annotatio
                 continue;
             }
             DotName mutinyImplBase = generatedBean.superName();
+            // Should probably never be 0 ... ?
             if (index.getIndex().getAllKnownSubclasses(mutinyImplBase).size() != 1) {
-                // Some class extends the mutiny impl base
-                continue;
+                // Some other class also extends the mutiny impl base -- 1 is this generated gRPC "bean" class
+                throw new IllegalStateException("Duplicated gRPC service: " + mutinyImplBase);
             }
             String mutinyImplBaseName = mutinyImplBase.toString();
             // Now derive the original impl base
             // e.g. examples.MutinyGreeterGrpc.GreeterImplBase -> examples.GreeterGrpc.GreeterImplBase
             DotName implBase = DotName.createSimple(mutinyImplBaseName.replace(MutinyGrpcGenerator.CLASS_PREFIX, ""));
             if (!index.getIndex().getAllKnownSubclasses(implBase).isEmpty()) {
-                // Some class extends the impl base
-                continue;
+                // Some class already extends the impl base
+                throw new IllegalStateException("Duplicated gRPC service: " + implBase);
             }
             // Finally, exclude some packages
             boolean excluded = false;
@@ -235,6 +236,7 @@ void discoverBindableServices(BuildProducer<BindableServiceBuildItem> bindables,
             if (Modifier.isAbstract(service.flags())) {
                 continue;
             }
+
             BindableServiceBuildItem item = new BindableServiceBuildItem(service.name());
             Set<String> blockingMethods = gatherBlockingOrVirtualMethodNames(service, index, false);
             Set<String> virtualMethods = gatherBlockingOrVirtualMethodNames(service, index, true);

diff --git a/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/DuplicateServiceTest.java b/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/DuplicateServiceTest.java
@@ -0,0 +1,32 @@
+package io.quarkus.grpc.server;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.grpc.examples.dups.Poke;
+import io.quarkus.grpc.server.dups.MutinyPokeService;
+import io.quarkus.grpc.server.dups.PokeService;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class DuplicateServiceTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest unitTest = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(MutinyPokeService.class, PokeService.class)
+                    .addPackage(Poke.class.getPackage()))
+            .assertException(t -> {
+                Assertions.assertTrue(t.getMessage().contains("Duplicated gRPC service"));
+            });
+
+    @Test
+    void testDuplicateService() {
+        fail("Should not be called");
+    }
+
+}
diff --git a/...sions/grpc/deployment/src/test/java/io/quarkus/grpc/server/VertxDuplicateServiceTest.java b/...sions/grpc/deployment/src/test/java/io/quarkus/grpc/server/VertxDuplicateServiceTest.java
@@ -0,0 +1,33 @@
+package io.quarkus.grpc.server;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.grpc.examples.dups.Poke;
+import io.quarkus.grpc.server.dups.MutinyPokeService;
+import io.quarkus.grpc.server.dups.PokeService;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class VertxDuplicateServiceTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest unitTest = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(MutinyPokeService.class, PokeService.class)
+                    .addPackage(Poke.class.getPackage()))
+            .overrideConfigKey("quarkus.grpc.server.use-separate-server", "false")
+            .assertException(t -> {
+                Assertions.assertTrue(t.getMessage().contains("Duplicated gRPC service"));
+            });
+
+    @Test
+    void testDuplicateService() {
+        fail("Should not be called");
+    }
+
+}
diff --git a/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/dups/MutinyPokeService.java b/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/dups/MutinyPokeService.java
@@ -0,0 +1,15 @@
+package io.quarkus.grpc.server.dups;
+
+import io.grpc.examples.dups.Poke;
+import io.grpc.examples.dups.PokeReply;
+import io.grpc.examples.dups.PokeRequest;
+import io.quarkus.grpc.GrpcService;
+import io.smallrye.mutiny.Uni;
+
+@GrpcService
+public class MutinyPokeService implements Poke {
+    @Override
+    public Uni<PokeReply> poke(PokeRequest request) {
+        return null;
+    }
+}
diff --git a/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/dups/PokeService.java b/extensions/grpc/deployment/src/test/java/io/quarkus/grpc/server/dups/PokeService.java
@@ -0,0 +1,14 @@
+package io.quarkus.grpc.server.dups;
+
+import io.grpc.examples.dups.PokeGrpc;
+import io.grpc.examples.dups.PokeReply;
+import io.grpc.examples.dups.PokeRequest;
+import io.grpc.stub.StreamObserver;
+import io.quarkus.grpc.GrpcService;
+
+@GrpcService
+public class PokeService extends PokeGrpc.PokeImplBase {
+    @Override
+    public void poke(PokeRequest request, StreamObserver<PokeReply> responseObserver) {
+    }
+}
diff --git a/extensions/grpc/deployment/src/test/proto/dups.proto b/extensions/grpc/deployment/src/test/proto/dups.proto
@@ -0,0 +1,20 @@
+syntax = "proto3";
+
+option java_multiple_files = true;
+option java_package = "io.grpc.examples.dups";
+option java_outer_classname = "DupsFooProto";
+option objc_class_prefix = "DF";
+
+package dups;
+
+service Poke {
+    rpc poke (PokeRequest) returns (PokeReply) {}
+}
+
+message PokeRequest {
+    string name = 1;
+}
+
+message PokeReply {
+    string message = 1;
+}
diff --git a/...grpc/reflection/src/main/java/io/quarkus/grpc/reflection/service/ReflectionServiceV1.java b/...grpc/reflection/src/main/java/io/quarkus/grpc/reflection/service/ReflectionServiceV1.java
@@ -28,8 +28,12 @@ public class ReflectionServiceV1 extends MutinyServerReflectionGrpc.ServerReflec
 
     private final GrpcServerIndex index;
 
+    public ReflectionServiceV1(GrpcServerIndex index) {
+        this.index = index;
+    }
+
     public ReflectionServiceV1(List<ServerServiceDefinition> definitions) {
-        index = new GrpcServerIndex(definitions);
+        this(new GrpcServerIndex(definitions));
     }
 
     @Override

diff --git a/...reflection/src/main/java/io/quarkus/grpc/reflection/service/ReflectionServiceV1alpha.java b/...reflection/src/main/java/io/quarkus/grpc/reflection/service/ReflectionServiceV1alpha.java
@@ -21,8 +21,12 @@ public class ReflectionServiceV1alpha extends MutinyServerReflectionGrpc.ServerR
 
     private final GrpcServerIndex index;
 
+    public ReflectionServiceV1alpha(GrpcServerIndex index) {
+        this.index = index;
+    }
+
     public ReflectionServiceV1alpha(List<ServerServiceDefinition> definitions) {
-        index = new GrpcServerIndex(definitions);
+        this(new GrpcServerIndex(definitions));
     }
 
     @Override

diff --git a/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java b/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java
@@ -177,7 +177,6 @@ private void buildGrpcServer(Vertx vertx, GrpcServerConfiguration configuration,
         }
 
         boolean reflectionServiceEnabled = configuration.enableReflectionService || launchMode == LaunchMode.DEVELOPMENT;
-
         if (reflectionServiceEnabled) {
             LOGGER.info("Registering gRPC reflection service");
             ReflectionServiceV1 reflectionServiceV1 = new ReflectionServiceV1(definitions);
@@ -261,7 +260,9 @@ private void prodStart(GrpcContainer grpcContainer, Vertx vertx, GrpcServerConfi
         } catch (TimeoutException e) {
             LOGGER.error("Unable to start the gRPC server, still not listening after 1 minute");
         } catch (ExecutionException e) {
-            LOGGER.error("Unable to start the gRPC server", e.getCause());
+            Throwable cause = e.getCause();
+            LOGGER.error("Unable to start the gRPC server", cause);
+            throw new RuntimeException(cause);
         }
     }
 
@@ -325,11 +326,11 @@ private void devModeStart(GrpcContainer grpcContainer, Vertx vertx, GrpcServerCo
             try {
                 future.get(1, TimeUnit.MINUTES);
             } catch (TimeoutException e) {
-                LOGGER.error("Failed to start grpc server in time", e);
+                LOGGER.error("Failed to start gRPC server in time", e);
             } catch (ExecutionException e) {
-                throw new RuntimeException("grpc server start failed", e);
+                throw new RuntimeException("Unable to start the gRPC server", e);
             } catch (InterruptedException e) {
-                LOGGER.warn("Waiting for grpc server start interrupted", e);
+                LOGGER.warn("Waiting for gRPC server start interrupted", e);
                 Thread.currentThread().interrupt();
             }