You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ni...@apache.org on 2008/04/25 10:16:40 UTC

svn commit: r651529 - in /struts/struts2/trunk: core/src/main/java/org/apache/struts2/components/ plugins/portlet/src/main/java/org/apache/struts2/components/ plugins/portlet/src/test/java/org/apache/struts2/views/jsp/

Author: nilsga
Date: Fri Apr 25 01:16:36 2008
New Revision: 651529

URL: http://svn.apache.org/viewvc?rev=651529&view=rev
Log:
WW-2504 Ignoring the includeParams for portlet urls. Had to extract some more code into the "UrlRenderer".

Modified:
    struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java
    struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/URL.java
    struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/UrlRenderer.java
    struts/struts2/trunk/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java
    struts/struts2/trunk/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java

Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java
URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java?rev=651529&r1=651528&r2=651529&view=diff
==============================================================================
--- struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java (original)
+++ struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java Fri Apr 25 01:16:36 2008
@@ -22,6 +22,10 @@
 
 import java.io.IOException;
 import java.io.Writer;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
 
 import org.apache.struts2.StrutsException;
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
@@ -197,4 +201,126 @@
 		// interceptor does allow validation eg. method is not filtered out)
 		formComponent.evaluateClientSideJsEnablement(actionName, namespace, actionMethod);
 	}
