Merge pull request apache#634 from apache/WW-5259-parser

[WW-5259] Extracts UrlHelper#parseQueryString into a dedicated bean

core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java

@@ -118,8 +118,10 @@
 import org.apache.struts2.conversion.StrutsTypeConverterHolder;
 import org.apache.struts2.dispatcher.HttpParameters;
 import org.apache.struts2.dispatcher.Parameter;
-import org.apache.struts2.url.ParametersStringBuilder;
-import org.apache.struts2.url.StrutsParametersStringBuilder;
+import org.apache.struts2.url.QueryStringBuilder;
+import org.apache.struts2.url.QueryStringParser;
+import org.apache.struts2.url.StrutsQueryStringBuilder;
+import org.apache.struts2.url.StrutsQueryStringParser;
 import org.apache.struts2.url.StrutsUrlDecoder;
 import org.apache.struts2.url.StrutsUrlEncoder;
 import org.apache.struts2.url.UrlDecoder;
@@ -236,7 +238,8 @@ public void register(ContainerBuilder builder, LocatableProperties props)
 
             .factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON)
 
-            .factory(ParametersStringBuilder.class, StrutsParametersStringBuilder.class, Scope.SINGLETON)
+            .factory(QueryStringBuilder.class, StrutsQueryStringBuilder.class, Scope.SINGLETON)
+            .factory(QueryStringParser.class, StrutsQueryStringParser.class, Scope.SINGLETON)
             .factory(UrlEncoder.class, StrutsUrlEncoder.class, Scope.SINGLETON)
             .factory(UrlDecoder.class, StrutsUrlDecoder.class, Scope.SINGLETON)
         ;

core/src/main/java/org/apache/struts2/StrutsConstants.java

@@ -458,7 +458,8 @@ public final class StrutsConstants {
     /** See {@link org.apache.struts2.components.Date#setDateFormatter(DateFormatter)} */
     public static final String STRUTS_DATE_FORMATTER = "struts.date.formatter";
 
-    public static final String STRUTS_URL_PARAMETERS_STRING_BUILDER = "struts.url.parametersStringBuilder";
+    public static final String STRUTS_URL_QUERY_STRING_BUILDER = "struts.url.queryStringBuilder";
+    public static final String STRUTS_URL_QUERY_STRING_PARSER = "struts.url.queryStringParser";
     public static final String STRUTS_URL_ENCODER = "struts.url.encoder";
     public static final String STRUTS_URL_DECODER = "struts.url.decoder";
 }

core/src/main/java/org/apache/struts2/components/ExtraParameterProvider.java

@@ -21,5 +21,7 @@
 import java.util.Map;
 
 public interface ExtraParameterProvider {
-    public Map getExtraParameters();
+
+    Map<String, Object> getExtraParameters();
+
 }

core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java

@@ -29,6 +29,7 @@
 import org.apache.struts2.StrutsException;
 import org.apache.struts2.dispatcher.mapper.ActionMapper;
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
+import org.apache.struts2.url.QueryStringParser;
 import org.apache.struts2.views.util.UrlHelper;
 
 import java.io.IOException;
