diff --git a/vertx-web-api-contract/src/main/java/io/vertx/ext/web/api/validation/impl/BaseValidationHandler.java b/vertx-web-api-contract/src/main/java/io/vertx/ext/web/api/validation/impl/BaseValidationHandler.java index 2642df07b5..e0d26e36b2 100644 --- a/vertx-web-api-contract/src/main/java/io/vertx/ext/web/api/validation/impl/BaseValidationHandler.java +++ b/vertx-web-api-contract/src/main/java/io/vertx/ext/web/api/validation/impl/BaseValidationHandler.java @@ -104,7 +104,7 @@ private Map validatePathParams(RoutingContext routingC for (ParameterValidationRule rule : pathParamsRules.values()) { String name = rule.getName(); - if (routingContext.hasPathParam(name)) { + if (name != null) { String pathParam = routingContext.pathParam(name); if (pathParam != null || !rule.isOptional() ) { RequestParameter parsedParam = rule.validateSingleParam(pathParam); diff --git a/vertx-web-validation/src/main/java/io/vertx/ext/web/validation/impl/ValidationHandlerImpl.java b/vertx-web-validation/src/main/java/io/vertx/ext/web/validation/impl/ValidationHandlerImpl.java index 62cb707937..751d5e31ed 100644 --- a/vertx-web-validation/src/main/java/io/vertx/ext/web/validation/impl/ValidationHandlerImpl.java +++ b/vertx-web-validation/src/main/java/io/vertx/ext/web/validation/impl/ValidationHandlerImpl.java @@ -187,8 +187,12 @@ private void runPredicates(RoutingContext context) throws BadRequestException { private Future> validatePathParams(RoutingContext routingContext) { // Validation process validate only params that are registered in the validation -> extra params are allowed - Map> pathParams = new HashMap<>(); - routingContext.forEachPathParam((name, params) -> pathParams.put(name, Collections.singletonList(params))); + Map> pathParams = routingContext + .pathParams() + .entrySet() + .stream() + .map(e -> new SimpleImmutableEntry<>(e.getKey(), Collections.singletonList(e.getValue()))) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); Map parsedParams = new HashMap<>(); diff --git a/vertx-web/src/main/java/io/vertx/ext/web/RoutingContext.java b/vertx-web/src/main/java/io/vertx/ext/web/RoutingContext.java index c6149bd490..782a13eda8 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/RoutingContext.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/RoutingContext.java @@ -34,7 +34,6 @@ import java.time.Instant; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import static io.vertx.codegen.annotations.GenIgnore.PERMITTED_TYPE; @@ -614,11 +613,10 @@ default LanguageHeader preferredLanguage() { } /** - * Returns a map of named parameters as defined in path declaration with their actual values - * - * @return the map of named parameters + * Add a new one or replace an existing path parameter + * @throws NullPointerException when the name or value is null */ - Map pathParams(); + void addOrReplacePathParam(String name, String value); /** * Remove a path parameter @@ -627,16 +625,11 @@ default LanguageHeader preferredLanguage() { boolean removePathParam(String s); /** - * Iterate over all the path parameters + * Returns an unmodifiable map of named parameters as defined in path declaration with their actual values * - * @param pathParamsConsumer the consumer for each path parameter - */ - void forEachPathParam(BiConsumer pathParamsConsumer); - - /** - * @return {@code true} if the context has a path parameter with the specified {@code name}, {@code false} otherwise. + * @return the map of named parameters */ - boolean hasPathParam(String name); + Map pathParams(); /** * Gets the value of a single path parameter diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteState.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteState.java index 5febac4c9a..736e9d8800 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteState.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RouteState.java @@ -1030,8 +1030,7 @@ public int matches(RoutingContextImplBase context, String mountPoint, boolean fa if (!exactPath) { context.matchRest = m.start("rest"); // always replace - context.pathParams() - .put("*", path.substring(context.matchRest)); + context.addOrReplacePathParam("*", path.substring(context.matchRest)); } if (!isEmpty(groups)) { @@ -1162,8 +1161,7 @@ private boolean pathMatches(String mountPoint, RoutingContext ctx) { if (exactPath) { // exact path has no "rest" - ctx.pathParams() - .remove("*"); + ctx.removePathParam("*"); return pathMatchesExact(thePath, requestPath, pathEndsWithSlash); } else { @@ -1184,8 +1182,7 @@ private boolean pathMatches(String mountPoint, RoutingContext ctx) { // because the mount path ended with a wildcard we are relaxed in the check if (thePath.regionMatches(0, requestPath, 0, pathLen - 1)) { // handle the "rest" as path param *, always known to be empty - ctx.pathParams() - .put("*", "/"); + ctx.addOrReplacePathParam("*", "/"); return true; } } @@ -1193,8 +1190,8 @@ private boolean pathMatches(String mountPoint, RoutingContext ctx) { if (requestPath.startsWith(thePath)) { // handle the "rest" as path param * - ctx.pathParams() - .put("*", URIDecoder.decodeURIComponent(requestPath.substring(thePath.length()), false)); + ctx.addOrReplacePathParam("*", + URIDecoder.decodeURIComponent(requestPath.substring(thePath.length()), false)); return true; } return false; @@ -1267,7 +1264,7 @@ private void addPathParam(RoutingContext context, String name, String value) { if (!request.params().contains(name)) { request.params().add(name, decodedValue); } - context.pathParams().put(name, decodedValue); + context.addOrReplacePathParam(name, decodedValue); } boolean hasNextContextHandler(RoutingContextImplBase context) { diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextDecorator.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextDecorator.java index b598376fdb..a88caaa08a 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextDecorator.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextDecorator.java @@ -254,6 +254,16 @@ public void reroute(HttpMethod method, String path) { decoratedContext.reroute(method, path); } + @Override + public void addOrReplacePathParam(String name, String value) { + decoratedContext.addOrReplacePathParam(name, value); + } + + @Override + public boolean removePathParam(String s) { + return decoratedContext.removePathParam(s); + } + @Override public Map pathParams() { return decoratedContext.pathParams(); diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java index 44a64f5a7e..2cc4e7238d 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextImpl.java @@ -441,28 +441,40 @@ public void reroute(HttpMethod method, String path) { } @Override - public void forEachPathParam(final BiConsumer pathParamsConsumer) { - if (pathParams != null) { - pathParams.forEach(pathParamsConsumer); - } + public void addOrReplacePathParam(final String name, final String value) { + Objects.requireNonNull(name, "name"); + Objects.requireNonNull(value, "value"); + getOrCreatePathParams().put(name, value); } @Override - public boolean hasPathParam(final String name) { - if (pathParams != null) { - return pathParams.containsKey(name); + public Map pathParams() { + if (pathParams == null || pathParams.isEmpty()) { + return Collections.emptyMap(); } - return false; + return Collections.unmodifiableMap(pathParams); } @Override - public Map pathParams() { - return getPathParams(); + public boolean removePathParam(String s) { + if (s == null) { + return false; + } + if (pathParams != null) { + return pathParams.remove(s) != null; + } + return false; } @Override public @Nullable String pathParam(String name) { - return getPathParams().get(name); + if (name == null) { + return null; + } + if (pathParams != null) { + return pathParams.get(name); + } + return null; } @Override @@ -506,9 +518,10 @@ private MultiMap getQueryParams(Charset charset) { return queryParams; } - private Map getPathParams() { + private Map getOrCreatePathParams() { if (pathParams == null) { - pathParams = new HashMap<>(); + // let's start small + pathParams = new HashMap<>(1); } return pathParams; } diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextWrapper.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextWrapper.java index 0836bb6eea..256857e3be 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextWrapper.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/impl/RoutingContextWrapper.java @@ -310,18 +310,18 @@ public void reroute(HttpMethod method, String path) { } @Override - public Map pathParams() { - return inner.pathParams(); + public void addOrReplacePathParam(final String name, final String value) { + inner.addOrReplacePathParam(name, value); } @Override - public boolean hasPathParam(final String name) { - return inner.hasPathParam(name); + public boolean removePathParam(final String s) { + return inner.removePathParam(s); } @Override - public void forEachPathParam(final BiConsumer pathParamsConsumer) { - inner.forEachPathParam(pathParamsConsumer); + public Map pathParams() { + return inner.pathParams(); } @Override diff --git a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java index 9310f91695..ae982dd734 100644 --- a/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java +++ b/vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java @@ -863,8 +863,7 @@ public void testPercentEncoding() throws Exception { @Test public void testPathParamsAreFulfilled() throws Exception { router.route("/blah/:abc/quux/:def/eep/:ghi").handler(rc -> { - Map params = rc.pathParams(); - rc.response().setStatusMessage(params.get("abc") + params.get("def") + params.get("ghi")).end(); + rc.response().setStatusMessage(rc.pathParam("abc") + rc.pathParam("def") + rc.pathParam("ghi")).end(); }); testPattern("/blah/tim/quux/julien/eep/nick", "timjuliennick"); } @@ -877,11 +876,10 @@ public void testPathParamsDoesNotOverrideQueryParam() throws Exception { final String queryParamValue2 = "queryParamValue2"; final String sep = ","; router.route("/blah/:" + paramName + "/test").handler(rc -> { - Map params = rc.pathParams(); MultiMap queryParams = rc.request().params(); List values = queryParams.getAll(paramName); String qValue = values.stream().collect(Collectors.joining(sep)); - rc.response().setStatusMessage(params.get(paramName) + "|" + qValue).end(); + rc.response().setStatusMessage(rc.pathParam(paramName) + "|" + qValue).end(); }); testRequest(HttpMethod.GET, "/blah/" + pathParamValue + "/test?" + paramName + "=" + queryParamValue1 + "&" + paramName + "=" + queryParamValue2,