+
+
+	public void beforeRenderUrl(URL urlComponent) {
+		if (urlComponent.value != null) {
+            urlComponent.value = urlComponent.findString(urlComponent.value);
+        }
+
+        // no explicit url set so attach params from current url, do
+        // this at start so body params can override any of these they wish.
+        try {
+            // ww-1266
+            String includeParams = (urlComponent.urlIncludeParams != null ? urlComponent.urlIncludeParams.toLowerCase() : URL.GET);
+
+            if (urlComponent.includeParams != null) {
+                includeParams = urlComponent.findString(urlComponent.includeParams);
+            }
+
+            if (URL.NONE.equalsIgnoreCase(includeParams)) {
+                mergeRequestParameters(urlComponent.value, urlComponent.parameters, Collections.EMPTY_MAP);
+            } else if (URL.ALL.equalsIgnoreCase(includeParams)) {
+                mergeRequestParameters(urlComponent.value, urlComponent.parameters, urlComponent.req.getParameterMap());
+
+                // for ALL also include GET parameters
+                includeGetParameters(urlComponent);
+                includeExtraParameters(urlComponent);
+            } else if (URL.GET.equalsIgnoreCase(includeParams) || (includeParams == null && urlComponent.value == null && urlComponent.action == null)) {
+                includeGetParameters(urlComponent);
+                includeExtraParameters(urlComponent);
+            } else if (includeParams != null) {
+                LOG.warn("Unknown value for includeParams parameter to URL tag: " + includeParams);
+            }
+        } catch (Exception e) {
+            LOG.warn("Unable to put request parameters (" + urlComponent.req.getQueryString() + ") into parameter map.", e);
+        }
+
+		
+	}
+	
+    private void includeExtraParameters(URL urlComponent) {
+        if (urlComponent.extraParameterProvider != null) {
+            mergeRequestParameters(urlComponent.value, urlComponent.parameters, urlComponent.extraParameterProvider.getExtraParameters());
+        }
+    }
+    private void includeGetParameters(URL urlComponent) {
+    	String query = extractQueryString(urlComponent);
+    	mergeRequestParameters(urlComponent.value, urlComponent.parameters, UrlHelper.parseQueryString(query));
+    }
+
+    private String extractQueryString(URL urlComponent) {
+        // Parse the query string to make sure that the parameters come from the query, and not some posted data
+        String query = urlComponent.req.getQueryString();
+        if (query == null) {
+            query = (String) urlComponent.req.getAttribute("javax.servlet.forward.query_string");
+        }
+
+        if (query != null) {
+            // Remove possible #foobar suffix
+            int idx = query.lastIndexOf('#');
+
+            if (idx != -1) {
+                query = query.substring(0, idx);
+            }
+        }
+        return query;
+    }
+    
+    /**
+     * Merge request parameters into current parameters. If a parameter is
+     * 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(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();
+
+            if (!parameters.containsKey(key)) {
+                parameters.put(key, entry.getValue());
+            }
+        }
+    }
 }

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=651529&r1=651528&r2=651529&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 Fri Apr 25 01:16:36 2008
@@ -175,71 +175,10 @@
 
     public boolean start(Writer writer) {
         boolean result = super.start(writer);
-
-        if (value != null) {
-            value = findString(value);
-        }
-
-        // no explicit url set so attach params from current url, do
-        // this at start so body params can override any of these they wish.
-        try {
-            // ww-1266
-            String includeParams = (urlIncludeParams != null ? urlIncludeParams.toLowerCase() : GET);
-
-            if (this.includeParams != null) {
-                includeParams = findString(this.includeParams);
-            }
-
-            if (NONE.equalsIgnoreCase(includeParams)) {
-                mergeRequestParameters(value, parameters, Collections.EMPTY_MAP);
-            } else if (ALL.equalsIgnoreCase(includeParams)) {
-                mergeRequestParameters(value, parameters, req.getParameterMap());
-
-                // for ALL also include GET parameters
-                includeGetParameters();
-                includeExtraParameters();
-            } else if (GET.equalsIgnoreCase(includeParams) || (includeParams == null && value == null && action == null)) {
-                includeGetParameters();
-                includeExtraParameters();
-            } else if (includeParams != null) {
-                LOG.warn("Unknown value for includeParams parameter to URL tag: " + includeParams);
-            }
-        } catch (Exception e) {
-            LOG.warn("Unable to put request parameters (" + req.getQueryString() + ") into parameter map.", e);
-        }
-
-
+        urlRenderer.beforeRenderUrl(this);
         return result;
     }
 
-    private void includeExtraParameters() {
-        if (extraParameterProvider != null) {
-            mergeRequestParameters(value, parameters, extraParameterProvider.getExtraParameters());
-        }
-    }
-    private void includeGetParameters() {
-    	String query = extractQueryString();
-    	mergeRequestParameters(value, parameters, UrlHelper.parseQueryString(query));
-    }
-
-    private String extractQueryString() {
-        // Parse the query string to make sure that the parameters come from the query, and not some posted data
-        String query = req.getQueryString();
-        if (query == null) {
-            query = (String) req.getAttribute("javax.servlet.forward.query_string");
-        }
-
-        if (query != null) {
-            // Remove possible #foobar suffix
-            int idx = query.lastIndexOf('#');
-
-            if (idx != -1) {
-                query = query.substring(0, idx);
-            }
-        }
-        return query;
-    }
-
     public boolean end(Writer writer, String body) {
     	urlRenderer.renderUrl(writer, this);
         return super.end(writer, body);
@@ -313,64 +252,6 @@
     @StrutsTagAttribute(description="Specifies whether to force the addition of scheme, host and port or not", type="Boolean", defaultValue="false")
     public void setForceAddSchemeHostAndPort(boolean forceAddSchemeHostAndPort) {
         this.forceAddSchemeHostAndPort = forceAddSchemeHostAndPort;
-    }
-
-
-    /**
-     * Merge request parameters into current parameters. If a parameter is
-     * 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(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();
-
-            if (!parameters.containsKey(key)) {
-                parameters.put(key, entry.getValue());
-            }
-        }
     }
 
     public static interface ExtraParameterProvider {

Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/UrlRenderer.java
URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/UrlRenderer.java?rev=651529&r1=651528&r2=651529&view=diff
==============================================================================
--- struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/UrlRenderer.java (original)
+++ struts/struts2/trunk/core/src/main/java/org/apache/struts2/components/UrlRenderer.java Fri Apr 25 01:16:36 2008
@@ -28,6 +28,13 @@
  *
  */
 public interface UrlRenderer {
+	
+	/**
+	 * Preprocessing step
+	 * @param urlComponent
+	 */
+	void beforeRenderUrl(URL urlComponent);
+	
 	/**
 	 * Render a URL.
 	 * @param writer A writer that the implementation can use to write the result to.
@@ -40,4 +47,5 @@
 	 * @param formComponent The {@link Form} component that "owns" this renderer.
 	 */
 	void renderFormUrl(Form formComponent);
+
 }

