refactor according to code review comments

apollo-adminservice/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>com.ctrip.framework.apollo</groupId>
 		<artifactId>apollo</artifactId>
-		<version>0.0.1</version>
+		<version>0.0.2-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>

apollo-assembly/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>com.ctrip.framework.apollo</groupId>
 		<artifactId>apollo</artifactId>
-		<version>0.0.1</version>
+		<version>0.0.2-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>

apollo-biz/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<artifactId>apollo</artifactId>
 		<groupId>com.ctrip.framework.apollo</groupId>
-		<version>0.0.1</version>
+		<version>0.0.2-SNAPSHOT</version>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>apollo-biz</artifactId>

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

@@ -49,7 +49,7 @@ public void createDefaultAppNamespace(String appId, String createBy) {
     }
     AppNamespace appNs = new AppNamespace();
     appNs.setAppId(appId);
-    appNs.setName(ConfigConsts.NAMESPACE_DEFAULT);
+    appNs.setName(ConfigConsts.NAMESPACE_APPLICATION);
     appNs.setComment("default app namespace");
     appNs.setDataChangeCreatedBy(createBy);
     appNs.setDataChangeLastModifiedBy(createBy);
@@ -74,7 +74,7 @@ public AppNamespace createAppNamespace(AppNamespace appNamespace, String createB
   }
 
   public List<AppNamespace> findPublicAppNamespaces(){
-    return appNamespaceRepository.findByNameNot(ConfigConsts.NAMESPACE_DEFAULT);
+    return appNamespaceRepository.findByNameNot(ConfigConsts.NAMESPACE_APPLICATION);
   }
 
   public AppNamespace update(AppNamespace appNamespace){

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

@@ -91,7 +91,7 @@ public void createDefaultNamespace(String appId, String createBy) {
     Namespace ns = new Namespace();
     ns.setAppId(appId);
     ns.setClusterName(ConfigConsts.CLUSTER_NAME_DEFAULT);
-    ns.setNamespaceName(ConfigConsts.NAMESPACE_DEFAULT);
+    ns.setNamespaceName(ConfigConsts.NAMESPACE_APPLICATION);
     ns.setDataChangeCreatedBy(createBy);
     ns.setDataChangeLastModifiedBy(createBy);
     namespaceRepository.save(ns);

apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/repository/AppNamespaceRepositoryTest.java

@@ -26,7 +26,7 @@ public class AppNamespaceRepositoryTest {
 
   @Test
   public void testFindAllPublicAppNamespaces(){
-    List<AppNamespace> appNamespaceList = repository.findByNameNot(ConfigConsts.NAMESPACE_DEFAULT);
+    List<AppNamespace> appNamespaceList = repository.findByNameNot(ConfigConsts.NAMESPACE_APPLICATION);
     Assert.assertEquals(4, appNamespaceList.size());
   }
 

apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/AdminServiceTest.java

@@ -64,7 +64,7 @@ public void testCreateNewApp() {
 
     List<Namespace> namespaces = namespaceService.findNamespaces(appId, clusters.get(0).getName());
     Assert.assertEquals(1, namespaces.size());
-    Assert.assertEquals(ConfigConsts.NAMESPACE_DEFAULT, namespaces.get(0).getNamespaceName());
+    Assert.assertEquals(ConfigConsts.NAMESPACE_APPLICATION, namespaces.get(0).getNamespaceName());
 
     List<Audit> audits = auditService.findByOwner(owner);
     Assert.assertEquals(4, audits.size());

apollo-buildtools/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>com.ctrip.framework.apollo</groupId>
 		<artifactId>apollo</artifactId>
-		<version>0.0.1</version>
+		<version>0.0.2-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>

apollo-client/README.md

@@ -37,6 +37,14 @@ Environment could be configured in 3 ways:
 	* And specify the environment in the file as `env=YOUR-ENVIRONMENT`
 	* Please note the key should be lower case
 
+Currently, `env` allows the following values (case-insensitive):
+
+* DEV
+* FWS
+* FAT
+* UAT
+* PRO
+
 ### I.II Optional Setup
 
 #### Cluster
@@ -76,7 +84,7 @@ If you need this functionality, you could specify the cluster as follows:
 		<dependency>
 			<groupId>com.ctrip.framework.apollo</groupId>
 			<artifactId>apollo-client</artifactId>
-			<version>0.0.1</version>
+			<version>0.0.2-SNAPSHOT</version>
 		</dependency>
 
 ## III. Client Usage

apollo-client/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>com.ctrip.framework.apollo</groupId>
 		<artifactId>apollo</artifactId>
-		<version>0.0.1</version>
+		<version>0.0.2-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

@@ -1,6 +1,7 @@
 package com.ctrip.framework.apollo;
 
 import com.ctrip.framework.apollo.core.ConfigConsts;
+import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
 import com.ctrip.framework.apollo.internals.ConfigManager;
 import com.ctrip.framework.apollo.spi.ConfigFactory;
 import com.ctrip.framework.apollo.spi.ConfigRegistry;
@@ -28,7 +29,7 @@ private ConfigService() {
    * @return config instance
    */
   public static Config getAppConfig() {
-    return getConfig(ConfigConsts.NAMESPACE_DEFAULT);
+    return getConfig(ConfigConsts.NAMESPACE_APPLICATION);
   }
 
   /**
@@ -37,30 +38,32 @@ public static Config getAppConfig() {
    * @return config instance
    */
   public static Config getConfig(String namespace) {
-    Cat.logEvent("Apollo.Client.Version", Apollo.VERSION);
+
     return getManager().getConfig(namespace);
   }
 
   private static ConfigManager getManager() {
     try {
       return s_instance.m_container.lookup(ConfigManager.class);
     } catch (ComponentLookupException ex) {
-      Cat.logError(ex);
-      throw new IllegalStateException("Unable to load ConfigManager!", ex);
+      ApolloConfigException exception = new ApolloConfigException("Unable to load ConfigManager!", ex);
+      Cat.logError(exception);
+      throw exception;
     }
   }
 
   private static ConfigRegistry getRegistry() {
     try {
       return s_instance.m_container.lookup(ConfigRegistry.class);
     } catch (ComponentLookupException ex) {
-      Cat.logError(ex);
-      throw new IllegalStateException("Unable to load ConfigRegistry!", ex);
+      ApolloConfigException exception = new ApolloConfigException("Unable to load ConfigRegistry!", ex);
+      Cat.logError(exception);
+      throw exception;
     }
   }
 
   static void setConfig(Config config) {
-    setConfig(ConfigConsts.NAMESPACE_DEFAULT, config);
+    setConfig(ConfigConsts.NAMESPACE_APPLICATION, config);
   }
 
   /**
@@ -78,7 +81,7 @@ public Config create(String namespace) {
   }
 
   static void setConfigFactory(ConfigFactory factory) {
-    setConfigFactory(ConfigConsts.NAMESPACE_DEFAULT, factory);
+    setConfigFactory(ConfigConsts.NAMESPACE_APPLICATION, factory);
   }
 
   /**

apollo-client/src/main/java/com/ctrip/framework/apollo/exceptions/ApolloConfigException.java

@@ -0,0 +1,14 @@
+package com.ctrip.framework.apollo.exceptions;
+
+/**
+ * @author Jason Song([email protected])
+ */
+public class ApolloConfigException extends RuntimeException {
+  public ApolloConfigException(String message) {
+    super(message);
+  }
+
+  public ApolloConfigException(String message, Throwable cause) {
+    super(message, cause);
+  }
+}

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

@@ -6,25 +6,37 @@
 
 import com.ctrip.framework.apollo.Config;
 import com.ctrip.framework.apollo.ConfigChangeListener;
+import com.ctrip.framework.apollo.core.utils.ApolloThreadFactory;
 import com.ctrip.framework.apollo.enums.PropertyChangeType;
 import com.ctrip.framework.apollo.model.ConfigChange;
 import com.ctrip.framework.apollo.model.ConfigChangeEvent;
 import com.dianping.cat.Cat;
+import com.dianping.cat.message.Message;
+import com.dianping.cat.message.Transaction;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.Properties;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 /**
  * @author Jason Song([email protected])
  */
 public abstract class AbstractConfig implements Config {
   private static final Logger logger = LoggerFactory.getLogger(AbstractConfig.class);
+  private static ExecutorService m_executorService;
   private List<ConfigChangeListener> m_listeners = Lists.newCopyOnWriteArrayList();
 
+  static {
+    m_executorService = Executors.newCachedThreadPool(ApolloThreadFactory
+        .create("Config", true));
+
+  }
+
   @Override
   public void addChangeListener(ConfigChangeListener listener) {
     if (!m_listeners.contains(listener)) {
@@ -80,19 +92,30 @@ public String[] getArrayProperty(String key, String delimiter, String[] defaultV
     return value == null ? defaultValue : value.split(delimiter);
   }
 
-  protected void fireConfigChange(ConfigChangeEvent changeEvent) {
-    for (ConfigChangeListener listener : m_listeners) {
-      try {
-        listener.onChange(changeEvent);
-      } catch (Throwable ex) {
-        Cat.logError(ex);
-        logger.error("Failed to invoke config change listener {}", listener.getClass(), ex);
-      }
+  protected void fireConfigChange(final ConfigChangeEvent changeEvent) {
+    for (final ConfigChangeListener listener : m_listeners) {
+      m_executorService.submit(new Runnable() {
+        @Override
+        public void run() {
+          String listenerName = listener.getClass().getName();
+          Transaction transaction = Cat.newTransaction("Apollo.ConfigChangeListener", listenerName);
+          try {
+            listener.onChange(changeEvent);
+            transaction.setStatus(Message.SUCCESS);
+          } catch (Throwable ex) {
+            transaction.setStatus(ex);
+            Cat.logError(ex);
+            logger.error("Failed to invoke config change listener {}", listenerName, ex);
+          } finally {
+            transaction.complete();
+          }
+        }
+      });
     }
   }
 
   List<ConfigChange> calcPropertyChanges(String namespace, Properties previous,
-      Properties current) {
+                                         Properties current) {
     if (previous == null) {
       previous = new Properties();
     }

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

@@ -14,9 +14,9 @@ public interface ConfigRepository {
 
   /**
    * Set the fallback repo for this repository.
-   * @param fallbackConfigRepository the fallback repo
+   * @param upstreamConfigRepository the fallback repo
    */
-  public void setFallback(ConfigRepository fallbackConfigRepository);
+  public void setUpstreamRepository(ConfigRepository upstreamConfigRepository);
 
   /**
    * Add change listener.

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

@@ -10,6 +10,7 @@
 
 import com.ctrip.framework.apollo.core.dto.ServiceDTO;
 import com.ctrip.framework.apollo.core.utils.ApolloThreadFactory;
+import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
 import com.ctrip.framework.apollo.util.ConfigUtil;
 import com.ctrip.framework.apollo.util.http.HttpRequest;
 import com.ctrip.framework.apollo.util.http.HttpResponse;
@@ -92,11 +93,8 @@ private void schedulePeriodicRefresh() {
           @Override
           public void run() {
             logger.debug("refresh config services");
-            Transaction transaction = Cat.newTransaction("Apollo.MetaService", "periodicRefresh");
-            boolean syncResult = tryUpdateConfigServices();
-            String status = syncResult ? Message.SUCCESS : "-1";
-            transaction.setStatus(status);
-            transaction.complete();
+            Cat.logEvent("Apollo.MetaService", "periodicRefresh");
+            tryUpdateConfigServices();
           }
         }, m_configUtil.getRefreshInterval(), m_configUtil.getRefreshInterval(),
         m_configUtil.getRefreshTimeUnit());
@@ -138,7 +136,7 @@ private synchronized void updateConfigServices() {
       }
     }
 
-    throw new RuntimeException(
+    throw new ApolloConfigException(
         String.format("Get config services failed from %s", url), exception);
   }
 

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

@@ -14,7 +14,7 @@
 /**
  * @author Jason Song([email protected])
  */
-@Named(type = ConfigManager.class, value = "default")
+@Named(type = ConfigManager.class)
 public class DefaultConfigManager implements ConfigManager {
   @Inject
   private ConfigFactoryManager m_factoryManager;