[ISSUE #13138] fix processServiceInfo bug (#13137)

* fix processServiceInfo bug * fix processServiceInfo bug fix * fix * fix * fix --------- Co-authored-by: yuecai.liu <[email protected]>
alibaba · Mar 3, 2025 · fa79474 · fa79474
1 parent f257fca
commit fa79474
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 25 deletions.
diff --git a/api/src/main/java/com/alibaba/nacos/api/naming/pojo/ServiceInfo.java b/api/src/main/java/com/alibaba/nacos/api/naming/pojo/ServiceInfo.java
@@ -215,14 +215,17 @@ public String getKey() {
 
     @JsonIgnore
     public static String getKey(String name, String clusters) {
-
         if (!isEmpty(clusters)) {
             return name + Constants.SERVICE_INFO_SPLITER + clusters;
         }
-
         return name;
     }
-
+
+    @JsonIgnore
+    public String getKeyWithoutClusters() {
+        return getGroupedServiceName();
+    }
+
     @JsonIgnore
     public String getKeyEncoded() {
         String serviceName = getGroupedServiceName();

diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/NacosNamingService.java b/client/src/main/java/com/alibaba/nacos/client/naming/NacosNamingService.java
@@ -344,15 +344,15 @@ private ServiceInfo getServiceInfo(String serviceName, String groupName, List<St
     }
 
     private ServiceInfo getServiceInfoByFailover(String serviceName, String groupName, NamingSelector clusterSelector) {
-        ServiceInfo result = serviceInfoHolder.getFailoverServiceInfo(serviceName, groupName, StringUtils.EMPTY);
+        ServiceInfo result = serviceInfoHolder.getFailoverServiceInfo(serviceName, groupName);
         return doSelectInstance(result, clusterSelector);
     }
 
     private ServiceInfo getServiceInfoBySubscribe(String serviceName, String groupName, List<String> clusters,
             NamingSelector selector, boolean subscribe) throws NacosException {
         ServiceInfo serviceInfo;
         if (subscribe) {
-            serviceInfo = serviceInfoHolder.getServiceInfo(serviceName, groupName, StringUtils.EMPTY);
+            serviceInfo = serviceInfoHolder.getServiceInfo(serviceName, groupName);
             serviceInfo = tryToSubscribe(serviceName, groupName, serviceInfo);
             serviceInfo = doSelectInstance(serviceInfo, selector);
         } else {
@@ -590,7 +590,7 @@ private void notifyIfSubscribed(String serviceName, String groupName, NamingSele
             NAMING_LOGGER.warn(
                     "Duplicate subscribe for groupName: {}, serviceName: {}; directly use current cached to notify.",
                     groupName, serviceName);
-            ServiceInfo serviceInfo = serviceInfoHolder.getServiceInfo(serviceName, groupName, Constants.NULL);
+            ServiceInfo serviceInfo = serviceInfoHolder.getServiceInfo(serviceName, groupName);
             InstancesChangeEvent event = transferToEvent(serviceInfo);
             wrapper.notifyListener(event);
         }

diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java b/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java
@@ -94,9 +94,8 @@ public Map<String, ServiceInfo> getServiceInfoMap() {
         return serviceInfoMap;
     }
 
-    public ServiceInfo getServiceInfo(final String serviceName, final String groupName, final String clusters) {
-        String groupedServiceName = NamingUtils.getGroupedName(serviceName, groupName);
-        String key = ServiceInfo.getKey(groupedServiceName, clusters);
+    public ServiceInfo getServiceInfo(final String serviceName, final String groupName) {
+        String key = NamingUtils.getGroupedName(serviceName, groupName);
         return serviceInfoMap.get(key);
     }
 
@@ -119,27 +118,27 @@ public ServiceInfo processServiceInfo(String json) {
      * @return service info
      */
     public ServiceInfo processServiceInfo(ServiceInfo serviceInfo) {
-        String serviceKey = serviceInfo.getKey();
+        String serviceKey = serviceInfo.getKeyWithoutClusters();
         if (serviceKey == null) {
             NAMING_LOGGER.warn("process service info but serviceKey is null, service host: {}",
                     JacksonUtils.toJson(serviceInfo.getHosts()));
             return null;
         }
-        ServiceInfo oldService = serviceInfoMap.get(serviceInfo.getKey());
+        ServiceInfo oldService = serviceInfoMap.get(serviceKey);
         if (isEmptyOrErrorPush(serviceInfo)) {
             //empty or error push, just ignore
             NAMING_LOGGER.warn("process service info but found empty or error push, serviceKey: {}, "
                     + "pushEmptyProtection: {}, hosts: {}", serviceKey, pushEmptyProtection, serviceInfo.getHosts());
             return oldService;
         }
-        serviceInfoMap.put(serviceInfo.getKey(), serviceInfo);
+        serviceInfoMap.put(serviceKey, serviceInfo);
         InstancesDiff diff = getServiceInfoDiff(oldService, serviceInfo);
         if (StringUtils.isBlank(serviceInfo.getJsonFromServer())) {
             serviceInfo.setJsonFromServer(JacksonUtils.toJson(serviceInfo));
         }
         MetricsMonitor.getServiceInfoMapSizeMonitor().set(serviceInfoMap.size());
         if (diff.hasDifferent()) {
-            NAMING_LOGGER.info("current ips:({}) service: {} -> {}", serviceInfo.ipCount(), serviceInfo.getKey(),
+            NAMING_LOGGER.info("current ips:({}) service: {} -> {}", serviceInfo.ipCount(), serviceKey,
                     JacksonUtils.toJson(serviceInfo.getHosts()));
 
             if (!failoverReactor.isFailoverSwitch(serviceKey)) {
@@ -168,9 +167,8 @@ public boolean isFailoverSwitch() {
         return failoverReactor.isFailoverSwitch();
     }
 
-    public ServiceInfo getFailoverServiceInfo(final String serviceName, final String groupName, final String clusters) {
-        String groupedServiceName = NamingUtils.getGroupedName(serviceName, groupName);
-        String key = ServiceInfo.getKey(groupedServiceName, clusters);
+    public ServiceInfo getFailoverServiceInfo(final String serviceName, final String groupName) {
+        String key = NamingUtils.getGroupedName(serviceName, groupName);
         return failoverReactor.getService(key);
     }
 

diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/NacosNamingServiceTest.java
@@ -527,7 +527,7 @@ void testGetAllInstanceFromFailover() throws NacosException {
         when(serviceInfoHolder.isFailoverSwitch()).thenReturn(true);
         ServiceInfo serviceInfo = new ServiceInfo("group1@@service1");
         serviceInfo.setHosts(Collections.singletonList(new Instance()));
-        when(serviceInfoHolder.getFailoverServiceInfo("service1", "group1", "")).thenReturn(serviceInfo);
+        when(serviceInfoHolder.getFailoverServiceInfo("service1", "group1")).thenReturn(serviceInfo);
         List<Instance> actual = client.getAllInstances("service1", "group1", false);
         verify(proxy, never()).queryInstancesOfService(anyString(), anyString(), anyString(), anyBoolean());
         assertEquals(1, actual.size());
@@ -538,7 +538,7 @@ void testGetAllInstanceFromFailover() throws NacosException {
     void testGetAllInstanceFromFailoverEmpty() throws NacosException {
         when(serviceInfoHolder.isFailoverSwitch()).thenReturn(true);
         ServiceInfo serviceInfo = new ServiceInfo("group1@@service1");
-        when(serviceInfoHolder.getFailoverServiceInfo("service1", "group1", "")).thenReturn(serviceInfo);
+        when(serviceInfoHolder.getFailoverServiceInfo("service1", "group1")).thenReturn(serviceInfo);
         List<Instance> actual = client.getAllInstances("service1", "group1", false);
         verify(proxy).queryInstancesOfService(anyString(), anyString(), anyString(), anyBoolean());
         assertEquals(0, actual.size());
@@ -550,7 +550,7 @@ void testGetAllInstanceWithCacheAndSubscribeException() throws NacosException {
         ServiceInfo serviceInfo = new ServiceInfo();
         serviceInfo.setName(serviceName);
         serviceInfo.addHost(new Instance());
-        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP, "")).thenReturn(serviceInfo);
+        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP)).thenReturn(serviceInfo);
         when(proxy.subscribe(serviceName, Constants.DEFAULT_GROUP, "")).thenThrow(new NacosException(500, "test"));
         List<Instance> result = client.getAllInstances(serviceName);
         assertEquals(serviceInfo.getHosts().get(0), result.get(0));
@@ -569,7 +569,7 @@ void testGetAllInstanceWithCacheAndSubscribed() throws NacosException {
         ServiceInfo serviceInfo = new ServiceInfo();
         serviceInfo.setName(serviceName);
         serviceInfo.addHost(new Instance());
-        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP, "")).thenReturn(serviceInfo);
+        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP)).thenReturn(serviceInfo);
         when(proxy.isSubscribed(serviceName, Constants.DEFAULT_GROUP, "")).thenReturn(true);
         List<Instance> result = client.getAllInstances(serviceName);
         assertEquals(serviceInfo.getHosts().get(0), result.get(0));
@@ -991,7 +991,7 @@ void testSubscribeDuplicate() throws NacosException {
         when(proxy.isSubscribed(serviceName, Constants.DEFAULT_GROUP, StringUtils.EMPTY)).thenReturn(true);
         ServiceInfo serviceInfo = new ServiceInfo(Constants.DEFAULT_GROUP + "@@" + serviceName);
         serviceInfo.addHost(new Instance());
-        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP, "")).thenReturn(serviceInfo);
+        when(serviceInfoHolder.getServiceInfo(serviceName, Constants.DEFAULT_GROUP)).thenReturn(serviceInfo);
         final AtomicBoolean flag = new AtomicBoolean(false);
         client.subscribe(serviceName, event -> flag.set(true));
         assertTrue(flag.get());

diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolderTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolderTest.java
@@ -164,8 +164,7 @@ void testGetServiceInfo() {
         ServiceInfo expect = holder.processServiceInfo(info);
         String serviceName = "b";
         String groupName = "a";
-        String clusters = "c";
-        ServiceInfo actual = holder.getServiceInfo(serviceName, groupName, clusters);
+        ServiceInfo actual = holder.getServiceInfo(serviceName, groupName);
         assertEquals(expect.getKey(), actual.getKey());
         assertEquals(expect.getHosts().size(), actual.getHosts().size());
         assertEquals(expect.getHosts().get(0), actual.getHosts().get(0));
@@ -205,8 +204,8 @@ void testIsFailoverSwitch() throws IllegalAccessException, NoSuchFieldException,
     void testGetFailoverServiceInfo() throws IllegalAccessException, NoSuchFieldException, NacosException {
         FailoverReactor mock = injectMockFailoverReactor();
         ServiceInfo serviceInfo = new ServiceInfo("a@@b@@c");
-        when(mock.getService("a@@b@@c")).thenReturn(serviceInfo);
-        assertEquals(serviceInfo, holder.getFailoverServiceInfo("b", "a", "c"));
+        when(mock.getService("a@@b")).thenReturn(serviceInfo);
+        assertEquals(serviceInfo, holder.getFailoverServiceInfo("b", "a"));
     }
 
     private FailoverReactor injectMockFailoverReactor()