Merge pull request #309 from nobodyiam/fix-client-url-encoding

apollo-adminservice/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>com.ctrip.framework.apollo</groupId>
 		<artifactId>apollo</artifactId>
-		<version>0.0.3</version>
+		<version>0.0.4</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.3</version>
+		<version>0.0.4</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.3</version>
+		<version>0.0.4</version>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>apollo-biz</artifactId>

apollo-buildtools/pom.xml

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

apollo-client/README.md

@@ -88,7 +88,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.3</version>
+			<version>0.0.4</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.3</version>
+		<version>0.0.4</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>

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

@@ -45,6 +45,7 @@ public class ConfigServiceLocator implements Initializable {
   private Type m_responseType;
   private ScheduledExecutorService m_executorService;
   private static final Joiner.MapJoiner MAP_JOINER = Joiner.on("&").withKeyValueSeparator("=");
+  private static final Escaper queryParamEscaper = UrlEscapers.urlFormParameterEscaper();
 
   /**
    * Create a config service locator.
@@ -145,11 +146,10 @@ public class ConfigServiceLocator implements Initializable {
     String appId = m_configUtil.getAppId();
     String localIp = m_configUtil.getLocalIp();
 
-    Escaper escaper = UrlEscapers.urlPathSegmentEscaper();
     Map<String, String> queryParams = Maps.newHashMap();
-    queryParams.put("appId", escaper.escape(appId));
+    queryParams.put("appId", queryParamEscaper.escape(appId));
     if (!Strings.isNullOrEmpty(localIp)) {
-      queryParams.put("ip", escaper.escape(localIp));
+      queryParams.put("ip", queryParamEscaper.escape(localIp));
     }
 
     return domainName + "/services/config?" + MAP_JOINER.join(queryParams);

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

@@ -65,6 +65,8 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
   private AtomicReference<ApolloConfigNotification> m_longPollResult;
   private RateLimiter m_longPollRateLimiter;
   private RateLimiter m_loadConfigRateLimiter;
+  private static final Escaper pathEscaper = UrlEscapers.urlPathSegmentEscaper();
+  private static final Escaper queryParamEscaper = UrlEscapers.urlFormParameterEscaper();
 
   static {
     m_executorService = Executors.newScheduledThreadPool(1,
@@ -235,26 +237,26 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
     throw new ApolloConfigException(message, exception);
   }
 
-  private String assembleQueryConfigUrl(String uri, String appId, String cluster, String namespace,
+  String assembleQueryConfigUrl(String uri, String appId, String cluster, String namespace,
                                         String dataCenter, ApolloConfig previousConfig) {
-    Escaper escaper = UrlEscapers.urlPathSegmentEscaper();
+
     String path = "configs/%s/%s/%s";
     List<String> pathParams =
-        Lists.newArrayList(escaper.escape(appId), escaper.escape(cluster),
-            escaper.escape(namespace));
+        Lists.newArrayList(pathEscaper.escape(appId), pathEscaper.escape(cluster),
+            pathEscaper.escape(namespace));
     Map<String, String> queryParams = Maps.newHashMap();
 
     if (previousConfig != null) {
-      queryParams.put("releaseKey", escaper.escape(String.valueOf(previousConfig.getReleaseKey())));
+      queryParams.put("releaseKey", queryParamEscaper.escape(previousConfig.getReleaseKey()));
     }
 
     if (!Strings.isNullOrEmpty(dataCenter)) {
-      queryParams.put("dataCenter", escaper.escape(dataCenter));
+      queryParams.put("dataCenter", queryParamEscaper.escape(dataCenter));
     }
 
     String localIp = m_configUtil.getLocalIp();
     if (!Strings.isNullOrEmpty(localIp)) {
-      queryParams.put("ip", escaper.escape(localIp));
+      queryParams.put("ip", queryParamEscaper.escape(localIp));
     }
 
     String pathExpanded = String.format(path, pathParams.toArray());
@@ -356,23 +358,22 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
     }
   }
 
-  private String assembleLongPollRefreshUrl(String uri, String appId, String cluster,
+  String assembleLongPollRefreshUrl(String uri, String appId, String cluster,
                                             String namespace, String dataCenter,
                                             ApolloConfigNotification previousResult) {
-    Escaper escaper = UrlEscapers.urlPathSegmentEscaper();
     Map<String, String> queryParams = Maps.newHashMap();
-    queryParams.put("appId", escaper.escape(appId));
-    queryParams.put("cluster", escaper.escape(cluster));
+    queryParams.put("appId", queryParamEscaper.escape(appId));
+    queryParams.put("cluster", queryParamEscaper.escape(cluster));
 
     if (!Strings.isNullOrEmpty(namespace)) {
-      queryParams.put("namespace", escaper.escape(namespace));
+      queryParams.put("namespace", queryParamEscaper.escape(namespace));
     }
     if (!Strings.isNullOrEmpty(dataCenter)) {
-      queryParams.put("dataCenter", escaper.escape(dataCenter));
+      queryParams.put("dataCenter", queryParamEscaper.escape(dataCenter));
     }
     String localIp = m_configUtil.getLocalIp();
     if (!Strings.isNullOrEmpty(localIp)) {
-      queryParams.put("ip", escaper.escape(localIp));
+      queryParams.put("ip", queryParamEscaper.escape(localIp));
     }
 
     if (previousResult != null) {

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

@@ -32,6 +32,7 @@ import java.util.concurrent.TimeUnit;
 import javax.servlet.http.HttpServletResponse;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -165,6 +166,46 @@ public class RemoteConfigRepositoryTest extends ComponentTestCase {
     assertEquals(newConfigurations, captor.getValue());
   }
 
+  @Test
+  public void testAssembleLongPollRefreshUrl() throws Exception {
+    String someUri = "http://someServer";
+    String someAppId = "someAppId";
+    String someCluster = "someCluster+ &.-_someSign";
+
+    RemoteConfigRepository remoteConfigRepository = new RemoteConfigRepository(someNamespace);
+
+    String longPollRefreshUrl =
+        remoteConfigRepository
+            .assembleLongPollRefreshUrl(someUri, someAppId, someCluster, someNamespace, null, null);
+
+    assertTrue(longPollRefreshUrl.contains("http://someServer/notifications?"));
+    assertTrue(longPollRefreshUrl.contains("appId=someAppId"));
+    assertTrue(longPollRefreshUrl.contains("cluster=someCluster%2B+%26.-_someSign"));
+    assertTrue(longPollRefreshUrl.contains("namespace=" + someNamespace));
+  }
+
+  @Test
+  public void testAssembleQueryConfigUrl() throws Exception {
+    String someUri = "http://someServer";
+    String someAppId = "someAppId";
+    String someCluster = "someCluster+ &.-_someSign";
+    String someReleaseKey = "20160705193346-583078ef5716c055+20160705193308-31c471ddf9087c3f";
+
+    RemoteConfigRepository remoteConfigRepository = new RemoteConfigRepository(someNamespace);
+    ApolloConfig someApolloConfig = mock(ApolloConfig.class);
+    when(someApolloConfig.getReleaseKey()).thenReturn(someReleaseKey);
+
+    String queryConfigUrl = remoteConfigRepository
+            .assembleQueryConfigUrl(someUri, someAppId, someCluster, someNamespace, null,
+                someApolloConfig);
+
+    assertTrue(queryConfigUrl
+        .contains("http://someServer/configs/someAppId/someCluster+%20&.-_someSign/" + someNamespace));
+    assertTrue(queryConfigUrl
+        .contains("releaseKey=20160705193346-583078ef5716c055%2B20160705193308-31c471ddf9087c3f"));
+
+  }
+
   private ApolloConfig assembleApolloConfig(Map<String, String> configurations) {
     String someAppId = "appId";
     String someClusterName = "cluster";

apollo-common/pom.xml

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

apollo-configservice/pom.xml

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

apollo-core/pom.xml

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

apollo-demo/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<artifactId>apollo</artifactId>
 		<groupId>com.ctrip.framework.apollo</groupId>
-		<version>0.0.3</version>
+		<version>0.0.4</version>
 	</parent>
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>apollo-demo</artifactId>

apollo-portal/pom.xml

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

pom.xml

@@ -4,7 +4,7 @@
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>com.ctrip.framework.apollo</groupId>
 	<artifactId>apollo</artifactId>
-	<version>0.0.3</version>
+	<version>0.0.4</version>
 	<name>Apollo</name>
 	<packaging>pom</packaging>
 	<description>Ctrip Configuration Center</description>