fix apollo config data loader with profiles (#3870)

* fix apollo config data loader with profiles

* CHANGES.md

* m_pureApolloConfig default to false

* rename field

* divide Config to 2 class

* add unit test

* change getPropertyFromRepository

* change getPropertyFromAdditional

* use spi to create PureApolloConfig, and remove the pureApolloConfig field

* modify unit test

* update getPropertyNames

* move to apollo-client-config-data

* extends DefaultConfig

* update unit test

* fix getPropertyNames

* update DefaultConfig unit test

* remove AbstractRepositoryConfig

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

Co-authored-by: Jason Song <nobodyiam@gmail.com>

Co-authored-by: Jason Song <nobodyiam@gmail.com>

CHANGES.md

@@ -73,6 +73,7 @@ Apollo 1.9.0
 * [add spring configuration metadata for property names cache](https://github.com/ctripcorp/apollo/pull/3879)
 * [Fix Multiple PropertySourcesPlaceholderConfigurer beans registered issue](https://github.com/ctripcorp/apollo/pull/3865)
 * [use jdk 8 to publish apollo-client-config-data](https://github.com/ctripcorp/apollo/pull/3880)
+* [fix apollo config data loader with profiles](https://github.com/ctripcorp/apollo/pull/3870)
 
 ------------------
 All issues and pull requests are [here](https://github.com/ctripcorp/apollo/milestone/6?closed=1)

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/extension/webclient/ApolloClientLongPollingExtensionInitializer.java

@@ -19,7 +19,8 @@ package com.ctrip.framework.apollo.config.data.extension.webclient;
 import com.ctrip.framework.apollo.config.data.extension.initialize.ApolloClientExtensionInitializer;
 import com.ctrip.framework.apollo.config.data.extension.properties.ApolloClientProperties;
 import com.ctrip.framework.apollo.config.data.extension.webclient.customizer.spi.ApolloClientWebClientCustomizerFactory;
-import com.ctrip.framework.apollo.config.data.extension.webclient.injector.ApolloClientCustomHttpClientInjectorCustomizer;
+import com.ctrip.framework.apollo.config.data.injector.ApolloConfigDataInjectorCustomizer;
+import com.ctrip.framework.apollo.util.http.HttpClient;
 import com.ctrip.framework.foundation.internals.ServiceBootstrap;
 import java.util.List;
 import org.apache.commons.logging.Log;
@@ -62,6 +63,7 @@ public class ApolloClientLongPollingExtensionInitializer implements
         }
       }
     }
-    ApolloClientCustomHttpClientInjectorCustomizer.setCustomWebClient(webClientBuilder.build());
+    HttpClient httpClient = new ApolloWebClientHttpClient(webClientBuilder.build());
+    ApolloConfigDataInjectorCustomizer.registerIfAbsent(HttpClient.class, () -> httpClient);
   }
 }

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/extension/webclient/injector/ApolloClientCustomHttpClientInjectorCustomizer.java

@@ -1,63 +0,0 @@
-/*
- * Copyright 2021 Apollo Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-package com.ctrip.framework.apollo.config.data.extension.webclient.injector;
-
-import com.ctrip.framework.apollo.config.data.extension.webclient.ApolloWebClientHttpClient;
-import com.ctrip.framework.apollo.core.spi.Ordered;
-import com.ctrip.framework.apollo.spi.ApolloInjectorCustomizer;
-import org.springframework.web.reactive.function.client.WebClient;
-
-/**
- * @author vdisk <vdisk@foxmail.com>
- */
-public class ApolloClientCustomHttpClientInjectorCustomizer implements ApolloInjectorCustomizer {
-
-  /**
-   * the order of the injector customizer
-   */
-  public static final int ORDER = Ordered.LOWEST_PRECEDENCE - 100;
-
-  private static ApolloWebClientHttpClient CUSTOM_HTTP_CLIENT;
-
-  /**
-   * set the webClient to use
-   *
-   * @param webClient webClient to use
-   */
-  public static void setCustomWebClient(WebClient webClient) {
-    CUSTOM_HTTP_CLIENT = new ApolloWebClientHttpClient(webClient);
-  }
-
-  @SuppressWarnings("unchecked")
-  @Override
-  public <T> T getInstance(Class<T> clazz) {
-    if (clazz.isInstance(CUSTOM_HTTP_CLIENT)) {
-      return (T) CUSTOM_HTTP_CLIENT;
-    }
-    return null;
-  }
-
-  @Override
-  public <T> T getInstance(Class<T> clazz, String name) {
-    return null;
-  }
-
-  @Override
-  public int getOrder() {
-    return ORDER;
-  }
-}

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/importer/ApolloConfigDataLoader.java

@@ -74,8 +74,7 @@ public class ApolloConfigDataLoader implements ConfigDataLoader<ApolloConfigData
     List<PropertySource<?>> propertySourceList = new ArrayList<>();
     propertySourceList.add(configPropertySource);
     propertySourceList.addAll(initialPropertySourceList);
-    log.debug(Slf4jLogMessageFormatter
-        .format("apollo client loaded namespace [{}]", resource.getNamespace()));
+    log.debug(Slf4jLogMessageFormatter.format("apollo client loaded namespace [{}]", namespace));
     return new ConfigData(propertySourceList);
   }
 

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/importer/ApolloConfigDataLoaderInitializer.java

