Throw exception if service is registered twice (Implements #365)

Author: ST-DDT

grpc-server-spring-boot-autoconfigure/src/main/java/net/devh/boot/grpc/server/serverfactory/AbstractGrpcServerFactory.java

@@ -19,17 +19,22 @@
 
 import static java.util.Objects.requireNonNull;
 
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.util.unit.DataSize;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 
 import io.grpc.Server;
 import io.grpc.ServerBuilder;
 import io.grpc.health.v1.HealthCheckResponse;
+import io.grpc.health.v1.HealthGrpc;
 import io.grpc.protobuf.services.ProtoReflectionService;
+import io.grpc.reflection.v1alpha.ServerReflectionGrpc;
 import io.grpc.services.HealthStatusManager;
 import lombok.extern.slf4j.Slf4j;
 import net.devh.boot.grpc.server.config.GrpcServerProperties;
@@ -52,7 +57,8 @@ public abstract class AbstractGrpcServerFactory<T extends ServerBuilder<T>> impl
     protected final List<GrpcServerConfigurer> serverConfigurers;
 
     @Autowired
-    private HealthStatusManager healthStatusManager;
+    @VisibleForTesting
+    HealthStatusManager healthStatusManager;
 
     /**
      * Creates a new server factory with the given properties.
@@ -102,16 +108,24 @@ protected void configure(final T builder) {
      * @param builder The server builder to configure.
      */
     protected void configureServices(final T builder) {
+        final Set<String> serviceNames = new LinkedHashSet<>();
+
         // support health check
         if (this.properties.isHealthServiceEnabled()) {
             builder.addService(this.healthStatusManager.getHealthService());
+            serviceNames.add(HealthGrpc.getServiceDescriptor().getName());
         }
+        // gRPC reflection
         if (this.properties.isReflectionServiceEnabled()) {
             builder.addService(ProtoReflectionService.newInstance());
+            serviceNames.add(ServerReflectionGrpc.getServiceDescriptor().getName());
         }
 
         for (final GrpcServiceDefinition service : this.serviceList) {
             final String serviceName = service.getDefinition().getServiceDescriptor().getName();
+            if (!serviceNames.add(serviceName)) {
+                throw new IllegalStateException("Found duplicate service implementation: " + serviceName);
+            }
             log.info("Registered gRPC service: " + serviceName + ", bean: " + service.getBeanName() + ", class: "
                     + service.getBeanClazz().getName());
             builder.addService(service.getDefinition());

grpc-server-spring-boot-autoconfigure/src/test/java/net/devh/boot/grpc/server/serverfactory/AbstractGrpcServerFactoryTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2016-2020 Michael Zhang <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+package net.devh.boot.grpc.server.serverfactory;
+
+import static java.util.Collections.emptyList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+
+import io.grpc.ServerBuilder;
+import io.grpc.netty.NettyServerBuilder;
+import io.grpc.protobuf.services.ProtoReflectionService;
+import io.grpc.reflection.v1alpha.ServerReflectionGrpc;
+import io.grpc.services.HealthStatusManager;
+import net.devh.boot.grpc.server.config.GrpcServerProperties;
+import net.devh.boot.grpc.server.service.GrpcServiceDefinition;
+
+/**
+ * Tests for {@link AbstractGrpcServerFactory}.
+ */
+class AbstractGrpcServerFactoryTest {
+
+    /**
+     * Tests {@link AbstractGrpcServerFactory#configureServices(ServerBuilder)}.
+     */
+    @Test
+    void testConfigureServices() {
+        final GrpcServerProperties properties = new GrpcServerProperties();
+        properties.setReflectionServiceEnabled(false);
+
+        final NettyGrpcServerFactory serverFactory = new NettyGrpcServerFactory(properties, emptyList());
+        serverFactory.healthStatusManager = new HealthStatusManager();
+
+        serverFactory.addService(new GrpcServiceDefinition("test1", ProtoReflectionService.class,
+                ProtoReflectionService.newInstance().bindService()));
+        serverFactory.addService(new GrpcServiceDefinition("test2", ProtoReflectionService.class,
+                ProtoReflectionService.newInstance().bindService()));
+
+        final NettyServerBuilder serverBuilder = serverFactory.newServerBuilder();
+
+        assertEquals("Found duplicate service implementation: " + ServerReflectionGrpc.SERVICE_NAME,
+                assertThrows(IllegalStateException.class, () -> serverFactory.configureServices(serverBuilder))
+                        .getMessage());
+    }
+
+}