@@ -48,6 +49,7 @@ public class ServletUrlRenderer implements UrlRenderer {
 
     private ActionMapper actionMapper;
     private UrlHelper urlHelper;
+    private QueryStringParser queryStringParser;
 
     @Override
     @Inject
@@ -60,6 +62,11 @@ public void setUrlHelper(UrlHelper urlHelper) {
         this.urlHelper = urlHelper;
     }
 
+    @Inject
+    public void setQueryStringParser(QueryStringParser queryStringParser) {
+        this.queryStringParser = queryStringParser;
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -152,10 +159,10 @@ public void renderFormUrl(Form formComponent) {
             }
         }
 
-        Map actionParams = null;
+        Map<String, Object> actionParams = null;
         if (action != null && action.indexOf('?') > 0) {
             String queryString = action.substring(action.indexOf('?') + 1);
-            actionParams = urlHelper.parseQueryString(queryString, false);
+            actionParams = queryStringParser.parse(queryString, false);
             action = action.substring(0, action.indexOf('?'));
         }
 
@@ -164,19 +171,19 @@ public void renderFormUrl(Form formComponent) {
         String actionMethod = nameMapping.getMethod();
 
         final ActionConfig actionConfig = formComponent.configuration.getRuntimeConfiguration().getActionConfig(
-                namespace, actionName);
+            namespace, actionName);
         if (actionConfig != null) {
 
             ActionMapping mapping = new ActionMapping(actionName, namespace, actionMethod, formComponent.parameters);
             String result = urlHelper.buildUrl(formComponent.actionMapper.getUriFromActionMapping(mapping),
-                    formComponent.request, formComponent.response, actionParams, scheme, formComponent.includeContext, true, false, false);
+                formComponent.request, formComponent.response, actionParams, scheme, formComponent.includeContext, true, false, false);
             formComponent.addParameter("action", result);
 
             // let's try to get the actual action class and name
             // this can be used for getting the list of validators
             formComponent.addParameter("actionName", actionName);
             try {
-                Class clazz = formComponent.objectFactory.getClassInstance(actionConfig.getClassName());
+                Class<?> clazz = formComponent.objectFactory.getClassInstance(actionConfig.getClassName());
                 formComponent.addParameter("actionClass", clazz);
             } catch (ClassNotFoundException e) {
                 // this is OK, we'll just move on
@@ -258,7 +265,7 @@ public void beforeRenderUrl(UrlProvider urlComponent) {
             }
 
             if (UrlProvider.NONE.equalsIgnoreCase(includeParams)) {
-                mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), Collections.<String, Object>emptyMap());
+                mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), Collections.emptyMap());
             } else if (UrlProvider.ALL.equalsIgnoreCase(includeParams)) {
                 mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), urlComponent.getHttpServletRequest().getParameterMap());
 