@@ -17,9 +17,12 @@
 package com.ctrip.framework.apollo.config.data.importer;
 
 import com.ctrip.framework.apollo.config.data.extension.initialize.ApolloClientExtensionInitializeFactory;
+import com.ctrip.framework.apollo.config.data.injector.ApolloConfigDataInjectorCustomizer;
+import com.ctrip.framework.apollo.config.data.internals.PureApolloConfigFactory;
 import com.ctrip.framework.apollo.config.data.system.ApolloClientSystemPropertyInitializer;
 import com.ctrip.framework.apollo.config.data.util.Slf4jLogMessageFormatter;
 import com.ctrip.framework.apollo.core.utils.DeferredLogger;
+import com.ctrip.framework.apollo.spi.ConfigFactory;
 import com.ctrip.framework.apollo.spring.config.PropertySourcesConstants;
 import java.util.Arrays;
 import java.util.Collections;
@@ -100,6 +103,8 @@ class ApolloConfigDataLoaderInitializer {
     new ApolloClientExtensionInitializeFactory(this.log,
         this.bootstrapContext).initializeExtension(this.binder, this.bindHandler);
     DeferredLogger.enable();
+    ApolloConfigDataInjectorCustomizer.register(ConfigFactory.class,
+        PureApolloConfigFactory::new);
   }
 
   private boolean forceDisableApolloBootstrap() {

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/injector/ApolloConfigDataInjectorCustomizer.java

@@ -0,0 +1,104 @@
+/*
+ * Copyright 2021 Apollo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.ctrip.framework.apollo.config.data.injector;
+
+import com.ctrip.framework.apollo.core.spi.Ordered;
+import com.ctrip.framework.apollo.spi.ApolloInjectorCustomizer;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
+
+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public class ApolloConfigDataInjectorCustomizer implements ApolloInjectorCustomizer {
+
+  /**
+   * the order of the injector customizer
+   */
+  public static final int ORDER = Ordered.LOWEST_PRECEDENCE - 200;
+
+  private static final Map<Class<?>, Supplier<?>> INSTANCE_SUPPLIERS = new ConcurrentHashMap<>();
+
+  private static final Map<Class<?>, Object> INSTANCES = new ConcurrentHashMap<>();
+
+  /**
+   * Register a specific type with the registry. If the specified type has already been registered,
+   * it will be replaced.
+   *
+   * @param <T>              the instance type
+   * @param type             the instance type
+   * @param instanceSupplier the instance supplier
+   */
+  public static <T> void register(Class<T> type, Supplier<T> instanceSupplier) {
+    INSTANCE_SUPPLIERS.put(type, instanceSupplier);
+  }
+
+  /**
+   * Register a specific type with the registry if one is not already present.
+   *
+   * @param <T>              the instance type
+   * @param type             the instance type
+   * @param instanceSupplier the instance supplier
+   */
+  public static <T> void registerIfAbsent(Class<T> type, Supplier<T> instanceSupplier) {
+    INSTANCE_SUPPLIERS.putIfAbsent(type, instanceSupplier);
+  }
+
+  /**
+   * Return if a registration exists for the given type.
+   *
+   * @param <T>  the instance type
+   * @param type the instance type
+   * @return {@code true} if the type has already been registered
+   */
+  public static <T> boolean isRegistered(Class<T> type) {
+    return INSTANCE_SUPPLIERS.containsKey(type);
+  }
+
+  @Override
+  public <T> T getInstance(Class<T> clazz) {
+    @SuppressWarnings("unchecked")
+    Supplier<T> instanceSupplier = (Supplier<T>) INSTANCE_SUPPLIERS.get(clazz);
+    if (instanceSupplier == null) {
+      return null;
+    }
+    return this.getInstance(clazz, instanceSupplier);
+  }
+
+  @SuppressWarnings("unchecked")
+  private <T> T getInstance(Class<T> type, Supplier<T> instanceSupplier) {
+    T instance = (T) INSTANCES.get(type);
+    if (instance != null) {
+      return instance;
+    }
+    // prebuild an newInstance to prevent dead lock when recursive call computeIfAbsent
+    // https://bugs.openjdk.java.net/browse/JDK-8062841
+    T newInstance = instanceSupplier.get();
+    return (T) INSTANCES.computeIfAbsent(type, key -> newInstance);
+  }
+
+  @Override
+  public <T> T getInstance(Class<T> clazz, String name) {
+    return null;
+  }
+
+  @Override
+  public int getOrder() {
+    return ORDER;
+  }
+}

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright 2021 Apollo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.ctrip.framework.apollo.config.data.internals;
+
+import com.ctrip.framework.apollo.internals.ConfigRepository;
+import com.ctrip.framework.apollo.internals.DefaultConfig;
+import com.ctrip.framework.apollo.internals.RepositoryChangeListener;
+import com.google.common.collect.Sets;
+import java.util.Set;
+import org.springframework.util.CollectionUtils;
+
+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public class PureApolloConfig extends DefaultConfig implements RepositoryChangeListener {
+
+  /**
+   * Constructor.
+   *
+   * @param namespace        the namespace of this config instance
+   * @param configRepository the config repository for this config instance
+   */
+  public PureApolloConfig(String namespace,
+      ConfigRepository configRepository) {
+    super(namespace, configRepository);
+  }
+
+  @Override
+  public String getProperty(String key, String defaultValue) {
+    // step 1: check local cached properties file
+    String value = this.getPropertyFromRepository(key);
+
+    // step 2: check properties file from classpath
+    if (value == null) {
+      value = this.getPropertyFromAdditional(key);
+    }
+
+    this.tryWarnLog(value);
+
+    return value == null ? defaultValue : value;
+  }
+
+  @Override
+  public Set<String> getPropertyNames() {
+    // pure apollo config only contains the property from repository and the property from additional
+    Set<String> fromRepository = this.getPropertyNamesFromRepository();
+    Set<String> fromAdditional = this.getPropertyNamesFromAdditional();
+    if (CollectionUtils.isEmpty(fromRepository)) {
+      return fromAdditional;
+    }
+    if (CollectionUtils.isEmpty(fromAdditional)) {
+      return fromRepository;
+    }
+    Set<String> propertyNames = Sets
+        .newLinkedHashSetWithExpectedSize(fromRepository.size() + fromAdditional.size());
+    propertyNames.addAll(fromRepository);
+    propertyNames.addAll(fromAdditional);
+    return propertyNames;
+  }
+}

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2021 Apollo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.ctrip.framework.apollo.config.data.internals;
+
+import com.ctrip.framework.apollo.Config;
+import com.ctrip.framework.apollo.internals.ConfigRepository;
+import com.ctrip.framework.apollo.spi.ConfigFactory;
+import com.ctrip.framework.apollo.spi.DefaultConfigFactory;
+
+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public class PureApolloConfigFactory extends DefaultConfigFactory implements ConfigFactory {
+
+  @Override
+  protected Config createRepositoryConfig(String namespace, ConfigRepository configRepository) {
+    return new PureApolloConfig(namespace, configRepository);
+  }
+}

