Merge branch 'release/0.8.2'

CHANGELOG

@@ -1,3 +1,8 @@
+Java 0.8.2 (2016-02-28)
+-----------------------
+Fixed GET requests not being properly encoded (#174)
+Upgraded commons-codec version (#172)
+
 Java 0.8.1 (2015-10-01)
 -----------------------
 Timing event field is incorrectly converted to a String (#166)

build.gradle

@@ -30,7 +30,7 @@ apply plugin: 'propdeps-maven'
 apply plugin: 'propdeps-idea'
 
 group = 'com.snowplowanalytics'
-version = '0.8.1'
+version = '0.8.2'
 sourceCompatibility = '1.7'
 targetCompatibility = '1.7'
 
@@ -58,7 +58,7 @@ sourceSets {
 dependencies {
 
     // Apache Commons
-    compile 'commons-codec:commons-codec:1.2'
+    compile 'commons-codec:commons-codec:1.10'
     compile 'commons-net:commons-net:3.3'
 
     // Apache HTTP

src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java

@@ -13,8 +13,9 @@
 package com.snowplowanalytics.snowplow.tracker;
 
 // Java
-import java.net.URL;
 import java.util.*;
+import java.net.URL;
+import java.net.URLEncoder;
 
 // Jackson
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -128,6 +129,49 @@ public static String mapToJSONString(Map map) {
         return jString;
     }
 
+    /**
+     * Builds a QueryString from a Map of Name-Value pairs.
+     * 
+     * @param map The map to convert
+     * @return the QueryString ready for sending
+     */
+    public static String mapToQueryString(Map<String, Object> map) {
+        StringBuilder sb = new StringBuilder();
+        for (String key : map.keySet()) {
+            if (sb.length() > 0) {
+                sb.append("&");
+            }
+
+            String encodedKey = urlEncodeUTF8(key);
+            String encodedVal = urlEncodeUTF8(map.get(key));
+
+            // Do not add empty Keys
+            if (encodedKey != null && !encodedKey.isEmpty()) {
+                sb.append(String.format("%s=%s", encodedKey, encodedVal));
+            }
+        }
+        return sb.toString();
+    }
+
+    /**
+     * Encodes an Object in UTF-8.  
+     * Will attempt to cast the object to a String and then encode.
+     *
+     * @param o The object to encode
+     * @return either the encoded String or an empty
+     *         String if it fails.
+     */
+    public static String urlEncodeUTF8(Object o) {
+        try {
+            String s = (String) o;
+            String encoded = URLEncoder.encode(s, "UTF-8");
+            return encoded.replaceAll("\\+", "%20");
+        } catch (Exception e) {
+            LOGGER.error("Object {} could not be encoded: {}", o, e.getMessage());
+            return "";
+        }
+    }
+
     /**
      * Count the number of bytes a string will occupy when UTF-8 encoded
      *

src/main/java/com/snowplowanalytics/snowplow/tracker/http/AbstractHttpClientAdapter.java

@@ -19,6 +19,7 @@
 import com.google.common.base.Preconditions;
 
 // This library
+import com.snowplowanalytics.snowplow.tracker.constants.Constants;
 import com.snowplowanalytics.snowplow.tracker.Utils;
 import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson;
 import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload;
@@ -82,8 +83,9 @@ public String getUrl() {
      */
     @Override
     public int post(SelfDescribingJson payload) {
+        String url = this.url + "/" + Constants.PROTOCOL_VENDOR + "/" + Constants.PROTOCOL_VERSION;
         String body = payload.toString();
-        return doPost(body);
+        return doPost(url, body);
     }
 
     /**
@@ -94,7 +96,8 @@ public int post(SelfDescribingJson payload) {
     @Override
     @SuppressWarnings("unchecked")
     public int get(TrackerPayload payload) {
-        return doGet(payload.getMap());
+        String url = this.url + "/i?" + Utils.mapToQueryString(payload.getMap());
+        return doGet(url);
     }
 
     /**
@@ -109,17 +112,18 @@ public int get(TrackerPayload payload) {
      * Sends the SelfDescribingJson string containing
      * the events as a POST request to the endpoint.
      *
+     * @param url the URL to send to
      * @param payload the event payload String
      * @return the result of the send
      */
-    protected abstract int doPost(String payload);
+    protected abstract int doPost(String url, String payload);
 
     /**
      * Sends the Map of key-value pairs for the event
      * as a GET request to the endpoint.
      *
-     * @param payload the event payload Map
+     * @param url the URL to send
      * @return the result of the send
      */
-    protected abstract int doGet(Map<String, Object> payload);
+    protected abstract int doGet(String url);
 }

src/main/java/com/snowplowanalytics/snowplow/tracker/http/ApacheHttpClientAdapter.java

@@ -94,17 +94,12 @@ public Object getHttpClient() {
      * Attempts to send a group of payloads with a
      * GET request to the configured endpoint.
      *
-     * @param payload the payload map to send
+     * @param url the URL send
      * @return the HttpResponse for the Request
      */
-    public int doGet(Map<String, Object> payload) {
+    public int doGet(String url) {
         try {
-            URIBuilder uriBuilder = new URIBuilder(this.url);
-            for (String key : payload.keySet()) {
-                String value = (String) payload.get(key);
-                uriBuilder.setParameter(key, value);
-            }
-            HttpGet httpGet = new HttpGet(uriBuilder.setPath("/i").build());
+            HttpGet httpGet = new HttpGet(url);
             HttpResponse httpResponse = httpClient.execute(httpGet);
             httpGet.releaseConnection();
             return httpResponse.getStatusLine().getStatusCode();
@@ -118,13 +113,13 @@ public int doGet(Map<String, Object> payload) {
      * Attempts to send a group of payloads with a
      * POST request to the configured endpoint.
      *
+     * @param url the URL to send to
      * @param payload the payload to send
      * @return the HttpResponse for the Request
      */
-    public int doPost(String payload) {
+    public int doPost(String url, String payload) {
         try {
-            URIBuilder uriBuilder = new URIBuilder(this.url);
-            HttpPost httpPost = new HttpPost(uriBuilder.setPath("/" + Constants.PROTOCOL_VENDOR + "/" + Constants.PROTOCOL_VERSION).build());
+            HttpPost httpPost = new HttpPost(url);
             httpPost.addHeader("Content-Type", Constants.POST_CONTENT_TYPE);
             StringEntity params = new StringEntity(payload);
             httpPost.setEntity(params);

src/main/java/com/snowplowanalytics/snowplow/tracker/http/OkHttpClientAdapter.java

@@ -95,21 +95,11 @@ public Object getHttpClient() {
      * Attempts to send a group of payloads with a
      * GET request to the configured endpoint.
      *
-     * @param payload the payload map to send
+     * @param url the URL send
      * @return the HttpResponse for the Request
      */
-    public int doGet(Map<String, Object> payload) {
-        StringBuilder urlBuilder = new StringBuilder(this.url).append("/i?");
-
-        Iterator<String> iterator = payload.keySet().iterator();
-        while (iterator.hasNext()) {
-            String key = iterator.next();
-            urlBuilder.append(key).append("=").append(payload.get(key));
-            if (iterator.hasNext()) {
-                urlBuilder.append("&");
-            }
-        }
-        Request request = new Request.Builder().url(urlBuilder.toString()).build();
+    public int doGet(String url) {
+        Request request = new Request.Builder().url(url).build();
 
         try {
             Response response = httpClient.newCall(request).execute();
@@ -124,14 +114,15 @@ public int doGet(Map<String, Object> payload) {
      * Attempts to send a group of payloads with a
      * POST request to the configured endpoint.
      *
+     * @param url the URL to send to
      * @param payload the payload to send
      * @return the HttpResponse for the Request
      */
-    public int doPost(String payload) {
+    public int doPost(String url, String payload) {
         try {
             RequestBody body = RequestBody.create(JSON, payload);
             Request request = new Request.Builder()
-                    .url(this.url + "/" + Constants.PROTOCOL_VENDOR + "/" + Constants.PROTOCOL_VERSION)
+                    .url(url)
                     .addHeader("Content-Type", Constants.POST_CONTENT_TYPE)
                     .post(body)
                     .build();

src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java

@@ -390,7 +390,7 @@ public void testTrackTimingWithSubject() {
     @Test
     public void testGetTrackerVersion() throws Exception {
         Tracker tracker = new Tracker.TrackerBuilder(emitter, "namespace", "an-app-id").build();
-        assertEquals("java-0.8.1", tracker.getTrackerVersion());
+        assertEquals("java-0.8.2", tracker.getTrackerVersion());
     }
 
     @Test

src/test/java/com/snowplowanalytics/snowplow/tracker/UtilsTest.java

@@ -18,6 +18,10 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+// Java
+import java.util.LinkedHashMap;
+import java.util.Map;
+
 public class UtilsTest {
 
     @Test
@@ -55,6 +59,24 @@ public void testIsUriValid() {
         assertTrue(!Utils.isValidUrl(badUri2));
     }
 
+    @Test
+    public void testMapToQueryString() {
+        Map<String, Object> payload = new LinkedHashMap<>();
+        payload.put("k1", "v1");
+        payload.put("k2", "s p a c e");
+        payload.put("k3", "s+p+a+c+e");
+
+        assertEquals("k1=v1&k2=s%20p%20a%20c%20e&k3=s%2Bp%2Ba%2Bc%2Be", Utils.mapToQueryString(payload));
+    }
+
+    @Test
+    public void testObjectToUTF8() {
+        assertEquals("", Utils.urlEncodeUTF8(null));
+        assertEquals(
+            "%3C%20%3E%20%23%20%25%20%7B%20%7D%20%7C%20%5C%20%5E%20%7E%20%5B%20%5D%20%60%20%3B%20%2F%20%3F%20%3A%20%40%20%3D%20%26%20%24%20%2B%20%22", 
+            Utils.urlEncodeUTF8("< > # % { } | \\ ^ ~ [ ] ` ; / ? : @ = & $ + \""));
+    }
+
     @Test
     public void testGetTimezone() {
         String tz = Utils.getTimezone();

src/test/java/com/snowplowanalytics/snowplow/tracker/http/HttpClientAdapterTest.java

@@ -98,12 +98,13 @@ public void get_withSuccessfulStatusCode_isOk() throws Exception {
         // When
         TrackerPayload data = new TrackerPayload();
         data.add("foo", "bar");
+        data.add("space", "b a r");
         adapter.get(data);
 
         // Then
         assertEquals(1, mockWebServer.getRequestCount());
         RecordedRequest recordedRequest = mockWebServer.takeRequest();
-        assertEquals("/i?foo=bar", recordedRequest.getPath());
+        assertEquals("/i?foo=bar&space=b%20a%20r", recordedRequest.getPath());
         assertEquals("GET", recordedRequest.getMethod());
     }