[ISSUE #4136] Fix the bug that cann't correctlly instantiate ServiceInfo from cache file #4249 (#4272)

api/src/main/java/com/alibaba/nacos/api/naming/pojo/ServiceInfo.java

@@ -31,6 +31,7 @@ import java.util.List;
  * ServiceInfo.
  *
  * @author nkorange
+ * @author shizhengxing
  */
 @JsonInclude(Include.NON_NULL)
 public class ServiceInfo {
@@ -67,19 +68,28 @@ public class ServiceInfo {
         this.allIPs = allIPs;
     }
     
+    /**
+     * There is only one form of the key:groupName@@name@clusters. This constuctor used by DiskCache.read(String) and
+     * FailoverReactor.FailoverFileReader,you should know that 'groupName' must not be null,and 'clusters' can be null.
+     */
     public ServiceInfo(String key) {
-        
         int maxIndex = 2;
-        int clusterIndex = 1;
+        int clusterIndex = 2;
-        int serviceNameIndex = 0;
+        int serviceNameIndex = 1;
+        int groupIndex = 0;
         
         String[] keys = key.split(Constants.SERVICE_INFO_SPLITER);
-        if (keys.length >= maxIndex) {
+        if (keys.length >= maxIndex + 1) {
+            this.groupName = keys[groupIndex];
             this.name = keys[serviceNameIndex];
             this.clusters = keys[clusterIndex];
+        } else if (keys.length == maxIndex) {
+            this.groupName = keys[groupIndex];
+            this.name = keys[serviceNameIndex];
+        } else {
+            //defensive programming
+            throw new IllegalArgumentException("Cann't parse out 'groupName',but it must not be null!");
         }
-        
-        this.name = keys[0];
     }
     
     public ServiceInfo(String name, String clusters) {
@@ -182,7 +192,8 @@ public class ServiceInfo {
     
     @JsonIgnore
     public String getKey() {
-        return getKey(name, clusters);
+        String serviceName = getGroupedServiceName();
+        return getKey(serviceName, clusters);
     }
     
     @JsonIgnore
@@ -197,11 +208,21 @@ public class ServiceInfo {
     
     @JsonIgnore
     public String getKeyEncoded() {
+        String serviceName = getGroupedServiceName();
         try {
-            return getKey(URLEncoder.encode(name, "UTF-8"), clusters);
+            serviceName = URLEncoder.encode(serviceName, "UTF-8");
         } catch (UnsupportedEncodingException e) {
-            return getKey();
+            //do nothing
         }
+        return getKey(serviceName, clusters);
+    }
+    
+    private String getGroupedServiceName() {
+        String serviceName = this.name;
+        if (!isEmpty(groupName) && serviceName.indexOf(Constants.SERVICE_INFO_SPLITER) == -1) {
+            serviceName = groupName + Constants.SERVICE_INFO_SPLITER + serviceName;
+        }
+        return serviceName;
     }
     
     /**

api/src/test/java/com/alibaba/nacos/api/naming/pojo/ServiceInfoTest.java

@@ -23,6 +23,8 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -38,13 +40,13 @@ public class ServiceInfoTest {
     public void setUp() throws Exception {
         mapper = new ObjectMapper();
         mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
-        serviceInfo = new ServiceInfo("testName", "testClusters");
+        serviceInfo = new ServiceInfo("G@@testName", "testClusters");
     }
     
     @Test
     public void testSerialize() throws JsonProcessingException {
         String actual = mapper.writeValueAsString(serviceInfo);
-        assertTrue(actual.contains("\"name\":\"testName\""));
+        assertTrue(actual.contains("\"name\":\"G@@testName\""));
         assertTrue(actual.contains("\"clusters\":\"testClusters\""));
         assertTrue(actual.contains("\"cacheMillis\":1000"));
         assertTrue(actual.contains("\"hosts\":[]"));
@@ -58,11 +60,11 @@ public class ServiceInfoTest {
     }
     
     @Test
-    @SuppressWarnings("checkstyle:linelength")
     public void testDeserialize() throws IOException {
-        String example = "{\"name\":\"testName\",\"clusters\":\"testClusters\",\"cacheMillis\":1000,\"hosts\":[],\"lastRefTime\":0,\"checksum\":\"\",\"allIPs\":false,\"valid\":true,\"groupName\":\"\"}";
+        String example = "{\"name\":\"G@@testName\",\"clusters\":\"testClusters\",\"cacheMillis\":1000,\"hosts\":[],"
+                + "\"lastRefTime\":0,\"checksum\":\"\",\"allIPs\":false,\"valid\":true,\"groupName\":\"\"}";
         ServiceInfo actual = mapper.readValue(example, ServiceInfo.class);
-        assertEquals("testName", actual.getName());
+        assertEquals("G@@testName", actual.getName());
         assertEquals("testClusters", actual.getClusters());
         assertEquals("", actual.getChecksum());
         assertEquals("", actual.getGroupName());
@@ -72,4 +74,32 @@ public class ServiceInfoTest {
         assertTrue(actual.isValid());
         assertFalse(actual.isAllIPs());
     }
+    
+    @Test
+    public void testGetKey() {
+        String key = serviceInfo.getKey();
+        assertEquals("G@@testName@@testClusters", key);
+    }
+    
+    @Test
+    public void testGetKeyEncode() {
+        String key = serviceInfo.getKeyEncoded();
+        String encodeName = null;
+        try {
+            encodeName = URLEncoder.encode("G@@testName", "utf-8");
+        } catch (UnsupportedEncodingException e) {
+            e.printStackTrace();
+        }
+        assertEquals(key, ServiceInfo.getKey(encodeName, "testClusters"));
+    }
+    
+    @Test
+    public void testServiceInfoConstructor() {
+        String key1 = "group@@name";
+        String key2 = "group@@name@@c2";
+        ServiceInfo s1 = new ServiceInfo(key1);
+        ServiceInfo s2 = new ServiceInfo(key2);
+        assertEquals(key1, s1.getKey());
+        assertEquals(key2, s2.getKey());
+    }
 }

client/src/test/java/com/alibaba/nacos/client/naming/cache/DiskCacheTest.java

@@ -41,7 +41,7 @@ public class DiskCacheTest {
     @Before
     public void setUp() throws Exception {
         System.out.println(CACHE_DIR);
-        serviceInfo = new ServiceInfo("testName", "testClusters");
+        serviceInfo = new ServiceInfo("G@@testName", "testClusters");
         instance = new Instance();
         instance.setClusterName("testClusters");
         instance.setIp("1.1.1.1");
@@ -66,8 +66,8 @@ public class DiskCacheTest {
         DiskCache.write(serviceInfo, CACHE_DIR);
         Map<String, ServiceInfo> actual = DiskCache.read(CACHE_DIR);
         assertEquals(1, actual.size());
-        assertTrue(actual.containsKey(serviceInfo.getKeyEncoded()));
+        assertTrue(actual.containsKey(serviceInfo.getKey()));
-        assertServiceInfo(actual.get(serviceInfo.getKeyEncoded()), serviceInfo);
+        assertServiceInfo(actual.get(serviceInfo.getKey()), serviceInfo);
     }
     
     private void assertServiceInfo(ServiceInfo actual, ServiceInfo expected) {