Author: tmjee Date: Thu Aug 31 07:34:32 2006 New Revision: 438940 URL: http://svn.apache.org/viewvc?rev=438940&view=rev Log: WW-1426 - URL Tag cause duplicates query string
Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java struts/struts2/trunk/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java?rev=438940&r1=438939&r2=438940&view=diff ============================================================================== --- struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java (original) +++ struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java Thu Aug 31 07:34:32 2006 @@ -33,7 +33,9 @@ import javax.servlet.http.HttpUtils; import java.io.IOException; import java.io.Writer; +import java.util.Collections; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.Map; /** @@ -157,9 +159,10 @@ } if (NONE.equalsIgnoreCase(includeParams)) { + mergeRequestParameters(value, parameters, Collections.EMPTY_MAP); ActionContext.getContext().put(XWorkContinuationConfig.CONTINUE_KEY, null); } else if (ALL.equalsIgnoreCase(includeParams)) { - mergeRequestParameters(parameters, req.getParameterMap()); + mergeRequestParameters(value, parameters, req.getParameterMap()); // for ALL also include GET parameters includeGetParameters(); @@ -179,10 +182,7 @@ private void includeGetParameters() { if(!(Dispatcher.getInstance().isPortletSupportActive() && PortletActionContext.isPortletRequest())) { String query = extractQueryString(); - if (query != null) { - //mergeRequestParameters(parameters, HttpUtils.parseQueryString(query)); - mergeRequestParameters(parameters, UrlHelper.parseQueryString(query)); - } + mergeRequestParameters(value, parameters, UrlHelper.parseQueryString(query)); } } @@ -221,7 +221,14 @@ result = PortletUrlHelper.buildResourceUrl(value, parameters); } else { - result = UrlHelper.buildUrl(value, req, res, parameters, scheme, includeContext, encode); + String _value = value; + + // We don't include the request parameters cause they would have been + // prioritised before this [in start(Writer) method] + if (_value != null && _value.indexOf("?") > 0) { + _value = _value.substring(0, _value.indexOf("?")); + } + result = UrlHelper.buildUrl(_value, req, res, parameters, scheme, includeContext, encode); } } if ( anchor != null && anchor.length() > 0 ) { @@ -345,13 +352,52 @@ /** * Merge request parameters into current parameters. If a parameter is - * already present, than the request parameter will not override its value. + * already present, than the request parameter in the current request and value atrribute + * will not override its value. + * + * The priority is as follows:- + * <ul> + * <li>parameter from the current request (least priority)</li> + * <li>parameter form the value attribute (more priority)</li> + * <li>parameter from the param tag (most priority)</li> + * </ul> * + * @param value the value attribute (url to be generated by this component) * @param parameters component parameters * @param contextParameters request parameters */ - protected void mergeRequestParameters(Map parameters, Map contextParameters){ - for (Iterator iterator = contextParameters.entrySet().iterator(); iterator.hasNext();) { + protected void mergeRequestParameters(String value, Map parameters, Map contextParameters){ + + Map mergedParams = new LinkedHashMap(contextParameters); + + // Merge contextParameters (from current request) with parameters specified in value attribute + // eg. value="someAction.action?id=someId&venue=someVenue" + // where the parameters specified in value attribute takes priority. + + if (value != null && value.trim().length() > 0 && value.indexOf("?") > 0) { + mergedParams = new LinkedHashMap(); + + String queryString = value.substring(value.indexOf("?")+1); + + mergedParams = UrlHelper.parseQueryString(queryString); + for (Iterator iterator = contextParameters.entrySet().iterator(); iterator.hasNext();) { + Map.Entry entry = (Map.Entry) iterator.next(); + Object key = entry.getKey(); + + if (!mergedParams.containsKey(key)) { + mergedParams.put(key, entry.getValue()); + } + } + } + + + // Merge parameters specified in value attribute + // eg. value="someAction.action?id=someId&venue=someVenue" + // with parameters specified though param tag + // eg. <param name="id" value="%{'someId'}" /> + // where parameters specified through param tag takes priority. + + for (Iterator iterator = mergedParams.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = (Map.Entry) iterator.next(); Object key = entry.getKey(); Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java?rev=438940&r1=438939&r2=438940&view=diff ============================================================================== --- struts/struts2/trunk/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java (original) +++ struts/struts2/trunk/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java Thu Aug 31 07:34:32 2006 @@ -267,18 +267,20 @@ if (queryString != null) { String[] params = queryString.split("&"); for (int a=0; a< params.length; a++) { - String[] tmpParams = params[a].split("="); - String paramName = null; - String paramValue = ""; - if (tmpParams.length > 0) { - paramName = tmpParams[0]; - } - if (tmpParams.length > 1) { - paramValue = tmpParams[1]; - } - if (paramName != null) { - String translatedParamValue = translateAndDecode(paramValue); - queryParams.put(paramName, translatedParamValue); + if (params[a].trim().length() > 0) { + String[] tmpParams = params[a].split("="); + String paramName = null; + String paramValue = ""; + if (tmpParams.length > 0) { + paramName = tmpParams[0]; + } + if (tmpParams.length > 1) { + paramValue = tmpParams[1]; + } + if (paramName != null) { + String translatedParamValue = translateAndDecode(paramValue); + queryParams.put(paramName, translatedParamValue); + } } } } Modified: struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java?rev=438940&r1=438939&r2=438940&view=diff ============================================================================== --- struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java (original) +++ struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java Thu Aug 31 07:34:32 2006 @@ -18,6 +18,7 @@ package org.apache.struts2.views.jsp; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import javax.servlet.jsp.JspWriter; @@ -35,6 +36,151 @@ public class URLTagTest extends AbstractUITagTest { private URLTag tag; + + + /** + * To test priority of parameter passed in to url component though + * various way + * - current request url + * - tag's value attribute + * - tag's nested param tag + * + * id1 + * === + * - found in current request url + * - found in tag's value attribute + * - found in tag's param tag + * CONCLUSION: tag's param tag takes precedence (paramId1) + * + * id2 + * === + * - found in current request url + * - found in tag's value attribute + * CONCLUSION: tag's value attribute take precedence (tagId2) + * + * urlParam1 + * ========= + * - found in current request url + * CONCLUSION: param in current request url will be used (urlValue1) + * + * urlParam2 + * ========= + * - found in current request url + * CONCLUSION: param in current request url will be used. (urlValue2) + * + * tagId + * ===== + * - found in tag's value attribute + * CONCLUSION: param in tag's value attribute wil; be used. (tagValue) + * + * param1 + * ====== + * - found in nested param tag + * CONCLUSION: param in nested param tag will be used. (param1value) + * + * param2 + * ====== + * - found in nested param tag + * CONCLUSION: param in nested param tag will be used. (param2value) + */ + public void testParametersPriority() throws Exception { + request.setQueryString("id1=urlId1&id2=urlId2&urlParam1=urlValue1&urlParam2=urlValue2"); + + tag.setValue("testAction.action?id1=tagId1&id2=tagId2&tagId=tagValue"); + + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("param1"); + param1.setValue("%{'param1value'}"); + + ParamTag param2 = new ParamTag(); + param2.setPageContext(pageContext); + param2.setName("param2"); + param2.setValue("%{'param2value'}"); + + ParamTag param3 = new ParamTag(); + param3.setPageContext(pageContext); + param3.setName("id1"); + param3.setValue("%{'paramId1'}"); + + + tag.doStartTag(); + param1.doStartTag(); + param1.doEndTag(); + param2.doStartTag(); + param2.doEndTag(); + param3.doStartTag(); + param3.doEndTag(); + + URL url = (URL) tag.getComponent(); + Map parameters = url.getParameters(); + + + assertNotNull(parameters); + assertEquals(parameters.size(), 7); + assertEquals(parameters.get("id1"), "paramId1"); + assertEquals(parameters.get("id2"), "tagId2"); + assertEquals(parameters.get("urlParam1"), "urlValue1"); + assertEquals(parameters.get("urlParam2"), "urlValue2"); + assertEquals(parameters.get("tagId"), "tagValue"); + assertEquals(parameters.get("param1"), "param1value"); + assertEquals(parameters.get("param2"), "param2value"); + } + + + /** + * To test priority of parameter passed in to url component though + * various way, with includeParams="NONE" + * - current request url + * - tag's value attribute + * - tag's nested param tag + * + * In this case only parameters from the tag itself is taken into account. + * Those from request will not count, only those in tag's value attribute + * and nested param tag. + * + * @throws Exception + */ + public void testParametersPriorityWithIncludeParamsAsNONE() throws Exception { + request.setQueryString("id1=urlId1&id2=urlId2&urlParam1=urlValue1&urlParam2=urlValue2"); + + tag.setValue("testAction.action?id1=tagId1&id2=tagId2&tagId=tagValue"); + tag.setIncludeParams("NONE"); + + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("param1"); + param1.setValue("%{'param1value'}"); + + ParamTag param2 = new ParamTag(); + param2.setPageContext(pageContext); + param2.setName("param2"); + param2.setValue("%{'param2value'}"); + + ParamTag param3 = new ParamTag(); + param3.setPageContext(pageContext); + param3.setName("id1"); + param3.setValue("%{'paramId1'}"); + + + tag.doStartTag(); + param1.doStartTag(); + param1.doEndTag(); + param2.doStartTag(); + param2.doEndTag(); + param3.doStartTag(); + param3.doEndTag(); + + URL url = (URL) tag.getComponent(); + Map parameters = url.getParameters(); + + assertEquals(parameters.size(), 5); + assertEquals(parameters.get("id1"), "paramId1"); + assertEquals(parameters.get("id2"), "tagId2"); + assertEquals(parameters.get("tagId"), "tagValue"); + assertEquals(parameters.get("param1"), "param1value"); + assertEquals(parameters.get("param2"), "param2value"); + } public void testIncludeParamsDefaultToGET() throws Exception { request.setQueryString("one=oneVal&two=twoVal&three=threeVal"); Modified: struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java?rev=438940&r1=438939&r2=438940&view=diff ============================================================================== --- struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java (original) +++ struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java Thu Aug 31 07:34:32 2006 @@ -25,6 +25,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; @@ -280,6 +281,21 @@ assertEquals(result.get("bbb"), "bbbval"); assertEquals(result.get("ccc"), ""); } + + public void testParseEmptyQuery() throws Exception { + Map result = UrlHelper.parseQueryString(""); + + assertNotNull(result); + assertEquals(result.size(), 0); + } + + public void testParseNullQuery() throws Exception { + Map result = UrlHelper.parseQueryString(null); + + assertNotNull(result); + assertEquals(result.size(), 0); + } + public void testTranslateAndEncode() throws Exception { String defaultI18nEncoding = Settings.get(StrutsConstants.STRUTS_I18N_ENCODING);