apollo-client-config-data/src/main/resources/META-INF/services/com.ctrip.framework.apollo.spi.ApolloInjectorCustomizer

@@ -1 +1 @@
-com.ctrip.framework.apollo.config.data.extension.webclient.injector.ApolloClientCustomHttpClientInjectorCustomizer
+com.ctrip.framework.apollo.config.data.injector.ApolloConfigDataInjectorCustomizer

apollo-client-config-data/src/test/java/com/ctrip/framework/apollo/config/data/importer/PureApolloConfigTest.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright 2021 Apollo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.ctrip.framework.apollo.config.data.importer;
+
+import com.ctrip.framework.apollo.Config;
+import com.ctrip.framework.apollo.build.ApolloInjector;
+import com.ctrip.framework.apollo.config.data.injector.ApolloMockInjectorCustomizer;
+import com.ctrip.framework.apollo.config.data.internals.PureApolloConfigFactory;
+import com.ctrip.framework.apollo.spi.ConfigFactory;
+import com.ctrip.framework.apollo.spi.DefaultConfigFactory;
+import com.github.stefanbirkner.systemlambda.SystemLambda;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public class PureApolloConfigTest {
+
+  @Before
+  public void before() {
+    System.setProperty("env", "local");
+  }
+
+  @After
+  public void after() {
+    System.clearProperty("spring.profiles.active");
+    System.clearProperty("env");
+    ApolloMockInjectorCustomizer.clear();
+  }
+
+  @Test
+  public void testDefaultConfigWithSystemProperties() {
+    System.setProperty("spring.profiles.active", "test");
+    ApolloMockInjectorCustomizer.register(ConfigFactory.class,
+        DefaultConfigFactory::new);
+    ConfigFactory configFactory = ApolloInjector.getInstance(ConfigFactory.class);
+    Config config = configFactory.create("application");
+    Assert.assertEquals("test", config.getProperty("spring.profiles.active", null));
+  }
+
+  @Test
+  public void testPureApolloConfigWithSystemProperties() {
+    System.setProperty("spring.profiles.active", "test");
+    ApolloMockInjectorCustomizer.register(ConfigFactory.class,
+        PureApolloConfigFactory::new);
+    ConfigFactory configFactory = ApolloInjector.getInstance(ConfigFactory.class);
+    Config config = configFactory.create("application");
+    Assert.assertNull(config.getProperty("spring.profiles.active", null));
+  }
+
+  @Test
+  public void testDefaultConfigWithEnvironmentVariables() throws Exception {
+    SystemLambda.withEnvironmentVariable(
+        "SPRING_PROFILES_ACTIVE",
+        "test-env")
+        .execute(() -> {
+          ApolloMockInjectorCustomizer.register(ConfigFactory.class,
+              DefaultConfigFactory::new);
+          ConfigFactory configFactory = ApolloInjector.getInstance(ConfigFactory.class);
+          Config config = configFactory.create("application");
+          Assert.assertEquals("test-env", config.getProperty("SPRING_PROFILES_ACTIVE", null));
+        });
+  }
+
+  @Test
+  public void testPureApolloConfigWithEnvironmentVariables() throws Exception {
+    SystemLambda.withEnvironmentVariable(
+        "SPRING_PROFILES_ACTIVE",
+        "test-env")
+        .execute(() -> {
+          ApolloMockInjectorCustomizer.register(ConfigFactory.class,
+              PureApolloConfigFactory::new);
+          ConfigFactory configFactory = ApolloInjector.getInstance(ConfigFactory.class);
+          Config config = configFactory.create("application");
+          Assert.assertNull(config.getProperty("SPRING_PROFILES_ACTIVE", null));
+        });
+  }
+}