Modified: struts/struts2/trunk/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java
URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java?rev=651529&r1=651528&r2=651529&view=diff
==============================================================================
--- struts/struts2/trunk/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java (original)
+++ struts/struts2/trunk/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java Fri Apr 25 01:16:36 2008
@@ -136,4 +136,7 @@
 		
 	}
 
+	public void beforeRenderUrl(URL urlComponent) {
+	}
+
 }

Modified: struts/struts2/trunk/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java
URL: http://svn.apache.org/viewvc/struts/struts2/trunk/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java?rev=651529&r1=651528&r2=651529&view=diff
==============================================================================
--- struts/struts2/trunk/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java (original)
+++ struts/struts2/trunk/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java Fri Apr 25 01:16:36 2008
@@ -38,6 +38,7 @@
 
 import junit.textui.TestRunner;
 
+import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.dispatcher.Dispatcher;
 import org.apache.struts2.portlet.PortletActionConstants;
 import org.apache.struts2.portlet.util.PortletUrlHelper;
@@ -49,6 +50,7 @@
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.ActionProxy;
+import com.opensymphony.xwork2.inject.Container;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.ValueStackFactory;
 
@@ -334,6 +336,57 @@
         mockPortletUrl.expects(once()).method("setWindowState").with(eq(WindowState.NORMAL));
     	tag.doStartTag();
     	tag.doEndTag();    	
+    }
+    
+    public void testUrlShouldNotIncludeParamsFromHttpQueryString() throws Exception {
+        PortletMode mode = PortletMode.VIEW;
+
+        mockHttpReq.stubs().method("getQueryString").will(returnValue("thisParamShouldNotBeIncluded=thisValueShouldNotBeIncluded"));
+
+        mockPortletRes.expects(once()).method("createRenderURL").will(
+                returnValue((PortletURL) mockPortletUrl.proxy()));
+
+        Map paramMap = new HashMap();
+        paramMap.put(PortletActionConstants.ACTION_PARAM, new String[]{"/view/testAction"});
+        paramMap.put("testParam1", new String[]{"testValue1"});
+        paramMap.put(PortletActionConstants.MODE_PARAM, new String[]{mode.toString()});
+
+        mockPortletUrl.expects(once()).method("setParameters").with(new ParamMapConstraint(paramMap));
+        mockPortletUrl.expects(once()).method("setPortletMode").with(eq(PortletMode.VIEW));
+        mockPortletUrl.expects(once()).method("setWindowState").with(eq(WindowState.NORMAL));
+        
+        // Mock the container. Need to do this to create test to reproduce WW-2504
+        Mock mockContainer = mock(Container.class);
+        mockContainer.stubs().method("getInstance").with(new Constraint[]{eq(String.class), eq(StrutsConstants.STRUTS_I18N_ENCODING)});
+        ActionContext ctx = ActionContext.getContext();
+        ctx.setContainer((Container)mockContainer.proxy());
+
+        tag.setAction("testAction?testParam1=testValue1");
+        tag.doStartTag();
+        tag.doEndTag();
+    }
+    
+    public void testUrlShouldIgnoreIncludeParams() throws Exception {
+        PortletMode mode = PortletMode.VIEW;
+
+        mockHttpReq.stubs().method("getQueryString").will(returnValue("thisParamShouldNotBeIncluded=thisValueShouldNotBeIncluded"));
+
+        mockPortletRes.expects(once()).method("createRenderURL").will(
+                returnValue((PortletURL) mockPortletUrl.proxy()));
+
+        Map paramMap = new HashMap();
+        paramMap.put(PortletActionConstants.ACTION_PARAM, new String[]{"/view/testAction"});
+        paramMap.put("testParam1", new String[]{"testValue1"});
+        paramMap.put(PortletActionConstants.MODE_PARAM, new String[]{mode.toString()});
+
+        mockPortletUrl.expects(once()).method("setParameters").with(new ParamMapConstraint(paramMap));
+        mockPortletUrl.expects(once()).method("setPortletMode").with(eq(PortletMode.VIEW));
+        mockPortletUrl.expects(once()).method("setWindowState").with(eq(WindowState.NORMAL));
+
+        tag.setAction("testAction?testParam1=testValue1");
+        tag.setIncludeParams("GET");
+        tag.doStartTag();
+        tag.doEndTag();
     }
     
     private static class ParamMapConstraint implements Constraint {