@@ -284,7 +291,7 @@ private void includeExtraParameters(UrlProvider urlComponent) {
 
     private void includeGetParameters(UrlProvider urlComponent) {
         String query = extractQueryString(urlComponent);
-        mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), urlHelper.parseQueryString(query, false));
+        mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), queryStringParser.parse(query, false));
     }
 
     private String extractQueryString(UrlProvider urlComponent) {
@@ -309,16 +316,16 @@ private String extractQueryString(UrlProvider urlComponent) {
      * Merge request parameters into current parameters. If a parameter is
      * already present, than the request parameter in the current request and value attribute
      * will not override its value.
-     *
+     * <p>
      * 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 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<String, Object> parameters, Map<String, ?> contextParameters) {
@@ -332,7 +339,7 @@ protected void mergeRequestParameters(String value, Map<String, Object> paramete
         if (StringUtils.contains(value, "?")) {
             String queryString = value.substring(value.indexOf('?') + 1);
 
-            mergedParams = urlHelper.parseQueryString(queryString, false);
+            mergedParams = queryStringParser.parse(queryString, false);
             for (Map.Entry<String, ?> entry : contextParameters.entrySet()) {
                 if (!mergedParams.containsKey(entry.getKey())) {
                     mergedParams.put(entry.getKey(), entry.getValue());

core/src/main/java/org/apache/struts2/components/UrlProvider.java

@@ -25,7 +25,7 @@
 import java.util.Map;
 
 /**
- * Implemntations of this interface can be used to build a URL
+ * Implementations of this interface can be used to build a URL
  */
 public interface UrlProvider {
     /**
@@ -37,9 +37,9 @@ public interface UrlProvider {
      * get  - include only GET parameters in the URL (default)
      * all  - include both GET and POST parameters in the URL
      */
-    public static final String NONE = "none";
-    public static final String GET = "get";
-    public static final String ALL = "all";
+    String NONE = "none";
+    String GET = "get";
+    String ALL = "all";
 
     boolean isPutInContext();
 
@@ -55,7 +55,7 @@ public interface UrlProvider {
 
     String getIncludeParams();
 
-    Map getParameters();
+    Map<String, Object> getParameters();
 
     HttpServletRequest getHttpServletRequest();
 
@@ -78,19 +78,19 @@ public interface UrlProvider {
     boolean isForceAddSchemeHostAndPort();
 
     boolean isEscapeAmp();
-    
+
     String getPortletMode();
-    
+
     String getWindowState();
 
-    String determineActionURL(String action, String namespace, String method, HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map parameters, String scheme, boolean includeContext, boolean encode, boolean forceAddSchemeHostAndPort, boolean escapeAmp);
-    
+    String determineActionURL(String action, String namespace, String method, HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Map<String, ?> parameters, String scheme, boolean includeContext, boolean encode, boolean forceAddSchemeHostAndPort, boolean escapeAmp);
+
     String determineNamespace(String namespace, ValueStack stack, HttpServletRequest req);
 
     String getAnchor();
-    
+
     String getPortletUrlType();
-    
+
     ValueStack getStack();
 
     void setUrlIncludeParams(String urlIncludeParams);

core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java

@@ -66,7 +66,8 @@
 import org.apache.struts2.dispatcher.StaticContentLoader;
 import org.apache.struts2.dispatcher.mapper.ActionMapper;
 import org.apache.struts2.dispatcher.multipart.MultiPartRequest;
-import org.apache.struts2.url.ParametersStringBuilder;
+import org.apache.struts2.url.QueryStringBuilder;
+import org.apache.struts2.url.QueryStringParser;
 import org.apache.struts2.url.UrlDecoder;
 import org.apache.struts2.url.UrlEncoder;
 import org.apache.struts2.util.ContentTypeMatcher;
@@ -432,7 +433,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) {
         alias(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, builder, props, Scope.SINGLETON);
         alias(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, builder, props, Scope.SINGLETON);
 
-        alias(ParametersStringBuilder.class, StrutsConstants.STRUTS_URL_PARAMETERS_STRING_BUILDER, builder, props, Scope.SINGLETON);
+        alias(QueryStringBuilder.class, StrutsConstants.STRUTS_URL_QUERY_STRING_BUILDER, builder, props, Scope.SINGLETON);
+        alias(QueryStringParser.class, StrutsConstants.STRUTS_URL_QUERY_STRING_PARSER, builder, props, Scope.SINGLETON);
         alias(UrlEncoder.class, StrutsConstants.STRUTS_URL_ENCODER, builder, props, Scope.SINGLETON);
         alias(UrlDecoder.class, StrutsConstants.STRUTS_URL_DECODER, builder, props, Scope.SINGLETON);
 

core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java

@@ -27,7 +27,7 @@
 import org.apache.struts2.ServletActionContext;
 import org.apache.struts2.StrutsStatics;
 import org.apache.struts2.dispatcher.HttpParameters;
-import org.apache.struts2.views.util.UrlHelper;
+import org.apache.struts2.url.QueryStringParser;
 
 import javax.servlet.RequestDispatcher;
 import javax.servlet.http.HttpServletRequest;
@@ -66,7 +66,7 @@
  * <!-- END SNIPPET: description -->
  *
  * <p><b>This result type takes the following parameters:</b></p>
- *
+ * <p>
  * <!-- START SNIPPET: params -->
  *
  * <ul>
@@ -76,7 +76,7 @@
  * <li><b>parse</b> - true by default. If set to false, the location param will not be parsed for Ognl expressions.</li>
  *
  * </ul>
- *
+ * <p>
  * <!-- END SNIPPET: params -->
  *
  * <p><b>Example:</b></p>
@@ -99,7 +99,7 @@ public class ServletDispatcherResult extends StrutsResultSupport {
 
     private static final Logger LOG = LogManager.getLogger(ServletDispatcherResult.class);
 
-    private UrlHelper urlHelper;
+    private QueryStringParser queryStringParser;
 
     public ServletDispatcherResult() {
         super();
@@ -110,8 +110,8 @@ public ServletDispatcherResult(String location) {
     }
 
     @Inject
-    public void setUrlHelper(UrlHelper urlHelper) {
-        this.urlHelper = urlHelper;
+    public void setQueryStringParser(QueryStringParser queryStringParser) {
+        this.queryStringParser = queryStringParser;
     }
 
     /**
@@ -140,7 +140,7 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws
             if (StringUtils.isNotEmpty(finalLocation) && finalLocation.indexOf('?') > 0) {
                 String queryString = finalLocation.substring(finalLocation.indexOf('?') + 1);
                 HttpParameters parameters = getParameters(invocation);
-                Map<String, Object> queryParams = urlHelper.parseQueryString(queryString, true);
+                Map<String, Object> queryParams = queryStringParser.parse(queryString, true);
                 if (queryParams != null && !queryParams.isEmpty()) {
                     parameters = HttpParameters.create(queryParams).withParent(parameters).build();
                     invocation.getInvocationContext().setParameters(parameters);

core/src/main/java/org/apache/struts2/result/ServletRedirectResult.java

@@ -29,7 +29,7 @@
 import org.apache.struts2.dispatcher.Dispatcher;
 import org.apache.struts2.dispatcher.mapper.ActionMapper;
 import org.apache.struts2.dispatcher.mapper.ActionMapping;
-import org.apache.struts2.views.util.UrlHelper;
+import org.apache.struts2.url.QueryStringBuilder;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -54,7 +54,7 @@
  * available. This is because actions are built on a single-thread model. The
  * only way to pass data is through the session or with web parameters
  * (url?name=value) which can be OGNL expressions.
- *
+ * <p>
  * <b>This result type takes the following parameters:</b>
  *
  * <ul>
@@ -65,7 +65,7 @@
  * "hash".  You can specify an anchor for a result.</li>
  * </ul>
  * This result follows the same rules from {@link StrutsResultSupport}.
- *
+ * <p>
  * <b>Example:</b>
  * <pre>
  * <!-- START SNIPPET: example -->
@@ -94,7 +94,7 @@ public class ServletRedirectResult extends StrutsResultSupport implements Reflec
     protected Map<String, Object> requestParameters = new LinkedHashMap<>();
     protected String anchor;
 
-    private UrlHelper urlHelper;
+    private QueryStringBuilder queryStringBuilder;
 
     public ServletRedirectResult() {
         super();
@@ -115,8 +115,8 @@ public void setActionMapper(ActionMapper mapper) {
     }
 
     @Inject
-    public void setUrlHelper(UrlHelper urlHelper) {
-        this.urlHelper = urlHelper;
+    public void setQueryStringBuilder(QueryStringBuilder queryStringBuilder) {
+        this.queryStringBuilder = queryStringBuilder;
     }
 
     public void setStatusCode(int code) {
@@ -202,7 +202,7 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro
         }
 
         StringBuilder tmpLocation = new StringBuilder(finalLocation);
-        urlHelper.buildParametersString(requestParameters, tmpLocation, "&");
+        queryStringBuilder.build(requestParameters, tmpLocation, "&");
 
         // add the anchor
         if (anchor != null) {
@@ -283,10 +283,7 @@ protected boolean isPathUrl(String url) {
                 LOG.debug("[{}] isn't absolute URI, assuming it's a path", url);
                 return true;
             }
-        } catch (IllegalArgumentException e) {
-            LOG.debug("[{}] isn't a valid URL, assuming it's a path", url, e);
-            return true;
-        } catch (MalformedURLException e) {
+        } catch (IllegalArgumentException | MalformedURLException e) {
             LOG.debug("[{}] isn't a valid URL, assuming it's a path", url, e);
             return true;
         }

core/src/main/java/org/apache/struts2/url/QueryStringBuilder.java

@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.url;
+
+import java.io.Serializable;
+import java.util.Map;
+
+/**
+ * A builder used to create a proper Query String out of a set of parameters
+ * @since Struts 6.1.0
+ */
+public interface QueryStringBuilder extends Serializable {
+
+    /**
+     * Builds a Query String with defined separator and appends it to the provided link
+     * @param params a Map used to build a Query String
+     * @param link to which the Query String should be added
+     * @param paramSeparator used to separate parameters in query string
+     */
+    void build(Map<String, Object> params, StringBuilder link, String paramSeparator);
+
+}