fix(#11895): grpc 服务注册失败时返回了成功 (#11909)

naming/src/main/java/com/alibaba/nacos/naming/core/v2/service/impl/EphemeralClientOperationServiceImpl.java

@@ -63,9 +63,7 @@ public class EphemeralClientOperationServiceImpl implements ClientOperationServi
                             singleton.getGroupedServiceName()));
         }
         Client client = clientManager.getClient(clientId);
-        if (!clientIsLegal(client, clientId)) {
+        checkClientIsLegal(client, clientId);
-            return;
-        }
         InstancePublishInfo instanceInfo = getPublishInfo(instance);
         client.addServiceInstance(singleton, instanceInfo);
         client.setLastUpdatedTime();
@@ -84,9 +82,7 @@ public class EphemeralClientOperationServiceImpl implements ClientOperationServi
                             singleton.getGroupedServiceName()));
         }
         Client client = clientManager.getClient(clientId);
-        if (!clientIsLegal(client, clientId)) {
+        checkClientIsLegal(client, clientId);
-            return;
-        }
         BatchInstancePublishInfo batchInstancePublishInfo = new BatchInstancePublishInfo();
         List<InstancePublishInfo> resultList = new ArrayList<>();
         for (Instance instance : instances) {
@@ -110,9 +106,7 @@ public class EphemeralClientOperationServiceImpl implements ClientOperationServi
         }
         Service singleton = ServiceManager.getInstance().getSingleton(service);
         Client client = clientManager.getClient(clientId);
-        if (!clientIsLegal(client, clientId)) {
+        checkClientIsLegal(client, clientId);
-            return;
-        }
         InstancePublishInfo removedInstance = client.removeServiceInstance(singleton);
         client.setLastUpdatedTime();
         client.recalculateRevision();
@@ -127,9 +121,7 @@ public class EphemeralClientOperationServiceImpl implements ClientOperationServi
     public void subscribeService(Service service, Subscriber subscriber, String clientId) {
         Service singleton = ServiceManager.getInstance().getSingletonIfExist(service).orElse(service);
         Client client = clientManager.getClient(clientId);
-        if (!clientIsLegal(client, clientId)) {
+        checkClientIsLegal(client, clientId);
-            return;
-        }
         client.addServiceSubscriber(singleton, subscriber);
         client.setLastUpdatedTime();
         NotifyCenter.publishEvent(new ClientOperationEvent.ClientSubscribeServiceEvent(singleton, clientId));
@@ -139,23 +131,24 @@ public class EphemeralClientOperationServiceImpl implements ClientOperationServi
     public void unsubscribeService(Service service, Subscriber subscriber, String clientId) {
         Service singleton = ServiceManager.getInstance().getSingletonIfExist(service).orElse(service);
         Client client = clientManager.getClient(clientId);
-        if (!clientIsLegal(client, clientId)) {
+        checkClientIsLegal(client, clientId);
-            return;
-        }
         client.removeServiceSubscriber(singleton);
         client.setLastUpdatedTime();
         NotifyCenter.publishEvent(new ClientOperationEvent.ClientUnsubscribeServiceEvent(singleton, clientId));
     }
 
-    private boolean clientIsLegal(Client client, String clientId) {
+    private void checkClientIsLegal(Client client, String clientId) {
         if (client == null) {
             Loggers.SRV_LOG.warn("Client connection {} already disconnect", clientId);
-            return false;
+            throw new NacosRuntimeException(NacosException.CLIENT_DISCONNECT,
+                    String.format("Client [%s] connection already disconnect, can't register ephemeral instance.",
+                            clientId));
         }
         if (!client.isEphemeral()) {
             Loggers.SRV_LOG.warn("Client connection {} type is not ephemeral", clientId);
-            return false;
+            throw new NacosRuntimeException(NacosException.INVALID_PARAM,
-        }
+                    String.format("Current client [%s] is persistent client, can't register ephemeral instance.",
-        return true;
+                            clientId));
+        }
     }
 }

naming/src/test/java/com/alibaba/nacos/naming/core/v2/service/impl/EphemeralClientOperationServiceImplTest.java

@@ -38,6 +38,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -148,4 +149,84 @@ public class EphemeralClientOperationServiceImplTest extends TestCase {
         ephemeralClientOperationServiceImpl.unsubscribeService(service, subscriber, ipPortBasedClientId);
         assertFalse(ipPortBasedClient.getAllSubscribeService().contains(service));
     }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testRegisterWhenClientNull() throws NacosException {
+        when(clientManager.getClient(anyString())).thenReturn(null);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.registerInstance(service, instance, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testRegisterWhenClientPersistent() throws NacosException {
+        Client persistentClient = new IpPortBasedClient(ipPortBasedClientId, false);
+        when(clientManager.getClient(anyString())).thenReturn(persistentClient);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.registerInstance(service, instance, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testBatchRegisterWhenClientNull() {
+        when(clientManager.getClient(anyString())).thenReturn(null);
+        // Excepted exception
+        List<Instance> instances = new ArrayList<>();
+        ephemeralClientOperationServiceImpl.batchRegisterInstance(service, instances, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testBatchRegisterWhenClientPersistent() {
+        Client persistentClient = new IpPortBasedClient(ipPortBasedClientId, false);
+        when(clientManager.getClient(anyString())).thenReturn(persistentClient);
+        // Excepted exception
+        List<Instance> instances = new ArrayList<>();
+        ephemeralClientOperationServiceImpl.batchRegisterInstance(service, instances, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testDeRegisterWhenClientNull() throws NacosException {
+        // Test register instance
+        ephemeralClientOperationServiceImpl.registerInstance(service, instance, ipPortBasedClientId);
+        when(clientManager.getClient(anyString())).thenReturn(null);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.registerInstance(service, instance, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testDeRegisterWhenClientPersistent() throws NacosException {
+        ephemeralClientOperationServiceImpl.registerInstance(service, instance, ipPortBasedClientId);
+        Client persistentClient = new IpPortBasedClient(ipPortBasedClientId, false);
+        when(clientManager.getClient(anyString())).thenReturn(persistentClient);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.deregisterInstance(service, instance, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testSubscribeWhenClientNull() {
+        when(clientManager.getClient(anyString())).thenReturn(null);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.subscribeService(service, subscriber, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testSubscribeWhenClientPersistent() {
+        Client persistentClient = new IpPortBasedClient(ipPortBasedClientId, false);
+        when(clientManager.getClient(anyString())).thenReturn(persistentClient);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.subscribeService(service, subscriber, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testUnSubscribeWhenClientNull() {
+        when(clientManager.getClient(anyString())).thenReturn(null);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.unsubscribeService(service, subscriber, ipPortBasedClientId);
+    }
+
+    @Test(expected = NacosRuntimeException.class)
+    public void testUnSubscribeWhenClientPersistent() {
+        Client persistentClient = new IpPortBasedClient(ipPortBasedClientId, false);
+        when(clientManager.getClient(anyString())).thenReturn(persistentClient);
+        // Excepted exception
+        ephemeralClientOperationServiceImpl.unsubscribeService(service, subscriber, ipPortBasedClientId);
+    }
 }