apollo-client-config-data/src/test/java/com/ctrip/framework/apollo/config/data/injector/ApolloMockInjectorCustomizer.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright 2021 Apollo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.ctrip.framework.apollo.config.data.injector;
+
+import com.ctrip.framework.apollo.core.spi.Ordered;
+import com.ctrip.framework.apollo.spi.ApolloInjectorCustomizer;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
+
+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public class ApolloMockInjectorCustomizer implements ApolloInjectorCustomizer {
+
+  private static final Map<Class<?>, Supplier<?>> INSTANCE_SUPPLIERS = new ConcurrentHashMap<>();
+
+  private static final Map<Class<?>, Object> INSTANCES = new ConcurrentHashMap<>();
+
+  /**
+   * Register a specific type with the registry. If the specified type has already been registered,
+   * it will be replaced.
+   *
+   * @param <T>              the instance type
+   * @param type             the instance type
+   * @param instanceSupplier the instance supplier
+   */
+  public static <T> void register(Class<T> type, Supplier<T> instanceSupplier) {
+    INSTANCE_SUPPLIERS.put(type, instanceSupplier);
+  }
+
+  /**
+   * Register a specific type with the registry if one is not already present.
+   *
+   * @param <T>              the instance type
+   * @param type             the instance type
+   * @param instanceSupplier the instance supplier
+   */
+  public static <T> void registerIfAbsent(Class<T> type, Supplier<T> instanceSupplier) {
+    INSTANCE_SUPPLIERS.putIfAbsent(type, instanceSupplier);
+  }
+
+  /**
+   * Return if a registration exists for the given type.
+   *
+   * @param <T>  the instance type
+   * @param type the instance type
+   * @return {@code true} if the type has already been registered
+   */
+  public static <T> boolean isRegistered(Class<T> type) {
+    return INSTANCE_SUPPLIERS.containsKey(type);
+  }
+
+  /**
+   * clear the instance cache and instance suppliers
+   */
+  public static void clear() {
+    INSTANCE_SUPPLIERS.clear();
+    INSTANCES.clear();
+  }
+
+  @Override
+  public <T> T getInstance(Class<T> clazz) {
+    @SuppressWarnings("unchecked")
+    Supplier<T> instanceSupplier = (Supplier<T>) INSTANCE_SUPPLIERS.get(clazz);
+    if (instanceSupplier == null) {
+      return null;
+    }
+    return this.getInstance(clazz, instanceSupplier);
+  }
+
+  @SuppressWarnings("unchecked")
+  private <T> T getInstance(Class<T> type, Supplier<T> instanceSupplier) {
+    T instance = (T) INSTANCES.get(type);
+    if (instance != null) {
+      return instance;
+    }
+    // prebuild an newInstance to prevent dead lock when recursive call computeIfAbsent
+    // https://bugs.openjdk.java.net/browse/JDK-8062841
+    T newInstance = instanceSupplier.get();
+    return (T) INSTANCES.computeIfAbsent(type, key -> newInstance);
+  }
+
+  @Override
+  public <T> T getInstance(Class<T> clazz, String name) {
+    return null;
+  }
+
+  @Override
+  public int getOrder() {
+    return Ordered.HIGHEST_PRECEDENCE;
+  }
+}

