Use a XML pull parser instead of pattern-matching for gcvote

Benchmark results on Nexus 5:
- XML GCVote parsing (current) in ms (1000 times): 2678
- Regex GCVote parsing (old) in ms (1000 times): 3986

Benchmark results on Nexus 7 (2013):
- XML GCVote parsing (current) in ms (1000 times): 3448
- Regex GCVote parsing (old) in ms (1000 times): 6075

3 files changed, 253 insertions, 126 deletions

diff --git a/main/src/cgeo/geocaching/gcvote/GCVote.java b/main/src/cgeo/geocaching/gcvote/GCVote.java
index d42bb34..ad460e5 100644
--- a/main/src/cgeo/geocaching/gcvote/GCVote.java
+++ b/main/src/cgeo/geocaching/gcvote/GCVote.java
@@ -8,30 +8,25 @@ import cgeo.geocaching.network.Parameters;
 import cgeo.geocaching.settings.Settings;
 import cgeo.geocaching.utils.LeastRecentlyUsedMap;
 import cgeo.geocaching.utils.Log;
-import cgeo.geocaching.utils.MatcherWrapper;
 
+import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.eclipse.jdt.annotation.NonNull;
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlPullParserFactory;
 
+import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Pattern;
 
 public final class GCVote {
     public static final float NO_RATING = 0;
-    private static final Pattern PATTERN_LOG_IN = Pattern.compile("loggedIn='([^']+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_GUID = Pattern.compile("cacheId='([^']+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_WAYPOINT = Pattern.compile("waypoint='([^']+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_RATING = Pattern.compile("voteAvg='([0-9.]+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_VOTES = Pattern.compile("voteCnt='([0-9]+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_VOTE = Pattern.compile("voteUser='([0-9.]+)'", Pattern.CASE_INSENSITIVE);
-    private static final Pattern PATTERN_VOTE_ELEMENT = Pattern.compile("<vote ([^>]+)>", Pattern.CASE_INSENSITIVE);
 
     private static final int MAX_CACHED_RATINGS = 1000;
     private static final LeastRecentlyUsedMap<String, GCVoteRating> RATINGS_CACHE = new LeastRecentlyUsedMap.LruCache<>(MAX_CACHED_RATINGS);
@@ -70,124 +65,64 @@ public final class GCVote {
      * @param geocodes
      * @return
      */
+    @NonNull
     private static Map<String, GCVoteRating> getRating(final List<String> guids, final List<String> geocodes) {
         if (guids == null && geocodes == null) {
-            return null;
+            return MapUtils.EMPTY_SORTED_MAP;
         }
 
-        final Map<String, GCVoteRating> ratings = new HashMap<>();
-
-        try {
-            final Parameters params = new Parameters();
-            if (Settings.isLogin()) {
-                final ImmutablePair<String, String> login = Settings.getGCvoteLogin();
-                if (login != null) {
-                    params.put("userName", login.left, "password", login.right);
-                }
-            }
-            // use guid or gccode for lookup
-            boolean requestByGuids = true;
-            if (guids != null && !guids.isEmpty()) {
-                params.put("cacheIds", StringUtils.join(guids.toArray(), ','));
-            } else {
-                params.put("waypoints", StringUtils.join(geocodes.toArray(), ','));
-                requestByGuids = false;
-            }
-            params.put("version", "cgeo");
-            final String page = Network.getResponseData(Network.getRequest("http://gcvote.com/getVotes.php", params));
-            if (page == null) {
-                return null;
-            }
-
-            final MatcherWrapper matcherVoteElement = new MatcherWrapper(PATTERN_VOTE_ELEMENT, page);
-            while (matcherVoteElement.find()) {
-                String voteData = matcherVoteElement.group(1);
-                if (voteData == null) {
-                    continue;
-                }
-
-                String id = null;
-                String guid = null;
-                final MatcherWrapper matcherGuid = new MatcherWrapper(PATTERN_GUID, voteData);
-                if (matcherGuid.find()) {
-                    if (matcherGuid.groupCount() > 0) {
-                        guid = matcherGuid.group(1);
-                        if (requestByGuids) {
-                            id = guid;
-                        }
-                    }
-                }
-                if (!requestByGuids) {
-                    final MatcherWrapper matcherWp = new MatcherWrapper(PATTERN_WAYPOINT, voteData);
-                    if (matcherWp.find()) {
-                        if (matcherWp.groupCount() > 0) {
-                            id = matcherWp.group(1);
-                        }
-                    }
-                }
-                if (id == null) {
-                    continue;
-                }
-
-                boolean loggedIn = false;
-                final MatcherWrapper matcherLoggedIn = new MatcherWrapper(PATTERN_LOG_IN, page);
-                if (matcherLoggedIn.find()) {
-                    if (matcherLoggedIn.groupCount() > 0) {
-                        if (matcherLoggedIn.group(1).equalsIgnoreCase("true")) {
-                            loggedIn = true;
-                        }
-                    }
-                }
-
-                float rating = NO_RATING;
-                try {
-                    final MatcherWrapper matcherRating = new MatcherWrapper(PATTERN_RATING, voteData);
-                    if (matcherRating.find()) {
-                        rating = Float.parseFloat(matcherRating.group(1));
-                    }
-                } catch (NumberFormatException e) {
-                    Log.w("GCVote.getRating: Failed to parse rating");
-                }
-                if (!isValidRating(rating)) {
-                    continue;
-                }
+        final Parameters params = new Parameters("version", "cgeo");
+        final ImmutablePair<String, String> login = Settings.getGCvoteLogin();
+        if (login != null) {
+            params.put("userName", login.left, "password", login.right);
+        }
 
-                int votes = -1;
-                try {
-                    final MatcherWrapper matcherVotes = new MatcherWrapper(PATTERN_VOTES, voteData);
-                    if (matcherVotes.find()) {
-                        votes = Integer.parseInt(matcherVotes.group(1));
-                    }
-                } catch (NumberFormatException e) {
-                    Log.w("GCVote.getRating: Failed to parse vote count");
-                }
-                if (votes < 0) {
-                    continue;
-                }
+        // use guid or gccode for lookup
+        final boolean requestByGuids = CollectionUtils.isNotEmpty(guids);
+        if (requestByGuids) {
+            params.put("cacheIds", StringUtils.join(guids, ','));
+        } else {
+            params.put("waypoints", StringUtils.join(geocodes, ','));
+        }
+        final String page = Network.getResponseData(Network.getRequest("http://gcvote.com/getVotes.php", params));
+        if (page == null) {
+            return MapUtils.EMPTY_SORTED_MAP;
+        }
+        return getRatingsFromXMLResponse(page, requestByGuids);
+    }
 
-                float myVote = NO_RATING;
-                if (loggedIn) {
-                    try {
-                        final MatcherWrapper matcherVote = new MatcherWrapper(PATTERN_VOTE, voteData);
-                        if (matcherVote.find()) {
-                            myVote = Float.parseFloat(matcherVote.group(1));
-                        }
-                    } catch (NumberFormatException e) {
-                        Log.w("GCVote.getRating: Failed to parse user's vote");
+    static Map<String, GCVoteRating> getRatingsFromXMLResponse(@NonNull final String page, final boolean requestByGuids) {
+        try {
+            final XmlPullParserFactory factory = XmlPullParserFactory.newInstance();
+            final XmlPullParser xpp = factory.newPullParser();
+            xpp.setInput(new StringReader(page));
+            boolean loggedIn = false;
+            final Map<String, GCVoteRating> ratings = new HashMap<>();
+            int eventType = xpp.getEventType();
+            while (eventType != XmlPullParser.END_DOCUMENT) {
+                if (eventType == XmlPullParser.START_TAG) {
+                    final String tagName = xpp.getName();
+                    if (StringUtils.equals(tagName, "vote")) {
+                        final String guid = xpp.getAttributeValue(null, "cacheId");
+                        final String id = requestByGuids ? guid : xpp.getAttributeValue(null, "waypoint");
+                        final float myVote = loggedIn ? Float.parseFloat(xpp.getAttributeValue(null, "voteUser")) : 0;
+                        final GCVoteRating voteRating = new GCVoteRating(Float.parseFloat(xpp.getAttributeValue(null, "voteAvg")),
+                                Integer.parseInt(xpp.getAttributeValue(null, "voteCnt")),
+                                myVote);
+                        ratings.put(id, voteRating);
+                    } else if (StringUtils.equals(tagName, "votes")) {
+                        loggedIn = StringUtils.equals(xpp.getAttributeValue(null, "loggedIn"), "true");
                     }
                 }
-
-                if (StringUtils.isNotBlank(id)) {
-                    GCVoteRating gcvoteRating = new GCVoteRating(rating, votes, myVote);
-                    ratings.put(id, gcvoteRating);
-                    RATINGS_CACHE.put(guid, gcvoteRating);
-                }
+                eventType = xpp.next();
             }
-        } catch (RuntimeException e) {
-            Log.e("GCVote.getRating", e);
-        }
+            RATINGS_CACHE.putAll(ratings);
+            return ratings;
+        } catch (final Exception e) {
+            Log.e("Cannot parse GC vote result", e);
+            return MapUtils.EMPTY_SORTED_MAP;
 
-        return ratings;
+        }
     }
 
     /**
@@ -235,16 +170,14 @@ public final class GCVote {
         try {
             final Map<String, GCVoteRating> ratings = GCVote.getRating(null, geocodes);
 
-            if (MapUtils.isNotEmpty(ratings)) {
-                // save found cache coordinates
-                for (Geocache cache : caches) {
-                    if (ratings.containsKey(cache.getGeocode())) {
-                        GCVoteRating rating = ratings.get(cache.getGeocode());
+            // save found cache coordinates
+            for (Geocache cache : caches) {
+                if (ratings.containsKey(cache.getGeocode())) {
+                    GCVoteRating rating = ratings.get(cache.getGeocode());
 
-                        cache.setRating(rating.getRating());
-                        cache.setVotes(rating.getVotes());
-                        cache.setMyVote(rating.getMyVote());
-                    }
+                    cache.setRating(rating.getRating());
+                    cache.setVotes(rating.getVotes());
+                    cache.setMyVote(rating.getMyVote());
                 }
             }
         } catch (Exception e) {
diff --git a/tests/res/raw/gcvote.xml b/tests/res/raw/gcvote.xml
new file mode 100644
index 0000000..c58100a
--- /dev/null
+++ b/tests/res/raw/gcvote.xml
@@ -0,0 +1,13 @@
+<votes userName='' currentVersion='3.2' securityState='locked' loggedIn='false'>
+<vote userName='' cacheId='120f6a89-94db-4897-9813-4d79d59ffb09' voteMedian='2.5' voteAvg='2.9' voteCnt='5' voteUser='0' waypoint='GC3MVFN' vote1='0' vote2='3' vote3='1' vote4='1' vote5='0' rawVotes='(2.5:3)(3.0:1)(4.0:1)'/>
+<vote userName='' cacheId='3683c154-4a2a-45cd-a20e-ae503ee8975c' voteMedian='2.5' voteAvg='2.75' voteCnt='6' voteUser='0' waypoint='GC3JCQG' vote1='0' vote2='4' vote3='1' vote4='1' vote5='0' rawVotes='(2.0:1)(2.5:3)(3.0:1)(4.0:1)'/>
+<vote userName='' cacheId='411fa2e5-7f03-458e-a2e0-f3eda8fd7140' voteMedian='4' voteAvg='4' voteCnt='1' voteUser='0' waypoint='GC2X3X2' vote1='0' vote2='0' vote3='0' vote4='1' vote5='0' rawVotes='(4.0:1)'/>
+<vote userName='' cacheId='44f9a8c1-97ac-451a-9269-4b7dc1322db5' voteMedian='3' voteAvg='3.3333333333333' voteCnt='3' voteUser='0' waypoint='GC30X77' vote1='0' vote2='0' vote3='2' vote4='1' vote5='0' rawVotes='(3.0:2)(4.0:1)'/>
+<vote userName='' cacheId='5520c33b-3941-45ca-9056-ea655dbaadf7' voteMedian='3.75' voteAvg='3.75' voteCnt='2' voteUser='0' waypoint='GC1WEVZ' vote1='0' vote2='0' vote3='1' vote4='1' vote5='0' rawVotes='(3.5:1)(4.0:1)'/>
+<vote userName='' cacheId='5be5cbe3-9094-4a48-b534-dbc52b337315' voteMedian='3.5' voteAvg='3.5' voteCnt='2' voteUser='0' waypoint='GC3JCY7' vote1='0' vote2='0' vote3='1' vote4='1' vote5='0' rawVotes='(3.0:1)(4.0:1)'/>
+<vote userName='' cacheId='735334dd-e284-4a7b-bc78-f3534f52a46d' voteMedian='2.5' voteAvg='2.6666666666667' voteCnt='3' voteUser='0' waypoint='GC3QD6N' vote1='0' vote2='2' vote3='1' vote4='0' vote5='0' rawVotes='(2.5:2)(3.0:1)'/>
+<vote userName='' cacheId='92a611f9-9ff3-41ac-ae4e-d2927c86e836' voteMedian='3.25' voteAvg='3.5625' voteCnt='8' voteUser='0' waypoint='GCKF13' vote1='0' vote2='0' vote3='5' vote4='2' vote5='1' rawVotes='(3.0:4)(3.5:1)(4.0:2)(5.0:1)'/>
+<vote userName='' cacheId='a02894bb-4a08-4c09-a73c-25939894ba15' voteMedian='3' voteAvg='3.1666666666667' voteCnt='9' voteUser='0' waypoint='GC2R3X3' vote1='0' vote2='2' vote3='5' vote4='2' vote5='0' rawVotes='(2.0:1)(2.5:1)(3.0:3)(3.5:2)(4.0:2)'/>
+<vote userName='' cacheId='a1711706-09c4-44de-bda8-36df7e53d57c' voteMedian='3.5' voteAvg='3.5' voteCnt='4' voteUser='0' waypoint='GC3MVN6' vote1='0' vote2='0' vote3='2' vote4='2' vote5='0' rawVotes='(3.0:2)(4.0:2)'/>
+<errorstring></errorstring>
+</votes>
diff --git a/tests/src/cgeo/geocaching/gcvote/GCVoteTest.java b/tests/src/cgeo/geocaching/gcvote/GCVoteTest.java
new file mode 100644
index 0000000..209810c
--- /dev/null
+++ b/tests/src/cgeo/geocaching/gcvote/GCVoteTest.java
@@ -0,0 +1,181 @@
+package cgeo.geocaching.gcvote;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import cgeo.geocaching.test.AbstractResourceInstrumentationTestCase;
+import cgeo.geocaching.test.R;
+import cgeo.geocaching.utils.LeastRecentlyUsedMap;
+import cgeo.geocaching.utils.Log;
+import cgeo.geocaching.utils.MatcherWrapper;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.regex.Pattern;
+
+public class GCVoteTest extends AbstractResourceInstrumentationTestCase {
+
+    private String response;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        response = getFileContent(R.raw.gcvote);
+    }
+
+    public void testGetRatingsByGeocode() {
+        final Map<String, GCVoteRating> ratings = GCVote.getRatingsFromXMLResponse(response, false);
+        assertThat(ratings).hasSize(10);
+        assertThat(ratings).containsKey("GCKF13");
+        assertThat(ratings.get("GC1WEVZ")).isEqualToComparingFieldByField(new GCVoteRating(3.75f, 2, 0));
+    }
+
+    public void testGetRatingsByGuid() {
+        final Map<String, GCVoteRating> ratings = GCVote.getRatingsFromXMLResponse(response, true);
+        assertThat(ratings).hasSize(10);
+        assertThat(ratings).containsKey("a02894bb-4a08-4c09-a73c-25939894ba15");
+        assertThat(ratings.get("5520c33b-3941-45ca-9056-ea655dbaadf7")).isEqualToComparingFieldByField(new GCVoteRating(3.75f, 2, 0));
+    }
+
+    public void testBenchmark() {
+        benchmarkXML(1000);
+        benchmarkRegex(1000);
+    }
+
+    public void testCompareResults() {
+        for (int i = 0; i < 2; i++) {
+            final boolean requestByGuids = i == 0;
+            final Map<String, GCVoteRating> xmlRatings = GCVote.getRatingsFromXMLResponse(response, requestByGuids);
+            final Map<String, GCVoteRating> regexRatings = getRatingsRegex(response, requestByGuids);
+            assertThat(xmlRatings.keySet()).containsExactlyElementsOf(regexRatings.keySet());
+            for (final Entry<String, GCVoteRating> entry: xmlRatings.entrySet()) {
+                assertThat(entry.getValue()).isEqualToComparingFieldByField(regexRatings.get(entry.getKey()));
+            }
+        }
+    }
+
+    private void benchmarkXML(final int occurrences) {
+        final long start = System.currentTimeMillis();
+        for (int i = 0; i < occurrences; i++) {
+            GCVote.getRatingsFromXMLResponse(response, false);
+        }
+        Log.d("XML GCVote parsing (current) in ms (" + occurrences + " times): " + (System.currentTimeMillis() - start));
+    }
+
+    private void benchmarkRegex(final int occurrences) {
+        final long start = System.currentTimeMillis();
+        for (int i = 0; i < occurrences; i++) {
+            getRatingsRegex(response, false);
+        }
+        Log.d("Regex GCVote parsing (old) in ms (" + occurrences + " times): " + (System.currentTimeMillis() - start));
+    }
+
+    public static final float NO_RATING = 0;
+    private static final Pattern PATTERN_LOG_IN = Pattern.compile("loggedIn='([^']+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_GUID = Pattern.compile("cacheId='([^']+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_WAYPOINT = Pattern.compile("waypoint='([^']+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_RATING = Pattern.compile("voteAvg='([0-9.]+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_VOTES = Pattern.compile("voteCnt='([0-9]+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_VOTE = Pattern.compile("voteUser='([0-9.]+)'", Pattern.CASE_INSENSITIVE);
+    private static final Pattern PATTERN_VOTE_ELEMENT = Pattern.compile("<vote ([^>]+)>", Pattern.CASE_INSENSITIVE);
+
+    private static final int MAX_CACHED_RATINGS = 1000;
+    private static final LeastRecentlyUsedMap<String, GCVoteRating> RATINGS_CACHE = new LeastRecentlyUsedMap.LruCache<>(MAX_CACHED_RATINGS);
+
+    private static Map<String, GCVoteRating> getRatingsRegex(final String page, final boolean requestByGuids) {
+        final Map<String, GCVoteRating> ratings = new HashMap<>();
+
+        try {
+            final MatcherWrapper matcherVoteElement = new MatcherWrapper(PATTERN_VOTE_ELEMENT, page);
+            while (matcherVoteElement.find()) {
+                String voteData = matcherVoteElement.group(1);
+                if (voteData == null) {
+                    continue;
+                }
+
+                String id = null;
+                String guid = null;
+                final MatcherWrapper matcherGuid = new MatcherWrapper(PATTERN_GUID, voteData);
+                if (matcherGuid.find()) {
+                    if (matcherGuid.groupCount() > 0) {
+                        guid = matcherGuid.group(1);
+                        if (requestByGuids) {
+                            id = guid;
+                        }
+                    }
+                }
+                if (!requestByGuids) {
+                    final MatcherWrapper matcherWp = new MatcherWrapper(PATTERN_WAYPOINT, voteData);
+                    if (matcherWp.find()) {
+                        if (matcherWp.groupCount() > 0) {
+                            id = matcherWp.group(1);
+                        }
+                    }
+                }
+                if (id == null) {
+                    continue;
+                }
+
+                boolean loggedIn = false;
+                final MatcherWrapper matcherLoggedIn = new MatcherWrapper(PATTERN_LOG_IN, page);
+                if (matcherLoggedIn.find()) {
+                    if (matcherLoggedIn.groupCount() > 0) {
+                        if (matcherLoggedIn.group(1).equalsIgnoreCase("true")) {
+                            loggedIn = true;
+                        }
+                    }
+                }
+
+                float rating = NO_RATING;
+                try {
+                    final MatcherWrapper matcherRating = new MatcherWrapper(PATTERN_RATING, voteData);
+                    if (matcherRating.find()) {
+                        rating = Float.parseFloat(matcherRating.group(1));
+                    }
+                } catch (NumberFormatException e) {
+                    Log.w("GCVote.getRating: Failed to parse rating", e);
+                }
+                if (!GCVote.isValidRating(rating)) {
+                    continue;
+                }
+
+                int votes = -1;
+                try {
+                    final MatcherWrapper matcherVotes = new MatcherWrapper(PATTERN_VOTES, voteData);
+                    if (matcherVotes.find()) {
+                        votes = Integer.parseInt(matcherVotes.group(1));
+                    }
+                } catch (NumberFormatException e) {
+                    Log.w("GCVote.getRating: Failed to parse vote count", e);
+                }
+                if (votes < 0) {
+                    continue;
+                }
+
+                float myVote = NO_RATING;
+                if (loggedIn) {
+                    try {
+                        final MatcherWrapper matcherVote = new MatcherWrapper(PATTERN_VOTE, voteData);
+                        if (matcherVote.find()) {
+                            myVote = Float.parseFloat(matcherVote.group(1));
+                        }
+                    } catch (NumberFormatException e) {
+                        Log.w("GCVote.getRating: Failed to parse user's vote", e);
+                    }
+                }
+
+                if (StringUtils.isNotBlank(id)) {
+                    GCVoteRating gcvoteRating = new GCVoteRating(rating, votes, myVote);
+                    ratings.put(id, gcvoteRating);
+                    RATINGS_CACHE.put(guid, gcvoteRating);
+                }
+            }
+        } catch (RuntimeException e) {
+            Log.e("GCVote.getRating", e);
+        }
+
+        return ratings;
+    }
+}