use linked hash map to keep server side config order and do some refactoring to apollo-client

apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ReleaseService.java

@@ -31,14 +31,7 @@ import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.CollectionUtils;
 
 import java.lang.reflect.Type;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -153,7 +146,7 @@ public class ReleaseService {
 
     Map<String, String> operateNamespaceItems = getNamespaceItems(namespace);
 
-    Map<String, Object> operationContext = Maps.newHashMap();
+    Map<String, Object> operationContext = Maps.newLinkedHashMap();
     operationContext.put(ReleaseOperationContext.SOURCE_BRANCH, branchName);
     operationContext.put(ReleaseOperationContext.BASE_RELEASE_ID, branchReleaseId);
     operationContext.put(ReleaseOperationContext.IS_EMERGENCY_PUBLISH, isEmergencyPublish);
@@ -188,7 +181,7 @@ public class ReleaseService {
     }
 
     //master release
-    Map<String, Object> operationContext = Maps.newHashMap();
+    Map<String, Object> operationContext = Maps.newLinkedHashMap();
     operationContext.put(ReleaseOperationContext.IS_EMERGENCY_PUBLISH, isEmergencyPublish);
 
     Release release = masterRelease(namespace, releaseName, releaseComment, operateNamespaceItems,
@@ -211,7 +204,7 @@ public class ReleaseService {
     Release parentLatestRelease = findLatestActiveRelease(parentNamespace);
     Map<String, String> parentConfigurations = parentLatestRelease != null ?
             gson.fromJson(parentLatestRelease.getConfigurations(),
-                    GsonType.CONFIG) : new HashMap<>();
+                    GsonType.CONFIG) : new LinkedHashMap<>();
     long baseReleaseId = parentLatestRelease == null ? 0 : parentLatestRelease.getId();
 
     Map<String, String> configsToPublish = mergeConfiguration(parentConfigurations, childNamespaceItems);
@@ -342,7 +335,7 @@ public class ReleaseService {
                                                       childNamespace.getNamespaceName());
     long previousReleaseId = previousRelease == null ? 0 : previousRelease.getId();
 
-    Map<String, Object> releaseOperationContext = Maps.newHashMap();
+    Map<String, Object> releaseOperationContext = Maps.newLinkedHashMap();
     releaseOperationContext.put(ReleaseOperationContext.BASE_RELEASE_ID, baseReleaseId);
     releaseOperationContext.put(ReleaseOperationContext.IS_EMERGENCY_PUBLISH, isEmergencyPublish);
     releaseOperationContext.put(ReleaseOperationContext.BRANCH_RELEASE_KEYS, branchReleaseKeys);
@@ -372,7 +365,7 @@ public class ReleaseService {
 
   private Map<String, String> mergeConfiguration(Map<String, String> baseConfigurations,
                                                  Map<String, String> coverConfigurations) {
-    Map<String, String> result = new HashMap<>();
+    Map<String, String> result = new LinkedHashMap<>();
     //copy base configuration
     for (Map.Entry<String, String> entry : baseConfigurations.entrySet()) {
       result.put(entry.getKey(), entry.getValue());
@@ -388,8 +381,8 @@ public class ReleaseService {
 
 
   private Map<String, String> getNamespaceItems(Namespace namespace) {
-    List<Item> items = itemService.findItemsWithoutOrdered(namespace.getId());
-    Map<String, String> configurations = new HashMap<>();
+    List<Item> items = itemService.findItemsWithOrdered(namespace.getId());
+    Map<String, String> configurations = new LinkedHashMap<>();
     for (Item item : items) {
       if (StringUtils.isEmpty(item.getKey())) {
         continue;
@@ -520,7 +513,7 @@ public class ReleaseService {
       Map<String, String> masterReleaseConfigs, Map<String, String> branchReleaseConfigs,
       Collection<String> branchReleaseKeys) {
 
-    Map<String, String> modifiedConfigs = new HashMap<>();
+    Map<String, String> modifiedConfigs = new LinkedHashMap<>();
 
     if (CollectionUtils.isEmpty(branchReleaseConfigs)) {
       return modifiedConfigs;

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java

@@ -88,7 +88,7 @@ public class DefaultConfig extends AbstractConfig implements RepositoryChangeLis
 
     // step 4: check properties file from classpath
     if (value == null && m_resourceProperties != null) {
-      value = (String) m_resourceProperties.getProperty(key);
+      value = m_resourceProperties.getProperty(key);
     }
 
     if (value == null && m_configProperties.get() == null && m_warnLogRateLimiter.tryAcquire()) {

apollo-client/src/main/java/com/ctrip/framework/apollo/util/OrderedProperties.java

@@ -107,7 +107,9 @@ public class OrderedProperties extends Properties {
 
   @Override
   public synchronized Object remove(Object key) {
+    if (key instanceof String) {
       this.propertyNames.remove(key);
+    }
     return super.remove(key);
   }
 

apollo-client/src/main/java/com/ctrip/framework/apollo/util/factory/DefaultPropertiesFactory.java

@@ -3,10 +3,7 @@ package com.ctrip.framework.apollo.util.factory;
 import com.ctrip.framework.apollo.build.ApolloInjector;
 import com.ctrip.framework.apollo.util.ConfigUtil;
 import com.ctrip.framework.apollo.util.OrderedProperties;
-import com.google.common.base.Strings;
 import java.util.Properties;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Default PropertiesFactory implementation.
@@ -15,8 +12,6 @@ import org.slf4j.LoggerFactory;
  */
 public class DefaultPropertiesFactory implements PropertiesFactory {
 
-  private static final Logger logger = LoggerFactory.getLogger(DefaultPropertiesFactory.class);
-
   private ConfigUtil m_configUtil;
 
   public DefaultPropertiesFactory() {

apollo-client/src/main/java/com/ctrip/framework/apollo/util/factory/PropertiesFactory.java

@@ -12,7 +12,7 @@ public interface PropertiesFactory {
   /**
    * Configuration to keep properties order as same as line order in .yml/.yaml/.properties file.
    */
-  public static final String APOLLO_PROPERTY_ORDER_ENABLE = "apollo.property.order.enable";
+  String APOLLO_PROPERTY_ORDER_ENABLE = "apollo.property.order.enable";
 
   /**
    * <pre>

apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java

@@ -44,6 +44,7 @@ public abstract class BaseIntegrationTest {
   protected static String someDataCenter;
   protected static int refreshInterval;
   protected static TimeUnit refreshTimeUnit;
+  protected static boolean propertiesOrderEnabled;
   private Server server;
   protected Gson gson = new Gson();
 
@@ -64,6 +65,7 @@ public abstract class BaseIntegrationTest {
     someDataCenter = "someDC";
     refreshInterval = 5;
     refreshTimeUnit = TimeUnit.MINUTES;
+    propertiesOrderEnabled = false;
 
     //as ConfigService is singleton, so we must manually clear its container
     ConfigService.reset();
@@ -140,6 +142,10 @@ public abstract class BaseIntegrationTest {
     BaseIntegrationTest.refreshTimeUnit = refreshTimeUnit;
   }
 
+  protected void setPropertiesOrderEnabled(boolean propertiesOrderEnabled) {
+    BaseIntegrationTest.propertiesOrderEnabled = propertiesOrderEnabled;
+  }
+
   public static class MockConfigUtil extends ConfigUtil {
 
     @Override
@@ -204,7 +210,7 @@ public abstract class BaseIntegrationTest {
 
     @Override
     public boolean isPropertiesOrderEnabled() {
-      return true;
+      return propertiesOrderEnabled;
     }
   }
 

apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java

@@ -5,6 +5,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import com.ctrip.framework.apollo.util.OrderedProperties;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -17,7 +18,6 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.server.handler.ContextHandler;
@@ -53,7 +53,6 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
   private String defaultNamespace;
   private String someOtherNamespace;
   private RemoteConfigLongPollService remoteConfigLongPollService;
-  private PropertiesFactory propertiesFactory;
 
   @Before
   public void setUp() throws Exception {
@@ -68,9 +67,6 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     }
     configDir.mkdirs();
     remoteConfigLongPollService = ApolloInjector.getInstance(RemoteConfigLongPollService.class);
-
-    System.setProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE, "true");
-    propertiesFactory = ApolloInjector.getInstance(PropertiesFactory.class);
   }
 
   @Override
@@ -78,7 +74,6 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
   public void tearDown() throws Exception {
     ReflectionTestUtils.invokeMethod(remoteConfigLongPollService, "stopLongPollingRefresh");
     recursiveDelete(configDir);
-    System.clearProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE);
     super.tearDown();
   }
 
@@ -117,6 +112,8 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
 
   @Test
   public void testOrderGetConfigWithNoLocalFileButWithRemoteConfig() throws Exception {
+    setPropertiesOrderEnabled(true);
+
     String someKey1 = "someKey1";
     String someValue1 = "someValue1";
     String someKey2 = "someKey2";
@@ -167,7 +164,9 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     String someKey2 = "someKey2";
     String someValue2 = "someValue2";
 
-    Properties properties = propertiesFactory.getPropertiesInstance();
+    setPropertiesOrderEnabled(true);
+
+    Properties properties = new OrderedProperties();
     properties.put(someKey, someValue);
     properties.put(someKey1, someValue1);
     properties.put(someKey2, someValue2);
@@ -230,7 +229,10 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     String someValue1 = "someValue1";
     String someKey2 = "someKey2";
     String someValue2 = "someValue2";
-    Properties properties = propertiesFactory.getPropertiesInstance();
+
+    setPropertiesOrderEnabled(true);
+
+    Properties properties = new OrderedProperties();
     properties.put(someKey1, someValue1);
     properties.put(someKey2, someValue2);
     createLocalCachePropertyFile(properties);

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java

@@ -10,7 +10,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
+import com.ctrip.framework.apollo.util.OrderedProperties;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -44,6 +44,8 @@ import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
 import com.google.common.util.concurrent.SettableFuture;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -54,12 +56,21 @@ public class DefaultConfigTest {
   private ConfigRepository configRepository;
   private Properties someProperties;
   private ConfigSourceType someSourceType;
+  private PropertiesFactory propertiesFactory;
 
   @Before
   public void setUp() throws Exception {
     MockInjector.reset();
     MockInjector.setInstance(ConfigUtil.class, new MockConfigUtil());
-    MockInjector.setInstance(PropertiesFactory.class, new DefaultPropertiesFactory());
+
+    propertiesFactory = mock(PropertiesFactory.class);
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
 
     someResourceDir = new File(ClassLoaderUtil.getClassPath() + "/META-INF/config");
     someResourceDir.mkdirs();
@@ -726,15 +737,15 @@ public class DefaultConfigTest {
   @Test
   public void testRemoveChangeListener() throws Exception {
     String someNamespace = "someNamespace";
-    final ConfigChangeEvent someConfigChangEvent = mock(ConfigChangeEvent.class);
-    ConfigChangeEvent anotherConfigChangEvent = mock(ConfigChangeEvent.class);
+    final ConfigChangeEvent someConfigChangeEvent = mock(ConfigChangeEvent.class);
+    ConfigChangeEvent anotherConfigChangeEvent = mock(ConfigChangeEvent.class);
 
     final SettableFuture<ConfigChangeEvent> someListenerFuture1 = SettableFuture.create();
     final SettableFuture<ConfigChangeEvent> someListenerFuture2 = SettableFuture.create();
     ConfigChangeListener someListener = new ConfigChangeListener() {
       @Override
       public void onChange(ConfigChangeEvent changeEvent) {
-        if (someConfigChangEvent == changeEvent) {
+        if (someConfigChangeEvent == changeEvent) {
           someListenerFuture1.set(changeEvent);
         } else {
           someListenerFuture2.set(changeEvent);
@@ -747,7 +758,7 @@ public class DefaultConfigTest {
     ConfigChangeListener anotherListener = new ConfigChangeListener() {
       @Override
       public void onChange(ConfigChangeEvent changeEvent) {
-        if (someConfigChangEvent == changeEvent) {
+        if (someConfigChangeEvent == changeEvent) {
           anotherListenerFuture1.set(changeEvent);
         } else {
           anotherListenerFuture2.set(changeEvent);
@@ -762,17 +773,17 @@ public class DefaultConfigTest {
     config.addChangeListener(someListener);
     config.addChangeListener(anotherListener);
 
-    config.fireConfigChange(someConfigChangEvent);
+    config.fireConfigChange(someConfigChangeEvent);
 
-    assertEquals(someConfigChangEvent, someListenerFuture1.get(500, TimeUnit.MILLISECONDS));
-    assertEquals(someConfigChangEvent, anotherListenerFuture1.get(500, TimeUnit.MILLISECONDS));
+    assertEquals(someConfigChangeEvent, someListenerFuture1.get(500, TimeUnit.MILLISECONDS));
+    assertEquals(someConfigChangeEvent, anotherListenerFuture1.get(500, TimeUnit.MILLISECONDS));
 
     assertFalse(config.removeChangeListener(yetAnotherListener));
     assertTrue(config.removeChangeListener(someListener));
 
-    config.fireConfigChange(anotherConfigChangEvent);
+    config.fireConfigChange(anotherConfigChangeEvent);
 
-    assertEquals(anotherConfigChangEvent, anotherListenerFuture2.get(500, TimeUnit.MILLISECONDS));
+    assertEquals(anotherConfigChangeEvent, anotherListenerFuture2.get(500, TimeUnit.MILLISECONDS));
 
     TimeUnit.MILLISECONDS.sleep(100);
 
@@ -792,8 +803,34 @@ public class DefaultConfigTest {
 
     when(configRepository.getConfig()).thenReturn(someProperties);
 
-    DefaultConfig defaultConfig =
-            new DefaultConfig(someNamespace, configRepository);
+    DefaultConfig defaultConfig = new DefaultConfig(someNamespace, configRepository);
+
+    Set<String> propertyNames = defaultConfig.getPropertyNames();
+
+    assertEquals(10, propertyNames.size());
+    assertEquals(someProperties.stringPropertyNames(), propertyNames);
+  }
+
+  @Test
+  public void testGetPropertyNamesWithOrderedProperties() {
+    String someKeyPrefix = "someKey";
+    String someValuePrefix = "someValue";
+
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new OrderedProperties();
+      }
+    });
+    //set up config repo
+    someProperties = new OrderedProperties();
+    for (int i = 0; i < 10; i++) {
+      someProperties.setProperty(someKeyPrefix + i, someValuePrefix + i);
+    }
+
+    when(configRepository.getConfig()).thenReturn(someProperties);
+
+    DefaultConfig defaultConfig = new DefaultConfig(someNamespace, configRepository);
 
     Set<String> propertyNames = defaultConfig.getPropertyNames();
 

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepositoryTest.java

 package com.ctrip.framework.apollo.internals;
 
-import static org.hamcrest.Matchers.both;
-import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.hamcrest.Matchers.everyItem;
-import static org.hamcrest.Matchers.isIn;
 import static org.hamcrest.core.IsEqual.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
@@ -14,15 +10,12 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.Map.Entry;
 import java.util.Properties;
 
-import org.hamcrest.Matcher;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -34,12 +27,13 @@ import com.ctrip.framework.apollo.util.ConfigUtil;
 import com.google.common.base.Charsets;
 import com.google.common.base.Joiner;
 import com.google.common.io.Files;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 /**
  * Created by Jason on 4/9/16.
  */
 public class LocalFileConfigRepositoryTest {
-
   private File someBaseDir;
   private String someNamespace;
   private ConfigRepository upstreamRepo;
@@ -49,7 +43,6 @@ public class LocalFileConfigRepositoryTest {
   private String defaultKey;
   private String defaultValue;
   private ConfigSourceType someSourceType;
-  private MockConfigUtil configUtil;
 
   @Before
   public void setUp() throws Exception {
@@ -67,10 +60,15 @@ public class LocalFileConfigRepositoryTest {
     when(upstreamRepo.getSourceType()).thenReturn(someSourceType);
 
     MockInjector.reset();
-    configUtil = new MockConfigUtil();
-    MockInjector.setInstance(ConfigUtil.class, configUtil);
-    MockInjector.setInstance(PropertiesFactory.class, new DefaultPropertiesFactory());
-
+    MockInjector.setInstance(ConfigUtil.class, new MockConfigUtil());
+    PropertiesFactory propertiesFactory = mock(PropertiesFactory.class);
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
   }
 
   @After
@@ -123,8 +121,7 @@ public class LocalFileConfigRepositoryTest {
 
     Files.write(defaultKey + "=" + someValue, file, Charsets.UTF_8);
 
-    LocalFileConfigRepository localRepo = new LocalFileConfigRepository(someNamespace,
-        upstreamRepo);
+    LocalFileConfigRepository localRepo = new LocalFileConfigRepository(someNamespace, upstreamRepo);
     localRepo.setLocalCacheDir(someBaseDir, true);
 
     Properties properties = localRepo.getConfig();
@@ -141,20 +138,9 @@ public class LocalFileConfigRepositoryTest {
 
     Properties result = localFileConfigRepository.getConfig();
 
-    if (!isJDK11() || (isJDK11() && configUtil.isPropertiesOrderEnabled())) {
     assertThat(
         "LocalFileConfigRepository's properties should be the same as fallback repo's when there is no local cache",
         result.entrySet(), equalTo(someProperties.entrySet()));
-    } else {
-      //In JDK11 Properties return EntrySet without customize equals implementation, so equalTo will check
-      //whether they are same object. This is why two statements are used to check items in entryset.
-      assertThat(
-          "LocalFileConfigRepository's properties should be the same as fallback repo's when there is no local cache",
-          result.entrySet(), everyItem(isIn(someProperties.entrySet())));
-      assertThat(
-          "LocalFileConfigRepository's properties should be the same as fallback repo's when there is no local cache",
-          someProperties.entrySet(), everyItem(isIn(result.entrySet())));
-    }
     assertEquals(someSourceType, localFileConfigRepository.getSourceType());
   }
 
@@ -173,20 +159,9 @@ public class LocalFileConfigRepositoryTest {
 
     Properties anotherProperties = anotherLocalRepoWithNoFallback.getConfig();
 
-    if (!isJDK11() || (isJDK11() && configUtil.isPropertiesOrderEnabled())) {
     assertThat(
         "LocalFileConfigRepository should persist local cache files and return that afterwards",
         someProperties.entrySet(), equalTo(anotherProperties.entrySet()));
-    } else {
-      //In JDK11 Properties return EntrySet without customize equals implementation, so equalTo will check
-      //whether they are same object. This is why two statements are used to check items in entryset.
-      assertThat(
-          "LocalFileConfigRepository's properties should be the same as fallback repo's when there is no local cache",
-          someProperties.entrySet(), everyItem(isIn(anotherProperties.entrySet())));
-      assertThat(
-          "LocalFileConfigRepository's properties should be the same as fallback repo's when there is no local cache",
-          anotherProperties.entrySet(), everyItem(isIn(someProperties.entrySet())));
-    }
     assertEquals(someSourceType, localRepo.getSourceType());
   }
 
@@ -221,7 +196,6 @@ public class LocalFileConfigRepositoryTest {
   }
 
   public static class MockConfigUtil extends ConfigUtil {
-
     @Override
     public String getAppId() {
       return someAppId;
@@ -231,10 +205,6 @@ public class LocalFileConfigRepositoryTest {
     public String getCluster() {
       return someCluster;
     }
-
-    public String getJavaVersion() {
-      return System.getProperty("java.version");
-    }
   }
 
   private File createLocalCachePropertyFile(Properties properties) throws IOException {
@@ -250,8 +220,4 @@ public class LocalFileConfigRepositoryTest {
     }
     return file;
   }
-
-  private boolean isJDK11() {
-    return configUtil.getJavaVersion().startsWith("11.");
-  }
 }

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java

@@ -10,8 +10,6 @@ import com.ctrip.framework.apollo.ConfigFileChangeListener;
 import com.ctrip.framework.apollo.build.MockInjector;
 import com.ctrip.framework.apollo.enums.PropertyChangeType;
 import com.ctrip.framework.apollo.model.ConfigFileChangeEvent;
-import com.ctrip.framework.apollo.util.ConfigUtil;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.Properties;
@@ -21,9 +19,11 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import com.ctrip.framework.apollo.core.enums.ConfigFileFormat;
+import org.mockito.stubbing.Answer;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -34,13 +34,20 @@ public class PropertiesConfigFileTest {
   private String someNamespace;
   @Mock
   private ConfigRepository configRepository;
+  @Mock
+  private PropertiesFactory propertiesFactory;
 
   @Before
   public void setUp() throws Exception {
     someNamespace = "someName";
     MockInjector.reset();
-    MockInjector.setInstance(ConfigUtil.class, new ConfigUtil());
-    MockInjector.setInstance(PropertiesFactory.class, new DefaultPropertiesFactory());
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
   }
 
   @Test

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

@@ -23,7 +23,7 @@ import com.ctrip.framework.apollo.core.signature.Signature;
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
 import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
 import com.ctrip.framework.apollo.util.ConfigUtil;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
+import com.ctrip.framework.apollo.util.OrderedProperties;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import com.ctrip.framework.apollo.util.http.HttpRequest;
 import com.ctrip.framework.apollo.util.http.HttpResponse;
@@ -40,7 +40,6 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 import javax.servlet.http.HttpServletResponse;
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -49,7 +48,6 @@ import org.mockito.Mock;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.mockito.stubbing.Answer;
-import org.springframework.test.util.ReflectionTestUtils;
 
 /**
  * Created by Jason on 4/9/16.
@@ -68,6 +66,8 @@ public class RemoteConfigRepositoryTest {
   @Mock
   private static HttpResponse<List<ApolloConfigNotification>> pollResponse;
   private RemoteConfigLongPollService remoteConfigLongPollService;
+  @Mock
+  private PropertiesFactory propertiesFactory;
 
   private static String someAppId;
   private static String someCluster;
@@ -98,21 +98,40 @@ public class RemoteConfigRepositoryTest {
 
     MockInjector.setInstance(RemoteConfigLongPollService.class, remoteConfigLongPollService);
 
-    System.setProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE, "true");
-    PropertiesFactory propertiesFactory = new DefaultPropertiesFactory();
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
     MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
 
     someAppId = "someAppId";
     someCluster = "someCluster";
   }
 
-  @After
-  public void tearDown() throws Exception {
-    System.clearProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE);
+  @Test
+  public void testLoadConfig() throws Exception {
+    String someKey = "someKey";
+    String someValue = "someValue";
+    Map<String, String> configurations = Maps.newHashMap();
+    configurations.put(someKey, someValue);
+    ApolloConfig someApolloConfig = assembleApolloConfig(configurations);
+
+    when(someResponse.getStatusCode()).thenReturn(200);
+    when(someResponse.getBody()).thenReturn(someApolloConfig);
+
+    RemoteConfigRepository remoteConfigRepository = new RemoteConfigRepository(someNamespace);
+
+    Properties config = remoteConfigRepository.getConfig();
+
+    assertEquals(configurations, config);
+    assertEquals(ConfigSourceType.REMOTE, remoteConfigRepository.getSourceType());
+    remoteConfigLongPollService.stopLongPollingRefresh();
   }
 
   @Test
-  public void testLoadConfig() throws Exception {
+  public void testLoadConfigWithOrderedProperties() throws Exception {
     String someKey = "someKey";
     String someValue = "someValue";
     Map<String, String> configurations = Maps.newLinkedHashMap();
@@ -122,22 +141,26 @@ public class RemoteConfigRepositoryTest {
 
     when(someResponse.getStatusCode()).thenReturn(200);
     when(someResponse.getBody()).thenReturn(someApolloConfig);
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new OrderedProperties();
+      }
+    });
 
     RemoteConfigRepository remoteConfigRepository = new RemoteConfigRepository(someNamespace);
 
     Properties config = remoteConfigRepository.getConfig();
 
-//    assertEquals(configurations, config);
-    assertTrue(configurations.equals(config));
+    assertTrue(config instanceof OrderedProperties);
+    assertEquals(configurations, config);
     assertEquals(ConfigSourceType.REMOTE, remoteConfigRepository.getSourceType());
     remoteConfigLongPollService.stopLongPollingRefresh();
 
-    if (configUtil.isPropertiesOrderEnabled()) {
     String[] actualArrays = config.keySet().toArray(new String[]{});
     String[] expectedArrays = {"someKey", "someKey2"};
     assertArrayEquals(expectedArrays, actualArrays);
   }
-  }
 
   @Test
   public void testLoadConfigWithAccessKeySecret() throws Exception {

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/SimpleConfigTest.java

@@ -6,16 +6,15 @@ import static org.mockito.Mockito.when;
 
 import com.ctrip.framework.apollo.build.MockInjector;
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import com.ctrip.framework.apollo.Config;
@@ -25,6 +24,7 @@ import com.ctrip.framework.apollo.model.ConfigChange;
 import com.ctrip.framework.apollo.model.ConfigChangeEvent;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.SettableFuture;
+import org.mockito.stubbing.Answer;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -35,20 +35,21 @@ public class SimpleConfigTest {
   private String someNamespace;
   @Mock
   private ConfigRepository configRepository;
+  @Mock
+  private PropertiesFactory propertiesFactory;
   private ConfigSourceType someSourceType;
 
   @Before
   public void setUp() throws Exception {
     someNamespace = "someName";
 
-    System.setProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE, "true");
-    PropertiesFactory propertiesFactory = new DefaultPropertiesFactory();
-    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
       }
-
-  @After
-  public void tearDown() throws Exception {
-    System.clearProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE);
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
   }
 
   @Test

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java

@@ -10,7 +10,6 @@ import com.ctrip.framework.apollo.ConfigFileChangeListener;
 import com.ctrip.framework.apollo.build.MockInjector;
 import com.ctrip.framework.apollo.enums.PropertyChangeType;
 import com.ctrip.framework.apollo.model.ConfigFileChangeEvent;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.Properties;
@@ -21,10 +20,12 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import com.ctrip.framework.apollo.core.ConfigConsts;
 import com.ctrip.framework.apollo.core.enums.ConfigFileFormat;
+import org.mockito.stubbing.Answer;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -35,13 +36,19 @@ public class XmlConfigFileTest {
   private String someNamespace;
   @Mock
   private ConfigRepository configRepository;
+  @Mock
+  private PropertiesFactory propertiesFactory;
 
   @Before
   public void setUp() throws Exception {
     someNamespace = "someName";
 
-    System.setProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE, "true");
-    PropertiesFactory propertiesFactory = new DefaultPropertiesFactory();
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
     MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
   }
 

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/YamlConfigFileTest.java

@@ -7,17 +7,17 @@ import com.ctrip.framework.apollo.build.MockInjector;
 import com.ctrip.framework.apollo.core.ConfigConsts;
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
 import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
-import com.ctrip.framework.apollo.util.ConfigUtil;
-import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
+import com.ctrip.framework.apollo.util.OrderedProperties;
 import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
 import com.ctrip.framework.apollo.util.yaml.YamlParser;
 import java.util.Properties;
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
 
 @RunWith(MockitoJUnitRunner.class)
 public class YamlConfigFileTest {
@@ -27,6 +27,8 @@ public class YamlConfigFileTest {
   private ConfigRepository configRepository;
   @Mock
   private YamlParser yamlParser;
+  @Mock
+  private PropertiesFactory propertiesFactory;
 
   private ConfigSourceType someSourceType;
 
@@ -34,17 +36,16 @@ public class YamlConfigFileTest {
   public void setUp() throws Exception {
     someNamespace = "someName";
 
-    System.setProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE, "true");
-
     MockInjector.reset();
     MockInjector.setInstance(YamlParser.class, yamlParser);
-    MockInjector.setInstance(ConfigUtil.class, new ConfigUtil());
-    MockInjector.setInstance(PropertiesFactory.class, new DefaultPropertiesFactory());
-  }
 
-  @After
-  public void tearDown() throws Exception {
-    System.clearProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE);
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new Properties();
+      }
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
   }
 
   @Test
@@ -70,6 +71,12 @@ public class YamlConfigFileTest {
 
   @Test
   public void testWhenHasContentWithOrder() throws Exception {
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new OrderedProperties();
+      }
+    });
     Properties someProperties = new Properties();
     String key = ConfigConsts.CONFIG_FILE_CONTENT_KEY;
     String someContent = "someKey: 'someValue'\nsomeKey2: 'someValue2'";

apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java

@@ -10,7 +10,6 @@ import org.junit.Test;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.when;
 
 /**
  * @author Jason Song(song_s@ctrip.com)

apollo-client/src/test/java/com/ctrip/framework/apollo/util/OrderedPropertiesTest.java

@@ -6,13 +6,11 @@ import java.util.Collection;
 import java.util.Enumeration;
 import java.util.Properties;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class OrderedPropertiesTest {
 
   private OrderedProperties orderedProperties;
-  private Properties legacyProperties;
 
   @Before
   public void setUp() {
@@ -23,7 +21,7 @@ public class OrderedPropertiesTest {
 
   @Test
   public void testOrderedPropertiesInvokedAsLegacyProperties() {
-    legacyProperties = orderedProperties;
+    Properties legacyProperties = orderedProperties;
     assertEquals(orderedProperties.size(), legacyProperties.size());
 
     legacyProperties.put("key3", "value3");
@@ -85,10 +83,9 @@ public class OrderedPropertiesTest {
 
   @Test
   public void testPropertyNames() {
-    Enumeration<String> propertyNames = (Enumeration<String>) orderedProperties.propertyNames();
-    assertTrue(propertyNames.nextElement().equals("key1"));
-    assertTrue(propertyNames.nextElement().equals("key2"));
-
+    Enumeration<?> propertyNames = orderedProperties.propertyNames();
+    assertEquals("key1", propertyNames.nextElement());
+    assertEquals("key2", propertyNames.nextElement());
   }
 
 

apollo-client/src/test/java/com/ctrip/framework/apollo/util/yaml/YamlParserTest.java

 package com.ctrip.framework.apollo.util.yaml;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import com.ctrip.framework.apollo.build.MockInjector;
+import com.ctrip.framework.apollo.util.OrderedProperties;
+import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
 import java.io.File;
+import java.io.IOException;
 import java.util.Properties;
-
 import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.springframework.beans.factory.config.YamlPropertiesFactoryBean;
 import org.springframework.core.io.ByteArrayResource;
 import org.yaml.snakeyaml.parser.ParserException;
 
-import com.google.common.base.Charsets;
-import com.google.common.io.Files;
-
 public class YamlParserTest {
 
   private YamlParser parser = new YamlParser();
@@ -37,14 +44,50 @@ public class YamlParserTest {
     testInvalid("case8.yaml");
   }
 
-  private void test(String caseName) throws Exception {
-    File file = new File("src/test/resources/yaml/" + caseName);
+  @Test
+  public void testOrderProperties() throws IOException {
+    String yamlContent = loadYaml("orderedcase.yaml");
 
-    String yamlContent = Files.toString(file, Charsets.UTF_8);
+    Properties nonOrderedProperties = parser.yamlToProperties(yamlContent);
+
+    MockInjector.reset();
+
+    PropertiesFactory propertiesFactory = mock(PropertiesFactory.class);;
+    when(propertiesFactory.getPropertiesInstance()).thenAnswer(new Answer<Properties>() {
+      @Override
+      public Properties answer(InvocationOnMock invocation) {
+        return new OrderedProperties();
+      }
+    });
+    MockInjector.setInstance(PropertiesFactory.class, propertiesFactory);
+
+    parser = new YamlParser();
+
+    Properties orderedProperties = parser.yamlToProperties(yamlContent);
+
+    assertTrue(orderedProperties instanceof OrderedProperties);
+
+    checkPropertiesEquals(nonOrderedProperties, orderedProperties);
+
+    String[] propertyNames = orderedProperties.stringPropertyNames().toArray(new String[0]);
+
+    assertEquals("k2", propertyNames[0]);
+    assertEquals("k4", propertyNames[1]);
+    assertEquals("k1", propertyNames[2]);
+  }
+
+  private void test(String caseName) throws Exception {
+    String yamlContent = loadYaml(caseName);
 
     check(yamlContent);
   }
 
+  private String loadYaml(String caseName) throws IOException {
+    File file = new File("src/test/resources/yaml/" + caseName);
+
+    return Files.toString(file, Charsets.UTF_8);
+  }
+
   private void testInvalid(String caseName) throws Exception {
     File file = new File("src/test/resources/yaml/" + caseName);
 

apollo-client/src/test/resources/yaml/orderedcase.yaml

+k2: "someValue"
+k4: "anotherValue"
+k1: "yetAnotherValue"
\ No newline at end of file

apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigController.java

@@ -177,7 +177,7 @@ public class ConfigController {
    * Release in lower index override those in higher index
    */
   Map<String, String> mergeReleaseConfigurations(List<Release> releases) {
-    Map<String, String> result = Maps.newHashMap();
+    Map<String, String> result = Maps.newLinkedHashMap();
     for (Release release : Lists.reverse(releases)) {
       result.putAll(gson.fromJson(release.getConfigurations(), configurationTypeReference));
     }