apollo-client-config-data/src/test/resources/META-INF/services/com.ctrip.framework.apollo.spi.ApolloInjectorCustomizer

@@ -0,0 +1 @@
+com.ctrip.framework.apollo.config.data.injector.ApolloMockInjectorCustomizer

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

@@ -18,10 +18,11 @@ package com.ctrip.framework.apollo.internals;
 
 import com.ctrip.framework.apollo.core.utils.DeferredLoggerFactory;
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -38,6 +39,7 @@ import com.ctrip.framework.apollo.tracer.Tracer;
 import com.ctrip.framework.apollo.util.ExceptionUtil;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.RateLimiter;
+import org.springframework.util.CollectionUtils;
 
 
 /**
@@ -83,17 +85,84 @@ public class DefaultConfig extends AbstractConfig implements RepositoryChangeLis
     }
   }
 
+  /**
+   * get property from cached repository properties file
+   *
+   * @param key property key
+   * @return value
+   */
+  protected String getPropertyFromRepository(String key) {
+    Properties properties = m_configProperties.get();
+    if (properties != null) {
+      return properties.getProperty(key);
+    }
+    return null;
+  }
+
+  /**
+   * get property from additional properties file on classpath
+   *
+   * @param key property key
+   * @return value
+   */
+  protected String getPropertyFromAdditional(String key) {
+    Properties properties = this.m_resourceProperties;
+    if (properties != null) {
+      return properties.getProperty(key);
+    }
+    return null;
+  }
+
+  /**
+   * try to print a warn log when can not find a property
+   *
+   * @param value value
+   */
+  protected void tryWarnLog(String value) {
+    if (value == null && m_configProperties.get() == null && m_warnLogRateLimiter.tryAcquire()) {
+      logger.warn(
+          "Could not load config for namespace {} from Apollo, please check whether the configs are released in Apollo! Return default value now!",
+          m_namespace);
+    }
+  }
+
+  /**
+   * get property names from cached repository properties file
+   *
+   * @return property names
+   */
+  protected Set<String> getPropertyNamesFromRepository() {
+    Properties properties = m_configProperties.get();
+    if (properties == null) {
+      return Collections.emptySet();
+    }
+    return this.stringPropertyNames(properties);
+  }
+
+  /**
+   * get property names from additional properties file on classpath
+   *
+   * @return property names
+   */
+  protected Set<String> getPropertyNamesFromAdditional() {
+    Properties properties = m_resourceProperties;
+    if (properties == null) {
+      return Collections.emptySet();
+    }
+    return this.stringPropertyNames(properties);
+  }
+
   @Override
   public String getProperty(String key, String defaultValue) {
     // step 1: check system properties, i.e. -Dkey=value
     String value = System.getProperty(key);
 
     // step 2: check local cached properties file
-    if (value == null && m_configProperties.get() != null) {
-      value = m_configProperties.get().getProperty(key);
+    if (value == null) {
+      value = this.getPropertyFromRepository(key);
     }
 
-    /**
+    /*
      * step 3: check env variable, i.e. PATH=...
      * normally system environment variables are in UPPERCASE, however there might be exceptions.
      * so the caller should provide the key in the right case
@@ -103,27 +172,31 @@ public class DefaultConfig extends AbstractConfig implements RepositoryChangeLis
     }
 
     // step 4: check properties file from classpath
-    if (value == null && m_resourceProperties != null) {
-      value = m_resourceProperties.getProperty(key);
+    if (value == null) {
+      value = this.getPropertyFromAdditional(key);
     }
 
-    if (value == null && m_configProperties.get() == null && m_warnLogRateLimiter.tryAcquire()) {
-      logger.warn(
-          "Could not load config for namespace {} from Apollo, please check whether the configs are released in Apollo! Return default value now!",
-          m_namespace);
-    }
+    this.tryWarnLog(value);
 
     return value == null ? defaultValue : value;
   }
 
   @Override
   public Set<String> getPropertyNames() {
-    Properties properties = m_configProperties.get();
-    if (properties == null) {
-      return Collections.emptySet();
+    // propertyNames include system property and system env might cause some compatibility issues, though that looks like the correct implementation.
+    Set<String> fromRepository = this.getPropertyNamesFromRepository();
+    Set<String> fromAdditional = this.getPropertyNamesFromAdditional();
+    if (CollectionUtils.isEmpty(fromRepository)) {
+      return fromAdditional;
     }
-
-    return stringPropertyNames(properties);
+    if (CollectionUtils.isEmpty(fromAdditional)) {
+      return fromRepository;
+    }
+    Set<String> propertyNames = Sets
+        .newLinkedHashSetWithExpectedSize(fromRepository.size() + fromAdditional.size());
+    propertyNames.addAll(fromRepository);
+    propertyNames.addAll(fromAdditional);
+    return propertyNames;
   }
 
   @Override
@@ -133,7 +206,7 @@ public class DefaultConfig extends AbstractConfig implements RepositoryChangeLis
 
   private Set<String> stringPropertyNames(Properties properties) {
     //jdk9以下版本Properties#enumerateStringProperties方法存在性能问题，keys() + get(k) 重复迭代, jdk9之后改为entrySet遍历.
-    Map<String, String> h = new LinkedHashMap<>();
+    Map<String, String> h = Maps.newLinkedHashMapWithExpectedSize(properties.size());
     for (Map.Entry<Object, Object> e : properties.entrySet()) {
       Object k = e.getKey();
       Object v = e.getValue();

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java

@@ -43,7 +43,7 @@ import com.ctrip.framework.apollo.util.ConfigUtil;
  */
 public class DefaultConfigFactory implements ConfigFactory {
   private static final Logger logger = LoggerFactory.getLogger(DefaultConfigFactory.class);
-  private ConfigUtil m_configUtil;
+  private final ConfigUtil m_configUtil;
 
   public DefaultConfigFactory() {
     m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
@@ -53,9 +53,13 @@ public class DefaultConfigFactory implements ConfigFactory {
   public Config create(String namespace) {
     ConfigFileFormat format = determineFileFormat(namespace);
     if (ConfigFileFormat.isPropertiesCompatible(format)) {
-      return new DefaultConfig(namespace, createPropertiesCompatibleFileConfigRepository(namespace, format));
+      return this.createRepositoryConfig(namespace, createPropertiesCompatibleFileConfigRepository(namespace, format));
     }
-    return new DefaultConfig(namespace, createLocalConfigRepository(namespace));
+    return this.createRepositoryConfig(namespace, createLocalConfigRepository(namespace));
+  }
+
+  protected Config createRepositoryConfig(String namespace, ConfigRepository configRepository) {
+    return new DefaultConfig(namespace, configRepository);
   }
 
   @Override