svn commit: r1863397 - 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:23:51 2019
New Revision: 1863397

URL: http://svn.apache.org/viewvc?rev=1863397&view=rev
Log:
Implemented: Add unit tests for ‘UtilHttp#getParameterMap’
(OFBIZ-11138)

Adapt slightly the implementation to make mocking easier.

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=1863397&r1=1863396&r2=1863397&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:23:51 2019
@@ -142,15 +142,13 @@ public final class UtilHttp {
         Map<String, Object> paramMap = new HashMap<>();
 
         // add all the actual HTTP request parameters
-        Enumeration<String> e = UtilGenerics.cast(request.getParameterNames());
-        while (e.hasMoreElements()) {
-            String name = e.nextElement();
+        Map<String, String[]> origParams = request.getParameterMap();
+        origParams.forEach((name, paramArr) -> {
             if (nameSet != null && (onlyIncludeOrSkipPrim ^ 
nameSet.contains(name))) {
-                continue;
+                return;
             }
 
             Object value = null;
-            String[] paramArr = request.getParameterValues(name);
             if (paramArr != null) {
                 if (paramArr.length > 1) {
                     value = Arrays.asList(paramArr);
@@ -160,7 +158,7 @@ public final class UtilHttp {
                 }
             }
             paramMap.put(name, value);
-        }
+        });
 
         paramMap.putAll(getPathInfoOnlyParameterMap(request.getPathInfo(), 
nameSet, onlyIncludeOrSkipPrim));
 

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=1863397&r1=1863396&r2=1863397&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:23:51 2019
@@ -22,14 +22,27 @@ import static org.apache.ofbiz.base.util
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.Matchers.hasEntry;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Map;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.HttpMethod;
+
 import org.hamcrest.Matchers;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class UtilHttpTest {
+    private HttpServletRequest req;
+
+    @Before
+    public void setup() {
+        req = Mockito.mock(HttpServletRequest.class);
+    }
 
     @Test
     public void basicGetPathInfoOnlyParameterMap() {
@@ -61,4 +74,50 @@ public class UtilHttpTest {
         assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", 
UtilMisc.toSet("foo"), true),
                 allOf(hasEntry("foo", "1"), not(hasEntry("bar", "2"))));
     }
+
+    @Test
+    public void basicGetParameterMap() {
+        when(req.getParameterMap()).thenReturn(UtilMisc.toMap(
+                "foo", new String[] {"1"},
+                "bar", new String[] {"2", "3"}));
+        when(req.getPathInfo()).thenReturn("/foo");
+        assertThat(UtilHttp.getParameterMap(req), Matchers.<Map<String, 
Object>>allOf(
+                hasEntry("foo", "1"),
+                hasEntry("bar", Arrays.asList("2", "3"))));
+    }
+
+    @Test
+    public void pathInfoOverrideGetParameterMap() {
+        when(req.getParameterMap()).thenReturn(UtilMisc.toMap(
+                "foo", new String[] {"1"},
+                "bar", new String[] {"2"}));
+        when(req.getPathInfo()).thenReturn("/foo/~bar=3");
+        assertThat(UtilHttp.getParameterMap(req), Matchers.<Map<String, 
Object>>allOf(
+                hasEntry("foo", "1"),
+                hasEntry("bar", "3")));
+    }
+
+    @Test
+    public void emptyParameterMap() {
+        when(req.getParameterMap()).thenReturn(Collections.emptyMap());
+        when(req.getPathInfo()).thenReturn("/foo/bar");
+        when(req.getMethod()).thenReturn(HttpMethod.POST);
+        UtilHttp.getParameterMap(req);
+        // Check that multi-part arguments are looked up
+        Mockito.verify(req).getContentType();
+    }
+
+    @Test
+    public void filteredGetParameterMap() {
+        when(req.getParameterMap()).thenReturn(UtilMisc.toMap(
+                "foo", new String[] {"1"},
+                "bar", new String[] {"2", "3"}));
+        when(req.getPathInfo()).thenReturn("/foo");
+        assertThat(UtilHttp.getParameterMap(req, UtilMisc.toSet("bar"), 
false), Matchers.<Map<String, Object>>allOf(
+                hasEntry("foo", "1"),
+                not(hasEntry("bar", Arrays.asList("2", "3")))));
+        assertThat(UtilHttp.getParameterMap(req, UtilMisc.toSet("bar"), true), 
Matchers.<Map<String, Object>>allOf(
+                not(hasEntry("foo", "1")),
+                hasEntry("bar", Arrays.asList("2", "3"))));
+    }
 }