[ISSUE-#4066] Make serviceNameList followed string order (#4087)

* make serviceNameList followed string order * code format
alibaba · Nov 5, 2020 · 4cd1999 · 4cd1999
1 parent ccea6ee
commit 4cd1999
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 2 deletions.
diff --git a/naming/src/main/java/com/alibaba/nacos/naming/core/ServiceManager.java b/naming/src/main/java/com/alibaba/nacos/naming/core/ServiceManager.java
@@ -59,6 +59,7 @@
 import java.util.Set;
 import java.util.StringJoiner;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.LinkedBlockingDeque;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
@@ -238,7 +239,7 @@ public void onDelete(String key) throws Exception {
             String persistInstanceListKey = KeyBuilder.buildInstanceListKey(namespace, name, false);
             consistencyService.remove(ephemeralInstanceListKey);
             consistencyService.remove(persistInstanceListKey);
-
+            
             // remove listeners of key to avoid mem leak
             consistencyService.unListen(ephemeralInstanceListKey, service);
             consistencyService.unListen(persistInstanceListKey, service);
@@ -867,7 +868,7 @@ public void putService(Service service) {
         if (!serviceMap.containsKey(service.getNamespaceId())) {
             synchronized (putServiceLock) {
                 if (!serviceMap.containsKey(service.getNamespaceId())) {
-                    serviceMap.put(service.getNamespaceId(), new ConcurrentHashMap<>(16));
+                    serviceMap.put(service.getNamespaceId(), new ConcurrentSkipListMap<>());
                 }
             }
         }

diff --git a/naming/src/test/java/com/alibaba/nacos/naming/core/ServiceManagerTest.java b/naming/src/test/java/com/alibaba/nacos/naming/core/ServiceManagerTest.java
@@ -39,7 +39,9 @@
 import org.springframework.test.util.ReflectionTestUtils;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -78,6 +80,8 @@ public class ServiceManagerTest extends BaseTest {
 
     private Instance instance2;
 
+    private List<String> serviceNames;
+
     @Before
     public void before() {
         super.before();
@@ -89,6 +93,7 @@ public void before() {
         mockService();
         mockCluster();
         mockInstance();
+        mockServiceName();
     }
 
     private void mockService() {
@@ -108,6 +113,13 @@ private void mockInstance() {
         instance2 = new Instance("2.2.2.2", 2);
     }
 
+    private void mockServiceName() {
+        serviceNames = new ArrayList<>(5);
+        for (int i = 0; i < 32; i++) {
+            serviceNames.add(String.valueOf(i));
+        }
+    }
+
     @Test
     public void testGetAllNamespaces() throws NacosException {
         assertTrue(serviceManager.getAllNamespaces().isEmpty());
@@ -127,6 +139,25 @@ public void testGetAllServiceNames() throws NacosException {
         assertEquals(TEST_SERVICE_NAME, serviceManager.getAllServiceNames(TEST_NAMESPACE).iterator().next());
     }
 
+    @Test
+    public void testGetAllServiceNamesOrder() throws NacosException {
+        assertTrue(serviceManager.getAllServiceNames().isEmpty());
+        for (String serviceName : serviceNames) {
+            serviceManager.createEmptyService(TEST_NAMESPACE, serviceName, true);
+        }
+        assertFalse(serviceManager.getAllServiceNames().isEmpty());
+        assertEquals(1, serviceManager.getAllServiceNames().size());
+        assertEquals(serviceNames.size(), serviceManager.getAllServiceNames(TEST_NAMESPACE).size());
+        Collections.sort(serviceNames);
+        Iterator<String> iterator = serviceManager.getAllServiceNames(TEST_NAMESPACE).iterator();
+        int index = 0;
+        while (iterator.hasNext()) {
+            String next = iterator.next();
+            assertEquals(next, serviceNames.get(index));
+            index++;
+        }
+    }
+
     @Test
     public void testGetAllServiceNameList() throws NacosException {
         assertTrue(serviceManager.getAllServiceNameList(TEST_NAMESPACE).isEmpty());
@@ -136,6 +167,21 @@ public void testGetAllServiceNameList() throws NacosException {
         assertEquals(TEST_SERVICE_NAME, serviceManager.getAllServiceNameList(TEST_NAMESPACE).get(0));
     }
 
+    @Test
+    public void testGetAllServiceNameListOrder() throws NacosException {
+        assertTrue(serviceManager.getAllServiceNameList(TEST_NAMESPACE).isEmpty());
+        for (String serviceName : serviceNames) {
+            serviceManager.createEmptyService(TEST_NAMESPACE, serviceName, true);
+        }
+        assertFalse(serviceManager.getAllServiceNameList(TEST_NAMESPACE).isEmpty());
+        assertEquals(serviceNames.size(), serviceManager.getAllServiceNameList(TEST_NAMESPACE).size());
+        List<String> allServiceNameList = serviceManager.getAllServiceNameList(TEST_NAMESPACE);
+        Collections.sort(serviceNames);
+        for (int i = 0; i < allServiceNameList.size(); i++) {
+            assertEquals(allServiceNameList.get(i), serviceNames.get(i));
+        }
+    }
+
     @Test
     public void testGetResponsibleServices() throws NacosException {
         when(distroMapper.responsible(TEST_SERVICE_NAME)).thenReturn(true);