Skip to content

Commit 39ba0a7

Browse files
committed
Improved: Use a predicate in ‘UtilHttp#getPathInfoOnlyParameterMap’
(OFBIZ-11138) git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1863400 13f79535-47bb-0310-9956-ffa450edef68
1 parent 54d873a commit 39ba0a7

File tree

2 files changed

+14
-14
lines changed

2 files changed

+14
-14
lines changed

Diff for: framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import java.util.StringTokenizer;
5454
import java.util.TimeZone;
5555
import java.util.function.Function;
56+
import java.util.function.Predicate;
5657

5758
import javax.net.ssl.SSLContext;
5859
import javax.servlet.http.HttpServletRequest;
@@ -155,7 +156,8 @@ public static Map<String, Object> getParameterMap(HttpServletRequest req, Set<?
155156
.collect(toMap(Map.Entry::getKey, pair -> transformParamValue(pair.getValue())));
156157

157158
// Pseudo-parameters passed in the URI path overrides the ones from the regular URI parameters
158-
params.putAll(getPathInfoOnlyParameterMap(req.getPathInfo(), nameSet, includeOrSkip));
159+
params.putAll(getPathInfoOnlyParameterMap(req.getPathInfo(),
160+
name -> nameSet == null || !(includeOrSkip ^ nameSet.contains(name))));
159161

160162
// If nothing is found in the parameters, try to find something in the multi-part map.
161163
Map<String, Object> multiPartMap = params.isEmpty() ? getMultiPartParameterMap(req) : Collections.emptyMap();
@@ -317,12 +319,10 @@ public static Map<String, Object> getQueryStringOnlyParameterMap(String queryStr
317319
* This is an obsolete syntax for passing parameters to request handlers.
318320
*
319321
* @param path the URI path part which can be {@code null}
320-
* @param nameSet the set of parameters keys to include or skip
321-
* @param includeOrSkip toggle where {@code true} means including and {@code false} means skipping
322+
* @param pred the predicate filtering parameter names
322323
* @return a canonicalized parameter map.
323324
*/
324-
static Map<String, Object> getPathInfoOnlyParameterMap(String path, Set<? extends String> nameSet,
325-
boolean includeOrSkip) {
325+
static Map<String, Object> getPathInfoOnlyParameterMap(String path, Predicate<String> pred) {
326326
String path$ = Optional.ofNullable(path).orElse("");
327327
Map<String, List<String>> allParams = Arrays.stream(path$.split("/"))
328328
.filter(segment -> segment.startsWith("~") && segment.contains("="))
@@ -332,15 +332,15 @@ static Map<String, Object> getPathInfoOnlyParameterMap(String path, Set<? extend
332332
// Filter and canonicalize the parameter map.
333333
Function<List<String>, Object> canonicalize = val -> (val.size() == 1) ? val.get(0) : val;
334334
return allParams.entrySet().stream()
335-
.filter(e -> nameSet == null || !(includeOrSkip ^ nameSet.contains(e.getKey())))
335+
.filter(pair -> pred.test(pair.getKey()))
336336
.collect(collectingAndThen(toMap(Map.Entry::getKey, canonicalize.compose(Map.Entry::getValue)),
337337
UtilHttp::canonicalizeParameterMap));
338338
}
339339

340340
public static Map<String, Object> getUrlOnlyParameterMap(HttpServletRequest request) {
341341
// NOTE: these have already been through canonicalizeParameterMap, so not doing it again here
342342
Map<String, Object> paramMap = getQueryStringOnlyParameterMap(request.getQueryString());
343-
paramMap.putAll(getPathInfoOnlyParameterMap(request.getPathInfo(), null, true));
343+
paramMap.putAll(getPathInfoOnlyParameterMap(request.getPathInfo(), x -> true));
344344
return paramMap;
345345
}
346346

Diff for: framework/base/src/test/java/org/apache/ofbiz/base/util/UtilHttpTest.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -46,32 +46,32 @@ public void setup() {
4646

4747
@Test
4848
public void basicGetPathInfoOnlyParameterMap() {
49-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", null, false),
49+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", x -> true),
5050
allOf(hasEntry("foo", "1"), hasEntry("bar", "2")));
5151

52-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2", null, false),
52+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2", x -> true),
5353
hasEntry("foo", Arrays.asList("1", "2")));
5454

55-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2/~foo=3/", null, false),
55+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~foo=2/~foo=3/", x -> true),
5656
hasEntry("foo", Arrays.asList("1", "2", "3")));
5757

58-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2/~foo=3/", null, false),
58+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2/~foo=3/", x -> true),
5959
Matchers.<Map<String,Object>>allOf(
6060
hasEntry("foo", Arrays.asList("1", "3")),
6161
hasEntry("bar", "2")));
6262
}
6363

6464
@Test
6565
public void emptyGetPathInfoOnlyParameterMap() {
66-
assertThat(getPathInfoOnlyParameterMap(null, null, false), is(anEmptyMap()));
66+
assertThat(getPathInfoOnlyParameterMap(null, x -> true), is(anEmptyMap()));
6767
}
6868

6969
@Test
7070
public void filteredGetPathInfoOnlyParameterMap() {
71-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", UtilMisc.toSet("foo"), false),
71+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", name -> !"foo".equals(name)),
7272
allOf(not(hasEntry("foo", "1")), hasEntry("bar", "2")));
7373

74-
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", UtilMisc.toSet("foo"), true),
74+
assertThat(getPathInfoOnlyParameterMap("/~foo=1/~bar=2", "foo"::equals),
7575
allOf(hasEntry("foo", "1"), not(hasEntry("bar", "2"))));
7676
}
7777

0 commit comments

Comments
 (0)