svn commit: r1863400 - in /ofbiz/ofbiz-framework/trunk/framework/base/src: main/java/org/apache/ofbiz/base/util/UtilHttp.java test/java/org/apache/ofbiz/base/util/UtilHttpTest.java

[mthl]

Author: mthl
Date: Fri Jul 19 14:27:23 2019
New Revision: 1863400

URL: http://svn.apache.org/viewvc?rev=1863400&view=rev
Log:
Improved: Use a predicate in ‘UtilHttp#getPathInfoOnlyParameterMap’
(OFBIZ-11138)

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java?rev=1863400&r1=1863399&r2=1863400&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java Fri Jul 19 14:27:23 2019
@@ -53,6 +53,7 @@ import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TimeZone;
 import java.util.function.Function;
+import java.util.function.Predicate;
 
 import javax.net.ssl.SSLContext;
 import javax.servlet.http.HttpServletRequest;
@@ -155,7 +156,8 @@ public final class UtilHttp {
                 .collect(toMap(Map.Entry::getKey, pair -> transformParamValue(pair.getValue())));
 
         // Pseudo-parameters passed in the URI path overrides the ones from the regular URI parameters
-        params.putAll(getPathInfoOnlyParameterMap(req.getPathInfo(), nameSet, includeOrSkip));
+        params.putAll(getPathInfoOnlyParameterMap(req.getPathInfo(),
+                name -> nameSet == null || !(includeOrSkip ^ nameSet.contains(name))));
 
         // If nothing is found in the parameters, try to find something in the multi-part map.
         Map<String, Object> multiPartMap = params.isEmpty() ? getMultiPartParameterMap(req) : Collections.emptyMap();
@@ -317,12 +319,10 @@ public final class UtilHttp {
      * This is an obsolete syntax for passing parameters to request handlers.
      *
      * @param path  the URI path part which can be {@code null}
-     * @param nameSet  the set of parameters keys to include or skip
-     * @param includeOrSkip  toggle where {@code true} means including and {@code false} means skipping
+     * @param pred  the predicate filtering parameter names
      * @return a canonicalized parameter map.
      */
-    static Map<String, Object> getPathInfoOnlyParameterMap(String path, Set<? extends String> nameSet,
-            boolean includeOrSkip) {
+    static Map<String, Object> getPathInfoOnlyParameterMap(String path, Predicate<String> pred) {
         String path$ = Optional.ofNullable(path).orElse("");
         Map<String, List<String>> allParams = Arrays.stream(path$.split("/"))
                 .filter(segment -> segment.startsWith("~") && segment.contains("="))
@@ -332,7 +332,7 @@ public final class UtilHttp {
         // Filter and canonicalize the parameter map.
         Function<List<String>, Object> canonicalize = val -> (val.size() == 1) ? val.get(0) : val;
         return allParams.entrySet().stream()
-                .filter(e -> nameSet == null || !(includeOrSkip ^ nameSet.contains(e.getKey())))
+                .filter(pair -> pred.test(pair.getKey()))
                 .collect(collectingAndThen(toMap(Map.Entry::getKey, canonicalize.compose(Map.Entry::getValue)),
                         UtilHttp::canonicalizeParameterMap));
     }
@@ -340,7 +340,7 @@ public final class UtilHttp {
     public static Map<String, Object> getUrlOnlyParameterMap(HttpServletRequest request) {
         // NOTE: these have already been through canonicalizeParameterMap, so not doing it again here
         Map<String, Object> paramMap = getQueryStringOnlyParameterMap(request.getQueryString());
-        paramMap.putAll(getPathInfoOnlyParameterMap(request.getPathInfo(), null, true));
+        paramMap.putAll(getPathInfoOnlyParameterMap(request.getPathInfo(), x -> true));
         return paramMap;
     }
 

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java?rev=1863400&r1=1863399&r2=1863400&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java Fri Jul 19 14:27:23 2019
@@ -46,16 +46,16 @@ public class UtilHttpTest {
 
     @Test
     public void basicGetPathInfoOnlyParameterMap() {
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", null, false),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", x -> true),
                 allOf(hasEntry("foo", "1"), hasEntry("bar", "2")));
 
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2", null, false),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2", x -> true),
                 hasEntry("foo", Arrays.asList("1", "2")));
 
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2/~foo=3/", null, false),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2/~foo=3/", x -> true),
                 hasEntry("foo", Arrays.asList("1", "2", "3")));
 
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2/~foo=3/", null, false),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2/~foo=3/", x -> true),
                 Matchers.<Map<String,Object>>allOf(
                         hasEntry("foo", Arrays.asList("1", "3")),
                         hasEntry("bar", "2")));
@@ -63,15 +63,15 @@ public class UtilHttpTest {
 
     @Test
     public void emptyGetPathInfoOnlyParameterMap() {
-        assertThat(getPathInfoOnlyParameterMap(null, null, false), is(anEmptyMap()));
+        assertThat(getPathInfoOnlyParameterMap(null, x -> true), is(anEmptyMap()));
     }
 
     @Test
     public void filteredGetPathInfoOnlyParameterMap() {
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", UtilMisc.toSet("foo"), false),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", name -> !"foo".equals(name)),
                 allOf(not(hasEntry("foo", "1")), hasEntry("bar", "2")));
 
-        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", UtilMisc.toSet("foo"), true),
+        assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", "foo"::equals),
                 allOf(hasEntry("foo", "1"), not(hasEntry("bar", "2"))));
     }