Hi,
I noticed that buildPageLink() uses a HashMap to keep track of query
string parameters. While this is perfectly okay from a web application
point of view, it makes testing a little more difficult and in
particular, RequestContextTest could break because it implicitly relies
on key ordering.
In the attached patch I propose using a TreeMap instead, which produces
a reliable ordering.
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
>From 984f3b48a370276bec7d3eedf0f114ed6ce07465 Mon Sep 17 00:00:00 2001
From: Silvio Moioli <smoi...@suse.de>
Date: Fri, 6 Sep 2013 14:23:58 +0200
Subject: [PATCH 01/22] RequestContext.buildPageLink: force parameter ordering
to ease testing
---
java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java | 8 +++++---
.../com/redhat/rhn/frontend/struts/test/RequestContextTest.java | 5 +----
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java b/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
index 52fc845..20dcd54 100644
--- a/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
+++ b/java/code/src/com/redhat/rhn/frontend/struts/RequestContext.java
@@ -47,6 +47,8 @@ import org.apache.log4j.Logger;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
import javax.servlet.http.HttpServletRequest;
@@ -589,7 +591,7 @@ public class RequestContext {
// if we already have this param in the query string we have to
// reset it to the new value
if (index >= 0) {
- Map parammap = new HashMap();
+ SortedMap<String, String> parammap = new TreeMap<String, String>();
String[] params = StringUtils.split(request.getQueryString(),
'&');
// Convert the parameters into a map so we can
@@ -601,9 +603,9 @@ public class RequestContext {
parammap.remove(name);
parammap.put(name, value);
page.append("?");
- Iterator i = parammap.keySet().iterator();
+ Iterator<String> i = parammap.keySet().iterator();
while (i.hasNext()) {
- String key = (String)i.next();
+ String key = i.next();
page.append(key + "=" + parammap.get(key));
if (i.hasNext()) {
page.append("&");
diff --git a/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java b/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
index 1394204..b34e2c1 100644
--- a/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
+++ b/java/code/src/com/redhat/rhn/frontend/struts/test/RequestContextTest.java
@@ -132,11 +132,8 @@ public class RequestContextTest extends MockObjectTestCase {
request.setupQueryString("otherparam=foo&barparam=beer&someparam=value");
url = requestContext.buildPageLink("someparam", "zzzzz");
- // we really can't guarantee the order of a map, so let's hope this
- // works long term. Thankfully in most cases we don't care about
- // the parameters order.
assertEquals("http://localhost/rhn/somePage.do?"; +
- "otherparam=foo&barparam=beer&someparam=zzzzz", url);
+ "barparam=beer&otherparam=foo&someparam=zzzzz", url);
}
}
--
1.8.1.4
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
