diff --git a/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java b/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java index dcf77e6..4dd2864 100644 --- a/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java +++ b/src/main/java/org/scribe/extractors/BaseStringExtractorImpl.java @@ -35,8 +35,7 @@ public class BaseStringExtractorImpl implements BaseStringExtractor params.addAll(request.getQueryStringParams()); params.addAll(request.getBodyParams()); params.addAll(new ParameterList(request.getOauthParameters())); - params.sort(); - return params.asOauthBaseString(); + return params.sort().asOauthBaseString(); } private void checkPreconditions(OAuthRequest request) diff --git a/src/main/java/org/scribe/model/ParameterList.java b/src/main/java/org/scribe/model/ParameterList.java index 3c71991..100f92e 100644 --- a/src/main/java/org/scribe/model/ParameterList.java +++ b/src/main/java/org/scribe/model/ParameterList.java @@ -26,7 +26,12 @@ public class ParameterList { params = new ArrayList(); } - + + ParameterList(List params) + { + this.params = new ArrayList(params); + } + public ParameterList(Map map) { this(); @@ -110,8 +115,10 @@ public class ParameterList return params.size(); } - public void sort() + public ParameterList sort() { - Collections.sort(params); + ParameterList sorted = new ParameterList(params); + Collections.sort(sorted.params); + return sorted; } } diff --git a/src/main/java/org/scribe/utils/MapUtils.java b/src/main/java/org/scribe/utils/MapUtils.java deleted file mode 100644 index 18f507e..0000000 --- a/src/main/java/org/scribe/utils/MapUtils.java +++ /dev/null @@ -1,72 +0,0 @@ -package org.scribe.utils; - -import java.util.*; - -/** - * Utils for {@link Map} manipulation - * - * @author Pablo Fernandez - */ -public class MapUtils -{ - private static final String EMPTY_STRING = ""; - private static final String PAIR_SEPARATOR = "="; - private static final String PARAM_SEPARATOR = "&"; - - /** - * Sorts a Map - * - * @param map unsorted map - * @return sorted map - */ - public static final Map sort(Map map) - { - Preconditions.checkNotNull(map, "Cannot sort a null object."); - - Map sorted = new LinkedHashMap(); - for (String key : getSortedKeys(map)) - { - sorted.put(key, map.get(key)); - } - return sorted; - } - - private static List getSortedKeys(Map map) - { - List keys = new ArrayList(map.keySet()); - Collections.sort(keys); - return keys; - } - - /** - * Form-urlDecodes and appends all keys from the source {@link Map} to the - * target {@link Map} - * - * @param source Map from where the keys get copied and decoded - * @param target Map where the decoded keys are copied to - */ - public static void decodeAndAppendEntries(Map source, Map target) - { - for (String key: source.keySet()) - { - target.put(URLUtils.percentEncode(key), URLUtils.percentEncode(source.get(key))); - } - } - - /** - * Concats a key-value map into a querystring-like String - * - * @param params key-value map - * @return querystring-like String - */ - public static String concatSortedPercentEncodedParams(Map params) - { - StringBuilder result = new StringBuilder(); - for (String key : params.keySet()) - { - result.append(key).append(PAIR_SEPARATOR); - result.append(params.get(key)).append(PARAM_SEPARATOR); - } - return result.toString().substring(0, result.length() - 1); - } -} diff --git a/src/test/java/org/scribe/extractors/BaseStringExtractorTest.java b/src/test/java/org/scribe/extractors/BaseStringExtractorTest.java index 8fe872f..edc101d 100644 --- a/src/test/java/org/scribe/extractors/BaseStringExtractorTest.java +++ b/src/test/java/org/scribe/extractors/BaseStringExtractorTest.java @@ -41,7 +41,7 @@ public class BaseStringExtractorTest OAuthRequest request = new OAuthRequest(Verb.GET, "http://example.com"); extractor.extract(request); } - + @Test public void shouldProperlyEncodeSpaces() { diff --git a/src/test/java/org/scribe/model/ParameterListTest.java b/src/test/java/org/scribe/model/ParameterListTest.java index 46f4007..bc818d0 100644 --- a/src/test/java/org/scribe/model/ParameterListTest.java +++ b/src/test/java/org/scribe/model/ParameterListTest.java @@ -1,12 +1,17 @@ package org.scribe.model; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.scribe.utils.URLUtils; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; + /** * @author: Pablo Fernandez */ @@ -59,4 +64,33 @@ public class ParameterListTest url = params.appendTo(url); Assert.assertEquals(url, expectedUrl); } + + @Test + public void shouldProperlySortParameters() + { + params.add("param1", "v1"); + params.add("param6", "v2"); + params.add("a_param", "v3"); + params.add("param2", "v4"); + Assert.assertEquals("a_param=v3¶m1=v1¶m2=v4¶m6=v2", params.sort().asFormUrlEncodedString()); + } + + @Test + public void shouldProperlySortParametersWithTheSameName() + { + params.add("param1", "v1"); + params.add("param6", "v2"); + params.add("a_param", "v3"); + params.add("param2", "v4"); + Assert.assertEquals("a_param=v3¶m1=v1¶m2=v4¶m6=v2", params.sort().asFormUrlEncodedString()); + } + + @Test + public void shouldNotModifyTheOriginalParameterList() + { + params.add("param1", "v1"); + params.add("param6", "v2"); + + assertNotSame(params, params.sort()); + } } diff --git a/src/test/java/org/scribe/utils/MapUtilsTest.java b/src/test/java/org/scribe/utils/MapUtilsTest.java deleted file mode 100644 index da7f5ca..0000000 --- a/src/test/java/org/scribe/utils/MapUtilsTest.java +++ /dev/null @@ -1,53 +0,0 @@ -package org.scribe.utils; - -import static org.junit.Assert.*; - -import java.util.*; - -import org.junit.*; - -public class MapUtilsTest -{ - - private Map unsorted; - - @Before - public void setup() - { - unsorted = new LinkedHashMap(); - unsorted.put("d", "fourth"); - unsorted.put("a", "first"); - unsorted.put("c", "third"); - unsorted.put("b", "second"); - } - - @Test - public void shouldSortMap() - { - Map sorted = MapUtils.sort(unsorted); - List values = new ArrayList(sorted.values()); - assertEquals("first", values.get(0)); - assertEquals("second", values.get(1)); - assertEquals("third", values.get(2)); - assertEquals("fourth", values.get(3)); - } - - @Test - public void shouldNotModifyTheOriginalMap() - { - Map sorted = MapUtils.sort(unsorted); - assertNotSame(sorted, unsorted); - - List values = new ArrayList(unsorted.values()); - assertEquals("fourth", values.get(0)); - assertEquals("first", values.get(1)); - assertEquals("third", values.get(2)); - assertEquals("second", values.get(3)); - } - - @Test(expected = IllegalArgumentException.class) - public void shouldThrowExceptionForNullMap() - { - MapUtils.sort(null); - } -}